Message-ID: <8e31a475-e856-ea78-5515-5dc7ff9d3dc6@quicinc.com>
Date:   Mon, 30 Jan 2023 14:39:22 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.2
Subject: Re: [PATCH v1 10/14] drm/msm/disp/dpu: add supports of DSC encoder
 v1.2 engine
Content-Language: en-US
To:     Marijn Suijten <marijn.suijten@somainline.org>
CC:     Kuogee Hsieh <quic_khsieh@quicinc.com>,
        <dri-devel@lists.freedesktop.org>, <robdclark@gmail.com>,
        <sean@poorly.run>, <swboyd@chromium.org>, <dianders@chromium.org>,
        <vkoul@kernel.org>, <daniel@ffwll.ch>, <airlied@gmail.com>,
        <agross@kernel.org>, <dmitry.baryshkov@linaro.org>,
        <andersson@kernel.org>, <quic_sbillaka@quicinc.com>,
        <freedreno@lists.freedesktop.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <1674498274-6010-1-git-send-email-quic_khsieh@quicinc.com>
 <1674498274-6010-11-git-send-email-quic_khsieh@quicinc.com>
 <20230123201133.zzt2zbyaw3pfkzi6@SoMainline.org>
 <926021c1-d9d1-4449-f01b-f405b48f326a@quicinc.com>
 <20230130201654.vlr7p7id3a7vxc2b@SoMainline.org>
 <0a0c3a12-51e9-b0d0-9307-847023dbe1f7@quicinc.com>
 <20230130223110.abjh7k32fjs4kola@SoMainline.org>
From:   Abhinav Kumar <quic_abhinavk@quicinc.com>
In-Reply-To: <20230130223110.abjh7k32fjs4kola@SoMainline.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 1/30/2023 2:31 PM, Marijn Suijten wrote:
> Abhinav,
> 
> On 2023-01-30 13:22:03, Abhinav Kumar wrote:
>> Hi Marijn
>>
>> On 1/30/2023 12:16 PM, Marijn Suijten wrote:
>>> On 2023-01-24 15:52:46, Kuogee Hsieh wrote:
>>>
>>> <snip>
>>>
>>> If only replying to a small chunk somewhere in the middle of a diff
>>> and/or large review, please cut out unnecessary bits to make your reply
>>> easier to find :)
>>>
>>>>>> +	data = (dsc->flatness_min_qp & 0x1f);
>>>>>> +	data |= (dsc->flatness_max_qp & 0x1f) << 5;
>>>>>> +	data |= (dsc_info->det_thresh_flatness & 0xff) << 10;
>>>>> dpu_hw_dsc.c computes this on the fly.  After removing that, and
>>>>> using initial_lines from the function parameters, only
>>>>> dsc_info->num_active_ss_per_enc remains.  Do you really need that
>>>>> msm_display_dsc_info struct here, do you need it at all?
>>>>
>>>> I ported these code from our down stream code base.
>>>>
>>>> I make it work first, then clean it up will follow.
>>>>
>>>> I submit it for review since it looks like you guy like to have code sooner.
>>>
>>> Correct, I was looking forward to these patches albeit complete with the
>>> promised DSI support from Jessica, which still seems to be pending.
>>>
>>
>> DSI support is still being worked upon.
>>
>> I dont think we promised DSC 1.2 will come with DSI together in the same
>> series. It was always going to be DSC 1.2 + DP followed by another
>> series from Jessica for DSI.
>>
>> Lets set the expectations right.
> 
> Not saying that these patches were promised as part of this series (as
> said, "which still seem to be pending"), just making clear that this
> series if of no use to me (no hurry to get the code in my hands sooner)
> until the DSI patches are also shared which I would have started working
> on myself if I didn't know QUIC was picking it up to distract from the
> current v1.1 broken-ness on SM8150 and SM8250.
> 

This is being by Quic for everyone's benefit. So that we can land a 
working DSC 1.2 solution for DSI as a working example for all future 
panels. We only took it up to help others like you and linaro team to 
give a working example of a DSC 1.2 panel with command mode in upstream.


> To set my (and at least Neil's) expectations straight as well: DSC 1.2
> HW support should come in a separate series without DP support.  Smaller
> series (not to mention appropriately split-up patches) lead to a
> decrease in scope, less dependencies and hopefully more efficient v2 -
> for all involved.
> 

As I already wrote earlier, we will fix the mistakes of v1, make v2 
better and it will be split up better. But DSC 1.2 HW support had to be 
pushed along with DP or DSI to show its working. We chose DP to go with 
it as it aligns better with our upstream plans.


> - Marijn