diff mbox

[2/2] drm/exynos: solve plane on/off disharmory issue

Message ID 1422530131-18122-2-git-send-email-jy0922.shim@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joonyoung Shim Jan. 29, 2015, 11:15 a.m. UTC
The exynos_update_plane functions can be called from set_plane as well
as set_crtc and pageflip. Currently the plane displayed by set_plane
isn't called exynos_plane_on function and if plane is disabled, it calls
exynos_plane_off, so it causes disharmory of plane on/off.

This is caused from commit e7cd81111041 ("drm/exynos: Don't touch DPMS
when updating overlay planes").

Make .update_plane function called only by set_plane and call
exynos_plane_on in it.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  |  4 ++--
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 21 ++++++++++++++++++++-
 drivers/gpu/drm/exynos/exynos_drm_plane.h |  2 +-
 3 files changed, 23 insertions(+), 4 deletions(-)

Comments

Gustavo Padovan Feb. 3, 2015, 1:28 p.m. UTC | #1
Hi Joonyoung,

2015-01-29 Joonyoung Shim <jy0922.shim@samsung.com>:

> The exynos_update_plane functions can be called from set_plane as well
> as set_crtc and pageflip. Currently the plane displayed by set_plane
> isn't called exynos_plane_on function and if plane is disabled, it calls
> exynos_plane_off, so it causes disharmory of plane on/off.
> 
> This is caused from commit e7cd81111041 ("drm/exynos: Don't touch DPMS
> when updating overlay planes").
> 
> Make .update_plane function called only by set_plane and call
> exynos_plane_on in it.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c  |  4 ++--
>  drivers/gpu/drm/exynos/exynos_drm_plane.c | 21 ++++++++++++++++++++-
>  drivers/gpu/drm/exynos/exynos_drm_plane.h |  2 +-
>  3 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index dac8f90..2765f7e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -129,7 +129,7 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  	crtc_w = fb->width - x;
>  	crtc_h = fb->height - y;
>  
> -	return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
> +	return exynos_plane_update(crtc->primary, crtc, fb, 0, 0,
>  				   crtc_w, crtc_h, x, y, crtc_w, crtc_h);

This patch goes in the opposite direction of the clean up to support atomic
modesetting on exynos (see my patches for atomic modesetting here[0]) In my
latest series there was an effort to unify all places we update a plane under
exynos_update_plane() and this is a essential step for atomic modesetting.

My proposal to solve this issue would just be calling exynos_plane_dpms()
on exynos_drm_crtc_mode_set_base() and leave the rest as is. This would fix
the disharmory of plane on/off. I'll send a patch for this in a bit.

[0] https://git.kernel.org/cgit/linux/kernel/git/padovan/drm-exynos.git/log/?h=atomic-next-2

	Gustavo
Joonyoung Shim Feb. 4, 2015, 1:58 a.m. UTC | #2
Hi,

On 02/03/2015 10:28 PM, Gustavo Padovan wrote:
> Hi Joonyoung,
> 
> 2015-01-29 Joonyoung Shim <jy0922.shim@samsung.com>:
> 
>> The exynos_update_plane functions can be called from set_plane as well
>> as set_crtc and pageflip. Currently the plane displayed by set_plane
>> isn't called exynos_plane_on function and if plane is disabled, it calls
>> exynos_plane_off, so it causes disharmory of plane on/off.
>>
>> This is caused from commit e7cd81111041 ("drm/exynos: Don't touch DPMS
>> when updating overlay planes").
>>
>> Make .update_plane function called only by set_plane and call
>> exynos_plane_on in it.
>>
>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c  |  4 ++--
>>  drivers/gpu/drm/exynos/exynos_drm_plane.c | 21 ++++++++++++++++++++-
>>  drivers/gpu/drm/exynos/exynos_drm_plane.h |  2 +-
>>  3 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> index dac8f90..2765f7e 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> @@ -129,7 +129,7 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>>  	crtc_w = fb->width - x;
>>  	crtc_h = fb->height - y;
>>  
>> -	return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
>> +	return exynos_plane_update(crtc->primary, crtc, fb, 0, 0,
>>  				   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
> 
> This patch goes in the opposite direction of the clean up to support atomic
> modesetting on exynos (see my patches for atomic modesetting here[0]) In my
> latest series there was an effort to unify all places we update a plane under
> exynos_update_plane() and this is a essential step for atomic modesetting.
> 

No, this change is just to rename function, actually
"exynos_plane_update" is more suitable because use exynos_plane_ prefix
like other functions of plane. This will conflict your patcheset but it
doesn't give any wrong operation effect, just need rename.

Thanks.
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index dac8f90..2765f7e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -129,7 +129,7 @@  static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	crtc_w = fb->width - x;
 	crtc_h = fb->height - y;
 
-	return exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+	return exynos_plane_update(crtc->primary, crtc, fb, 0, 0,
 				   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
 }
 
@@ -203,7 +203,7 @@  static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 		crtc->primary->fb = fb;
 		crtc_w = fb->width - crtc->x;
 		crtc_h = fb->height - crtc->y;
-		ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
+		ret = exynos_plane_update(crtc->primary, crtc, fb, 0, 0,
 					  crtc_w, crtc_h, crtc->x, crtc->y,
 					  crtc_w, crtc_h);
 		if (ret) {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index aa87d79..88674d9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -176,7 +176,7 @@  void exynos_plane_off(struct drm_plane *plane)
 }
 
 int
-exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+exynos_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
 		     struct drm_framebuffer *fb, int crtc_x, int crtc_y,
 		     unsigned int crtc_w, unsigned int crtc_h,
 		     uint32_t src_x, uint32_t src_y,
@@ -201,6 +201,25 @@  exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	return 0;
 }
 
+static int
+exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+		     struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+		     unsigned int crtc_w, unsigned int crtc_h,
+		     uint32_t src_x, uint32_t src_y,
+		     uint32_t src_w, uint32_t src_h)
+{
+	int ret;
+
+	ret = exynos_plane_update(plane, crtc, fb, crtc_x, crtc_y,
+				  crtc_w, crtc_h, src_x, src_y, src_w, src_h);
+	if (ret < 0)
+		return ret;
+
+	exynos_plane_on(plane);
+
+	return 0;
+}
+
 static int exynos_disable_plane(struct drm_plane *plane)
 {
 	exynos_plane_off(plane);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.h b/drivers/gpu/drm/exynos/exynos_drm_plane.h
index c778295..239eff5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.h
@@ -15,7 +15,7 @@  void exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
 			   unsigned int crtc_w, unsigned int crtc_h,
 			   uint32_t src_x, uint32_t src_y,
 			   uint32_t src_w, uint32_t src_h);
-int exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+int exynos_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
 			struct drm_framebuffer *fb, int crtc_x, int crtc_y,
 			unsigned int crtc_w, unsigned int crtc_h,
 			uint32_t src_x, uint32_t src_y,