Re: [PATCH v9 02/13] drm/mediatek: Implement Display Stream Compression support

[Roman Vivchar <rva333@protonmail.com>]

Hi Angelo,

I have tested this series with my out-of-tree mt6595 (command mode 
panel) and mt6768 (video mode) DRM. There's no regressions, but I 
spotted an issue with kconfig.

On 9/16/26 14:08, AngeloGioacchino Del Regno wrote:
> Add a real driver for the Display Stream Compression (DSC) Display
> Controller IP, implementing support for DSC v1.1 to v1.2.
> 
> In order to do this, it was necessary to remove the basic DSC IP
> bypass setup from mtk_ddp_comp: this functionality is retained in
> the new mtk_disp_dsc driver, which checks if DSC was actually
> requested by other components (with the only one that currently
> supports this being DSI) and, if not, it will set BYPASS mode in
> the DSC IP.
> 
> Like before, the BYPASS mode is set before starting the DSC IP,
> but unlike before, this is being done in the component start
> callback instead of the config one.
> Notably, the config callback is called by mtk_crtc always
> immediately before the calling start callback, so the order of
> register writes is retained.
> The only real difference is that now this is being done through
> CPU writes instead of CMDQ, but since that's called only once
> and since it's just three registers, the performance impact will
> not be minimal and not even measurable.
> 
> As anticipated, DSC handling was also introduced in the mtk_dsi
> driver: when performing dsi_host_attach, the driver now checks
> if the DSI panel adds the DSC configuration structure to the
> mipi_dsi_device structure and, if it does, it will store a
> pointer in the driver-local mtk_dsi structure's `dsc` member.
> 
> The DSI driver will then check whether the DSC configuration
> that comes from the panel is valid (in regard to MediaTek DSI)
> and will call the DRM API's DSC helpers to calculate and set
> all of the const and RC parameters for the actual DSC setup.
> 
> For the time being, even though the latest MediaTek SoCs do
> support DSC v1.2, only DSC v1.1 pre-scr support is implemented
> as an initial contribution (which is rather big, and 1.2 would
> make it even bigger - but that can anyway be implemented later).
> 
> As a last step for validation of DSC parameters in DSI, a check
> for the hdisplay against DSC slice sidth and one for vdisplay
> against DSC slice height was added to the mode_valid callback,
> making sure that H/V are, as expected, multiples of slice W/H.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

...

> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -18,6 +18,8 @@
>   #include <video/mipi_display.h>
>   #include <video/videomode.h>
>   
> +#include <drm/display/drm_dsc.h>
> +#include <drm/display/drm_dsc_helper.h>

...

>   static void mtk_dsi_config_vdo_timing_per_frame_lp(struct mtk_dsi *dsi)
> @@ -565,26 +610,68 @@ static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
>   	writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
>   }
>   
> -static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> +static int mtk_dsi_set_dsc_params(struct mtk_dsi *dsi)
> +{
> +	struct drm_dsc_config *dsc = dsi->dsc;
> +	struct device *dev = dsi->host.dev;
> +	int ret;
> +
> +	if (dsc->bits_per_pixel & GENMASK(3, 0)) {
> +		dev_err(dev, "Fractional bits_per_pixel not supported\n");
> +		return -EINVAL;
> +	}
> +
> +	if (dsc->bits_per_component != 8) {
> +		dev_err(dev, "%u bits per component is not supported\n",
> +			dsc->bits_per_component);
> +		return -EINVAL;
> +	}
> +
> +	dsc->simple_422 = false;
> +	dsc->convert_rgb = true;
> +	dsc->vbr_enable = false;
> +
> +	drm_dsc_set_const_params(dsc);
> +	drm_dsc_set_rc_buf_thresh(dsc);
> +
> +	ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_2_444);

If config doesn't have DRM_DISPLAY_DSC_HELPER (selected by few panels), 
linking will fail with "error: undefined symbol: drm_dsc_set_const_params".
Should MediaTek DRM select it?

Best regards,
Roman