Message-ID: <5d45e541-ceb7-4931-a46f-cd55da33a525@linaro.org>
Date:   Fri, 5 May 2023 01:19:15 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc:     Kuogee Hsieh <quic_khsieh@...cinc.com>,
        dri-devel@...ts.freedesktop.org, robdclark@...il.com,
        sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
        vkoul@...nel.org, daniel@...ll.ch, airlied@...il.com,
        agross@...nel.org, andersson@...nel.org, quic_sbillaka@...cinc.com,
        freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/7] drm/msm/dpu: add DPU_PINGPONG_DSC bits into PP_BLK
 and PP_BLK_TE marcos

On 05/05/2023 00:39, Marijn Suijten wrote:
> On 2023-05-04 12:50:57, Abhinav Kumar wrote:
>>
>>
>> On 5/4/2023 12:36 PM, Marijn Suijten wrote:
>>> On 2023-05-04 11:25:44, Abhinav Kumar wrote:
>>> <snip>
>>>>> Sure, if you really prefer a split I'd go for two patches:
>>>>> 1. Add the flag to the enum and catalog;
>>>>> 2. Add the ops guard (functional change).
>>>>>
>>>>> Then don't forget to reword the commit message, following the guidelines
>>>>> below and the suggestion for 2/7.
>>>>>
>>>>> - Marijn
>>>>
>>>> Plan sounds good to me.
>>>>
>>>> Marijn, we will wait for a couple of days to post the next rev but would
>>>> be hard more than that as we need to pick up other things which are
>>>> pending on top of this. Hence would appreciate if you can finish reviews
>>>> by then.
>>>
>>> It depends on how many more revisions are needed after that, and not all
>>> patches in this series have an r-b just yet.  Given the amount of review
>>> comments that are still trickling in (also on patches that already have
>>> maintainer r-b) I don't think we're quite there to start thinging about
>>> picking this up in drm-msm just yet.  I doubt anyone wants a repeat of
>>> the original DSC series, which went through many review rounds yet still
>>> required multiple series of bugfixes (some of which were pointed out and
>>> ignored in review) to be brought to a working state.  But the split
>>> across topics per series already makes this a lot less likely, many
>>> thanks for that.
>>>
>>
>> I think the outstanding comments shouldnt last more than 1-2 revs more
>> on this one as its mostly due to multiple patches on the list touching
>> catalog at the same time. I have been monitoring the comments closely
>> even though I dont respond to all of them.
>>
>> One of the major reasons of the number of issues with DSC 1.1 was QC
>> didn't really have the devices or panels to support it. Thats why I
>> changed that this time around to take more control of validation of DSC
>> 1.2 and ofcourse decided to break up of series into the least amount of
>> functionality needed to keep the DPU driver intact.
> 
> Really glad that you are able to test and validate it now, that goes a
> long way.  Does that also mean you can post the panel patches quickly,
> so that everyone has a point of reference?  As you said that is one of
> the main points where DSC 1.1 "went wrong" (a misunderstanding of
> drm_dsc_config).
> 
>> All that being said, we still value your comments and would gladly wait
>> for a couple of days like I already wrote. But there are more
>> incremental series on top of this:
>>
>> -> DSI changes for DSC 1.2
>> -> proper teardown for DSC
>> -> DSC pair allocation support
>> -> DSC 1.2 over DP
> 
> Yeah, I'm familiar with the concept of having many dependent series, and
> now DSC series are even rebasing on DPU (catalog) cleanups to preempt
> conflicts, which is really hard to follow.
> Dmitry just pushed v5 of "drm/i915/dsc: change DSC param tables to
> follow the DSC model" [1] and seems to have dropped some patches that
> some of these series are depending on, resulting in compilation
> failures.  Other series don't seem to fully mention all their
> dependencies.

We'd have to pick them into our code or submit directly to drm-misc. I 
removed the patches which we can get in w/o Intel review.

> 
> [1]: https://lore.kernel.org/linux-arm-msm/20230504153511.4007320-1-dmitry.baryshkov@linaro.org/T/#u
> 
> So, for this to go as convenient as possible, do you have a list of
> series, in a desired order that they should be reviewed and tested?
> That way I can direct my priorities and help achieve the goal of picking
> base dependencies as early as possible.
> 
> For example, one of the many series regresses DSC on the Xperia XZ3
> (SDM845), but I have yet to bisect and understand which patch it is.
> Will let you know as soon as I get my tree in order.
> 
> - Marijn

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists