[PATCH v6 09/10] drm/msm/dpu: merge DPU_SSPP_SCALER_QSEED3, QSEED3LITE, QSEED4
Abhinav Kumar
quic_abhinavk at quicinc.com
Wed Nov 1 18:43:26 UTC 2023
On 10/31/2023 1:19 AM, Dmitry Baryshkov wrote:
> On Mon, 30 Oct 2023 at 22:24, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>>
>>
>>
>> On 10/6/2023 6:14 AM, Dmitry Baryshkov wrote:
>>> Three different features, DPU_SSPP_SCALER_QSEED3, QSEED3LITE and QSEED4
>>> are all related to different versions of the same HW scaling block.
>>> Corresponding driver parts use scaler_blk.version to identify the
>>> correct way to program the hardware. In order to simplify the driver
>>> codepath, merge these three feature bits.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +-----
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 9 ++-------
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 3 +--
>>> 4 files changed, 6 insertions(+), 16 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 32c396abf877..eb867c8123d7 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> @@ -31,10 +31,10 @@
>>> (VIG_SDM845_MASK | 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_QSEED3))
>>>
>>> #define VIG_SM6125_MASK \
>>> - (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
>>> + (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3))
>>>
>>
>> This merging is coming at a cost of inaccuracy. We are marking sc7180
>> and sm6125 as scaler_qseed3. But they are not. Let me know what you
>> think of below idea instead.
>>
>>> #define VIG_SC7180_MASK_SDMA \
>>> (VIG_SC7180_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
>>> 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 fc5027b0123a..ba262b3f0bdc 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>>> @@ -51,9 +51,7 @@ enum {
>>> /**
>>> * SSPP sub-blocks/features
>>> * @DPU_SSPP_SCALER_QSEED2, QSEED2 algorithm support
>>> - * @DPU_SSPP_SCALER_QSEED3, QSEED3 alogorithm support
>>> - * @DPU_SSPP_SCALER_QSEED3LITE, QSEED3 Lite alogorithm support
>>> - * @DPU_SSPP_SCALER_QSEED4, QSEED4 algorithm support
>>> + * @DPU_SSPP_SCALER_QSEED3, QSEED3 alogorithm support (also QSEED3LITE and QSEED4)
>>> * @DPU_SSPP_SCALER_RGB, RGB Scaler, supported by RGB pipes
>>> * @DPU_SSPP_CSC, Support of Color space converion
>>> * @DPU_SSPP_CSC_10BIT, Support of 10-bit Color space conversion
>>> @@ -72,8 +70,6 @@ enum {
>>> enum {
>>> DPU_SSPP_SCALER_QSEED2 = 0x1,
>>> DPU_SSPP_SCALER_QSEED3,
>>> - DPU_SSPP_SCALER_QSEED3LITE,
>>> - DPU_SSPP_SCALER_QSEED4,
>>> DPU_SSPP_SCALER_RGB,
>>> DPU_SSPP_CSC,
>>> DPU_SSPP_CSC_10BIT,
>>> 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 7e9c87088e17..d1b70cf72eef 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>>> @@ -594,9 +594,7 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
>>> test_bit(DPU_SSPP_SMART_DMA_V2, &c->cap->features))
>>> c->ops.setup_multirect = dpu_hw_sspp_setup_multirect;
>>>
>>> - if (test_bit(DPU_SSPP_SCALER_QSEED3, &features) ||
>>> - test_bit(DPU_SSPP_SCALER_QSEED3LITE, &features) ||
>>> - test_bit(DPU_SSPP_SCALER_QSEED4, &features))
>>> + if (test_bit(DPU_SSPP_SCALER_QSEED3, &features))
>>> c->ops.setup_scaler = _dpu_hw_sspp_setup_scaler3;
>>
>> Can we just do sblk->scaler_blk.version >= 0x3000 instead of this
>> merging? That way you can still drop those enums without inaccuracy.
>
> No. QSEED3 from sdm845 has version 1.3, msm8998, sdm660 and sdm630
> have version 1.2.
>
Ah got it.
HW versioning is getting harder to generalize with the example you have
given. In my opinion, in that case lets keep these enums intact because
we dont have any other way of knowing the Qseed version otherwise and in
terms of LOC, we are not really saving so much in this change.
In the prev change I agreed because along with the name and the version,
we could still interpret the version but with compressing the enums into
just QSEED3, this becomes very confusing. So now, in the future if we
have QSEED5 HW, we will have to change this anyway as its LUT
programming can change. So we need this distinction.
The below two changes seem fine and that way we are eliminating the
usages of the enum and we will end up with only one place using this.
>>
>>>
>>> if (test_bit(DPU_SSPP_CDP, &features))
>>> @@ -629,10 +627,7 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
>>> cfg->len,
>>> kms);
>>>
>>> - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
>>> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
>>> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
>>> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
>>> + if (sblk->scaler_blk.len)
>>
>> This part seems fine.
>>
>>> dpu_debugfs_create_regset32("scaler_blk", 0400,
>>> debugfs_root,
>>> sblk->scaler_blk.base + cfg->base,
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> index 43135894263c..ba3ee4ba25b3 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> @@ -438,8 +438,7 @@ static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
>>> scale_cfg->src_height[i] /= chroma_subsmpl_v;
>>> }
>>>
>>> - if (pipe_hw->cap->features &
>>> - BIT(DPU_SSPP_SCALER_QSEED4)) {
>>> + if (pipe_hw->cap->sblk->scaler_blk.version >= 0x3000) {
>> This is fine too.
>>> scale_cfg->preload_x[i] = DPU_QSEED4_DEFAULT_PRELOAD_H;
>>> scale_cfg->preload_y[i] = DPU_QSEED4_DEFAULT_PRELOAD_V;
>>> } else {
>
>
>
More information about the dri-devel
mailing list