[Freedreno] [RFC PATCH v2 10/13] drm/msm/dpu: add list of supported formats to the DPU caps

[Dmitry Baryshkov]

On 07/06/2023 01:57, Abhinav Kumar wrote:
> 
> 
> On 6/6/2023 3:50 PM, Dmitry Baryshkov wrote:
>> On 07/06/2023 01:47, Abhinav Kumar wrote:
>>>
>>>
>>> On 6/6/2023 2:52 PM, Dmitry Baryshkov wrote:
>>>> On 07/06/2023 00:47, Abhinav Kumar wrote:
>>>>>
>>>>>
>>>>> On 6/6/2023 2:29 PM, Dmitry Baryshkov wrote:
>>>>>> On 07/06/2023 00:14, Abhinav Kumar wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 5/24/2023 6:47 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Thu, 25 May 2023 at 02:16, Abhinav Kumar 
>>>>>>>> <quic_abhinavk at quicinc.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote:
>>>>>>>>>> As we are going to add virtual planes, add the list of 
>>>>>>>>>> supported formats
>>>>>>>>>> to the hw catalog entry. It will be used to setup universal 
>>>>>>>>>> planes, with
>>>>>>>>>> later selecting a pipe depending on whether the YUV format is 
>>>>>>>>>> used for
>>>>>>>>>> the framebuffer.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If your usage of format_list is going to be internal to 
>>>>>>>>> dpu_plane.c, I
>>>>>>>>> can think of another idea for this change.
>>>>>>>>>
>>>>>>>>> This essentially translates to if (num_vig >= 1)
>>>>>>>>>
>>>>>>>>> If we can just have a small helper to detect that from the 
>>>>>>>>> catalog can
>>>>>>>>> we use that instead of adding formats to the dpu caps?
>>>>>>>>
>>>>>>>> I'd prefer to be explicit here. The list of supported formats might
>>>>>>>> vary between generations, might it not? Also we don't have a 
>>>>>>>> case of
>>>>>>>> the devices not having VIG planes. Even the qcm2290 (which doesn't
>>>>>>>> have CSC) lists YUV as supported.
>>>>>>>>
>>>>>>>
>>>>>>> the list of formats is tied to the sspps the dpu generation has 
>>>>>>> and the capabilities of those sspps.
>>>>>>>
>>>>>>> qcm2290 is really an interesting case. It has one vig sspp but no 
>>>>>>> csc block for that vig sspp, hence it cannot support non-RGB 
>>>>>>> formats.
>>>>>>>
>>>>>>> I have confirmed that downstream is incorrect to populate yuv 
>>>>>>> formats for qcm2290.
>>>>>>>
>>>>>>> I still think that having atleast one vig sspp with csc sub-blk 
>>>>>>> is the right condition to check if we want to decide if the dpu 
>>>>>>> for that chipset supports yuv formats. Extra csc-blk check to 
>>>>>>> handle qcm2290.
>>>>>>>
>>>>>>> Having a small helper in dpu_plane.c is good enough for that 
>>>>>>> because with virtual planes, you only need to know "if such a 
>>>>>>> plane exists and not which plane" and a full catalog change isnt 
>>>>>>> needed IMO
>>>>>>
>>>>>> This goes down to the question: is the list of YUV and non-YUV 
>>>>>> formats static or not? Do all DPU devices support the same set of 
>>>>>> YUV and non-YUV formats? If it is static, we might as well drop 
>>>>>> dpu_sspp_sub_blks::format_list.
>>>>>>
>>>>>
>>>>> I would say yes based on the below reference:
>>>>>
>>>>> https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/clo/main/msm/sde/sde_hw_catalog.c#L3858
>>>>>
>>>>> We always add the same set of YUV formats for all Vig SSPPs 
>>>>> irrespective of the chipsets.
>>>>
>>>> Well, as your example pointed out, few lines below it starts adding 
>>>> formats to the list, so the format list is not static and depends on 
>>>> the generation.
>>>>
>>>
>>> No, the DPU revision checks are there to add P010 UBWC formats on top 
>>> of the Vig formats.
>>>
>>> At the moment, the latest downstream code (code which is not on CLO 
>>> hence I cannot share) has dropped all that and just checks if P010 
>>> UBWC is supported for the Vig SSPP and adds all those.
>>>
>>> So its still tied to the feature bit whether P010 UBWC is supported 
>>> in the Vig SSPP and not at the chipset level.
>>
>> So, what is the difference? This means that depending on some 
>> conditions either we can support P010 UBWC or we can not. So the list 
>> of all suppored formats is not static.
>>
> 
> The difference is SSPP level vs chipset level. This patch was made with 
> chipset level in mind and had to expand the format list for each chipset.
> 
> What I am suggesting is its a SSPP level decision. Please note its not 
> just P010 UBWC but P010 UBWC FOR Vig SSPP.
> 
> So expanding each chipset's format list is not needed when the decision 
> can be made just on the basis of the SSPP's feature flag OR the presence 
> of the csc blk.

Maybe I misunderstand something. Is P010 UBWC format supported on VIG 
SSPPs on all generations? The code that you have pointed me suggests 
that is is not.

> 
>>>
>>>>>
>>>>>> Note to myself: consider 
>>>>>> dpu_mdss_cfg::{dma_formats,cursor_formats,vig_formats}. Either 
>>>>>> migrate dpu_sspp_sub_blks::format_list users to these fields or 
>>>>>> drop them.
>>>>>>
>>>>>
>>>>> Yes, I agree. There is no need to have format list in the sub_blk 
>>>>> as for one type of SSPP, it supports the same format across DPU 
>>>>> generations.
>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Note: I think at some point we should have a closer look at the 
>>>>>>>> list
>>>>>>>> of supported formats and crosscheck that we do not have either
>>>>>>>> unsupported formats being listed, or missing formats which are not
>>>>>>>> listed as supported.
>>>>>>>>
>>

-- 
With best wishes
Dmitry