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!). > 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! > >>>>> 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
