| Message ID | 1357591415-17092-1-git-send-email-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 1/7/2013 10:43 AM, Sean Paul wrote: > Add a property to the hdmi node so we can specify the HDMI version in > the device tree instead of just defaulting to v1.4 with the existence of > the dt node. > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > .../devicetree/bindings/drm/exynos/hdmi.txt | 3 +++ > drivers/gpu/drm/exynos/exynos_hdmi.c | 19 ++++++++++--------- > 2 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > index 589edee..d1c7d91 100644 > --- a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > +++ b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > @@ -11,6 +11,8 @@ Required properties: > c) pin function mode. > d) optional flags and pull up/down. > e) drive strength. > +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 > +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 a) This seems pretty generic, not at all samsung-specific, as the HDMI version numbering space is well-defined by the HDMI spec. b) It would be better to make it an integer property whose value encodes the version number, thus eliminating the need to add new properties as new HDMI versions appear. > > Example: > > @@ -19,4 +21,5 @@ Example: > reg = <0x14530000 0x100000>; > interrupts = <0 95 0>; > hpd-gpio = <&gpx3 7 0xf 1 3>; > + samsung,supports-hdmi-1.4; > }; > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 2c46b6c..9834ae5 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -2444,7 +2444,6 @@ static struct platform_device_id hdmi_driver_types[] = { > static struct of_device_id hdmi_match_types[] = { > { > .compatible = "samsung,exynos5-hdmi", > - .data = (void *)HDMI_TYPE14, > }, { > /* end node */ > } > @@ -2498,16 +2497,18 @@ static int __devinit hdmi_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, drm_hdmi_ctx); > > - if (dev->of_node) { > - const struct of_device_id *match; > - match = of_match_node(of_match_ptr(hdmi_match_types), > - pdev->dev.of_node); > - if (match == NULL) > - return -ENODEV; > - hdata->type = (enum hdmi_type)match->data; > - } else { > + if (!dev->of_node) { > hdata->type = (enum hdmi_type)platform_get_device_id > (pdev)->driver_data; > + } else if (of_get_property(dev->of_node, "samsung,supports-hdmi-1.4", > + NULL)) { > + hdata->type = HDMI_TYPE14; > + } else if (of_get_property(dev->of_node, "samsung,supports-hdmi-1.3", > + NULL)) { > + hdata->type = HDMI_TYPE13; > + } else { > + DRM_ERROR("Could not resolve HDMI version support\n"); > + return -ENODEV; > } > > hdata->hpd_gpio = pdata->hpd_gpio; >
On Mon, Jan 7, 2013 at 3:54 PM, Mitch Bradley <wmb@firmworks.com> wrote: > On 1/7/2013 10:43 AM, Sean Paul wrote: >> Add a property to the hdmi node so we can specify the HDMI version in >> the device tree instead of just defaulting to v1.4 with the existence of >> the dt node. >> >> Signed-off-by: Sean Paul <seanpaul@chromium.org> >> --- >> .../devicetree/bindings/drm/exynos/hdmi.txt | 3 +++ >> drivers/gpu/drm/exynos/exynos_hdmi.c | 19 ++++++++++--------- >> 2 files changed, 13 insertions(+), 9 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >> index 589edee..d1c7d91 100644 >> --- a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >> +++ b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >> @@ -11,6 +11,8 @@ Required properties: >> c) pin function mode. >> d) optional flags and pull up/down. >> e) drive strength. >> +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 >> +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 > > a) This seems pretty generic, not at all samsung-specific, as the HDMI > version numbering space is well-defined by the HDMI spec. > > b) It would be better to make it an integer property whose value > encodes the version number, thus eliminating the need to add new > properties as new HDMI versions appear. > Thanks for the quick review, Mitch. How about: - hdmi-version: 0=v1.3, 1=v1.4 >> >> Example: >> >> @@ -19,4 +21,5 @@ Example: >> reg = <0x14530000 0x100000>; >> interrupts = <0 95 0>; >> hpd-gpio = <&gpx3 7 0xf 1 3>; >> + samsung,supports-hdmi-1.4; >> }; >> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c >> index 2c46b6c..9834ae5 100644 >> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c >> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c >> @@ -2444,7 +2444,6 @@ static struct platform_device_id hdmi_driver_types[] = { >> static struct of_device_id hdmi_match_types[] = { >> { >> .compatible = "samsung,exynos5-hdmi", >> - .data = (void *)HDMI_TYPE14, >> }, { >> /* end node */ >> } >> @@ -2498,16 +2497,18 @@ static int __devinit hdmi_probe(struct platform_device *pdev) >> >> platform_set_drvdata(pdev, drm_hdmi_ctx); >> >> - if (dev->of_node) { >> - const struct of_device_id *match; >> - match = of_match_node(of_match_ptr(hdmi_match_types), >> - pdev->dev.of_node); >> - if (match == NULL) >> - return -ENODEV; >> - hdata->type = (enum hdmi_type)match->data; >> - } else { >> + if (!dev->of_node) { >> hdata->type = (enum hdmi_type)platform_get_device_id >> (pdev)->driver_data; >> + } else if (of_get_property(dev->of_node, "samsung,supports-hdmi-1.4", >> + NULL)) { >> + hdata->type = HDMI_TYPE14; >> + } else if (of_get_property(dev->of_node, "samsung,supports-hdmi-1.3", >> + NULL)) { >> + hdata->type = HDMI_TYPE13; >> + } else { >> + DRM_ERROR("Could not resolve HDMI version support\n"); >> + return -ENODEV; >> } >> >> hdata->hpd_gpio = pdata->hpd_gpio; >>
On 01/07/2013 01:43 PM, Sean Paul wrote: > Add a property to the hdmi node so we can specify the HDMI version in > the device tree instead of just defaulting to v1.4 with the existence of > the dt node. > diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > @@ -11,6 +11,8 @@ Required properties: > c) pin function mode. > d) optional flags and pull up/down. > e) drive strength. > +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 > +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 Which device; the HDMI controller in the SoC, or the HDMI sink? The HDMI sync reports what it supports in the EDID, so there shouldn't be a need to duplicate this in the device tree (besides, how can you know what the user plugged in without parsing the EDID?)
On Jan 7, 2013 5:32 PM, "Stephen Warren" <swarren@wwwdotorg.org> wrote: > > On 01/07/2013 01:43 PM, Sean Paul wrote: > > Add a property to the hdmi node so we can specify the HDMI version in > > the device tree instead of just defaulting to v1.4 with the existence of > > the dt node. > > > diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > > > @@ -11,6 +11,8 @@ Required properties: > > c) pin function mode. > > d) optional flags and pull up/down. > > e) drive strength. > > +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 > > +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 > > Which device; the HDMI controller in the SoC, or the HDMI sink? > It's the controller. > The HDMI sync reports what it supports in the EDID, so there shouldn't > be a need to duplicate this in the device tree (besides, how can you > know what the user plugged in without parsing the EDID?)
On 01/07/2013 04:12 PM, Sean Paul wrote: > > On Jan 7, 2013 5:32 PM, "Stephen Warren" <swarren@wwwdotorg.org > <mailto:swarren@wwwdotorg.org>> wrote: >> >> On 01/07/2013 01:43 PM, Sean Paul wrote: >> > Add a property to the hdmi node so we can specify the HDMI version in >> > the device tree instead of just defaulting to v1.4 with the existence of >> > the dt node. >> >> > diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >> >> > @@ -11,6 +11,8 @@ Required properties: >> > c) pin function mode. >> > d) optional flags and pull up/down. >> > e) drive strength. >> > +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 >> > +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 >> >> Which device; the HDMI controller in the SoC, or the HDMI sink? >> > > It's the controller. Ah OK. Is it different versions of the controller HW IP block that support the different HDMI versions, or is it some kind of fusing/configuration with the same HW block? If different versions of HW, wouldn't this difference usually be represented by different compatible values, that indicate the exact HW version? I guess if the only difference really is just the HDMI support and there are no other bugs/quirks/enhancements, a separate property might make sense.
On Tue, Jan 8, 2013 at 11:38 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 01/07/2013 04:12 PM, Sean Paul wrote: >> >> On Jan 7, 2013 5:32 PM, "Stephen Warren" <swarren@wwwdotorg.org >> <mailto:swarren@wwwdotorg.org>> wrote: >>> >>> On 01/07/2013 01:43 PM, Sean Paul wrote: >>> > Add a property to the hdmi node so we can specify the HDMI version in >>> > the device tree instead of just defaulting to v1.4 with the existence of >>> > the dt node. >>> >>> > diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >> b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >>> >>> > @@ -11,6 +11,8 @@ Required properties: >>> > c) pin function mode. >>> > d) optional flags and pull up/down. >>> > e) drive strength. >>> > +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 >>> > +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 >>> >>> Which device; the HDMI controller in the SoC, or the HDMI sink? >>> >> >> It's the controller. > > Ah OK. Is it different versions of the controller HW IP block that > support the different HDMI versions, or is it some kind of > fusing/configuration with the same HW block? I think the answer is yes :). I'm not 100% sure, maybe the Samsung guys can chime in, but this is what I've inferred. Exynos5 is purely v1.4, Exynos4 can be v1.3 or v1.4. However an IP block that is compatible with 1.4 is *not* compatible with 1.3. So I think encoding the version as an integer makes sense since it prevents us from defining both (which is incorrect but harmless in the current version), and is future-safe. Sound good? Sean > If different versions of > HW, wouldn't this difference usually be represented by different > compatible values, that indicate the exact HW version? I guess if the > only difference really is just the HDMI support and there are no other > bugs/quirks/enhancements, a separate property might make sense.
Am Montag, den 07.01.2013, 16:12 -0500 schrieb Sean Paul: > On Mon, Jan 7, 2013 at 3:54 PM, Mitch Bradley <wmb@firmworks.com> wrote: > > On 1/7/2013 10:43 AM, Sean Paul wrote: > >> Add a property to the hdmi node so we can specify the HDMI version in > >> the device tree instead of just defaulting to v1.4 with the existence of > >> the dt node. > >> > >> Signed-off-by: Sean Paul <seanpaul@chromium.org> > >> --- > >> .../devicetree/bindings/drm/exynos/hdmi.txt | 3 +++ > >> drivers/gpu/drm/exynos/exynos_hdmi.c | 19 ++++++++++--------- > >> 2 files changed, 13 insertions(+), 9 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > >> index 589edee..d1c7d91 100644 > >> --- a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > >> +++ b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt > >> @@ -11,6 +11,8 @@ Required properties: > >> c) pin function mode. > >> d) optional flags and pull up/down. > >> e) drive strength. > >> +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 > >> +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 > > > > a) This seems pretty generic, not at all samsung-specific, as the HDMI > > version numbering space is well-defined by the HDMI spec. > > > > b) It would be better to make it an integer property whose value > > encodes the version number, thus eliminating the need to add new > > properties as new HDMI versions appear. > > > > Thanks for the quick review, Mitch. > > How about: > > - hdmi-version: 0=v1.3, 1=v1.4 Why bother obfuscating the real version number? Why not just 13=v1.3, 14=v1.4? Regards, Lucas
On Tue, Jan 8, 2013 at 11:45 AM, Lucas Stach <dev@lynxeye.de> wrote: > Am Montag, den 07.01.2013, 16:12 -0500 schrieb Sean Paul: >> On Mon, Jan 7, 2013 at 3:54 PM, Mitch Bradley <wmb@firmworks.com> wrote: >> > On 1/7/2013 10:43 AM, Sean Paul wrote: >> >> Add a property to the hdmi node so we can specify the HDMI version in >> >> the device tree instead of just defaulting to v1.4 with the existence of >> >> the dt node. >> >> >> >> Signed-off-by: Sean Paul <seanpaul@chromium.org> >> >> --- >> >> .../devicetree/bindings/drm/exynos/hdmi.txt | 3 +++ >> >> drivers/gpu/drm/exynos/exynos_hdmi.c | 19 ++++++++++--------- >> >> 2 files changed, 13 insertions(+), 9 deletions(-) >> >> >> >> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >> >> index 589edee..d1c7d91 100644 >> >> --- a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >> >> +++ b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >> >> @@ -11,6 +11,8 @@ Required properties: >> >> c) pin function mode. >> >> d) optional flags and pull up/down. >> >> e) drive strength. >> >> +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 >> >> +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 >> > >> > a) This seems pretty generic, not at all samsung-specific, as the HDMI >> > version numbering space is well-defined by the HDMI spec. >> > >> > b) It would be better to make it an integer property whose value >> > encodes the version number, thus eliminating the need to add new >> > properties as new HDMI versions appear. >> > >> >> Thanks for the quick review, Mitch. >> >> How about: >> >> - hdmi-version: 0=v1.3, 1=v1.4 > Why bother obfuscating the real version number? Why not just 13=v1.3, > 14=v1.4? > Sure, that sounds good to me. Sean > Regards, > Lucas > >
On 1/8/2013 6:48 AM, Sean Paul wrote: > On Tue, Jan 8, 2013 at 11:45 AM, Lucas Stach <dev@lynxeye.de> wrote: >> Am Montag, den 07.01.2013, 16:12 -0500 schrieb Sean Paul: >>> On Mon, Jan 7, 2013 at 3:54 PM, Mitch Bradley <wmb@firmworks.com> wrote: >>>> On 1/7/2013 10:43 AM, Sean Paul wrote: >>>>> Add a property to the hdmi node so we can specify the HDMI version in >>>>> the device tree instead of just defaulting to v1.4 with the existence of >>>>> the dt node. >>>>> >>>>> Signed-off-by: Sean Paul <seanpaul@chromium.org> >>>>> --- >>>>> .../devicetree/bindings/drm/exynos/hdmi.txt | 3 +++ >>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 19 ++++++++++--------- >>>>> 2 files changed, 13 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >>>>> index 589edee..d1c7d91 100644 >>>>> --- a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >>>>> +++ b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt >>>>> @@ -11,6 +11,8 @@ Required properties: >>>>> c) pin function mode. >>>>> d) optional flags and pull up/down. >>>>> e) drive strength. >>>>> +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 >>>>> +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 >>>> >>>> a) This seems pretty generic, not at all samsung-specific, as the HDMI >>>> version numbering space is well-defined by the HDMI spec. >>>> >>>> b) It would be better to make it an integer property whose value >>>> encodes the version number, thus eliminating the need to add new >>>> properties as new HDMI versions appear. >>>> >>> >>> Thanks for the quick review, Mitch. >>> >>> How about: >>> >>> - hdmi-version: 0=v1.3, 1=v1.4 >> Why bother obfuscating the real version number? Why not just 13=v1.3, >> 14=v1.4? It's hard to predict how the HDMI committee will assign version numbers in the future, but... In the EDID structure that HDMI devices use for identification, the major.minor version number is encoded as two bytes, one for major and one for minor, thus it is conceivable that the minor could exceed 9. (reference CEA-861-D section A.2.5 or http://en.wikipedia.org/wiki/Extended_display_identification_data#EDID_1.3_data_format The EDID version number is not the same as the HDMI version number, but I expect that the committees share people and thus ways of doing things. I think it would be safer to encode the version number as (major<<8) | minor. >> > > Sure, that sounds good to me. > > Sean > >> Regards, >> Lucas >> >> >
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt index 589edee..d1c7d91 100644 --- a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt @@ -11,6 +11,8 @@ Required properties: c) pin function mode. d) optional flags and pull up/down. e) drive strength. +- samsung,supports-hdmi-1.4: Define if device supports HDMI v1.4 +- samsung,supports-hdmi-1.3: Define if device supports HDMI v1.3 Example: @@ -19,4 +21,5 @@ Example: reg = <0x14530000 0x100000>; interrupts = <0 95 0>; hpd-gpio = <&gpx3 7 0xf 1 3>; + samsung,supports-hdmi-1.4; }; diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 2c46b6c..9834ae5 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2444,7 +2444,6 @@ static struct platform_device_id hdmi_driver_types[] = { static struct of_device_id hdmi_match_types[] = { { .compatible = "samsung,exynos5-hdmi", - .data = (void *)HDMI_TYPE14, }, { /* end node */ } @@ -2498,16 +2497,18 @@ static int __devinit hdmi_probe(struct platform_device *pdev) platform_set_drvdata(pdev, drm_hdmi_ctx); - if (dev->of_node) { - const struct of_device_id *match; - match = of_match_node(of_match_ptr(hdmi_match_types), - pdev->dev.of_node); - if (match == NULL) - return -ENODEV; - hdata->type = (enum hdmi_type)match->data; - } else { + if (!dev->of_node) { hdata->type = (enum hdmi_type)platform_get_device_id (pdev)->driver_data; + } else if (of_get_property(dev->of_node, "samsung,supports-hdmi-1.4", + NULL)) { + hdata->type = HDMI_TYPE14; + } else if (of_get_property(dev->of_node, "samsung,supports-hdmi-1.3", + NULL)) { + hdata->type = HDMI_TYPE13; + } else { + DRM_ERROR("Could not resolve HDMI version support\n"); + return -ENODEV; } hdata->hpd_gpio = pdata->hpd_gpio;
Add a property to the hdmi node so we can specify the HDMI version in the device tree instead of just defaulting to v1.4 with the existence of the dt node. Signed-off-by: Sean Paul <seanpaul@chromium.org> --- .../devicetree/bindings/drm/exynos/hdmi.txt | 3 +++ drivers/gpu/drm/exynos/exynos_hdmi.c | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 9 deletions(-)