linux-kernel - Re: [PATCH] drm/panel: Add prepare_prev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c7c5c8f0-16e6-47bd-94e8-ce924163dfd3@linaro.org>
Date:   Fri, 18 Aug 2023 10:25:48 +0200
From:   neil.armstrong@...aro.org
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Jessica Zhang <quic_jesszhan@...cinc.com>,
        Sam Ravnborg <sam@...nborg.org>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Douglas Anderson <dianders@...omium.org>,
        Rob Clark <robdclark@...il.com>,
        Maxime Ripard <mripard@...nel.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Rob Clark <robdclark@...il.com>
Cc:     quic_parellan@...cinc.com, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/panel: Add prepare_prev_first flag to Visionox
 VTDR6130

Hi Dmitry,

On 17/08/2023 20:35, Dmitry Baryshkov wrote:
> On 16/08/2023 10:51, neil.armstrong@...aro.org wrote:
>> Hi Abhinav,
>>
>> On 14/08/2023 20:02, Abhinav Kumar wrote:

<snip>

>>
>> Sending HS commands will always work on any controller, it's all about LP commands.
>> The Samsung panels you listed only send HS commands so they can use prepare_prev_first
>> and work on any controllers.
> 
> I think there is some misunderstanding there, supported by the description of the flag.
> 
> If I remember correctly, some hosts (sunxi) can not send DCS commands after enabling video stream and switching to HS mode, see [1]. Thus, as you know, most of the drivers have all DSI panel setup commands in drm_panel_funcs::prepare() / drm_bridge_funcs::pre_enable() callbacks, not paying attention whether these commands are to be sent in LP or in HS mode.
> 
> Previously DSI source drivers could power on the DSI link either in mode_set() or in pre_enable() callbacks, with mode_set() being the hack to make panel/bridge drivers to be able to send commands from their prepare() / pre_enable() callbacks.
> 
> With the prev_first flags being introduced, we have established that DSI link should be enabled in DSI host's pre_enable() callback and switched to HS mode (be it command or video) in the enable() callback.
> 
> So far so good.

It seems coherent, I would like first to have a state of all DSI host drivers and make this would actually work first before adding the prev_first flag to all the required panels.

> 
> Unfortunately this change is not fully backwards-compatible. This requires that all DSI panels sending commands from prepare() should have the prepare_prev_first flag. In some sense, all such patches might have Fixes: 5ea6b1702781 ("drm/panel: Add prepare_prev_first flag to drm_panel").

This kind of migration should be done *before* any possible regression, not the other way round.

If all panels sending commands from prepare() should have the prepare_prev_first flag, then it should be first, check for regressions then continue.

<snip>

>>
>> I understand, but this patch doesn't qualify as a fix for 9e15123eca79 and is too late to be merged in drm-misc-next for v6.6,
>> and since 9e15123eca79 actually breaks some support it should be reverted (+ deps) since we are late in the rc cycles.
> 
> If we go this way, we can never reapply these patches. There will be no guarantee that all panel drivers are completely converted. We already have a story without an observable end - DRM_BRIDGE_ATTACH_NO_CONNECTOR.

I don't understand this point, who would block re-applying the patches ?

The migration to DRM_BRIDGE_ATTACH_NO_CONNECTOR was done over multiple Linux version and went smoothly because we reverted
regressing patches and restarted when needed, I don't understand why we can't do this here aswell.

> 
> I'd consider that the DSI driver is correct here and it is about the panel drivers that require fixes patches. If you care about the particular Fixes tag, I have provided one several lines above.

Unfortunately it should be done in the other way round, prepare for migration, then migrate,

I mean if it's a required migration, then it should be done and I'll support it from both bridge and panel PoV.

So, first this patch has the wrong Fixes tag, and I would like a better explanation on the commit message in any case.
Then I would like to have an ack from some drm-misc maintainers before applying it because it fixes a patch that
was sent via the msm tree thus per the drm-misc rules I cannot apply it via the drm-misc-next-fixes tree.

Neil

<snip>