Hi, Hsiao-chien:
On Fri, 2023-10-06 at 15:38 +0800, Hsiao Chien Sung wrote:
> Different from OVL, OVL adaptor is a pseudo device so we didn't
> define it in the device tree, consequently,
> pm_runtime_resume_and_get()
> called by .atomic_enable() powers on no device in OVL adaptor and
> leads to power outage in the corresponding IOMMU.
>
> To resolve the issue, we implement a function to power on the RDMAs
> in OVL adaptor, and the system will make sure the IOMMU is powered on
> as well because of the device link (iommus) in the RDMA nodes in DTS.
>
> Fixes: 5db12f5d843b ("media: drm/mediatek: Add pm runtime support for
> ovl and rdma")
>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 +
> .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 49
> +++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 9 ++++
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 1 +
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 9 ++++
> 5 files changed, 69 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 45b30a2fe11a..971d64261fb9 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -107,6 +107,7 @@ void mtk_ovl_adaptor_connect(struct device *dev,
> struct device *mmsys_dev,
> unsigned int next);
> void mtk_ovl_adaptor_disconnect(struct device *dev, struct device
> *mmsys_dev,
> unsigned int next);
> +int mtk_ovl_adaptor_power_on(struct device *dev);
> int mtk_ovl_adaptor_clk_enable(struct device *dev);
> void mtk_ovl_adaptor_clk_disable(struct device *dev);
> void mtk_ovl_adaptor_config(struct device *dev, unsigned int w,
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> index c326a658dc63..ae3b6ba655b1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> @@ -242,6 +242,55 @@ void mtk_ovl_adaptor_stop(struct device *dev)
> }
> }
>
> +/**
> + * mtk_ovl_adaptor_power_on - Power on devices in OVL adaptor
> + * @dev: device to be powered on
> + *
> + * Different from OVL, OVL adaptor is a pseudo device so
> + * we didn't define it in the device tree,
> pm_runtime_resume_and_get()
> + * called by .atomic_enable() power on no device in OVL adaptor,
> + * we have to implement a function to do the job instead.
> + *
> + * returns:
> + * zero on success, errno on failure.
> + */
> +int mtk_ovl_adaptor_power_on(struct device *dev)
> +{
> + int i, ret;
> + struct device *comp;
> + struct mtk_disp_ovl_adaptor *ovl_adaptor =
> dev_get_drvdata(dev);
> +
> + for (i = 0; i < OVL_ADAPTOR_ID_MAX; i++) {
> + comp = ovl_adaptor->ovl_adaptor_comp[i];
> +
> + if (!comp)
> + continue;
> +
> + if (comp_matches[i].type != OVL_ADAPTOR_TYPE_MDP_RDMA)
> + continue;
> +
> + ret = pm_runtime_resume_and_get(comp);
> + if (ret < 0) {
> + dev_err(dev, "Failed to power on comp(%u):
> %d\n", i, ret);
> + goto error;
> + }
> + }
> + return 0;
> +error:
> + while (--i >= 0) {
> + comp = ovl_adaptor->ovl_adaptor_comp[i];
> +
> + if (!comp)
> + continue;
> +
> + if (comp_matches[i].type != OVL_ADAPTOR_TYPE_MDP_RDMA)
> + continue;
> +
> + pm_runtime_put(comp);
> + }
> + return ret;
> +}
> +
> int mtk_ovl_adaptor_clk_enable(struct device *dev)
> {
> int i;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index b6fa4ad2f94d..5bd62027190b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -364,6 +364,15 @@ static int mtk_crtc_ddp_hw_init(struct
> mtk_drm_crtc *mtk_crtc)
> return ret;
> }
>
> + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> + ret = mtk_ddp_comp_power_on(mtk_crtc->ddp_comp[i]);
> + if (ret) {
> + DRM_ERROR("Failed to power on %s: %d\n",
> + dev_name(mtk_crtc->ddp_comp[i]->dev),
> ret);
> + return ret;
> + }
> + }
I would like to replace below statement in mtk_drm_crtc_atomic_enable()
ret = pm_runtime_resume_and_get(comp-dev);
with
ret = mtk_ddp_comp_power_on(comp);
And define mtk_ddp_comp_power_on() as
static inline int mtk_ddp_comp_power_on(struct mtk_ddp_comp *comp)
{
if (comp->funcs && comp->funcs->power_on)
return comp->funcs->power_on(comp->dev);
else
return pm_runtime_resume_and_get(comp-dev);
}
And you should also define mtk_ddp_comp_power_off().
Regards,
CK
> +
> ret = mtk_mutex_prepare(mtk_crtc->mutex);
> if (ret < 0) {
> DRM_ERROR("Failed to enable mutex clock: %d\n", ret);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 771f4e173353..e39860f2be78 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -394,6 +394,7 @@ static const struct mtk_ddp_comp_funcs ddp_ufoe =
> {
> };
>
> static const struct mtk_ddp_comp_funcs ddp_ovl_adaptor = {
> + .power_on = mtk_ovl_adaptor_power_on,
> .clk_enable = mtk_ovl_adaptor_clk_enable,
> .clk_disable = mtk_ovl_adaptor_clk_disable,
> .config = mtk_ovl_adaptor_config,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index febcaeef16a1..4fef283f17d2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -46,6 +46,7 @@ enum mtk_ddp_comp_type {
> struct mtk_ddp_comp;
> struct cmdq_pkt;
> struct mtk_ddp_comp_funcs {
> + int (*power_on)(struct device *dev);
> int (*clk_enable)(struct device *dev);
> void (*clk_disable)(struct device *dev);
> void (*config)(struct device *dev, unsigned int w,
> @@ -89,6 +90,14 @@ struct mtk_ddp_comp {
> const struct mtk_ddp_comp_funcs *funcs;
> };
>
> +static inline int mtk_ddp_comp_power_on(struct mtk_ddp_comp *comp)
> +{
> + if (comp->funcs && comp->funcs->power_on)
> + return comp->funcs->power_on(comp->dev);
> +
> + return 0;
> +}
> +
> static inline int mtk_ddp_comp_clk_enable(struct mtk_ddp_comp *comp)
> {
> if (comp->funcs && comp->funcs->clk_enable)
