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

[Olof Johansson]

On Thu, Jan 31, 2013 at 7:04 AM, Sean Paul <seanpaul at chromium.org> wrote:
> On Wed, Jan 30, 2013 at 10:03 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 01/30/2013 06:16 PM, Inki Dae wrote:
>>> 2013/1/30 Sylwester Nawrocki <sylvester.nawrocki at 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.
>>
>> 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