Re: [PATCH 1/3] drm/msm/dpu: Read previously-uninitialized SSPP scaler version from hw

[Dmitry Baryshkov]

On Thu, 16 Feb 2023 at 01:02, Marijn Suijten
<marijn.suijten@xxxxxxxxxxxxxx> wrote:
>
> DPU's catalog never assigned dpu_scaler_blk::version leading to
> initialization code in dpu_hw_setup_scaler3 to wander the wrong
> codepaths.  Instead of hardcoding the correct QSEED algorithm version,
> read it back from a hardware register.
>
> Note that this register is only available starting with QSEED3, where
> 0x1002 corresponds to QSEED3, 0x2004 to QSEED3LITE and 0x3000 to QSEED4.

This is not purely accurate. 0x1003 (sdm845) also corresponds to QSEED3.
I'd say instead that there are several variations of QSEED3 scalers,
where starting from 0x2004 it is called QSEED3LITE and starting from
0x3000 it is called QSEED4.

>
> Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 --
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c    | 8 +++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h    | 3 +++
>  3 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index ddab9caebb18..96ce1766f4a1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -324,11 +324,9 @@ struct dpu_src_blk {
>  /**
>   * struct dpu_scaler_blk: Scaler information
>   * @info:   HW register and features supported by this sub-blk
> - * @version: qseed block revision
>   */
>  struct dpu_scaler_blk {
>         DPU_HW_SUBBLK_INFO;
> -       u32 version;

No. Please keep the version in the scaler subblk.  It is a version of
the QSEED (scaler block), not the SSPP's version.

There is a block called DS (destination scaler), which can be used to
scale the resulting image after the LM. This block also uses the
QSEED3(,LITE,4) scaler block.

>  };
>
>  struct dpu_csc_blk {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> index 4246ab0b3bee..d4e181e1378c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -430,7 +430,7 @@ static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_pipe *ctx,
>                 return;
>
>         dpu_hw_setup_scaler3(&ctx->hw, scaler3_cfg, idx,
> -                       ctx->cap->sblk->scaler_blk.version,
> +                       ctx->version,
>                         sspp->layout.format);
>  }
>
> @@ -807,6 +807,12 @@ struct dpu_hw_pipe *dpu_hw_sspp_init(enum dpu_sspp idx,
>         hw_pipe->mdp = &catalog->mdp[0];
>         hw_pipe->idx = idx;
>         hw_pipe->cap = cfg;
> +
> +       if (test_bit(DPU_SSPP_SCALER_QSEED3, &cfg->features) ||
> +                       test_bit(DPU_SSPP_SCALER_QSEED3LITE, &cfg->features) ||
> +                       test_bit(DPU_SSPP_SCALER_QSEED4, &cfg->features))
> +               hw_pipe->version = _dpu_hw_sspp_get_scaler3_ver(hw_pipe);
> +
>         _setup_layer_ops(hw_pipe, hw_pipe->cap->features);
>
>         return hw_pipe;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> index 0c95b7e64f6c..eeaf16c6af15 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> @@ -352,6 +352,7 @@ struct dpu_hw_sspp_ops {
>   * @hw: block hardware details
>   * @catalog: back pointer to catalog
>   * @mdp: pointer to associated mdp portion of the catalog
> + * @version: qseed block revision
>   * @idx: pipe index
>   * @cap: pointer to layer_cfg
>   * @ops: pointer to operations possible for this pipe
> @@ -362,6 +363,8 @@ struct dpu_hw_pipe {
>         const struct dpu_mdss_cfg *catalog;
>         const struct dpu_mdp_cfg *mdp;
>
> +       u32 version;
> +
>         /* Pipe */
>         enum dpu_sspp idx;
>         const struct dpu_sspp_cfg *cap;
>
> --
> 2.39.2
>


-- 
With best wishes
Dmitry