diff mbox

[8/9] drm/mediatek: Use drm_plane_helper_check_state()

Message ID 1469549224-1860-9-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjala July 26, 2016, 4:07 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replace the use of drm_plane_helper_check_update() with
drm_plane_helper_check_state() since we have a plane state.

This also eliminates the double clipping the driver was doing
in both check and commit phases). And it should fix src coordinate
addr adjustement. Previously the driver was expecting negative dst
coordinates after clipping, which is not going happen, so any clipping
induced addr adjustment simply didn't happen. Neither did the driver
respect any user configured src coordinates, so panning and such would
have been totally broken. It should be all good now.

Cc: CK Hu <ck.hu@mediatek.com>
Cc: linux-mediatek@lists.infradead.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_plane.c | 72 +++++++++-----------------------
 1 file changed, 20 insertions(+), 52 deletions(-)

Comments

Sean Paul Aug. 1, 2016, 3:10 p.m. UTC | #1
On Tue, Jul 26, 2016 at 12:07 PM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Replace the use of drm_plane_helper_check_update() with
> drm_plane_helper_check_state() since we have a plane state.
>
> This also eliminates the double clipping the driver was doing
> in both check and commit phases). And it should fix src coordinate
> addr adjustement. Previously the driver was expecting negative dst
> coordinates after clipping, which is not going happen, so any clipping
> induced addr adjustment simply didn't happen. Neither did the driver
> respect any user configured src coordinates, so panning and such would
> have been totally broken. It should be all good now.
>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: linux-mediatek@lists.infradead.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c | 72 +++++++++-----------------------
>  1 file changed, 20 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 3995765a90dc..5f2516fca079 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -30,15 +30,20 @@ static const u32 formats[] = {
>         DRM_FORMAT_RGB565,
>  };
>
> -static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable,
> -                            dma_addr_t addr, struct drm_rect *dest)
> +static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane,
> +                            dma_addr_t addr)
>  {
>         struct drm_plane *plane = &mtk_plane->base;
>         struct mtk_plane_state *state = to_mtk_plane_state(plane->state);
>         unsigned int pitch, format;
> -       int x, y;
> +       bool enable;
>
> -       if (WARN_ON(!plane->state || (enable && !plane->state->fb)))
> +       if (WARN_ON(!plane->state))
> +               return;
> +
> +       enable = state->base.visible;
> +
> +       if (WARN_ON(enable && !plane->state->fb))
>                 return;
>
>         if (plane->state->fb) {
> @@ -49,27 +54,17 @@ static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable,
>                 format = DRM_FORMAT_RGBA8888;
>         }
>
> -       x = plane->state->crtc_x;
> -       y = plane->state->crtc_y;
> -
> -       if (x < 0) {
> -               addr -= x * 4;
> -               x = 0;
> -       }
> -
> -       if (y < 0) {
> -               addr -= y * pitch;
> -               y = 0;
> -       }
> +       addr += (state->base.src.x1 >> 16) * 4;
> +       addr += (state->base.src.y1 >> 16) * pitch;
>
>         state->pending.enable = enable;
>         state->pending.pitch = pitch;
>         state->pending.format = format;
>         state->pending.addr = addr;
> -       state->pending.x = x;
> -       state->pending.y = y;
> -       state->pending.width = dest->x2 - dest->x1;
> -       state->pending.height = dest->y2 - dest->y1;
> +       state->pending.x = state->base.dst.x1;
> +       state->pending.y = state->base.dst.y1;
> +       state->pending.width = drm_rect_width(&state->base.dst);
> +       state->pending.height = drm_rect_height(&state->base.dst);
>         wmb(); /* Make sure the above parameters are set before update */
>         state->pending.dirty = true;
>  }
> @@ -134,20 +129,6 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
>  {
>         struct drm_framebuffer *fb = state->fb;
>         struct drm_crtc_state *crtc_state;
> -       bool visible;
> -       struct drm_rect dest = {
> -               .x1 = state->crtc_x,
> -               .y1 = state->crtc_y,
> -               .x2 = state->crtc_x + state->crtc_w,
> -               .y2 = state->crtc_y + state->crtc_h,
> -       };
> -       struct drm_rect src = {
> -               /* 16.16 fixed point */
> -               .x1 = state->src_x,
> -               .y1 = state->src_y,
> -               .x2 = state->src_x + state->src_w,
> -               .y2 = state->src_y + state->src_h,
> -       };
>         struct drm_rect clip = { 0, };
>
>         if (!fb)
> @@ -168,12 +149,10 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
>         clip.x2 = crtc_state->mode.hdisplay;
>         clip.y2 = crtc_state->mode.vdisplay;
>
> -       return drm_plane_helper_check_update(plane, state->crtc, fb,
> -                                            &src, &dest, &clip,
> -                                            state->rotation,
> -                                            DRM_PLANE_HELPER_NO_SCALING,
> -                                            DRM_PLANE_HELPER_NO_SCALING,
> -                                            true, true, &visible);
> +       return drm_plane_helper_check_state(state, &clip,
> +                                           DRM_PLANE_HELPER_NO_SCALING,
> +                                           DRM_PLANE_HELPER_NO_SCALING,
> +                                           true, true);
>  }
>
>  static void mtk_plane_atomic_update(struct drm_plane *plane,
> @@ -184,24 +163,13 @@ static void mtk_plane_atomic_update(struct drm_plane *plane,
>         struct drm_gem_object *gem;
>         struct mtk_drm_gem_obj *mtk_gem;
>         struct mtk_drm_plane *mtk_plane = to_mtk_plane(plane);
> -       struct drm_rect dest = {
> -               .x1 = state->base.crtc_x,
> -               .y1 = state->base.crtc_y,
> -               .x2 = state->base.crtc_x + state->base.crtc_w,
> -               .y2 = state->base.crtc_y + state->base.crtc_h,
> -       };
> -       struct drm_rect clip = { 0, };
>
>         if (!crtc)
>                 return;
>
> -       clip.x2 = state->base.crtc->state->mode.hdisplay;
> -       clip.y2 = state->base.crtc->state->mode.vdisplay;
> -       drm_rect_intersect(&dest, &clip);
> -
>         gem = mtk_fb_get_gem_obj(state->base.fb);
>         mtk_gem = to_mtk_gem_obj(gem);
> -       mtk_plane_enable(mtk_plane, true, mtk_gem->dma_addr, &dest);
> +       mtk_plane_enable(mtk_plane, mtk_gem->dma_addr);
>  }
>
>  static void mtk_plane_atomic_disable(struct drm_plane *plane,
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Bibby Hsieh Aug. 2, 2016, 5:31 a.m. UTC | #2
On Tue, 2016-07-26 at 19:07 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace the use of drm_plane_helper_check_update() with
> drm_plane_helper_check_state() since we have a plane state.
> 
> This also eliminates the double clipping the driver was doing
> in both check and commit phases). And it should fix src coordinate
> addr adjustement. Previously the driver was expecting negative dst
> coordinates after clipping, which is not going happen, so any clipping
> induced addr adjustment simply didn't happen. Neither did the driver
> respect any user configured src coordinates, so panning and such would
> have been totally broken. It should be all good now.
> 
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: linux-mediatek@lists.infradead.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This patch looks fine to me, thanks for your patch.

Reviewed-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
Tested-by: Bibby Hsieh <bibby.hsieh@mediatek.com>

> ---
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c | 72 +++++++++-----------------------
>  1 file changed, 20 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 3995765a90dc..5f2516fca079 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -30,15 +30,20 @@ static const u32 formats[] = {
>  	DRM_FORMAT_RGB565,
>  };
>  
> -static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable,
> -			     dma_addr_t addr, struct drm_rect *dest)
> +static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane,
> +			     dma_addr_t addr)
>  {
>  	struct drm_plane *plane = &mtk_plane->base;
>  	struct mtk_plane_state *state = to_mtk_plane_state(plane->state);
>  	unsigned int pitch, format;
> -	int x, y;
> +	bool enable;
>  
> -	if (WARN_ON(!plane->state || (enable && !plane->state->fb)))
> +	if (WARN_ON(!plane->state))
> +		return;
> +
> +	enable = state->base.visible;
> +
> +	if (WARN_ON(enable && !plane->state->fb))
>  		return;
>  
>  	if (plane->state->fb) {
> @@ -49,27 +54,17 @@ static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable,
>  		format = DRM_FORMAT_RGBA8888;
>  	}
>  
> -	x = plane->state->crtc_x;
> -	y = plane->state->crtc_y;
> -
> -	if (x < 0) {
> -		addr -= x * 4;
> -		x = 0;
> -	}
> -
> -	if (y < 0) {
> -		addr -= y * pitch;
> -		y = 0;
> -	}
> +	addr += (state->base.src.x1 >> 16) * 4;
> +	addr += (state->base.src.y1 >> 16) * pitch;
>  
>  	state->pending.enable = enable;
>  	state->pending.pitch = pitch;
>  	state->pending.format = format;
>  	state->pending.addr = addr;
> -	state->pending.x = x;
> -	state->pending.y = y;
> -	state->pending.width = dest->x2 - dest->x1;
> -	state->pending.height = dest->y2 - dest->y1;
> +	state->pending.x = state->base.dst.x1;
> +	state->pending.y = state->base.dst.y1;
> +	state->pending.width = drm_rect_width(&state->base.dst);
> +	state->pending.height = drm_rect_height(&state->base.dst);
>  	wmb(); /* Make sure the above parameters are set before update */
>  	state->pending.dirty = true;
>  }
> @@ -134,20 +129,6 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
>  {
>  	struct drm_framebuffer *fb = state->fb;
>  	struct drm_crtc_state *crtc_state;
> -	bool visible;
> -	struct drm_rect dest = {
> -		.x1 = state->crtc_x,
> -		.y1 = state->crtc_y,
> -		.x2 = state->crtc_x + state->crtc_w,
> -		.y2 = state->crtc_y + state->crtc_h,
> -	};
> -	struct drm_rect src = {
> -		/* 16.16 fixed point */
> -		.x1 = state->src_x,
> -		.y1 = state->src_y,
> -		.x2 = state->src_x + state->src_w,
> -		.y2 = state->src_y + state->src_h,
> -	};
>  	struct drm_rect clip = { 0, };
>  
>  	if (!fb)
> @@ -168,12 +149,10 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->mode.hdisplay;
>  	clip.y2 = crtc_state->mode.vdisplay;
>  
> -	return drm_plane_helper_check_update(plane, state->crtc, fb,
> -					     &src, &dest, &clip,
> -					     state->rotation,
> -					     DRM_PLANE_HELPER_NO_SCALING,
> -					     DRM_PLANE_HELPER_NO_SCALING,
> -					     true, true, &visible);
> +	return drm_plane_helper_check_state(state, &clip,
> +					    DRM_PLANE_HELPER_NO_SCALING,
> +					    DRM_PLANE_HELPER_NO_SCALING,
> +					    true, true);
>  }
>  
>  static void mtk_plane_atomic_update(struct drm_plane *plane,
> @@ -184,24 +163,13 @@ static void mtk_plane_atomic_update(struct drm_plane *plane,
>  	struct drm_gem_object *gem;
>  	struct mtk_drm_gem_obj *mtk_gem;
>  	struct mtk_drm_plane *mtk_plane = to_mtk_plane(plane);
> -	struct drm_rect dest = {
> -		.x1 = state->base.crtc_x,
> -		.y1 = state->base.crtc_y,
> -		.x2 = state->base.crtc_x + state->base.crtc_w,
> -		.y2 = state->base.crtc_y + state->base.crtc_h,
> -	};
> -	struct drm_rect clip = { 0, };
>  
>  	if (!crtc)
>  		return;
>  
> -	clip.x2 = state->base.crtc->state->mode.hdisplay;
> -	clip.y2 = state->base.crtc->state->mode.vdisplay;
> -	drm_rect_intersect(&dest, &clip);
> -
>  	gem = mtk_fb_get_gem_obj(state->base.fb);
>  	mtk_gem = to_mtk_gem_obj(gem);
> -	mtk_plane_enable(mtk_plane, true, mtk_gem->dma_addr, &dest);
> +	mtk_plane_enable(mtk_plane, mtk_gem->dma_addr);
>  }
>  
>  static void mtk_plane_atomic_disable(struct drm_plane *plane,
CK Hu Aug. 2, 2016, 5:45 a.m. UTC | #3
On Tue, 2016-07-26 at 19:07 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace the use of drm_plane_helper_check_update() with
> drm_plane_helper_check_state() since we have a plane state.
> 
> This also eliminates the double clipping the driver was doing
> in both check and commit phases). And it should fix src coordinate
> addr adjustement. Previously the driver was expecting negative dst
> coordinates after clipping, which is not going happen, so any clipping
> induced addr adjustment simply didn't happen. Neither did the driver
> respect any user configured src coordinates, so panning and such would
> have been totally broken. It should be all good now.
> 
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: linux-mediatek@lists.infradead.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

Acked-by: CK Hu <ck.hu@mediatek.com>

>  drivers/gpu/drm/mediatek/mtk_drm_plane.c | 72 +++++++++-----------------------
>  1 file changed, 20 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 3995765a90dc..5f2516fca079 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -30,15 +30,20 @@ static const u32 formats[] = {
>  	DRM_FORMAT_RGB565,
>  };
>  
> -static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable,
> -			     dma_addr_t addr, struct drm_rect *dest)
> +static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane,
> +			     dma_addr_t addr)
>  {
>  	struct drm_plane *plane = &mtk_plane->base;
>  	struct mtk_plane_state *state = to_mtk_plane_state(plane->state);
>  	unsigned int pitch, format;
> -	int x, y;
> +	bool enable;
>  
> -	if (WARN_ON(!plane->state || (enable && !plane->state->fb)))
> +	if (WARN_ON(!plane->state))
> +		return;
> +
> +	enable = state->base.visible;
> +
> +	if (WARN_ON(enable && !plane->state->fb))
>  		return;
>  
>  	if (plane->state->fb) {
> @@ -49,27 +54,17 @@ static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable,
>  		format = DRM_FORMAT_RGBA8888;
>  	}
>  
> -	x = plane->state->crtc_x;
> -	y = plane->state->crtc_y;
> -
> -	if (x < 0) {
> -		addr -= x * 4;
> -		x = 0;
> -	}
> -
> -	if (y < 0) {
> -		addr -= y * pitch;
> -		y = 0;
> -	}
> +	addr += (state->base.src.x1 >> 16) * 4;
> +	addr += (state->base.src.y1 >> 16) * pitch;
>  
>  	state->pending.enable = enable;
>  	state->pending.pitch = pitch;
>  	state->pending.format = format;
>  	state->pending.addr = addr;
> -	state->pending.x = x;
> -	state->pending.y = y;
> -	state->pending.width = dest->x2 - dest->x1;
> -	state->pending.height = dest->y2 - dest->y1;
> +	state->pending.x = state->base.dst.x1;
> +	state->pending.y = state->base.dst.y1;
> +	state->pending.width = drm_rect_width(&state->base.dst);
> +	state->pending.height = drm_rect_height(&state->base.dst);
>  	wmb(); /* Make sure the above parameters are set before update */
>  	state->pending.dirty = true;
>  }
> @@ -134,20 +129,6 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
>  {
>  	struct drm_framebuffer *fb = state->fb;
>  	struct drm_crtc_state *crtc_state;
> -	bool visible;
> -	struct drm_rect dest = {
> -		.x1 = state->crtc_x,
> -		.y1 = state->crtc_y,
> -		.x2 = state->crtc_x + state->crtc_w,
> -		.y2 = state->crtc_y + state->crtc_h,
> -	};
> -	struct drm_rect src = {
> -		/* 16.16 fixed point */
> -		.x1 = state->src_x,
> -		.y1 = state->src_y,
> -		.x2 = state->src_x + state->src_w,
> -		.y2 = state->src_y + state->src_h,
> -	};
>  	struct drm_rect clip = { 0, };
>  
>  	if (!fb)
> @@ -168,12 +149,10 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->mode.hdisplay;
>  	clip.y2 = crtc_state->mode.vdisplay;
>  
> -	return drm_plane_helper_check_update(plane, state->crtc, fb,
> -					     &src, &dest, &clip,
> -					     state->rotation,
> -					     DRM_PLANE_HELPER_NO_SCALING,
> -					     DRM_PLANE_HELPER_NO_SCALING,
> -					     true, true, &visible);
> +	return drm_plane_helper_check_state(state, &clip,
> +					    DRM_PLANE_HELPER_NO_SCALING,
> +					    DRM_PLANE_HELPER_NO_SCALING,
> +					    true, true);
>  }
>  
>  static void mtk_plane_atomic_update(struct drm_plane *plane,
> @@ -184,24 +163,13 @@ static void mtk_plane_atomic_update(struct drm_plane *plane,
>  	struct drm_gem_object *gem;
>  	struct mtk_drm_gem_obj *mtk_gem;
>  	struct mtk_drm_plane *mtk_plane = to_mtk_plane(plane);
> -	struct drm_rect dest = {
> -		.x1 = state->base.crtc_x,
> -		.y1 = state->base.crtc_y,
> -		.x2 = state->base.crtc_x + state->base.crtc_w,
> -		.y2 = state->base.crtc_y + state->base.crtc_h,
> -	};
> -	struct drm_rect clip = { 0, };
>  
>  	if (!crtc)
>  		return;
>  
> -	clip.x2 = state->base.crtc->state->mode.hdisplay;
> -	clip.y2 = state->base.crtc->state->mode.vdisplay;
> -	drm_rect_intersect(&dest, &clip);
> -
>  	gem = mtk_fb_get_gem_obj(state->base.fb);
>  	mtk_gem = to_mtk_gem_obj(gem);
> -	mtk_plane_enable(mtk_plane, true, mtk_gem->dma_addr, &dest);
> +	mtk_plane_enable(mtk_plane, mtk_gem->dma_addr);
>  }
>  
>  static void mtk_plane_atomic_disable(struct drm_plane *plane,
diff mbox

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 3995765a90dc..5f2516fca079 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -30,15 +30,20 @@  static const u32 formats[] = {
 	DRM_FORMAT_RGB565,
 };
 
-static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable,
-			     dma_addr_t addr, struct drm_rect *dest)
+static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane,
+			     dma_addr_t addr)
 {
 	struct drm_plane *plane = &mtk_plane->base;
 	struct mtk_plane_state *state = to_mtk_plane_state(plane->state);
 	unsigned int pitch, format;
-	int x, y;
+	bool enable;
 
-	if (WARN_ON(!plane->state || (enable && !plane->state->fb)))
+	if (WARN_ON(!plane->state))
+		return;
+
+	enable = state->base.visible;
+
+	if (WARN_ON(enable && !plane->state->fb))
 		return;
 
 	if (plane->state->fb) {
@@ -49,27 +54,17 @@  static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable,
 		format = DRM_FORMAT_RGBA8888;
 	}
 
-	x = plane->state->crtc_x;
-	y = plane->state->crtc_y;
-
-	if (x < 0) {
-		addr -= x * 4;
-		x = 0;
-	}
-
-	if (y < 0) {
-		addr -= y * pitch;
-		y = 0;
-	}
+	addr += (state->base.src.x1 >> 16) * 4;
+	addr += (state->base.src.y1 >> 16) * pitch;
 
 	state->pending.enable = enable;
 	state->pending.pitch = pitch;
 	state->pending.format = format;
 	state->pending.addr = addr;
-	state->pending.x = x;
-	state->pending.y = y;
-	state->pending.width = dest->x2 - dest->x1;
-	state->pending.height = dest->y2 - dest->y1;
+	state->pending.x = state->base.dst.x1;
+	state->pending.y = state->base.dst.y1;
+	state->pending.width = drm_rect_width(&state->base.dst);
+	state->pending.height = drm_rect_height(&state->base.dst);
 	wmb(); /* Make sure the above parameters are set before update */
 	state->pending.dirty = true;
 }
@@ -134,20 +129,6 @@  static int mtk_plane_atomic_check(struct drm_plane *plane,
 {
 	struct drm_framebuffer *fb = state->fb;
 	struct drm_crtc_state *crtc_state;
-	bool visible;
-	struct drm_rect dest = {
-		.x1 = state->crtc_x,
-		.y1 = state->crtc_y,
-		.x2 = state->crtc_x + state->crtc_w,
-		.y2 = state->crtc_y + state->crtc_h,
-	};
-	struct drm_rect src = {
-		/* 16.16 fixed point */
-		.x1 = state->src_x,
-		.y1 = state->src_y,
-		.x2 = state->src_x + state->src_w,
-		.y2 = state->src_y + state->src_h,
-	};
 	struct drm_rect clip = { 0, };
 
 	if (!fb)
@@ -168,12 +149,10 @@  static int mtk_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->mode.hdisplay;
 	clip.y2 = crtc_state->mode.vdisplay;
 
-	return drm_plane_helper_check_update(plane, state->crtc, fb,
-					     &src, &dest, &clip,
-					     state->rotation,
-					     DRM_PLANE_HELPER_NO_SCALING,
-					     DRM_PLANE_HELPER_NO_SCALING,
-					     true, true, &visible);
+	return drm_plane_helper_check_state(state, &clip,
+					    DRM_PLANE_HELPER_NO_SCALING,
+					    DRM_PLANE_HELPER_NO_SCALING,
+					    true, true);
 }
 
 static void mtk_plane_atomic_update(struct drm_plane *plane,
@@ -184,24 +163,13 @@  static void mtk_plane_atomic_update(struct drm_plane *plane,
 	struct drm_gem_object *gem;
 	struct mtk_drm_gem_obj *mtk_gem;
 	struct mtk_drm_plane *mtk_plane = to_mtk_plane(plane);
-	struct drm_rect dest = {
-		.x1 = state->base.crtc_x,
-		.y1 = state->base.crtc_y,
-		.x2 = state->base.crtc_x + state->base.crtc_w,
-		.y2 = state->base.crtc_y + state->base.crtc_h,
-	};
-	struct drm_rect clip = { 0, };
 
 	if (!crtc)
 		return;
 
-	clip.x2 = state->base.crtc->state->mode.hdisplay;
-	clip.y2 = state->base.crtc->state->mode.vdisplay;
-	drm_rect_intersect(&dest, &clip);
-
 	gem = mtk_fb_get_gem_obj(state->base.fb);
 	mtk_gem = to_mtk_gem_obj(gem);
-	mtk_plane_enable(mtk_plane, true, mtk_gem->dma_addr, &dest);
+	mtk_plane_enable(mtk_plane, mtk_gem->dma_addr);
 }
 
 static void mtk_plane_atomic_disable(struct drm_plane *plane,