[PATCH] drm/exynos: Get HDMI version from device tree

[PATCH] drm/exynos: Get HDMI version from device tree

[Sean Paul]

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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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
 
 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;
-- 
1.7.7.3

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Mitch Bradley]

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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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;
> 

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Sean Paul]

On Mon, Jan 7, 2013 at 3:54 PM, Mitch Bradley <wmb-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> 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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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;
>>

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Stephen Warren]

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?)

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Sean Paul]

[-- Attachment #1.1: Type: text/plain, Size: 1028 bytes --]

On Jan 7, 2013 5:32 PM, "Stephen Warren" <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.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?)

[-- Attachment #1.2: Type: text/html, Size: 1370 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Stephen Warren]

On 01/07/2013 04:12 PM, Sean Paul wrote:
> 
> On Jan 7, 2013 5:32 PM, "Stephen Warren" <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org
> <mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.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.

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Sean Paul]

On Tue, Jan 8, 2013 at 11:38 AM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 01/07/2013 04:12 PM, Sean Paul wrote:
>>
>> On Jan 7, 2013 5:32 PM, "Stephen Warren" <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org
>> <mailto:swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.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.

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Lucas Stach]

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

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Sean Paul]

On Tue, Jan 8, 2013 at 11:45 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> 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-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> 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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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
>
>

Re: [PATCH] drm/exynos: Get HDMI version from device tree

[Mitch Bradley]

On 1/8/2013 6:48 AM, Sean Paul wrote:
> On Tue, Jan 8, 2013 at 11:45 AM, Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org> 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-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> 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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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
>>
>>
> 

[PATCH v2] drm/exynos: Get HDMI version from device tree

[Sean Paul]

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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 .../devicetree/bindings/drm/exynos/hdmi.txt        |    2 +
 drivers/gpu/drm/exynos/exynos_hdmi.c               |   22 ++++++++++----------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
index 589edee..228ede6 100644
--- a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
+++ b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
@@ -11,6 +11,7 @@ Required properties:
 	c) pin function mode.
 	d) optional flags and pull up/down.
 	e) drive strength.
+- hdmi-version: (major << 8) | minor
 
 Example:
 
@@ -19,4 +20,5 @@ Example:
 		reg = <0x14530000 0x100000>;
 		interrupts = <0 95 0>;
 		hpd-gpio = <&gpx3 7 0xf 1 3>;
+		hdmi-version = <0x104>; /* version 1.4 */
 	};
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2c46b6c..71736f9 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -74,8 +74,8 @@ enum HDMI_PACKET_TYPE {
 };
 
 enum hdmi_type {
-	HDMI_TYPE13,
-	HDMI_TYPE14,
+	HDMI_TYPE13 = (1 << 8) | 3,
+	HDMI_TYPE14 = (1 << 8) | 4,
 };
 
 struct hdmi_resources {
@@ -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 */
 	}
@@ -2459,6 +2458,7 @@ static int __devinit hdmi_probe(struct platform_device *pdev)
 	struct s5p_hdmi_platform_data *pdata;
 	struct resource *res;
 	int ret;
+	u32 ver;
 
 	DRM_DEBUG_KMS("[%d]\n", __LINE__);
 
@@ -2498,16 +2498,16 @@ 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 {
+		ret = of_property_read_u32(dev->of_node, "hdmi-version", &ver);
+		if (ret) {
+			DRM_ERROR("Could not resolve HDMI version support\n");
+			return ret;
+		}
+		hdata->type = (enum hdmi_type)ver;
 	}
 
 	hdata->hpd_gpio = pdata->hpd_gpio;
-- 
1.7.7.3

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Stephen Warren]

On 01/08/2013 01:16 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.

I guess this seems OK to me if required, although I'd certainly like to
see someone familiar with the Exynos HW confirm whether this should be
driven purely by DT compatible value for the HDMI IP block instead though.

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Sean Paul]

On Tue, Jan 8, 2013 at 5:56 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 01/08/2013 01:16 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.
>
> I guess this seems OK to me if required, although I'd certainly like to
> see someone familiar with the Exynos HW confirm whether this should be
> driven purely by DT compatible value for the HDMI IP block instead though.

+inki

Inki, does this seem reasonable to you?


Sean

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Sylwester Nawrocki]

Hi,

On 01/08/2013 11:56 PM, Stephen Warren wrote:
> On 01/08/2013 01:16 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.
>
> I guess this seems OK to me if required, although I'd certainly like to
> see someone familiar with the Exynos HW confirm whether this should be
> driven purely by DT compatible value for the HDMI IP block instead though.

I think the supported HDMI standard is something that could well be derived
from the compatible property. The IP supporting v1.3 and v1.4 will be
significantly different, so this would anyway already need to be reflected
in the compatible property. The only issue I see here is that people tend
to make the compatible string overly generic, so it is hardly usable for
anything but matching an IP with its driver. For instance for exynos5 we
have now (Documentation/devicetree/bindings/drm/exynos/hdmi.txt):

	compatible = "samsung,exynos5-hdmi";

For Exynos4 series there were already some patches proposed [1], but I 
believe
this isn't a clean solution. Instead of things like:

compatible = "samsung,exynos4-hdmi13";
compatible = "samsung,exynos4-hdmi14";

I would much more like to see the SoC version embedded in the compatible
string, e.g.

compatible = "samsung,exynos4210-hdmi"; /* among others it carries an
					  information this IP supports
					  HDMI v1.3 */	

compatible = "samsung,exynos4212-hdmi"; /* HDMI v1.4, IIRC */

[1] http://www.spinics.net/lists/linux-samsung-soc/msg15289.html

--

Thanks,
Sylwester

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Inki Dae]

2013/1/30 Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>:
> On Tue, Jan 8, 2013 at 5:56 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>> On 01/08/2013 01:16 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.
>>
>> I guess this seems OK to me if required, although I'd certainly like to
>> see someone familiar with the Exynos HW confirm whether this should be
>> driven purely by DT compatible value for the HDMI IP block instead though.
>
> +inki
>
> Inki, does this seem reasonable to you?
>

Looks good to me. At least it's better than old one. I'll apply it if
there is no any opinion. And one more thing, Exynos hdmi controller
has no version info register so we need such thing. But in case of
Mixer controller, it has the version info register so we could support
all mixer controllers of Exynos SoC checking that register. So if you
are planning on such thing for Mixer also then I'd recommend you to
consider checking that register.

Thanks,
Inki Dae

>
> Sean
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Inki Dae]

2013/1/30 Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Hi,
>
>
> On 01/08/2013 11:56 PM, Stephen Warren wrote:
>>
>> On 01/08/2013 01:16 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.
>>
>>
>> I guess this seems OK to me if required, although I'd certainly like to
>> see someone familiar with the Exynos HW confirm whether this should be
>> driven purely by DT compatible value for the HDMI IP block instead though.
>
>
> I think the supported HDMI standard is something that could well be derived
> from the compatible property. The IP supporting v1.3 and v1.4 will be
> significantly different, so this would anyway already need to be reflected
> in the compatible property. The only issue I see here is that people tend
> to make the compatible string overly generic, so it is hardly usable for
> anything but matching an IP with its driver. For instance for exynos5 we
> have now (Documentation/devicetree/bindings/drm/exynos/hdmi.txt):
>
>         compatible = "samsung,exynos5-hdmi";
>
> For Exynos4 series there were already some patches proposed [1], but I
> believe
> this isn't a clean solution. Instead of things like:
>
> compatible = "samsung,exynos4-hdmi13";
> compatible = "samsung,exynos4-hdmi14";
>
> I would much more like to see the SoC version embedded in the compatible
> string, e.g.
>

Hi Sylwester. long time no see.

I think that if we use the SoC version embedded in the compatible
string then each driver shoud aware of the ip version to the SoC to
use version specific feature so I think it's better to use it without
the SoC version embedded in the compatible string like this,
compatible = "samsung,exynos4-hdmi"
version = "0x104" or "0x103"

With this, all each driver to do  is to check version property and set
version specific feature properly. And we have some dtsi file to can
be used commonly.

For example,
exynos4.dtsi : have all Exynos4 series SoCs common properties and also
use common compatible string.
exynos4412.dtsi, exynos4212.dtsi and so on: have Exynos42xx specific
properties. So the hdmi version string could be used here as "version
= "0x104" or "0x103"
exynos4412-board.dts: have board specific properties.

compatible = "samsung,exynos5-hdmi" is reasonable to me.
any opinions?

> compatible = "samsung,exynos4210-hdmi"; /* among others it carries an
>                                           information this IP supports
>                                           HDMI v1.3 */
>
> compatible = "samsung,exynos4212-hdmi"; /* HDMI v1.4, IIRC */
>
> [1] http://www.spinics.net/lists/linux-samsung-soc/msg15289.html
>
> --
>
> Thanks,
> Sylwester
>
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Stephen Warren]

On 01/30/2013 06:16 PM, Inki Dae wrote:
> 2013/1/30 Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> Hi,
>>
>>
>> On 01/08/2013 11:56 PM, Stephen Warren wrote:
>>>
>>> On 01/08/2013 01:16 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.
>>>
>>>
>>> I guess this seems OK to me if required, although I'd certainly like to
>>> see someone familiar with the Exynos HW confirm whether this should be
>>> driven purely by DT compatible value for the HDMI IP block instead though.
>>
>>
>> I think the supported HDMI standard is something that could well be derived
>> from the compatible property. The IP supporting v1.3 and v1.4 will be
>> significantly different, so this would anyway already need to be reflected
>> in the compatible property. The only issue I see here is that people tend
>> to make the compatible string overly generic, so it is hardly usable for
>> anything but matching an IP with its driver. For instance for exynos5 we
>> have now (Documentation/devicetree/bindings/drm/exynos/hdmi.txt):
>>
>>         compatible = "samsung,exynos5-hdmi";
>>
>> For Exynos4 series there were already some patches proposed [1], but I
>> believe
>> this isn't a clean solution. Instead of things like:
>>
>> compatible = "samsung,exynos4-hdmi13";
>> compatible = "samsung,exynos4-hdmi14";
>>
>> I would much more like to see the SoC version embedded in the compatible
>> string, e.g.
>>
> 
> Hi Sylwester. long time no see.
> 
> I think that if we use the SoC version embedded in the compatible
> string then each driver shoud aware of the ip version to the SoC to

The driver only needs to be aware of one SoC version for each IP version.

So with Sylwester's proposal:

>> compatible = "samsung,exynos4210-hdmi"; /* among others it carries an
>>                                           information this IP supports
>>                                           HDMI v1.3 */
>>
>> compatible = "samsung,exynos4212-hdmi"; /* HDMI v1.4, IIRC */

The driver woulud only ever have to know about those two compatible
values (unless further incompatible HW revisions exist); any other SoC
would be listed as being compatible with one of those two strings (but
in addition to the specific value for the specific SoC, e.g. compatible
= "samsung,exynox5xxx-hdmi", "samsung,exynos4212-hdmi").

> use version specific feature so I think it's better to use it without
> the SoC version embedded in the compatible string like this,
> compatible = "samsung,exynos4-hdmi"
> version = "0x104" or "0x103"

That would be quite non-typical.

> With this, all each driver to do  is to check version property and set
> version specific feature properly. And we have some dtsi file to can
> be used commonly.
> 
> For example,
> exynos4.dtsi : have all Exynos4 series SoCs common properties and also
> use common compatible string.
> exynos4412.dtsi, exynos4212.dtsi and so on: have Exynos42xx specific
> properties. So the hdmi version string could be used here as "version
> = "0x104" or "0x103"
> exynos4412-board.dts: have board specific properties.
> 
> compatible = "samsung,exynos5-hdmi" is reasonable to me.
> any opinions?

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Inki Dae]

2013/1/31 Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>:
> On 01/30/2013 06:16 PM, Inki Dae wrote:
>> 2013/1/30 Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>> Hi,
>>>
>>>
>>> On 01/08/2013 11:56 PM, Stephen Warren wrote:
>>>>
>>>> On 01/08/2013 01:16 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.
>>>>
>>>>
>>>> I guess this seems OK to me if required, although I'd certainly like to
>>>> see someone familiar with the Exynos HW confirm whether this should be
>>>> driven purely by DT compatible value for the HDMI IP block instead though.
>>>
>>>
>>> I think the supported HDMI standard is something that could well be derived
>>> from the compatible property. The IP supporting v1.3 and v1.4 will be
>>> significantly different, so this would anyway already need to be reflected
>>> in the compatible property. The only issue I see here is that people tend
>>> to make the compatible string overly generic, so it is hardly usable for
>>> anything but matching an IP with its driver. For instance for exynos5 we
>>> have now (Documentation/devicetree/bindings/drm/exynos/hdmi.txt):
>>>
>>>         compatible = "samsung,exynos5-hdmi";
>>>
>>> For Exynos4 series there were already some patches proposed [1], but I
>>> believe
>>> this isn't a clean solution. Instead of things like:
>>>
>>> compatible = "samsung,exynos4-hdmi13";
>>> compatible = "samsung,exynos4-hdmi14";
>>>
>>> I would much more like to see the SoC version embedded in the compatible
>>> string, e.g.
>>>
>>
>> Hi Sylwester. long time no see.
>>
>> I think that if we use the SoC version embedded in the compatible
>> string then each driver shoud aware of the ip version to the SoC to
>
> The driver only needs to be aware of one SoC version for each IP version.
>

I know that device tree describes hw information and the information
includes ip version also. So shouldn't the driver aware of the ip
version itself and should the driver only aware of it through device
tree? So I thought using the version property is proper way.


> So with Sylwester's proposal:
>
>>> compatible = "samsung,exynos4210-hdmi"; /* among others it carries an
>>>                                           information this IP supports
>>>                                           HDMI v1.3 */
>>>
>>> compatible = "samsung,exynos4212-hdmi"; /* HDMI v1.4, IIRC */
>
> The driver woulud only ever have to know about those two compatible
> values (unless further incompatible HW revisions exist); any other SoC
> would be listed as being compatible with one of those two strings (but
> in addition to the specific value for the specific SoC, e.g. compatible
> = "samsung,exynox5xxx-hdmi", "samsung,exynos4212-hdmi").
>
>> use version specific feature so I think it's better to use it without
>> the SoC version embedded in the compatible string like this,
>> compatible = "samsung,exynos4-hdmi"
>> version = "0x104" or "0x103"
>
> That would be quite non-typical.
>
>> With this, all each driver to do  is to check version property and set
>> version specific feature properly. And we have some dtsi file to can
>> be used commonly.
>>
>> For example,
>> exynos4.dtsi : have all Exynos4 series SoCs common properties and also
>> use common compatible string.
>> exynos4412.dtsi, exynos4212.dtsi and so on: have Exynos42xx specific
>> properties. So the hdmi version string could be used here as "version
>> = "0x104" or "0x103"
>> exynos4412-board.dts: have board specific properties.
>>
>> compatible = "samsung,exynos5-hdmi" is reasonable to me.
>> any opinions?
>
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Rahul Sharma]

On Thu, Jan 31, 2013 at 10:22 AM, Inki Dae <inki.dae@samsung.com> wrote:
> 2013/1/31 Stephen Warren <swarren@wwwdotorg.org>:
>> On 01/30/2013 06:16 PM, Inki Dae wrote:
>>> 2013/1/30 Sylwester Nawrocki <sylvester.nawrocki@gmail.com>:
>>>> Hi,
>>>>
>>>>
>>>> On 01/08/2013 11:56 PM, Stephen Warren wrote:
>>>>>
>>>>> On 01/08/2013 01:16 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.
>>>>>
>>>>>
>>>>> I guess this seems OK to me if required, although I'd certainly like to
>>>>> see someone familiar with the Exynos HW confirm whether this should be
>>>>> driven purely by DT compatible value for the HDMI IP block instead though.
>>>>
>>>>
>>>> I think the supported HDMI standard is something that could well be derived
>>>> from the compatible property. The IP supporting v1.3 and v1.4 will be
>>>> significantly different, so this would anyway already need to be reflected
>>>> in the compatible property. The only issue I see here is that people tend
>>>> to make the compatible string overly generic, so it is hardly usable for
>>>> anything but matching an IP with its driver. For instance for exynos5 we
>>>> have now (Documentation/devicetree/bindings/drm/exynos/hdmi.txt):
>>>>
>>>>         compatible = "samsung,exynos5-hdmi";
>>>>
>>>> For Exynos4 series there were already some patches proposed [1], but I
>>>> believe
>>>> this isn't a clean solution. Instead of things like:
>>>>
>>>> compatible = "samsung,exynos4-hdmi13";
>>>> compatible = "samsung,exynos4-hdmi14";
>>>>
>>>> I would much more like to see the SoC version embedded in the compatible
>>>> string, e.g.
>>>>
>>>
>>> Hi Sylwester. long time no see.
>>>
>>> I think that if we use the SoC version embedded in the compatible
>>> string then each driver shoud aware of the ip version to the SoC to
>>
>> The driver only needs to be aware of one SoC version for each IP version.
>>
>
> I know that device tree describes hw information and the information
> includes ip version also. So shouldn't the driver aware of the ip
> version itself and should the driver only aware of it through device
> tree? So I thought using the version property is proper way.
>

I want to second Sylwester's proposal. Compatibility information
should be carried in compatible string. 'version' property is diluting
its purpose. With this implementation, all compatible strings :
"exynos-hdmi", "exynos4-hdmi", "exynos4x12-hdmi",
"exynos5-hdmi" ... have same relevance.

IMO, it make sense with compatible strings specifying the
first Soc with same IP version:
compatible = "samsung,exynos4210-hdmi";
compatible = "samsung,exynos4212-hdmi";

compatible = "samsung,exynos-hdmi-1.3" and
compatible = "samsung,exynos-hdmi-1.4" will also do the needful.

While representing hdmi version information we should consider
the possibility of intermediate releases like 1.3ab, 1.4a etc.

regards,
Rahul Sharma.

>
>> So with Sylwester's proposal:
>>
>>>> compatible = "samsung,exynos4210-hdmi"; /* among others it carries an
>>>>                                           information this IP supports
>>>>                                           HDMI v1.3 */
>>>>
>>>> compatible = "samsung,exynos4212-hdmi"; /* HDMI v1.4, IIRC */
>>
>> The driver woulud only ever have to know about those two compatible
>> values (unless further incompatible HW revisions exist); any other SoC
>> would be listed as being compatible with one of those two strings (but
>> in addition to the specific value for the specific SoC, e.g. compatible
>> = "samsung,exynox5xxx-hdmi", "samsung,exynos4212-hdmi").
>>
>>> use version specific feature so I think it's better to use it without
>>> the SoC version embedded in the compatible string like this,
>>> compatible = "samsung,exynos4-hdmi"
>>> version = "0x104" or "0x103"
>>
>> That would be quite non-typical.
>>
>>> With this, all each driver to do  is to check version property and set
>>> version specific feature properly. And we have some dtsi file to can
>>> be used commonly.
>>>
>>> For example,
>>> exynos4.dtsi : have all Exynos4 series SoCs common properties and also
>>> use common compatible string.
>>> exynos4412.dtsi, exynos4212.dtsi and so on: have Exynos42xx specific
>>> properties. So the hdmi version string could be used here as "version
>>> = "0x104" or "0x103"
>>> exynos4412-board.dts: have board specific properties.
>>>
>>> compatible = "samsung,exynos5-hdmi" is reasonable to me.
>>> any opinions?
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Sean Paul]

On Wed, Jan 30, 2013 at 10:03 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 01/30/2013 06:16 PM, Inki Dae wrote:
>> 2013/1/30 Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>> Hi,
>>>
>>>
>>> On 01/08/2013 11:56 PM, Stephen Warren wrote:
>>>>
>>>> On 01/08/2013 01:16 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.
>>>>
>>>>
>>>> I guess this seems OK to me if required, although I'd certainly like to
>>>> see someone familiar with the Exynos HW confirm whether this should be
>>>> driven purely by DT compatible value for the HDMI IP block instead though.
>>>
>>>
>>> I think the supported HDMI standard is something that could well be derived
>>> from the compatible property. The IP supporting v1.3 and v1.4 will be
>>> significantly different, so this would anyway already need to be reflected
>>> in the compatible property. The only issue I see here is that people tend
>>> to make the compatible string overly generic, so it is hardly usable for
>>> anything but matching an IP with its driver. For instance for exynos5 we
>>> have now (Documentation/devicetree/bindings/drm/exynos/hdmi.txt):
>>>
>>>         compatible = "samsung,exynos5-hdmi";
>>>
>>> For Exynos4 series there were already some patches proposed [1], but I
>>> believe
>>> this isn't a clean solution. Instead of things like:
>>>
>>> compatible = "samsung,exynos4-hdmi13";
>>> compatible = "samsung,exynos4-hdmi14";
>>>
>>> I would much more like to see the SoC version embedded in the compatible
>>> string, e.g.
>>>
>>
>> Hi Sylwester. long time no see.
>>
>> I think that if we use the SoC version embedded in the compatible
>> string then each driver shoud aware of the ip version to the SoC to
>
> The driver only needs to be aware of one SoC version for each IP version.
>
> So with Sylwester's proposal:
>
>>> compatible = "samsung,exynos4210-hdmi"; /* among others it carries an
>>>                                           information this IP supports
>>>                                           HDMI v1.3 */
>>>
>>> compatible = "samsung,exynos4212-hdmi"; /* HDMI v1.4, IIRC */
>
> The driver woulud only ever have to know about those two compatible
> values (unless further incompatible HW revisions exist); any other SoC
> would be listed as being compatible with one of those two strings (but
> in addition to the specific value for the specific SoC, e.g. compatible
> = "samsung,exynox5xxx-hdmi", "samsung,exynos4212-hdmi").
>

I think if we take a step back, we're really not discussing HDMI
version 1.3 vs. 1.4, we're really talking about the HDMI IP block
version. The blocks just happen to implement different versions of the
HDMI spec. The initial naming in the driver is unfortunate.

That said, I think the above solution is fine, but it's a little
misleading.  I'd much rather encode the version of the IP block
instead of the SoC that contains it. Something like:

compatible = "samsung,exynos-hdmiXXX"

In this case, XXX is just some integer in the bindings that maps to an
SoC. For example,

+----------------------+-------------+
| HDMI IP version      | Exynos SoC  |
+----------------------+-------------+
| samsung,exynos-hdmi1 | 4210        |
| samsung,exynos-hdmi2 | 4212, 5250  |
+----------------------+-------------+

The reason I like this better is that it's clear which value to use
when gating features in the driver. Using the scheme above, it might
be tempting to gate a feature/fix on exynos5xxx-hdmi when it really
works with both 4212 && 5xxx.

Sean



>> use version specific feature so I think it's better to use it without
>> the SoC version embedded in the compatible string like this,
>> compatible = "samsung,exynos4-hdmi"
>> version = "0x104" or "0x103"
>
> That would be quite non-typical.
>
>> With this, all each driver to do  is to check version property and set
>> version specific feature properly. And we have some dtsi file to can
>> be used commonly.
>>
>> For example,
>> exynos4.dtsi : have all Exynos4 series SoCs common properties and also
>> use common compatible string.
>> exynos4412.dtsi, exynos4212.dtsi and so on: have Exynos42xx specific
>> properties. So the hdmi version string could be used here as "version
>> = "0x104" or "0x103"
>> exynos4412-board.dts: have board specific properties.
>>
>> compatible = "samsung,exynos5-hdmi" is reasonable to me.
>> any opinions?
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Stephen Warren]

On 01/31/2013 08:04 AM, Sean Paul wrote:
...
> I think if we take a step back, we're really not discussing HDMI
> version 1.3 vs. 1.4, we're really talking about the HDMI IP block
> version.

Absolutely.

> The blocks just happen to implement different versions of the
> HDMI spec.

Yes.

> The initial naming in the driver is unfortunate.
> 
> That said, I think the above solution is fine, but it's a little
> misleading.  I'd much rather encode the version of the IP block
> instead of the SoC that contains it. Something like:
> 
> compatible = "samsung,exynos-hdmiXXX"
> 
> In this case, XXX is just some integer in the bindings that maps to an
> SoC. For example,
> 
> +----------------------+-------------+
> | HDMI IP version      | Exynos SoC  |
> +----------------------+-------------+
> | samsung,exynos-hdmi1 | 4210        |
> | samsung,exynos-hdmi2 | 4212, 5250  |
> +----------------------+-------------+

That seems reasonable to me. (But, does the documentation for these IP
blocks specify the version in the format "1" and "2"? It'd be best if
the compatible value encoded the same version scheme as the IP block
documentation).

>From all the bindings I've seen though, the style in the table above
would be unusual though; most compatible values simply specify the first
(or first SW-supported) SoC version that incorporated that IP version.
Still, I imagine that's simply historical precedent rather than some
hard-and-fast rule. So, I'd say go ahead with the table above, but you
may want to ping some DT maintainers or old-hands to make sure there
isn't something I forgot about.

> The reason I like this better is that it's clear which value to use
> when gating features in the driver. Using the scheme above, it might
> be tempting to gate a feature/fix on exynos5xxx-hdmi when it really
> works with both 4212 && 5xxx.

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Olof Johansson]

On Thu, Jan 31, 2013 at 7:04 AM, Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Wed, Jan 30, 2013 at 10:03 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>> On 01/30/2013 06:16 PM, Inki Dae wrote:
>>> 2013/1/30 Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>>>> Hi,
>>>>
>>>>
>>>> On 01/08/2013 11:56 PM, Stephen Warren wrote:
>>>>>
>>>>> On 01/08/2013 01:16 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.
>>>>>
>>>>>
>>>>> I guess this seems OK to me if required, although I'd certainly like to
>>>>> see someone familiar with the Exynos HW confirm whether this should be
>>>>> driven purely by DT compatible value for the HDMI IP block instead though.
>>>>
>>>>
>>>> I think the supported HDMI standard is something that could well be derived
>>>> from the compatible property. The IP supporting v1.3 and v1.4 will be
>>>> significantly different, so this would anyway already need to be reflected
>>>> in the compatible property. The only issue I see here is that people tend
>>>> to make the compatible string overly generic, so it is hardly usable for
>>>> anything but matching an IP with its driver. For instance for exynos5 we
>>>> have now (Documentation/devicetree/bindings/drm/exynos/hdmi.txt):
>>>>
>>>>         compatible = "samsung,exynos5-hdmi";
>>>>
>>>> For Exynos4 series there were already some patches proposed [1], but I
>>>> believe
>>>> this isn't a clean solution. Instead of things like:
>>>>
>>>> compatible = "samsung,exynos4-hdmi13";
>>>> compatible = "samsung,exynos4-hdmi14";
>>>>
>>>> I would much more like to see the SoC version embedded in the compatible
>>>> string, e.g.
>>>>
>>>
>>> Hi Sylwester. long time no see.
>>>
>>> I think that if we use the SoC version embedded in the compatible
>>> string then each driver shoud aware of the ip version to the SoC to
>>
>> The driver only needs to be aware of one SoC version for each IP version.
>>
>> So with Sylwester's proposal:
>>
>>>> compatible = "samsung,exynos4210-hdmi"; /* among others it carries an
>>>>                                           information this IP supports
>>>>                                           HDMI v1.3 */
>>>>
>>>> compatible = "samsung,exynos4212-hdmi"; /* HDMI v1.4, IIRC */
>>
>> The driver woulud only ever have to know about those two compatible
>> values (unless further incompatible HW revisions exist); any other SoC
>> would be listed as being compatible with one of those two strings (but
>> in addition to the specific value for the specific SoC, e.g. compatible
>> = "samsung,exynox5xxx-hdmi", "samsung,exynos4212-hdmi").
>>
>
> I think if we take a step back, we're really not discussing HDMI
> version 1.3 vs. 1.4, we're really talking about the HDMI IP block
> version. The blocks just happen to implement different versions of the
> HDMI spec. The initial naming in the driver is unfortunate.
>
> That said, I think the above solution is fine, but it's a little
> misleading.  I'd much rather encode the version of the IP block
> instead of the SoC that contains it. Something like:
>
> compatible = "samsung,exynos-hdmiXXX"
>
> In this case, XXX is just some integer in the bindings that maps to an
> SoC. For example,
>
> +----------------------+-------------+
> | HDMI IP version      | Exynos SoC  |
> +----------------------+-------------+
> | samsung,exynos-hdmi1 | 4210        |
> | samsung,exynos-hdmi2 | 4212, 5250  |
> +----------------------+-------------+
>
> The reason I like this better is that it's clear which value to use
> when gating features in the driver. Using the scheme above, it might
> be tempting to gate a feature/fix on exynos5xxx-hdmi when it really
> works with both 4212 && 5xxx.

If you instead use the first SoC when the corresponding IP showed up,
you could, in the 5250 dts, have compatible =
"samsung,exynos5250-hdmi", "samsung,exynos4212-hdmi",
"samsung,exynos4-hdmi".

The HDMI driver would only know about the 4212 vs 4210 case, but the
device tree specifies the more specific value if needed in the future
(don't fall in the trap thinking you _need_ to have all of them in the
driver, as long as it only cares about 4210 vs 4212).


-Olof

Re: [PATCH v2] drm/exynos: Get HDMI version from device tree

[Stephen Warren]

On 01/31/2013 10:36 AM, Stephen Warren wrote:
> On 01/31/2013 08:04 AM, Sean Paul wrote:
> ...
>> I think if we take a step back, we're really not discussing HDMI
>> version 1.3 vs. 1.4, we're really talking about the HDMI IP block
>> version.
> 
> Absolutely.
> 
>> The blocks just happen to implement different versions of the
>> HDMI spec.
> 
> Yes.
> 
>> The initial naming in the driver is unfortunate.
>>
>> That said, I think the above solution is fine, but it's a little
>> misleading.  I'd much rather encode the version of the IP block
>> instead of the SoC that contains it. Something like:
>>
>> compatible = "samsung,exynos-hdmiXXX"
>>
>> In this case, XXX is just some integer in the bindings that maps to an
>> SoC. For example,
>>
>> +----------------------+-------------+
>> | HDMI IP version      | Exynos SoC  |
>> +----------------------+-------------+
>> | samsung,exynos-hdmi1 | 4210        |
>> | samsung,exynos-hdmi2 | 4212, 5250  |
>> +----------------------+-------------+
> 
> That seems reasonable to me. (But, does the documentation for these IP
> blocks specify the version in the format "1" and "2"? It'd be best if
> the compatible value encoded the same version scheme as the IP block
> documentation).

The thing I forgot here is:

Even if the IP block is identical between two different SoCs, it's quite
possible it was tweaked just a tiny bit between the two SoCs, or
something about the environment into which the IP block was placed
differs. Either of those conditions could mean that the same IP block
instantiated into two different SoCs could end up requiring different
workarounds/bugfixes/... that the driver needs to know about. As such,
encoding the exact SoC into the compatible value, and then deriving the
IP block version from the SoC-specific compatible value, makes the most
sense.

This is really just what Olof was saying rephrased. I'm just following
up to change my mind on my assertion that the table above appears
reasonable.

And of course as Olof cares, the compatible value can contain multiple
values; the most specific first, and then progressively more generic
values, and if the driver doesn't care (yet?) about the differences
between some specific values, it can always /just/ match on the more
generic values until some specific WAR/bugfix is needed.