dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] More IPU cleanups v2
@ 2020-07-30 14:48 Paul Cercueil
  2020-07-30 14:48 ` [PATCH v2 1/3] drm/ingenic: ipu: Only restart manually on older SoCs Paul Cercueil
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Paul Cercueil @ 2020-07-30 14:48 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter
  Cc: Paul Cercueil, od, Sam Ravnborg, linux-kernel, dri-devel

Patch [1/3] unchanged. Patches [2/3] and [3/3] have had their commit
message slightly modified, but the patches themselves are the same as
before.

Cheers,
-Paul

Paul Cercueil (3):
  drm/ingenic: ipu: Only restart manually on older SoCs
  drm/ingenic: ipu: Remove YUV422 from supported formats on JZ4725B
  drm/ingenic: ipu: Only enable clock when needed

 drivers/gpu/drm/ingenic/ingenic-ipu.c | 38 +++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 5 deletions(-)

-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] drm/ingenic: ipu: Only restart manually on older SoCs
  2020-07-30 14:48 [PATCH v2 0/3] More IPU cleanups v2 Paul Cercueil
@ 2020-07-30 14:48 ` Paul Cercueil
  2020-07-30 14:48 ` [PATCH v2 2/3] drm/ingenic: ipu: Remove YUV422 from supported formats on JZ4725B Paul Cercueil
  2020-07-30 14:48 ` [PATCH v2 3/3] drm/ingenic: ipu: Only enable clock when needed Paul Cercueil
  2 siblings, 0 replies; 8+ messages in thread
From: Paul Cercueil @ 2020-07-30 14:48 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter
  Cc: Paul Cercueil, od, Sam Ravnborg, linux-kernel, dri-devel

On older SoCs, it is necessary to restart manually the IPU when a frame
is done processing. Doing so on newer SoCs (JZ4760/70) kinds of work
too, until the input or output resolutions or the framerate are too
high.

Make it work properly on newer SoCs by letting the LCD controller
trigger the IPU frame restart signal.

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-ipu.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index 7a0a8bd865d3..7eae56fa92ea 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -35,6 +35,7 @@ struct soc_info {
 	const u32 *formats;
 	size_t num_formats;
 	bool has_bicubic;
+	bool manual_restart;
 
 	void (*set_coefs)(struct ingenic_ipu *ipu, unsigned int reg,
 			  unsigned int sharpness, bool downscale,
@@ -645,7 +646,8 @@ static irqreturn_t ingenic_ipu_irq_handler(int irq, void *arg)
 	unsigned int dummy;
 
 	/* dummy read allows CPU to reconfigure IPU */
-	regmap_read(ipu->map, JZ_REG_IPU_STATUS, &dummy);
+	if (ipu->soc_info->manual_restart)
+		regmap_read(ipu->map, JZ_REG_IPU_STATUS, &dummy);
 
 	/* ACK interrupt */
 	regmap_write(ipu->map, JZ_REG_IPU_STATUS, 0);
@@ -656,7 +658,8 @@ static irqreturn_t ingenic_ipu_irq_handler(int irq, void *arg)
 	regmap_write(ipu->map, JZ_REG_IPU_V_ADDR, ipu->addr_v);
 
 	/* Run IPU for the new frame */
-	regmap_set_bits(ipu->map, JZ_REG_IPU_CTRL, JZ_IPU_CTRL_RUN);
+	if (ipu->soc_info->manual_restart)
+		regmap_set_bits(ipu->map, JZ_REG_IPU_CTRL, JZ_IPU_CTRL_RUN);
 
 	drm_crtc_handle_vblank(crtc);
 
@@ -806,6 +809,7 @@ static const struct soc_info jz4725b_soc_info = {
 	.formats	= jz4725b_ipu_formats,
 	.num_formats	= ARRAY_SIZE(jz4725b_ipu_formats),
 	.has_bicubic	= false,
+	.manual_restart	= true,
 	.set_coefs	= jz4725b_set_coefs,
 };
 
@@ -831,6 +835,7 @@ static const struct soc_info jz4760_soc_info = {
 	.formats	= jz4760_ipu_formats,
 	.num_formats	= ARRAY_SIZE(jz4760_ipu_formats),
 	.has_bicubic	= true,
+	.manual_restart	= false,
 	.set_coefs	= jz4760_set_coefs,
 };
 
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] drm/ingenic: ipu: Remove YUV422 from supported formats on JZ4725B
  2020-07-30 14:48 [PATCH v2 0/3] More IPU cleanups v2 Paul Cercueil
  2020-07-30 14:48 ` [PATCH v2 1/3] drm/ingenic: ipu: Only restart manually on older SoCs Paul Cercueil
@ 2020-07-30 14:48 ` Paul Cercueil
  2020-07-30 15:29   ` Sam Ravnborg
  2020-07-30 14:48 ` [PATCH v2 3/3] drm/ingenic: ipu: Only enable clock when needed Paul Cercueil
  2 siblings, 1 reply; 8+ messages in thread
From: Paul Cercueil @ 2020-07-30 14:48 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter
  Cc: Paul Cercueil, od, Sam Ravnborg, linux-kernel, dri-devel

When configuring the IPU for packed YUV 4:2:2, depending on the scaling
ratios given by the source and destination resolutions, it is possible
to crash the IPU block, to the point where a software reset of the IP
does not fix it. This can happen anytime, in the first few frames, or
after dozens of minutes. The same crash also happens when the IPU is
fully controlled by the LCD controller (in that case no HW register is
written at any moment after startup), which points towards a hardware
bug.

Thanksfully multiplanar YUV is not affected.

Until this bug is fixed or worked around, address this issue by removing
support for YUV 4:2:2 on the IPU of the JZ4725B.

v2: Update commit message (remove the "crash beyond repair" bit)

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-ipu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index 7eae56fa92ea..7dd2a6ae4994 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -795,10 +795,16 @@ static int ingenic_ipu_remove(struct platform_device *pdev)
 }
 
 static const u32 jz4725b_ipu_formats[] = {
+	/*
+	 * While officially supported, packed YUV 4:2:2 formats can cause
+	 * random hardware crashes on JZ4725B under certain circumstances.
+	 * It seems to happen with some specific resize ratios.
+	 * Until a proper workaround or fix is found, disable these formats.
 	DRM_FORMAT_YUYV,
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
+	*/
 	DRM_FORMAT_YUV411,
 	DRM_FORMAT_YUV420,
 	DRM_FORMAT_YUV422,
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] drm/ingenic: ipu: Only enable clock when needed
  2020-07-30 14:48 [PATCH v2 0/3] More IPU cleanups v2 Paul Cercueil
  2020-07-30 14:48 ` [PATCH v2 1/3] drm/ingenic: ipu: Only restart manually on older SoCs Paul Cercueil
  2020-07-30 14:48 ` [PATCH v2 2/3] drm/ingenic: ipu: Remove YUV422 from supported formats on JZ4725B Paul Cercueil
@ 2020-07-30 14:48 ` Paul Cercueil
  2020-07-30 15:29   ` Sam Ravnborg
  2 siblings, 1 reply; 8+ messages in thread
From: Paul Cercueil @ 2020-07-30 14:48 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter
  Cc: Paul Cercueil, od, Sam Ravnborg, linux-kernel, dri-devel

Instead of keeping the IPU clock enabled constantly, enable and disable
it on demand, when the IPU plane is used. That way, we won't use any
extra power when the IPU is not used.

v2: Explain the reason of this patch

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-ipu.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index 7dd2a6ae4994..fc8c6e970ee3 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -49,6 +49,7 @@ struct ingenic_ipu {
 	struct regmap *map;
 	struct clk *clk;
 	const struct soc_info *soc_info;
+	bool clk_enabled;
 
 	unsigned int num_w, num_h, denom_w, denom_h;
 
@@ -288,12 +289,23 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane,
 	const struct drm_format_info *finfo;
 	u32 ctrl, stride = 0, coef_index = 0, format = 0;
 	bool needs_modeset, upscaling_w, upscaling_h;
+	int err;
 
 	if (!state || !state->fb)
 		return;
 
 	finfo = drm_format_info(state->fb->format->format);
 
+	if (!ipu->clk_enabled) {
+		err = clk_enable(ipu->clk);
+		if (err) {
+			dev_err(ipu->dev, "Unable to enable clock: %d\n", err);
+			return;
+		}
+
+		ipu->clk_enabled = true;
+	}
+
 	/* Reset all the registers if needed */
 	needs_modeset = drm_atomic_crtc_needs_modeset(state->crtc->state);
 	if (needs_modeset) {
@@ -578,6 +590,11 @@ static void ingenic_ipu_plane_atomic_disable(struct drm_plane *plane,
 	regmap_clear_bits(ipu->map, JZ_REG_IPU_CTRL, JZ_IPU_CTRL_CHIP_EN);
 
 	ingenic_drm_plane_disable(ipu->master, plane);
+
+	if (ipu->clk_enabled) {
+		clk_disable(ipu->clk);
+		ipu->clk_enabled = false;
+	}
 }
 
 static const struct drm_plane_helper_funcs ingenic_ipu_plane_helper_funcs = {
@@ -761,9 +778,9 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d)
 	drm_object_attach_property(&plane->base, ipu->sharpness_prop,
 				   ipu->sharpness);
 
-	err = clk_prepare_enable(ipu->clk);
+	err = clk_prepare(ipu->clk);
 	if (err) {
-		dev_err(dev, "Unable to enable clock\n");
+		dev_err(dev, "Unable to prepare clock\n");
 		return err;
 	}
 
@@ -775,7 +792,7 @@ static void ingenic_ipu_unbind(struct device *dev,
 {
 	struct ingenic_ipu *ipu = dev_get_drvdata(dev);
 
-	clk_disable_unprepare(ipu->clk);
+	clk_unprepare(ipu->clk);
 }
 
 static const struct component_ops ingenic_ipu_ops = {
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] drm/ingenic: ipu: Remove YUV422 from supported formats on JZ4725B
  2020-07-30 14:48 ` [PATCH v2 2/3] drm/ingenic: ipu: Remove YUV422 from supported formats on JZ4725B Paul Cercueil
@ 2020-07-30 15:29   ` Sam Ravnborg
  0 siblings, 0 replies; 8+ messages in thread
From: Sam Ravnborg @ 2020-07-30 15:29 UTC (permalink / raw)
  To: Paul Cercueil; +Cc: David Airlie, od, dri-devel, linux-kernel

Hi Paul

On Thu, Jul 30, 2020 at 04:48:29PM +0200, Paul Cercueil wrote:
> When configuring the IPU for packed YUV 4:2:2, depending on the scaling
> ratios given by the source and destination resolutions, it is possible
> to crash the IPU block, to the point where a software reset of the IP
> does not fix it. This can happen anytime, in the first few frames, or
> after dozens of minutes. The same crash also happens when the IPU is
> fully controlled by the LCD controller (in that case no HW register is
> written at any moment after startup), which points towards a hardware
> bug.
> 
> Thanksfully multiplanar YUV is not affected.
> 
> Until this bug is fixed or worked around, address this issue by removing
> support for YUV 4:2:2 on the IPU of the JZ4725B.
> 
> v2: Update commit message (remove the "crash beyond repair" bit)
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/gpu/drm/ingenic/ingenic-ipu.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> index 7eae56fa92ea..7dd2a6ae4994 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> @@ -795,10 +795,16 @@ static int ingenic_ipu_remove(struct platform_device *pdev)
>  }
>  
>  static const u32 jz4725b_ipu_formats[] = {
> +	/*
> +	 * While officially supported, packed YUV 4:2:2 formats can cause
> +	 * random hardware crashes on JZ4725B under certain circumstances.
> +	 * It seems to happen with some specific resize ratios.
> +	 * Until a proper workaround or fix is found, disable these formats.
>  	DRM_FORMAT_YUYV,
>  	DRM_FORMAT_YVYU,
>  	DRM_FORMAT_UYVY,
>  	DRM_FORMAT_VYUY,
> +	*/
>  	DRM_FORMAT_YUV411,
>  	DRM_FORMAT_YUV420,
>  	DRM_FORMAT_YUV422,
> -- 
> 2.27.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] drm/ingenic: ipu: Only enable clock when needed
  2020-07-30 14:48 ` [PATCH v2 3/3] drm/ingenic: ipu: Only enable clock when needed Paul Cercueil
@ 2020-07-30 15:29   ` Sam Ravnborg
  2020-07-30 16:21     ` Paul Cercueil
  0 siblings, 1 reply; 8+ messages in thread
From: Sam Ravnborg @ 2020-07-30 15:29 UTC (permalink / raw)
  To: Paul Cercueil; +Cc: David Airlie, od, dri-devel, linux-kernel

On Thu, Jul 30, 2020 at 04:48:30PM +0200, Paul Cercueil wrote:
> Instead of keeping the IPU clock enabled constantly, enable and disable
> it on demand, when the IPU plane is used. That way, we won't use any
> extra power when the IPU is not used.
> 
> v2: Explain the reason of this patch
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

And thanks for the quick update!

	Sam

> ---
>  drivers/gpu/drm/ingenic/ingenic-ipu.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> index 7dd2a6ae4994..fc8c6e970ee3 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> @@ -49,6 +49,7 @@ struct ingenic_ipu {
>  	struct regmap *map;
>  	struct clk *clk;
>  	const struct soc_info *soc_info;
> +	bool clk_enabled;
>  
>  	unsigned int num_w, num_h, denom_w, denom_h;
>  
> @@ -288,12 +289,23 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane,
>  	const struct drm_format_info *finfo;
>  	u32 ctrl, stride = 0, coef_index = 0, format = 0;
>  	bool needs_modeset, upscaling_w, upscaling_h;
> +	int err;
>  
>  	if (!state || !state->fb)
>  		return;
>  
>  	finfo = drm_format_info(state->fb->format->format);
>  
> +	if (!ipu->clk_enabled) {
> +		err = clk_enable(ipu->clk);
> +		if (err) {
> +			dev_err(ipu->dev, "Unable to enable clock: %d\n", err);
> +			return;
> +		}
> +
> +		ipu->clk_enabled = true;
> +	}
> +
>  	/* Reset all the registers if needed */
>  	needs_modeset = drm_atomic_crtc_needs_modeset(state->crtc->state);
>  	if (needs_modeset) {
> @@ -578,6 +590,11 @@ static void ingenic_ipu_plane_atomic_disable(struct drm_plane *plane,
>  	regmap_clear_bits(ipu->map, JZ_REG_IPU_CTRL, JZ_IPU_CTRL_CHIP_EN);
>  
>  	ingenic_drm_plane_disable(ipu->master, plane);
> +
> +	if (ipu->clk_enabled) {
> +		clk_disable(ipu->clk);
> +		ipu->clk_enabled = false;
> +	}
>  }
>  
>  static const struct drm_plane_helper_funcs ingenic_ipu_plane_helper_funcs = {
> @@ -761,9 +778,9 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d)
>  	drm_object_attach_property(&plane->base, ipu->sharpness_prop,
>  				   ipu->sharpness);
>  
> -	err = clk_prepare_enable(ipu->clk);
> +	err = clk_prepare(ipu->clk);
>  	if (err) {
> -		dev_err(dev, "Unable to enable clock\n");
> +		dev_err(dev, "Unable to prepare clock\n");
>  		return err;
>  	}
>  
> @@ -775,7 +792,7 @@ static void ingenic_ipu_unbind(struct device *dev,
>  {
>  	struct ingenic_ipu *ipu = dev_get_drvdata(dev);
>  
> -	clk_disable_unprepare(ipu->clk);
> +	clk_unprepare(ipu->clk);
>  }
>  
>  static const struct component_ops ingenic_ipu_ops = {
> -- 
> 2.27.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] drm/ingenic: ipu: Only enable clock when needed
  2020-07-30 15:29   ` Sam Ravnborg
@ 2020-07-30 16:21     ` Paul Cercueil
  2020-08-04  9:31       ` Daniel Vetter
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Cercueil @ 2020-07-30 16:21 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: David Airlie, od, dri-devel, linux-kernel



Le jeu. 30 juil. 2020 à 17:29, Sam Ravnborg <sam@ravnborg.org> a 
écrit :
> On Thu, Jul 30, 2020 at 04:48:30PM +0200, Paul Cercueil wrote:
>>  Instead of keeping the IPU clock enabled constantly, enable and 
>> disable
>>  it on demand, when the IPU plane is used. That way, we won't use any
>>  extra power when the IPU is not used.
>> 
>>  v2: Explain the reason of this patch
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> And thanks for the quick update!

Pushed to drm-misc-next. Thanks!

Cheers,
-Paul

> 
> 	Sam
> 
>>  ---
>>   drivers/gpu/drm/ingenic/ingenic-ipu.c | 23 ++++++++++++++++++++---
>>   1 file changed, 20 insertions(+), 3 deletions(-)
>> 
>>  diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c 
>> b/drivers/gpu/drm/ingenic/ingenic-ipu.c
>>  index 7dd2a6ae4994..fc8c6e970ee3 100644
>>  --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
>>  +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
>>  @@ -49,6 +49,7 @@ struct ingenic_ipu {
>>   	struct regmap *map;
>>   	struct clk *clk;
>>   	const struct soc_info *soc_info;
>>  +	bool clk_enabled;
>> 
>>   	unsigned int num_w, num_h, denom_w, denom_h;
>> 
>>  @@ -288,12 +289,23 @@ static void 
>> ingenic_ipu_plane_atomic_update(struct drm_plane *plane,
>>   	const struct drm_format_info *finfo;
>>   	u32 ctrl, stride = 0, coef_index = 0, format = 0;
>>   	bool needs_modeset, upscaling_w, upscaling_h;
>>  +	int err;
>> 
>>   	if (!state || !state->fb)
>>   		return;
>> 
>>   	finfo = drm_format_info(state->fb->format->format);
>> 
>>  +	if (!ipu->clk_enabled) {
>>  +		err = clk_enable(ipu->clk);
>>  +		if (err) {
>>  +			dev_err(ipu->dev, "Unable to enable clock: %d\n", err);
>>  +			return;
>>  +		}
>>  +
>>  +		ipu->clk_enabled = true;
>>  +	}
>>  +
>>   	/* Reset all the registers if needed */
>>   	needs_modeset = drm_atomic_crtc_needs_modeset(state->crtc->state);
>>   	if (needs_modeset) {
>>  @@ -578,6 +590,11 @@ static void 
>> ingenic_ipu_plane_atomic_disable(struct drm_plane *plane,
>>   	regmap_clear_bits(ipu->map, JZ_REG_IPU_CTRL, JZ_IPU_CTRL_CHIP_EN);
>> 
>>   	ingenic_drm_plane_disable(ipu->master, plane);
>>  +
>>  +	if (ipu->clk_enabled) {
>>  +		clk_disable(ipu->clk);
>>  +		ipu->clk_enabled = false;
>>  +	}
>>   }
>> 
>>   static const struct drm_plane_helper_funcs 
>> ingenic_ipu_plane_helper_funcs = {
>>  @@ -761,9 +778,9 @@ static int ingenic_ipu_bind(struct device *dev, 
>> struct device *master, void *d)
>>   	drm_object_attach_property(&plane->base, ipu->sharpness_prop,
>>   				   ipu->sharpness);
>> 
>>  -	err = clk_prepare_enable(ipu->clk);
>>  +	err = clk_prepare(ipu->clk);
>>   	if (err) {
>>  -		dev_err(dev, "Unable to enable clock\n");
>>  +		dev_err(dev, "Unable to prepare clock\n");
>>   		return err;
>>   	}
>> 
>>  @@ -775,7 +792,7 @@ static void ingenic_ipu_unbind(struct device 
>> *dev,
>>   {
>>   	struct ingenic_ipu *ipu = dev_get_drvdata(dev);
>> 
>>  -	clk_disable_unprepare(ipu->clk);
>>  +	clk_unprepare(ipu->clk);
>>   }
>> 
>>   static const struct component_ops ingenic_ipu_ops = {
>>  --
>>  2.27.0


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] drm/ingenic: ipu: Only enable clock when needed
  2020-07-30 16:21     ` Paul Cercueil
@ 2020-08-04  9:31       ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2020-08-04  9:31 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: David Airlie, Linux Kernel Mailing List, dri-devel, od, Sam Ravnborg

On Thu, Jul 30, 2020 at 06:21:05PM +0200, Paul Cercueil wrote:
>
>
> Le jeu. 30 juil. 2020 à 17:29, Sam Ravnborg <sam@ravnborg.org> a écrit :
> > On Thu, Jul 30, 2020 at 04:48:30PM +0200, Paul Cercueil wrote:
> > >  Instead of keeping the IPU clock enabled constantly, enable and
> > > disable
> > >  it on demand, when the IPU plane is used. That way, we won't use any
> > >  extra power when the IPU is not used.
> > >
> > >  v2: Explain the reason of this patch
> > >
> > >  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> >
> > And thanks for the quick update!
>
> Pushed to drm-misc-next. Thanks!

Hi Paul,

Maybe I'm mixing up people, but iirc you're the one very much interested
in running fbdev userspace. Did you think about building up a set of
regression tests in igt (which we could eventually even run on something
like vkms for hw-free regression testing) so that we make sure fbdev keeps
wroking?

Defacto drm_fb_helper.c has become the fbdev reference implementation
already anyway (e.g. with stuff like stanardizing vblank waiting
behaviour), so this would only be the next logical step.

Also adding Bart.

Cheers, Daniel


>
> Cheers,
> -Paul
>
> >
> >     Sam
> >
> > >  ---
> > >   drivers/gpu/drm/ingenic/ingenic-ipu.c | 23 ++++++++++++++++++++---
> > >   1 file changed, 20 insertions(+), 3 deletions(-)
> > >
> > >  diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> > > b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> > >  index 7dd2a6ae4994..fc8c6e970ee3 100644
> > >  --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> > >  +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> > >  @@ -49,6 +49,7 @@ struct ingenic_ipu {
> > >           struct regmap *map;
> > >           struct clk *clk;
> > >           const struct soc_info *soc_info;
> > >  +        bool clk_enabled;
> > >
> > >           unsigned int num_w, num_h, denom_w, denom_h;
> > >
> > >  @@ -288,12 +289,23 @@ static void
> > > ingenic_ipu_plane_atomic_update(struct drm_plane *plane,
> > >           const struct drm_format_info *finfo;
> > >           u32 ctrl, stride = 0, coef_index = 0, format = 0;
> > >           bool needs_modeset, upscaling_w, upscaling_h;
> > >  +        int err;
> > >
> > >           if (!state || !state->fb)
> > >                   return;
> > >
> > >           finfo = drm_format_info(state->fb->format->format);
> > >
> > >  +        if (!ipu->clk_enabled) {
> > >  +                err = clk_enable(ipu->clk);
> > >  +                if (err) {
> > >  +                        dev_err(ipu->dev, "Unable to enable clock: %d\n", err);
> > >  +                        return;
> > >  +                }
> > >  +
> > >  +                ipu->clk_enabled = true;
> > >  +        }
> > >  +
> > >           /* Reset all the registers if needed */
> > >           needs_modeset = drm_atomic_crtc_needs_modeset(state->crtc->state);
> > >           if (needs_modeset) {
> > >  @@ -578,6 +590,11 @@ static void
> > > ingenic_ipu_plane_atomic_disable(struct drm_plane *plane,
> > >           regmap_clear_bits(ipu->map, JZ_REG_IPU_CTRL, JZ_IPU_CTRL_CHIP_EN);
> > >
> > >           ingenic_drm_plane_disable(ipu->master, plane);
> > >  +
> > >  +        if (ipu->clk_enabled) {
> > >  +                clk_disable(ipu->clk);
> > >  +                ipu->clk_enabled = false;
> > >  +        }
> > >   }
> > >
> > >   static const struct drm_plane_helper_funcs
> > > ingenic_ipu_plane_helper_funcs = {
> > >  @@ -761,9 +778,9 @@ static int ingenic_ipu_bind(struct device *dev,
> > > struct device *master, void *d)
> > >           drm_object_attach_property(&plane->base, ipu->sharpness_prop,
> > >                                      ipu->sharpness);
> > >
> > >  -        err = clk_prepare_enable(ipu->clk);
> > >  +        err = clk_prepare(ipu->clk);
> > >           if (err) {
> > >  -                dev_err(dev, "Unable to enable clock\n");
> > >  +                dev_err(dev, "Unable to prepare clock\n");
> > >                   return err;
> > >           }
> > >
> > >  @@ -775,7 +792,7 @@ static void ingenic_ipu_unbind(struct device
> > > *dev,
> > >   {
> > >           struct ingenic_ipu *ipu = dev_get_drvdata(dev);
> > >
> > >  -        clk_disable_unprepare(ipu->clk);
> > >  +        clk_unprepare(ipu->clk);
> > >   }
> > >
> > >   static const struct component_ops ingenic_ipu_ops = {
> > >  --
> > >  2.27.0
>
>

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-08-04  9:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 14:48 [PATCH v2 0/3] More IPU cleanups v2 Paul Cercueil
2020-07-30 14:48 ` [PATCH v2 1/3] drm/ingenic: ipu: Only restart manually on older SoCs Paul Cercueil
2020-07-30 14:48 ` [PATCH v2 2/3] drm/ingenic: ipu: Remove YUV422 from supported formats on JZ4725B Paul Cercueil
2020-07-30 15:29   ` Sam Ravnborg
2020-07-30 14:48 ` [PATCH v2 3/3] drm/ingenic: ipu: Only enable clock when needed Paul Cercueil
2020-07-30 15:29   ` Sam Ravnborg
2020-07-30 16:21     ` Paul Cercueil
2020-08-04  9:31       ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).