| Message ID | 1614597746-4563-1-git-send-email-pthombar@cadence.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | enable HDCP in Cadence MHDP bridge driver | expand |
Hi Parshuram, Thank you for the patch. On Mon, Mar 01, 2021 at 12:22:26PM +0100, Parshuram Thombare wrote: > Add binding changes for HDCP in the MHDP8546 DPI/DP bridge binding. > This binding is not used in any upstreamed DTS yet, so changing > index of property 'j721e-intg' should not affect anything. > > Signed-off-by: Parshuram Thombare <pthombar@cadence.com> > --- > .../display/bridge/cdns,mhdp8546.yaml | 29 ++++++++++++------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml > index 63427878715e..5fdadadaac16 100644 > --- a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml > @@ -17,21 +17,24 @@ properties: > - ti,j721e-mhdp8546 > > reg: > - minItems: 1 > - maxItems: 2 > + minItems: 2 > + maxItems: 3 > items: > - description: > Register block of mhdptx apb registers up to PHY mapped area (AUX_CONFIG_P). > The AUX and PMA registers are not part of this range, they are instead > included in the associated PHY. > + - description: > + Register block of mhdptx sapb registers. > - description: > Register block for DSS_EDP0_INTG_CFG_VP registers in case of TI J7 SoCs. > > reg-names: > - minItems: 1 > - maxItems: 2 > + minItems: 2 > + maxItems: 3 > items: > - const: mhdptx > + - const: mhdptx-sapb > - const: j721e-intg > > clocks: > @@ -53,6 +56,11 @@ properties: > power-domains: > maxItems: 1 > > + hdcp-config: > + maxItems: 1 > + description: > + HDCP version supported. Bit [0]:HDCP2.2 [1]:HDCP1.4. > + Is this a property of the hardware, that is, are there multiple versions of this IP core covered by the same compatible string that support HDCP 1.4 only, DHCP 2.2 only or both ? Or is it a way to select what a given system will offer ? > interrupts: > maxItems: 1 > > @@ -98,15 +106,15 @@ allOf: > then: > properties: > reg: > - minItems: 2 > + minItems: 3 > reg-names: > - minItems: 2 > + minItems: 3 > else: > properties: > reg: > - maxItems: 1 > + maxItems: 2 > reg-names: > - maxItems: 1 > + maxItems: 2 > > required: > - compatible > @@ -129,8 +137,9 @@ examples: > > mhdp: dp-bridge@f0fb000000 { > compatible = "cdns,mhdp8546"; > - reg = <0xf0 0xfb000000 0x0 0x1000000>; > - reg-names = "mhdptx"; > + reg = <0xf0 0xfb000000 0x0 0x1000000>, > + <0x0 0x4f48000 0x0 0x74>; > + reg-names = "mhdptx", "mhdptx-sapb"; > clocks = <&mhdp_clock>; > phys = <&dp_phy>; > phy-names = "dpphy";
Hi Laurent, >Is this a property of the hardware, that is, are there multiple versions >of this IP core covered by the same compatible string that support HDCP >1.4 only, DHCP 2.2 only or both ? Or is it a way to select what a given >system will offer ?[] MHDP hardware supports both HDCP 2.2 and 1.4. So, this is a way to select the version of HDCP, system wish to support. Regards, Parshuram Thombare
Hi Parshuram, On Mon, Mar 01, 2021 at 04:14:54PM +0000, Parshuram Raju Thombare wrote: > Hi Laurent, > > > Is this a property of the hardware, that is, are there multiple versions > > of this IP core covered by the same compatible string that support HDCP > > 1.4 only, DHCP 2.2 only or both ? Or is it a way to select what a given > > system will offer ?[] > > MHDP hardware supports both HDCP 2.2 and 1.4. So, this is a way > to select the version of HDCP, system wish to support. Then I'm not sure this qualifies as a DT property, which should describe the system, not configure it. A way for userspace to configure this would be better.
Hi Laurent, >> > Is this a property of the hardware, that is, are there multiple versions >> > of this IP core covered by the same compatible string that support HDCP >> > 1.4 only, DHCP 2.2 only or both ? Or is it a way to select what a given >> > system will offer ?[] >> >> MHDP hardware supports both HDCP 2.2 and 1.4. So, this is a way >> to select the version of HDCP, system wish to support. > >Then I'm not sure this qualifies as a DT property, which should describe >the system, not configure it. A way for userspace to configure this >would be better. Since this is for source device, I am not sure how useful it is to allow user to change HDCP version supported. I think doing it in DTS gives more control over HDCP to system designer/integrator. Regards, Parshuram Thombare
On Mon, Mar 01, 2021 at 12:22:26PM +0100, Parshuram Thombare wrote: > Add binding changes for HDCP in the MHDP8546 DPI/DP bridge binding. > This binding is not used in any upstreamed DTS yet, so changing > index of property 'j721e-intg' should not affect anything. TI folks might disagree, but weren't Cc'ed. > > Signed-off-by: Parshuram Thombare <pthombar@cadence.com> > --- > .../display/bridge/cdns,mhdp8546.yaml | 29 ++++++++++++------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml > index 63427878715e..5fdadadaac16 100644 > --- a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml > @@ -17,21 +17,24 @@ properties: > - ti,j721e-mhdp8546 > > reg: > - minItems: 1 > - maxItems: 2 > + minItems: 2 > + maxItems: 3 > items: > - description: > Register block of mhdptx apb registers up to PHY mapped area (AUX_CONFIG_P). > The AUX and PMA registers are not part of this range, they are instead > included in the associated PHY. > + - description: > + Register block of mhdptx sapb registers. > - description: > Register block for DSS_EDP0_INTG_CFG_VP registers in case of TI J7 SoCs. > > reg-names: > - minItems: 1 > - maxItems: 2 > + minItems: 2 > + maxItems: 3 > items: > - const: mhdptx > + - const: mhdptx-sapb > - const: j721e-intg > > clocks: > @@ -53,6 +56,11 @@ properties: > power-domains: > maxItems: 1 > > + hdcp-config: > + maxItems: 1 > + description: > + HDCP version supported. Bit [0]:HDCP2.2 [1]:HDCP1.4. 2.2 is not backwards compatible with 1.4? What's the setting if not present? Maybe just a 'disable 2.2 boolean' if that's the non-common case. In any case, it needs a type and constraints on the values. > + > interrupts: > maxItems: 1 > > @@ -98,15 +106,15 @@ allOf: > then: > properties: > reg: > - minItems: 2 > + minItems: 3 > reg-names: > - minItems: 2 > + minItems: 3 > else: > properties: > reg: > - maxItems: 1 > + maxItems: 2 > reg-names: > - maxItems: 1 > + maxItems: 2 > > required: > - compatible > @@ -129,8 +137,9 @@ examples: > > mhdp: dp-bridge@f0fb000000 { > compatible = "cdns,mhdp8546"; > - reg = <0xf0 0xfb000000 0x0 0x1000000>; > - reg-names = "mhdptx"; > + reg = <0xf0 0xfb000000 0x0 0x1000000>, > + <0x0 0x4f48000 0x0 0x74>; > + reg-names = "mhdptx", "mhdptx-sapb"; > clocks = <&mhdp_clock>; > phys = <&dp_phy>; > phy-names = "dpphy"; > -- > 2.25.1 >
Hi Parshuram, On Tue, Mar 02, 2021 at 12:53:50PM +0000, Parshuram Raju Thombare wrote: > Hi Laurent, > > >>> Is this a property of the hardware, that is, are there multiple versions > >>> of this IP core covered by the same compatible string that support HDCP > >>> 1.4 only, DHCP 2.2 only or both ? Or is it a way to select what a given > >>> system will offer ?[] > >> > >> MHDP hardware supports both HDCP 2.2 and 1.4. So, this is a way > >> to select the version of HDCP, system wish to support. > > > > Then I'm not sure this qualifies as a DT property, which should describe > > the system, not configure it. A way for userspace to configure this > > would be better. > > Since this is for source device, I am not sure how useful it is to allow > user to change HDCP version supported. I think doing it in DTS > gives more control over HDCP to system designer/integrator. But how would they do so ? What would be the rationale for selecting a particular version in DT ? I'm not thinking about giving control of this parameter to the end-user, but in the context of an embedded system, it may be useful to select which HDCP versions to offer based on different constraints at runtime. This really seems like a system configuration parameter to me, not a system description.
>> >>> Is this a property of the hardware, that is, are there multiple versions >> >>> of this IP core covered by the same compatible string that support HDCP >> >>> 1.4 only, DHCP 2.2 only or both ? Or is it a way to select what a given >> >>> system will offer ?[] >> >> >> >> MHDP hardware supports both HDCP 2.2 and 1.4. So, this is a way >> >> to select the version of HDCP, system wish to support. >> > >> > Then I'm not sure this qualifies as a DT property, which should describe >> > the system, not configure it. A way for userspace to configure this >> > would be better. >> >> Since this is for source device, I am not sure how useful it is to allow >> user to change HDCP version supported. I think doing it in DTS >> gives more control over HDCP to system designer/integrator. > >But how would they do so ? What would be the rationale for selecting a >particular version in DT ? > >I'm not thinking about giving control of this parameter to the end-user, >but in the context of an embedded system, it may be useful to select >which HDCP versions to offer based on different constraints at runtime. >This really seems like a system configuration parameter to me, not a >system description. Ok, we can remove this parameter from DTS and let driver attempt both HDCP 2.2 and HDCP 1.4 in the same sequence so that HDCP version is selected based on sink capabilities. But then user space application will not have information about the HDCP version with which content will be protected. I suppose most UHD 4k content would need HDCP 2.2. So, forcing HDCP version in DTS was the easiest way. Regards, Parshuram Thombare
Hi Rob, Thanks for your comments. >> Add binding changes for HDCP in the MHDP8546 DPI/DP bridge binding. >> This binding is not used in any upstreamed DTS yet, so changing >> index of property 'j721e-intg' should not affect anything. > >TI folks might disagree, but weren't Cc'ed. Kishon and Nikhil from TI are Cc'ed. Hi Kishon / Nikhil, Do you think this can be an issue ? >In any case, it needs a type and constraints on the values. Ok, I will add type and constraints on the values. Regards, Parshuram Thombare
diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml index 63427878715e..5fdadadaac16 100644 --- a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml @@ -17,21 +17,24 @@ properties: - ti,j721e-mhdp8546 reg: - minItems: 1 - maxItems: 2 + minItems: 2 + maxItems: 3 items: - description: Register block of mhdptx apb registers up to PHY mapped area (AUX_CONFIG_P). The AUX and PMA registers are not part of this range, they are instead included in the associated PHY. + - description: + Register block of mhdptx sapb registers. - description: Register block for DSS_EDP0_INTG_CFG_VP registers in case of TI J7 SoCs. reg-names: - minItems: 1 - maxItems: 2 + minItems: 2 + maxItems: 3 items: - const: mhdptx + - const: mhdptx-sapb - const: j721e-intg clocks: @@ -53,6 +56,11 @@ properties: power-domains: maxItems: 1 + hdcp-config: + maxItems: 1 + description: + HDCP version supported. Bit [0]:HDCP2.2 [1]:HDCP1.4. + interrupts: maxItems: 1 @@ -98,15 +106,15 @@ allOf: then: properties: reg: - minItems: 2 + minItems: 3 reg-names: - minItems: 2 + minItems: 3 else: properties: reg: - maxItems: 1 + maxItems: 2 reg-names: - maxItems: 1 + maxItems: 2 required: - compatible @@ -129,8 +137,9 @@ examples: mhdp: dp-bridge@f0fb000000 { compatible = "cdns,mhdp8546"; - reg = <0xf0 0xfb000000 0x0 0x1000000>; - reg-names = "mhdptx"; + reg = <0xf0 0xfb000000 0x0 0x1000000>, + <0x0 0x4f48000 0x0 0x74>; + reg-names = "mhdptx", "mhdptx-sapb"; clocks = <&mhdp_clock>; phys = <&dp_phy>; phy-names = "dpphy";
Add binding changes for HDCP in the MHDP8546 DPI/DP bridge binding. This binding is not used in any upstreamed DTS yet, so changing index of property 'j721e-intg' should not affect anything. Signed-off-by: Parshuram Thombare <pthombar@cadence.com> --- .../display/bridge/cdns,mhdp8546.yaml | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-)