diff mbox series

[v7,22/23] drm/mediatek: Power on devices in OVL adaptor when atomic enable

Message ID 20231006073831.10402-23-shawn.sung@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Add display driver for MT8188 VDOSYS1 | expand

Commit Message

Shawn Sung (宋孝謙) Oct. 6, 2023, 7:38 a.m. UTC
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@mediatek.com>
---
 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(+)

Comments

CK Hu (胡俊光) Oct. 6, 2023, 9:47 a.m. UTC | #1
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@mediatek.com>
> ---
>  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)
diff mbox series

Patch

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;
+		}
+	}
+
 	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)