[Freedreno] [PATCH v3 18/27] drm/msm/dpu: populate SmartDMA features in hw catalog

Sun Feb 5 00:29:27 UTC 2023

On Sun, 5 Feb 2023 at 01:20, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>
>
>
> On 2/4/2023 1:08 PM, Dmitry Baryshkov wrote:
> > On 04/02/2023 20:35, Abhinav Kumar wrote:
> >>
> >>
> >> On 2/4/2023 2:43 AM, Dmitry Baryshkov wrote:
> >>> On 04/02/2023 07:10, Abhinav Kumar wrote:
> >>>>
> >>>>
> >>>> On 2/3/2023 8:10 PM, Dmitry Baryshkov wrote:
> >>>>> On 04/02/2023 04:43, Abhinav Kumar wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 2/3/2023 6:29 PM, Dmitry Baryshkov wrote:
> >>>>>>> On 04/02/2023 01:35, Abhinav Kumar wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
> >>>>>>>>> Downstream driver uses dpu->caps->smart_dma_rev to update
> >>>>>>>>> sspp->cap->features with the bit corresponding to the supported
> >>>>>>>>> SmartDMA
> >>>>>>>>> version. Upstream driver does not do this, resulting in SSPP
> >>>>>>>>> subdriver
> >>>>>>>>> not enbaling setup_multirect callback. Add corresponding
> >>>>>>>>> SmartDMA SSPP
> >>>>>>>>> feature bits to dpu hw catalog.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> While reviewing this patch, I had a first hand experience of how
> >>>>>>>> we are reusing SSPP bitmasks for so many chipsets but I think
> >>>>>>>> overall you got them right here :)
> >>>>>>>>
> >>>>>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> >>>>>>>>> ---
> >>>>>>>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 +++++++---
> >>>>>>>>>   1 file changed, 7 insertions(+), 3 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>>>>>>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>>>>>>>> index cf053e8f081e..fc818b0273e7 100644
> >>>>>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>>>>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>>>>>>>> @@ -21,13 +21,16 @@
> >>>>>>>>>       (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3))
> >>>>>>>>>   #define VIG_SDM845_MASK \
> >>>>>>>>> -    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) |
> >>>>>>>>> BIT(DPU_SSPP_SCALER_QSEED3))
> >>>>>>>>> +    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) |
> >>>>>>>>> BIT(DPU_SSPP_SCALER_QSEED3) |\
> >>>>>>>>> +    BIT(DPU_SSPP_SMART_DMA_V2))
> >>>>>>>>>   #define VIG_SC7180_MASK \
> >>>>>>>>> -    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) |
> >>>>>>>>> BIT(DPU_SSPP_SCALER_QSEED4))
> >>>>>>>>> +    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) |
> >>>>>>>>> BIT(DPU_SSPP_SCALER_QSEED4) |\
> >>>>>>>>> +    BIT(DPU_SSPP_SMART_DMA_V2))
> >>>>>>>>>   #define VIG_SM8250_MASK \
> >>>>>>>>> -    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) |
> >>>>>>>>> BIT(DPU_SSPP_SCALER_QSEED3LITE))
> >>>>>>>>> +    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) |
> >>>>>>>>> BIT(DPU_SSPP_SCALER_QSEED3LITE) |\
> >>>>>>>>> +    BIT(DPU_SSPP_SMART_DMA_V2))
> >>>>>>>>>   #define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL))
> >>>>>>>>> @@ -42,6 +45,7 @@
> >>>>>>>>>   #define DMA_SDM845_MASK \
> >>>>>>>>>       (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) |
> >>>>>>>>> BIT(DPU_SSPP_QOS_8LVL) |\
> >>>>>>>>>       BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\
> >>>>>>>>> +    BIT(DPU_SSPP_SMART_DMA_V2) |\
> >>>>>>>>>       BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_EXCL_RECT))
> >>>>>>>>>   #define DMA_CURSOR_SDM845_MASK \
> >>>>>>>>
> >>>>>>>> VIG_SDM845_MASK and DMA_SDM845_MASK are used for many other
> >>>>>>>> chipsets like 8250, 8450, 8550.
> >>>>>>>>
> >>>>>>>> At the moment, for visual validation of this series, I only have
> >>>>>>>> sc7180/sc7280. We are leaving the rest for CI.
> >>>>>>>>
> >>>>>>>> Was that an intentional approach?
> >>>>>>>>
> >>>>>>>> If so, we will need tested-by tags from folks having
> >>>>>>>> 8350/8450/8550/sc8280x,qcm2290?
> >>>>>>>>
> >>>>>>>> I am only owning the visual validation on sc7280 atm.
> >>>>>>>
> >>>>>>> I'm not quite sure what is your intent here. Are there any SoCs
> >>>>>>> after 845 that do not have SmartDMA 2.5? Or do you propose to
> >>>>>>> enable SmartDMA only for the chipsets that we can visually test?
> >>>>>>> That sounds strange.
> >>>>>>>
> >>>>>>
> >>>>>> Yes I was thinking to enable smartDMA at the moment on chipsets
> >>>>>> which we can validate visually that display comes up. But I am not
> >>>>>> sure if thats entirely practical.
> >>>>>>
> >>>>>> But the intent was I just want to make sure basic display does
> >>>>>> come up with smartDMA enabled if we are enabling it for all chipsets.
> >>>>>
> >>>>> I don't think it is practical or logical. We don't require
> >>>>> validating other changes on all possible chipsets, so what is so
> >>>>> different with this one?
> >>>>>
> >>>>
> >>>> Thats because with smartDMA if the programming of stages goes wrong
> >>>> we could potentially just see a blank screen. Its not about other
> >>>> changes, this change in particular controls enabling a feature.
> >>>>
> >>>> But thats just my thought. I am not going to request to ensure this
> >>>> or block this for this.
> >>>>
> >>>> You can still have my
> >>>>
> >>>> Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
> >>>>
> >>>> But think of the validations that have to be done before we merge it.
> >>>
> >>> The usual way: verify as much as feasible and let anybody else
> >>> complain during the development cycle.
> >>>
> >>
> >> Well, our perspective is to enable the feature on devices on which you
> >> are able to test and not enable then wait for others to complain.
> >
> > This would not be really practical. There are plenty of people who can
> > test things on obscure platforms, but unfortunately far less amount of
> > people who tightly follow the development and can track which new
> > feature applies to a particular platform. I hope to be able to fix that
> > slightly with the hw catalog rework. However enabling features on other
> > platforms definitely requires more knowledge than simply testing the
> > kernel.
> >
> >>
> >> I did not say test all devices. My point was to enable smartDMA on
> >> devices which we are able to test.
> >>
> >> There are other examples of this, like inline rotation, writeback etc.
> >> which are at the moment enabled only on devices which QC or others
> >> have tested on.
> >
> > But at the time it was added, inline rotation 2.0 could only be
> > supported on sc7280. Probably we should expand it not to sc8280xp and
> > sm8[345]50.
> >
> > For WB I don't remember which platforms were supported at the moment it
> > was added. But it's also worth expanding support to new platforms.
> >
> > And, as we speak about testing, is there an easy way to setup the plane
> > with UBWC format modifier? Also, did the WB support patches land into
> > libdrm?
> >
>
> I will check the compositor code and update you on the UWBC format
> modifier as I am not too familiar with it.

Ideally it would be nice to support ubwc planes in some simple tool,
e.g. modetest.

>
> libdrm always supported virtual encoder
> https://github.com/grate-driver/libdrm/blob/master/include/drm/drm_mode.h#L352
>
> What other support patches are needed? Right now we only use IGT to
> validate writeback.

I remember there was a patchset to make modeset to support using
writeback. What was its fate?

>
> >> So when i said my suggestion was not practical, yes because if you
> >> want to go ahead with this change in the current form, you would have
> >> to validate all the chipsets as you are enabling smartDMA on all of them.
> >>
> >> If you enable smartDMA only on the chipsets you OR others can validate
> >> and give Tested-by for like I was planning to do for sc7280, then I am
> >> not sure why it doesnt sound logical.
> >>
> >> But like I said, thats my perspective. I will let you decide as you
> >> would know how confident you are with this getting enabled for all
> >> chipsets upstream.
> >
> > I'd say, that once tested on some of the platforms and granted that even
> > smalled (qcm2290, sm6115) platforms support smartdma, it will be safe to
> > enable smart DMA globablly for every SoC >= sdm845. If I remember
> > correctly, msm8998 (and sdm660/630) support smartdma/rect only on DMA
> > planes. Is it correct?
> >
> >
> Yes thats right msm8998 supports smartdma only on DMA sspps.

Good

-- 
With best wishes
Dmitry