On 22/09/25 13:11, Harikrishna Shenoy wrote:
On 9/18/25 07:00, Krzysztof Kozlowski wrote:
On Mon, Sep 15, 2025 at 04:00:40PM +0530, Harikrishna Shenoy wrote:
From: Swapnil Jakhade<[email protected]>
Add binding changes for DSC(Display Stream Compression) in the MHDP8546
DPI/DP bridge.
Signed-off-by: Swapnil Jakhade<[email protected]>
Signed-off-by: Harikrishna Shenoy<[email protected]>
---
.../display/bridge/cdns,mhdp8546.yaml | 24 ++++++++++++-------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
index c2b369456e4e..2a05a7d5847f 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
@@ -27,13 +27,12 @@ properties:
Register block for DSS_EDP0_INTG_CFG_VP registers in case of TI J7
SoCs.
- description:
Register block of mhdptx sapb registers.
+ - description:
+ Register block for mhdptx DSC encoder registers.
reg-names:
- minItems: 1
- items:
- - const: mhdptx
- - const: j721e-intg
- - const: mhdptx-sapb
+ description:
+ Names corresponding to entries in the reg property.
No, top-level should have broadest constraints. In your case it is
min/maxItems.
Description is completely redundant. Wasn't here before, so why adding
it?
Noted, will remove description and add minItems:1.
clocks:
maxItems: 1
@@ -100,18 +99,25 @@ allOf:
properties:
reg:
minItems: 2
- maxItems: 3
+ maxItems: 4
reg-names:
minItems: 2
- maxItems: 3
+ items:
+ - const: mhdptx
+ - const: j721e-intg
+ - const: mhdptx-sapb
+ - const: dsc
else:
properties:
reg:
minItems: 1
- maxItems: 2
+ maxItems: 3
reg-names:
minItems: 1
- maxItems: 2
+ items:
+ - const: mhdptx
+ - const: mhdptx-sapb
This is wrong. Previously CDNS variant had two items means it had
"j721e-intg". Now it's something else.
First, this is an ABI break.
Second, there is no explanation at all for it in the commit msg! Looks
like random change.
Read carefully writing-bindings doc.
Best regards,
Krzysztof
Hi Krzysztof,
Keeping this patch series aside, The existing binding-docs clearly have
a bug.
Since even for cadence specific compatible "cdns,mhdp8546" it
compulsorily expects "j721e-intg" register space
which is NOT part of the cadence IP block mhdp8546 and hence not
applicable to "cdns,mhdp8546".
This was also discussed here [1] and can also be referred in this TRM
section [2],
which clearly show that "j721e-intg" is part of TI wrapper IP block and
should be
applicable to "ti,j721e-mhdp8546" compatible.
Yes agreed it breaks the ABI but it also fixes a bug and I don't see any
one using only "cdns,mhdp8546" yet.
so I am thinking it's more appropriate to fix this as a separate patch
independent of this series.
Kindly let me know if I should submit a separate patch to fix this bug
or I should just ignore this bug.
Depending on your suggestion, if it's agreed upon to send the bug fix
patch first, I will send out an independent
bug fix to remove "j721e-intg" for compatible "cdns,mhdp8546" and then
rebase the series for adding DSC reg blocks
on top of bug fix.
[1]: https://lore.kernel.org/all/20250903220312.GA2903503-
[email protected]/ <https://lore.kernel.org/all/20250903220312.GA2903503-
[email protected]/>
[2]: Link to TRM ZIP:https://www.ti.com/lit/zip/spruil1 <https://
www.ti.com/lit/zip/spruil1>
Table 2-1. MAIN Domain Memory Map
DSS_EDP0_V2A_CORE_VP_REGS_APB are EDP core register identified by name
mhdptx in DT.
DSS_EDP0_INTG_CFG_VP identified by j721e-intg in DT.
Section 12.6.6.16.4: EDP_CFG Registers
Driver use: TI j721e Cadence MHDP8546 DP wrapper(drivers/gpu/drm/bridge/
cadence/cdns-mhdp8546-j721e.c)
Regards.
Hi Krzysztof,
Could you please let us know your thoughts on above, will re-spin it
accordingly.
Regards.
