On 9/15/26 6:05 PM, AngeloGioacchino Del Regno wrote:
[...]
>>> + ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
>>> + if (ret) {
>>> + dev_err(dev, "Cannot find DSC RC params\n");
>>> + return ret;
>>> + }
>>
>> The defaults in MediaTek's downstream driver match DRM_DSC_1_2_444, not
>> DRM_DSC_1_1_PRE_SCR, and the panel on my test device requires that even
>> though it uses DSC v1.1. This may be a panel-specific problem, but have
>> you tested this with any actual panels that need 1_1_PRE_SCR?
>>
>
> My downstream uses 1_1_PRE_SCR... that's why it is like this.
>
> I can resolve it like
>
> if (dsc->dsc_version_major == 1 && dsc->dsc_version_minor == 1)
> type = DRM_DSC_1_1_PRE_SCR;
> else
> type = DRM_DSC_1_2_444;
>
> ret = drm_dsc_setup_rc_params(dsc, type);
>
> ...that will work, granted that the panel declares the correct DSC version.
>
Unfortunately not, because DRM_DSC_1_2_444 is also needed for some v1.1
panels. From drivers/gpu/drm/drm_dsc_helper.c:
/*
* Selected Rate Control Related Parameter Recommended Values from DSC v1.2,
v1.2a, v1.2b and
* DSC_v1.1_E1 specs.
*
* Cross-checked against C Model releases: DSC_model_20161212 and 20210623
*/
static const struct rc_parameters_data rc_parameters_1_2_444[] = {
...
I'm pretty sure that my device is not using DSC v1.2.
Since this is definitely panel-specific and there seems to be no good
way to detect the required parameters at the moment, this is probably
something to fix later.
[...]
>> I don't quite understand what is going on here, even in the original
>> driver before this patch. What is the point of setting VACT_NL twice?
>> Does mtk_dsi_ps_control() really need to be called in mtk_dsi_poweron()
>> if mtk_dsi_config_vdo_timing() ends up calling it a second time?
>
> I didn't do much research, but MT8173 and 8192 if I remember correctly
> won't work
> without this sequence. I saw that 2 years ago or something, so I don't
> remember
> all that clearly.
>
Okay, it's fine to keep it as-is then to avoid breaking older SoCs. One
change I would definitely recommend though is to move
mtk_dsi_set_dsc_params() to the very beginning of the sequence, maybe
even into mtk_dsi_host_attach().
Otherwise some parts of the code might end up relying on unpopulated
parameters by accident, and it's harder to debug if they change in the
middle of the sequence.
[...]
> We can always come back to this in the future with a separate patch, and
> we should
> do just that, otherwise series versions will never come to an end :-)
>
Agreed. My work is currently also based on the restructured mediatek-drm
driver, but I will try to send follow-up patches for MT6858 support once
some of this stuff lands upstream.
Best regards,
Nikolai
