Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/14/2023 2:41 PM, Marijn Suijten wrote:
On 2023-06-14 13:39:57, Abhinav Kumar wrote:
On 6/14/2023 12:54 PM, Abhinav Kumar wrote:
On 6/14/2023 12:35 PM, Abhinav Kumar wrote:
On 6/14/2023 5:23 AM, Marijn Suijten wrote:
On 2023-06-14 15:01:59, Dmitry Baryshkov wrote:
On 14/06/2023 14:42, Marijn Suijten wrote:
On 2023-06-13 18:57:11, Jessica Zhang wrote:
DPU 5.x+ supports a databus widen mode that allows more data to be
sent
per pclk. Enable this feature flag on all relevant chipsets.

Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
---
    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++-
    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
    2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 36ba3f58dcdf..0be7bf0bfc41 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -103,7 +103,8 @@
        (BIT(DPU_INTF_INPUT_CTRL) | \
         BIT(DPU_INTF_TE) | \
         BIT(DPU_INTF_STATUS_SUPPORTED) | \
-     BIT(DPU_DATA_HCTL_EN))
+     BIT(DPU_DATA_HCTL_EN) | \
+     BIT(DPU_INTF_DATABUS_WIDEN))

This doesn't work.  DPU 5.0.0 is SM8150, which has DSI 6G 2.3.  In the
last patch for DSI you state and enable widebus for DSI 6G 2.5+ only,
meaning DPU and DSI are now desynced, and the output is completely
corrupted.


I looked at the internal docs and also this change. This change is
incorrect because this will try to enable widebus for DPU >= 5.0 and DSI
  >= 2.5

That was not the intended right condition as thats not what the docs say.

We should enable for DPU >= 7.0 and DSI >= 2.5

That makes more sense, DSI 2.5 is SM8350 which has DPU 7.0.

Is there any combination where this compatibility is broken? That would
be the strange thing for me ( not DPU 5.0 and DSI 2.5 as that was incorrect)

No clue if there are any interim SoCs...

Part of this confusion is because of catalog macro re-use again.

Somewhat agreed.  SC7180 is a DPU 6.2 SoC, and for this mask to be used
across DPU 5.x and above it should have been renamed to SM8150 and as
suggested by Dmitry, have DPU_5_x_` as suffix.

As I've asked many times before, we should inline these masks (not just
the macros) (disclaimer: haven't reviewed if Dmitry's series actually
does so!).


Yes it does inline it. I am halfway through that rework got stuck in one of the patches.

This series is a good candidate and infact I think we should only do
core_revision based check on DPU and DSI to avoid bringing the catalog
mess into this.

Tested this on SM8350 which actually has DSI 2.5, and it is also
corrupted with this series so something else on this series might be
broken.


Missed this response. That seems strange.

No worries.  But don't forget to look at the comments on patch 2/3
either.  Some of it is a continuation of pclk scaling factor for DSC
which discussion seems to have ceased on.

This series was tested on SM8350 HDK with a command mode panel.

We will fix the DPU-DSI handshake and post a v2 but your issue needs
investigation in parallel.

So another bug to track that would be great.

Will see if I can set that up for you!


Now, it seems this is resolved so bug is not needed?

Is the bound in dsi_host wrong, or do DPU and DSI need to communicate
when widebus will be enabled, based on DPU && DSI supporting it?

I'd prefer to follow the second approach, as we did for DP. DPU asks
the
actual video output driver if widebus is to be enabled.


I was afraid of this. This series was made on an assumption that the
DPU version of widebus and DSI version of widebus would be compatible
but looks like already SM8150 is an outlier.

Fwiw SM8250 would have been an outlier as well :)

Yes, I think we have to go with second approach.

DPU queries DSI if it supports widebus and enables it.

Thanks for your responses. We will post a v2.

No hurry, btw.  As alluded to above, let's also look at the comments on
patch 2/3 and discuss how this affects pclk.

Doesn't it seem very strange that DPU 5.x+ comes with a widebus feature,
but the DSI does not until two revisions later?  Or is this available on
every interface, but only for a different (probably DP) encoder block?


Yes its strange.


I have clarified this above. Its not strange but appeared strange
because we were checking wrong conditions.

Hehe :)

- Marijn



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux