On 6/6/2023 4:21 PM, Dmitry Baryshkov wrote:
On 07/06/2023 02:14, Abhinav Kumar wrote:
On 6/6/2023 3:59 PM, Dmitry Baryshkov wrote:
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@xxxxxxxxxxx> 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.
No, your understanding is correct that P010 UBWC format is supported
only on Vig SSPPs of certain generations.
But my point is that, its still a SSPP level decision.
So even if we add P010 UBWC formats later to the list of supported
formats, what we will do is add that feature bit alone in the Vig
SSPP's feature mask of that chipset and based on that all the P010
UBWC formats for the virtual planes.
But if we follow your approach, we will add another array called
plane_formats_p010_ubwc and add that to each chipset which will
support it.
sspp->sblk->formats_list is constant, so we will have to add another
formats array anyway.
Not to each chipset. Yes you will have one new array for P010 UBWC
formats which you will use to do the plane_init() if any of the Vig
SSPPs have the feature bit set.
As opposed to adding the format_list to each chipset like you are doing now.
Its just a decision of whether you want to expand the catalog to have
this for each chipset OR not when you can easily make that decision at
run time using the SSPP's capabilities.
BTW, all of this is just discussion for future. For this series, I think
we both agree that you can just check if the chipset has any Vigs with
CSC to populate YUV in the format list.
The only difference in idea is that, based on the SSPP's feature set
of that chipset we can still determine that Vs adding it to each chipset.
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.