Re: [PATCH] clk: qcom: x1e80100-dispcc: Stop disp_cc_mdss_mdp_clk_src from getting parked

[Hans de Goede <johannes.goede@oss.qualcomm.com>]

Hi Val,

On 25-Apr-26 18:10, Val Packett wrote:
> 
> On 4/25/26 9:44 AM, Dmitry Baryshkov wrote:
>> On Sat, 25 Apr 2026 at 15:33, Hans de Goede
>> <johannes.goede@oss.qualcomm.com> wrote:
>>> Parking disp_cc_mdss_mdp_clk_src at 19.2MHz causing the EFI GOP framebuffer
>>> to stop functioning. The EFI GOP framebuffer should keep working until
>>> the msm display driver loads, to help with boot debugging and to ensure
>>> display output when the msm module is not in the initramfs.
>>>
>>> Switch disp_cc_mdss_mdp_clk_src over to clk_rcg2_shared_no_init_park_ops
>>> to keep the EFI GOP working after binding the x1e80100-dispcc driver.
>>>
>>> Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Thanks for finding this!!
>> Most likely we need this to be performed for all dispcc drivers. At
>> least for all laptop usecases.
> 
> This is relevant for phones (ex-Android) as well actually, though there have been attempts to fix that by adding stuff like
> 
>             clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
>                  <&dispcc DISP_CC_MDSS_MDP_CLK>,
>                  <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
>                  <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
>                  <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
>                  <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
>             power-domains = <&dispcc DISP_CC_MDSS_CORE_GDSC>;
> 
> to the simple-framebuffer node. However there have also been some "random" issues with the handover to mdss drivers (even with the mdss reset) so some postmarketOS kernel builds completely disable the simplefb driver.

Interesting. Note that just adding the clocks does not fully fix
the parking issue. The clock gets parked as soon as the dispcc-x1e80100
driver binds and the simpledrm driver binds later, so the clock will
still get parked for at least a while, leading at best to a display
glitch and as worse to the hw being in a confused state.

We do still need these clocks to have a chance for things to work
without needing clk_ignore_unused.

I've been having quite a bit of trouble with getting a
simple-framebuffer with similar clocks listed to work on a Snapdragon
X1E78100 based ThinkPad T14s too work.

The problem is that every other boot or so once the msm driver
loads the display goes black with the following errors:

[    2.980181] disp_cc_mdss_dptx3_pixel0_clk_src: rcg didn't update its configuration.
[    2.980272] WARNING: drivers/clk/qcom/clk-rcg2.c:136 at update_config+0xdc/0x100, CPU#8: kworker/8:1/138

I've tried putting a delay between the simpledrm driver turning
off the clocks and the msm driver loading but that does not help.

My conclusion is that the current simple-framebuffer resource
handling code concept/design is broken. The intention of listing the
resources in the simple-framebuffer DT node is to avoid them getting
turned off, IOW leave them untouched.

The turning them on at bind time of the driver is a no-op since all
of them are already on and we also don't need to worry about
power-sequencing because of the resources already being on.

But on unbind of the simpledrm driver / removal of the sysfb
platform-device we actually do start touching resources and
turning them off causing these problems (and not knowing
anything about the correct power-off sequence).

So as said I believe the current design is broken, on unbind any
usage counters like struct clk_core enable and prepare count
should be decremented to allow the e.g. clk to turn off later;
but at this point in time the hw-state should not be touched,
so that the actual display driver inherits the hw in the exact
state as setup by the firmware/bootloader.

A first approach to stop simpledrm from turning off the clocks
just before the msm driver loads is here:

https://github.com/jwrdegoede/linux-sunxi/commits/efifb-simplefb-resources-wip

and then specifically the last 2 commits, which switch
to setting the CLK_IGNORE_UNUSED flag on the clocks
instead of doing clk_prepare_enable() on probe() followed
by a problematic clk_disable_unprepare() on remove()

This works for my case and likely for more Qualcomm hw, but
as the Self NACK in the commit messages explains:

This will not work some other device/driver uses a clock with
the same parent and then runtime-suspends.

Because we don't increment the enable account, so the parent
will then get disabled on the runtime-suspend of the other
consumer.

Instead I'm thinking about adding a clk_decrement_disable_unprepare_count()
function which goes through all the normal clk_disable_unprepare() motions
except for actually calling the clock-driver's disable() and unprepare()
callbacks (for both the clock itself and its parents).

I'll probably give that a shot tomorrow.

Or since just setting CLK_IGNORE_UNUSED works for qcom (no shared
parents, or at least none with runtime suspending other consumers?),
just patch dispcc-x1e80100.c

Regards,

Hans