<pre>
Hi Angelo,

On Thu, 2023-10-19 at 11:07 +0200, AngeloGioacchino Del Regno wrote:
> Il 19/10/23 07:56, Hsiao Chien Sung ha scritto:
> > Display modules will be powered on when .atomic_enable(),
> > there is no need to do it again in mtk_crtc_ddp_hw_init().
> > Besides, the DRM devices are created manually when mtk-mmsys
> > is probed and there is no power domain linked to it.
> >
> > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC
> > MT8173.")
> >
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 18 +++---------------
> > 1 file changed, 3 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index bc4cc75cca18..c7edd80be428 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -6,7 +6,6 @@
> > #include <linux/clk.h>
> > #include <linux/dma-mapping.h>
> > #include <linux/mailbox_controller.h>
> > -#include <linux/pm_runtime.h>
> > #include <linux/soc/mediatek/mtk-cmdq.h>
> > #include <linux/soc/mediatek/mtk-mmsys.h>
> > #include <linux/soc/mediatek/mtk-mutex.h>
> > @@ -362,22 +361,16 @@ static int mtk_crtc_ddp_hw_init(struct
> > mtk_drm_crtc *mtk_crtc, struct drm_atomic
> > drm_connector_list_iter_end(&conn_iter);
> > }
> >
> > -ret = pm_runtime_resume_and_get(crtc->dev->dev);
> > -if (ret < 0) {
> > -DRM_ERROR("Failed to enable power domain: %d\n", ret);
> > -return ret;
> > -}
> > -
>
> Are you really sure that writes to DISP_REG_OVL_xxx and others in
> other modules,
> called by the .layer_config() callback, can be successfully done on
> an unpowered
> and/or unclocked module, on all MediaTek SoCs?
> This looks a bit odd.

Not sure if I get your point correctly. We removed this PM API because:

1. mtk_crtc_ddp_hw_init() is called by mtk_drm_crtc_atomic_enable(),
and the new inline function mtk_ddp_comp_power_on() is called before hw
init, we can make sure the power is on before configuring the hardware.

2. The device "crtc->dev->dev" here is assigned by the probe function
of mtk-mmsys, which will be look like "mediatek-drm.auto.13", and this
device is not linked to any power domain.

>
> > ret = mtk_mutex_prepare(mtk_crtc->mutex);
> > if (ret < 0) {
> > DRM_ERROR("Failed to enable mutex clock: %d\n", ret);
> > -goto err_pm_runtime_put;
> > +goto error;
> > }
> >
> > ret = mtk_crtc_ddp_clk_enable(mtk_crtc);
> > if (ret < 0) {
> > DRM_ERROR("Failed to enable component clocks: %d\n",
> > ret);
> > -goto err_mutex_unprepare;
> > +goto error;
> > }
> >
> > for (i = 0; i < mtk_crtc->ddp_comp_nr - 1; i++) {
> > @@ -426,16 +419,13 @@ static int mtk_crtc_ddp_hw_init(struct
> > mtk_drm_crtc *mtk_crtc, struct drm_atomic
> >
>
> ...because you could otherwise just call pm_runtime_put() here,
> instead of removing
> the pm_runtime_resume_and_get() call, which is something I would
> advise to do.
>
> Regards,
> Angelo
>

Thanks,
Shawn

</pre><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any 
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be 
conveyed only to the designated recipient(s). Any use, dissemination, 
distribution, printing, retaining or copying of this e-mail (including its 
attachments) by unintended recipient(s) is strictly prohibited and may 
be unlawful. If you are not an intended recipient of this e-mail, or believe 
that you have received this e-mail in error, please notify the sender 
immediately (by replying to this e-mail), delete any and all copies of 
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->