Messages in this thread | | | Date | Mon, 8 May 2023 14:46:10 -0700 | Subject | Re: [PATCH v2 3/4] drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag | From | Jessica Zhang <> |
| |
On 5/7/2023 9:00 AM, Marijn Suijten wrote: > On 2023-05-05 14:23:50, Jessica Zhang wrote: >> Add DATA_COMPRESS feature flag to DPU INTF block. >> >> In DPU 7.x and later, DSC/DCE enablement registers have been moved from >> PINGPONG to INTF. >> >> As core_rev (and related macros) was removed from the dpu_kms struct, the >> most straightforward way to indicate the presence of this register would be >> to have a feature flag. > > Irrelevant. Even though core_rev was still in mainline until recently, > we always hardcoded the features in the catalog and only used core_rev > to select a dpu_mdss_cfg catalog entry. There is no "if version >= X > then enable feature Y" logic, this manually-enabled feature flag is the > only, correct way to do it.
Hi Marijn,
Understood. FWIW, if we do find more register bit-level differences between HW versions in the future, it might make more sense to keep the HW catalog small and bring core_rev back, rather than keep adding these kinds of small differences to caps.
Thanks,
Jessica Zhang
> >> Changes in v2: >> - Changed has_data_compress dpu_cap to a DATA_COMPRESS INTF feature flag >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> 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 7944481d0a33..c74051906d05 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> @@ -104,7 +104,7 @@ >> #define INTF_SC7180_MASK \ >> (BIT(DPU_INTF_INPUT_CTRL) | BIT(DPU_INTF_TE) | BIT(DPU_INTF_STATUS_SUPPORTED)) >> >> -#define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) >> +#define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) | BIT(DPU_INTF_DATA_COMPRESS) > > Konrad: Your SM6350/SM6375 series v3 [1] switched from INTF_SC7180_MASK > to INTF_SC7280_MASK to enable HCTL on SM6375, but that will now > erroneously also receive this feature flag and write the new > DATA_COMPESS mask even if it's DPU 6.9 (< 7.x where it got added). > > [1]: https://lore.kernel.org/linux-arm-msm/80b46fcb-d6d0-1998-c273-5401fa924c7d@linaro.org/T/#u > > Depending on who lands first, this flag should be split. > > I still see value in inlining and removing these defines, though that > brings a host of other complexity. > > - Marijn > >> #define WB_SM8250_MASK (BIT(DPU_WB_LINE_MODE) | \ >> BIT(DPU_WB_UBWC) | \ >> 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 4eda2cc847ef..01c65f940f2a 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> @@ -185,6 +185,7 @@ enum { >> * @DPU_DATA_HCTL_EN Allows data to be transferred at different rate >> * than video timing >> * @DPU_INTF_STATUS_SUPPORTED INTF block has INTF_STATUS register >> + * @DPU_INTF_DATA_COMPRESS INTF block has DATA_COMPRESS register >> * @DPU_INTF_MAX >> */ >> enum { >> @@ -192,6 +193,7 @@ enum { >> DPU_INTF_TE, >> DPU_DATA_HCTL_EN, >> DPU_INTF_STATUS_SUPPORTED, >> + DPU_INTF_DATA_COMPRESS, >> DPU_INTF_MAX >> }; >> >> >> -- >> 2.40.1 >>
| |