On 9/15/26 20:51, Nikolai Burov wrote:
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.
Ok, let's just go with 1_2_444 then - makes sense... that's something
that must be accounted at the API level, because I know of some panels
that need the PRE_SCR set... and that's even if they're using v1.2, just
because - for whatever reason - some of the DSC params are stored in and
read from an OTP area of the DriverIC (and that would mismatch then with
the other params that are runtime-programmable from the DrIC view, which
results in pixel-garbage artifacts).
Not sure how many panels are doing that, but this is of course not a MTK
specific thing... I looked around, and apparently there's a mention of
that in the Intel driver as well - yay I'm not the only one who knows, lol.
(And I definitely didn't expect that to be the case anyway).
[...]
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.
Honestly, since it works right now, I would be happier to push this upstream
as-is, and then shuffle things around (for an optimization, or else) after
the fact.
The point is to have a working baseline upstream, then if needed improve it
later when further testing can be done - point is that there's a huge set
of patches that is restructuring everything, so I want to play safe as much
as I can in this moment - I think (and hope) you understand my feeling here.
[...]
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.
That's... a w e s o m e !
This is a practical test of the restructured code on yet another platform,
and I'm happier now because it's a different type of device (as in, not a
laptop and not an aiot board).
Thanks for that, highly appreciated.
Cheers,
Angelo
