[PATCH 07/17] drm/msm/dpu: disallow widebus en in INTF_CONFIG2 when DP is YUV420

[Abhinav Kumar]

On 1/27/2024 9:33 PM, Dmitry Baryshkov wrote:
> On Sun, 28 Jan 2024 at 07:16, Paloma Arellano <quic_parellan at quicinc.com> wrote:
>>
>>
>> On 1/25/2024 1:26 PM, Dmitry Baryshkov wrote:
>>> On 25/01/2024 21:38, Paloma Arellano wrote:
>>>> INTF_CONFIG2 register cannot have widebus enabled when DP format is
>>>> YUV420. Therefore, program the INTF to send 1 ppc.
>>>
>>> I think this is handled in the DP driver, where we disallow wide bus
>>> for YUV 4:2:0 modes.
>> Yes we do disallow wide bus for YUV420 modes, but we still need to
>> program the INTF_CFG2_DATA_HCTL_EN. Therefore, it is necessary to add
>> this check.
> 
> As I wrote in my second email, I'd prefer to have one if which guards
> HCTL_EN and another one for WIDEN
> 
Its hard to separate out the conditions just for HCTL_EN . Its more 
about handling the various pixel per clock combinations.

But, here is how I can best summarize it.

Lets consider DSI and DP separately:

1) For DSI, for anything > DSI version 2.5 ( DPU version 7 ).

This is same the same condition as widebus today in 
msm_dsi_host_is_wide_bus_enabled().

Hence no changes needed for DSI.

2) For DP, whenever widebus is enabled AND YUV420 uncompressed case
as they are independent cases. We dont support YUV420 + DSC case.

There are other cases which fall outside of this bucket but they are 
optional ones. We only follow the "required" ones.

With this summary in mind, I am fine with what we have except perhaps 
better documentation above this block.

When DSC over DP gets added, I am expecting no changes to this block as 
it will fall under the widebus_en case.

With this information, how else would you like the check?

>>>
>>>>
>>>> Signed-off-by: Paloma Arellano <quic_parellan at quicinc.com>
>>>> ---
>>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
>>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
>>>> index 6bba531d6dc41..bfb93f02fe7c1 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
>>>> @@ -168,7 +168,9 @@ static void
>>>> dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
>>>>         * video timing. It is recommended to enable it for all cases,
>>>> except
>>>>         * if compression is enabled in 1 pixel per clock mode
>>>>         */
>>>> -    if (p->wide_bus_en)
>>>> +    if (dp_intf && fmt->base.pixel_format == DRM_FORMAT_YUV420)
>>>> +        intf_cfg2 |= INTF_CFG2_DATA_HCTL_EN;
>>>> +    else if (p->wide_bus_en)
>>>>            intf_cfg2 |= INTF_CFG2_DATABUS_WIDEN | INTF_CFG2_DATA_HCTL_EN;
>>>>          data_width = p->width;
>>>
> 
> 
>