On 29/08/2023 10:55, Laurent Pinchart wrote:
Hi Jinjie,
(CC'ing Tomi)
Thank you for the patch.
On Fri, Aug 25, 2023 at 03:23:24PM +0800, Jinjie Ruan wrote:
The devm_clk_get_enabled() helper:
- calls devm_clk_get()
- calls clk_prepare_enable() and registers what is needed in order to
call clk_disable_unprepare() when needed, as a managed resource.
This simplifies the code.
While this indeed simplifies the code, I think we should instead control
the clock dynamically at runtime.
I don't have access to the hardware at the moment. Tomi, would you be
able to give this a go ? I can also write a patch and let you test it if
desired.
I have a small patch that adds runtime resume & suspend callbacks, and
enables & disables the clock there. But the driver doesn't seem to do a
proper job of power management:
- It accesses registers without pm_runtime_get
- It initializes registers at probe time, apparently presuming that the
IP is never turned off, which might cause registers getting reset.
- It uses pm_runtime_get in a couple of places where it's starting the
video stream, but it seems to forget that there's also the DP AUX and HPD.
Then again, as the HPD is, I think, supposed to be always enabled, the
device is also always enabled, making the PM management a bit pointless.
I'll look at this a bit more to see if I can sort this out.
Tomi
