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,

On 26-Apr-26 20:20, Hans de Goede wrote:
> 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.

I've implemented this solution now and this seems to work nicely, see:

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

Regards,

Hans