Re: [PATCH v2 07/14] drm/imx: dc: Add DPR channel support

[Liu Ying <victor.liu@nxp.com>]

On 09/23/2025, Frank Li wrote:
> On Tue, Sep 23, 2025 at 10:07:57AM +0800, Liu Ying wrote:

[...]

>> +void dc_dprc_disable_at_boot(struct dc_dprc *dprc)
>> +{
>> +	dc_prg_disable_at_boot(dprc->prg);
>> +
>> +	clk_bulk_disable_unprepare(dprc->num_clks, dprc->clks);
>> +
> 
> you have runtime functions dc_dprc_runtime_suspend()
> 
> If runtime pm status is correct, needn't call clk_bulk_disable_unprepare().

The problem is that once drm_dev_unplug() is called from dc_drm_unbind()
due to imx8_dc_drm module removal, the RPM status of DC, DPRC and PRG is
"unsupported", which doesn't reflect the fact that the display controller
is running if it is before the removal.  When the module is installed
again, these clocks need to be disabled out of the RPM management to
avoid clock enable/prepare count leaks if necessary.

See kerneldoc in drm_drv.c:
/*
 * DOC: driver instance overview

...

 * Drivers that want to support device unplugging (USB, DT overlay unload) should
 * use drm_dev_unplug() instead of drm_dev_unregister(). The driver must protect
 * regions that is accessing device resources to prevent use after they're
 * released. This is done using drm_dev_enter() and drm_dev_exit(). There is one
 * shortcoming however, drm_dev_unplug() marks the drm_device as unplugged before
 * drm_atomic_helper_shutdown() is called. This means that if the disable code
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * paths are protected, they will not run on regular driver module unload,
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * possibly leaving the hardware enabled.
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 */

> 
> Look like call pm_runtime_put() here to let runtime pm management clks.
> 
> otherwise, runtime pm state will not match clock enable/disable state.
> 
>> +	dev_dbg(dprc->dev, "disable at boot\n");
>> +}


-- 
Regards,
Liu Ying