| Message ID | 1357676181-16728-1-git-send-email-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
On Tue, Jan 8, 2013 at 5:56 PM, Stephen Warren <swarren@wwwdotorg.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
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
2013/1/30 Sean Paul <seanpaul@chromium.org>: > On Tue, Jan 8, 2013 at 5:56 PM, Stephen Warren <swarren@wwwdotorg.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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
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 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
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. 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?
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. > 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
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
On Wed, Jan 30, 2013 at 10:03 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > 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. > > 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@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss
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.
On Thu, Jan 31, 2013 at 7:04 AM, Sean Paul <seanpaul@chromium.org> wrote: > On Wed, Jan 30, 2013 at 10:03 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> 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. >> >> 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
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.
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;
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 | 2 + drivers/gpu/drm/exynos/exynos_hdmi.c | 22 ++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-)