[PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While trawling the tree I spotted some issues with the way vmwgfx
uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
Do note that I haven't actually tested the resulting code at all,
but it does build at least.

And while touching that general area I took up Daniel's suggestion from
long ago that drm_plane_helper_check_state() should be renamed and
relocated to better reflect its status.

Here's a branch with the entire series:
git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff

Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Daniel Vetter <daniel@ffwll.ch>

Ville Syrjälä (5):
  drm/vmwgfx: Remove bogus crtc coords vs fb size check
  drm/vmwgfx: Use drm_plane_helper_check_state()
  drm/vmwgfx: Try to fix plane clipping
  drm: Check crtc_state->enable rather than crtc->enabled in
    drm_plane_helper_check_state()
  drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c

 drivers/gpu/drm/arm/hdlcd_crtc.c            |   8 +-
 drivers/gpu/drm/arm/malidp_planes.c         |   3 +-
 drivers/gpu/drm/drm_atomic_helper.c         |  95 ++++++++++++++++++++++++
 drivers/gpu/drm/drm_plane_helper.c          | 111 +++-------------------------
 drivers/gpu/drm/drm_simple_kms_helper.c     |   9 ++-
 drivers/gpu/drm/i915/intel_display.c        |  20 ++---
 drivers/gpu/drm/imx/ipuv3-plane.c           |   8 +-
 drivers/gpu/drm/mediatek/mtk_drm_plane.c    |   8 +-
 drivers/gpu/drm/meson/meson_plane.c         |   8 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
 drivers/gpu/drm/nouveau/nv50_display.c      |  18 +++--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
 drivers/gpu/drm/tegra/dc.c                  |   4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  40 ++++------
 drivers/gpu/drm/zte/zx_plane.c              |  15 ++--
 include/drm/drm_atomic_helper.h             |   7 ++
 include/drm/drm_plane_helper.h              |   5 --
 17 files changed, 187 insertions(+), 183 deletions(-)

-- 
2.13.6

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

[PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Throw away the bugs crtc coords vs. fb size check. Crtc coords don't
define the viewport inside the fb, that's a job for the src coords,
which have been checked by the core already.

Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 0545740b3724..a4b56699679a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -476,12 +476,6 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 
 		vcs = vmw_connector_state_to_vcs(du->connector.state);
 
-		if ((dest.x2 > new_fb->width ||
-		     dest.y2 > new_fb->height)) {
-			DRM_ERROR("CRTC area outside of framebuffer\n");
-			return -EINVAL;
-		}
-
 		/* Only one active implicit framebuffer at a time. */
 		mutex_lock(&dev_priv->global_kms_state_mutex);
 		if (vcs->is_implicit && dev_priv->implicit_fb &&
-- 
2.13.6

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

Re: [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check

[Daniel Vetter]

On Wed, Nov 01, 2017 at 08:29:16PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Throw away the bugs crtc coords vs. fb size check. Crtc coords don't
> define the viewport inside the fb, that's a job for the src coords,
> which have been checked by the core already.
> 
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 0545740b3724..a4b56699679a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -476,12 +476,6 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
>  
>  		vcs = vmw_connector_state_to_vcs(du->connector.state);
>  
> -		if ((dest.x2 > new_fb->width ||
> -		     dest.y2 > new_fb->height)) {
> -			DRM_ERROR("CRTC area outside of framebuffer\n");
> -			return -EINVAL;
> -		}
> -
>  		/* Only one active implicit framebuffer at a time. */
>  		mutex_lock(&dev_priv->global_kms_state_mutex);
>  		if (vcs->is_implicit && dev_priv->implicit_fb &&
> -- 
> 2.13.6
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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

Re: [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check

[Laurent Pinchart]

Hi Ville,

Thank you for the patch.

On Wednesday, 1 November 2017 20:29:16 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Throw away the bugs crtc coords vs. fb size check. Crtc coords don't
> define the viewport inside the fb, that's a job for the src coords,
> which have been checked by the core already.
> 
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 0545740b3724..a4b56699679a
> 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -476,12 +476,6 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
> *plane,
> 
>  		vcs = vmw_connector_state_to_vcs(du->connector.state);
> 
> -		if ((dest.x2 > new_fb->width ||
> -		     dest.y2 > new_fb->height)) {
> -			DRM_ERROR("CRTC area outside of framebuffer\n");
> -			return -EINVAL;
> -		}
> -
>  		/* Only one active implicit framebuffer at a time. */
>  		mutex_lock(&dev_priv->global_kms_state_mutex);
>  		if (vcs->is_implicit && dev_priv->implicit_fb &&


-- 
Regards,

Laurent Pinchart

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Atomic drivers have no reason to use drm_plane_helper_check_update()
instead of drm_plane_helper_check_state(). So let's switch over.

Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index a4b56699679a..515b67783a41 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -442,29 +442,18 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 				      struct drm_plane_state *state)
 {
 	struct drm_framebuffer *new_fb = state->fb;
-	bool visible;
-
-	struct drm_rect src = {
-		.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 dest = {
+	struct drm_rect clip = {
 		.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 clip = dest;
 	int ret;
 
-	ret = drm_plane_helper_check_update(plane, state->crtc, new_fb,
-					    &src, &dest, &clip,
-					    DRM_MODE_ROTATE_0,
+	ret = drm_plane_helper_check_state(state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
-					    false, true, &visible);
+					    false, true);
 
 
 	if (!ret && new_fb) {
-- 
2.13.6

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

Re: [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state()

[Daniel Vetter]

On Wed, Nov 01, 2017 at 08:29:17PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Atomic drivers have no reason to use drm_plane_helper_check_update()
> instead of drm_plane_helper_check_state(). So let's switch over.
> 
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I think the new atomic helper landed at about the same time vmwgfx landed,
hence why it wasn't converted.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index a4b56699679a..515b67783a41 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -442,29 +442,18 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
>  				      struct drm_plane_state *state)
>  {
>  	struct drm_framebuffer *new_fb = state->fb;
> -	bool visible;
> -
> -	struct drm_rect src = {
> -		.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 dest = {
> +	struct drm_rect clip = {
>  		.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 clip = dest;
>  	int ret;
>  
> -	ret = drm_plane_helper_check_update(plane, state->crtc, new_fb,
> -					    &src, &dest, &clip,
> -					    DRM_MODE_ROTATE_0,
> +	ret = drm_plane_helper_check_state(state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
> -					    false, true, &visible);
> +					    false, true);
>  
>  
>  	if (!ret && new_fb) {
> -- 
> 2.13.6
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state()

[Laurent Pinchart]

Hi Ville,

Thank you for the patch.

On Wednesday, 1 November 2017 20:29:17 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Atomic drivers have no reason to use drm_plane_helper_check_update()
> instead of drm_plane_helper_check_state(). So let's switch over.
> 
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index a4b56699679a..515b67783a41
> 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -442,29 +442,18 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
> *plane, struct drm_plane_state *state)
>  {
>  	struct drm_framebuffer *new_fb = state->fb;
> -	bool visible;
> -
> -	struct drm_rect src = {
> -		.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 dest = {
> +	struct drm_rect clip = {
>  		.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 clip = dest;
>  	int ret;
> 
> -	ret = drm_plane_helper_check_update(plane, state->crtc, new_fb,
> -					    &src, &dest, &clip,
> -					    DRM_MODE_ROTATE_0,
> +	ret = drm_plane_helper_check_state(state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
> -					    false, true, &visible);
> +					    false, true);
> 
> 
>  	if (!ret && new_fb) {


-- 
Regards,

Laurent Pinchart

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

[PATCH 3/5] drm/vmwgfx: Try to fix plane clipping

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Try to fix the code to actually clip the plane to the crtc bounds
instead of the user provided crtc coordinates (which would be a no-op
since those are exactly the coordinates before clipping).

Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 515b67783a41..efa41c086198 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
 int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 				      struct drm_plane_state *state)
 {
+	struct drm_crtc_state *crtc_state = NULL;
 	struct drm_framebuffer *new_fb = state->fb;
-	struct drm_rect clip = {
-		.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 clip = {};
 	int ret;
 
-	ret = drm_plane_helper_check_state(state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    false, true);
+	if (state->crtc)
+		crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
 
+	if (crtc_state && crtc_state->enable) {
+		clip.x2 = crtc_state->adjusted_mode.hdisplay;
+		clip.y2 = crtc_state->adjusted_mode.vdisplay;
+	}
+
+	ret = drm_plane_helper_check_state(state, &clip,
+					   DRM_PLANE_HELPER_NO_SCALING,
+					   DRM_PLANE_HELPER_NO_SCALING,
+					   false, true);
 
 	if (!ret && new_fb) {
 		struct drm_crtc *crtc = state->crtc;
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping

[Daniel Vetter]

On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Try to fix the code to actually clip the plane to the crtc bounds
> instead of the user provided crtc coordinates (which would be a no-op
> since those are exactly the coordinates before clipping).
> 
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I kinda wonder whether we shouldn't push this down into the helper ...

Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 515b67783a41..efa41c086198 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
>  int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
>  				      struct drm_plane_state *state)
>  {
> +	struct drm_crtc_state *crtc_state = NULL;
>  	struct drm_framebuffer *new_fb = state->fb;
> -	struct drm_rect clip = {
> -		.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 clip = {};
>  	int ret;
>  
> -	ret = drm_plane_helper_check_state(state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    false, true);
> +	if (state->crtc)
> +		crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
>  
> +	if (crtc_state && crtc_state->enable) {
> +		clip.x2 = crtc_state->adjusted_mode.hdisplay;
> +		clip.y2 = crtc_state->adjusted_mode.vdisplay;
> +	}
> +
> +	ret = drm_plane_helper_check_state(state, &clip,
> +					   DRM_PLANE_HELPER_NO_SCALING,
> +					   DRM_PLANE_HELPER_NO_SCALING,
> +					   false, true);
>  
>  	if (!ret && new_fb) {
>  		struct drm_crtc *crtc = state->crtc;
> -- 
> 2.13.6
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping

[Ville Syrjälä]

On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Try to fix the code to actually clip the plane to the crtc bounds
> > instead of the user provided crtc coordinates (which would be a no-op
> > since those are exactly the coordinates before clipping).
> > 
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Sinclair Yeh <syeh@vmware.com>
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I kinda wonder whether we shouldn't push this down into the helper ...

Would require quite going through all drivers making sure they are
happy with using the adjusted_mode.crtc_ timings. I think most drivers
use the other adjusted_mode timings currently, and some even use the
user mode timings (which could be an actual bug).

> 
> Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
> >  1 file changed, 13 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 515b67783a41..efa41c086198 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
> >  int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> >  				      struct drm_plane_state *state)
> >  {
> > +	struct drm_crtc_state *crtc_state = NULL;
> >  	struct drm_framebuffer *new_fb = state->fb;
> > -	struct drm_rect clip = {
> > -		.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 clip = {};
> >  	int ret;
> >  
> > -	ret = drm_plane_helper_check_state(state, &clip,
> > -					    DRM_PLANE_HELPER_NO_SCALING,
> > -					    DRM_PLANE_HELPER_NO_SCALING,
> > -					    false, true);
> > +	if (state->crtc)
> > +		crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
> >  
> > +	if (crtc_state && crtc_state->enable) {
> > +		clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > +		clip.y2 = crtc_state->adjusted_mode.vdisplay;
> > +	}
> > +
> > +	ret = drm_plane_helper_check_state(state, &clip,
> > +					   DRM_PLANE_HELPER_NO_SCALING,
> > +					   DRM_PLANE_HELPER_NO_SCALING,
> > +					   false, true);
> >  
> >  	if (!ret && new_fb) {
> >  		struct drm_crtc *crtc = state->crtc;
> > -- 
> > 2.13.6
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping

[Ville Syrjälä]

On Thu, Nov 02, 2017 at 03:19:30PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> > On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Try to fix the code to actually clip the plane to the crtc bounds
> > > instead of the user provided crtc coordinates (which would be a no-op
> > > since those are exactly the coordinates before clipping).
> > > 
> > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I kinda wonder whether we shouldn't push this down into the helper ...
> 
> Would require quite going through all drivers making sure they are
> happy with using the adjusted_mode.crtc_ timings. I think most drivers
> use the other adjusted_mode timings currently, and some even use the
> user mode timings (which could be an actual bug).

Let me take that back. What we want is to clip to the user mode
timings. Stereo 3D needs the special treatment from
drm_mode_get_hv_timing(). I'm getting confused by all these
different timings we have all over the place.

I think for i915 all we would need is to change the double wide/dual
link adjustent for pipe_src_w to simply reject odd widths instead.
That would guarantee that the user mode timings match the pipe_src_w/h
100%.

For the other driver we'd need to figure out why they're using
adjusted_mode timings for clipping. I guess that's just a mistake,
which I repeated here with vmwgfx after getting confused by looking
at the other drivers.

I guess I just volunteered myself to do this. Just needs plenty of
acks from driver maintainers I suppose.

> 
> > 
> > Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > > ---
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
> > >  1 file changed, 13 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > index 515b67783a41..efa41c086198 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
> > >  int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> > >  				      struct drm_plane_state *state)
> > >  {
> > > +	struct drm_crtc_state *crtc_state = NULL;
> > >  	struct drm_framebuffer *new_fb = state->fb;
> > > -	struct drm_rect clip = {
> > > -		.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 clip = {};
> > >  	int ret;
> > >  
> > > -	ret = drm_plane_helper_check_state(state, &clip,
> > > -					    DRM_PLANE_HELPER_NO_SCALING,
> > > -					    DRM_PLANE_HELPER_NO_SCALING,
> > > -					    false, true);
> > > +	if (state->crtc)
> > > +		crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
> > >  
> > > +	if (crtc_state && crtc_state->enable) {
> > > +		clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > > +		clip.y2 = crtc_state->adjusted_mode.vdisplay;
> > > +	}
> > > +
> > > +	ret = drm_plane_helper_check_state(state, &clip,
> > > +					   DRM_PLANE_HELPER_NO_SCALING,
> > > +					   DRM_PLANE_HELPER_NO_SCALING,
> > > +					   false, true);
> > >  
> > >  	if (!ret && new_fb) {
> > >  		struct drm_crtc *crtc = state->crtc;
> > > -- 
> > > 2.13.6
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping

[Daniel Vetter]

On Mon, Nov 06, 2017 at 08:04:38PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 03:19:30PM +0200, Ville Syrjälä wrote:
> > On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> > > On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Try to fix the code to actually clip the plane to the crtc bounds
> > > > instead of the user provided crtc coordinates (which would be a no-op
> > > > since those are exactly the coordinates before clipping).
> > > > 
> > > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > I kinda wonder whether we shouldn't push this down into the helper ...
> > 
> > Would require quite going through all drivers making sure they are
> > happy with using the adjusted_mode.crtc_ timings. I think most drivers
> > use the other adjusted_mode timings currently, and some even use the
> > user mode timings (which could be an actual bug).
> 
> Let me take that back. What we want is to clip to the user mode
> timings. Stereo 3D needs the special treatment from
> drm_mode_get_hv_timing(). I'm getting confused by all these
> different timings we have all over the place.
> 
> I think for i915 all we would need is to change the double wide/dual
> link adjustent for pipe_src_w to simply reject odd widths instead.
> That would guarantee that the user mode timings match the pipe_src_w/h
> 100%.
> 
> For the other driver we'd need to figure out why they're using
> adjusted_mode timings for clipping. I guess that's just a mistake,
> which I repeated here with vmwgfx after getting confused by looking
> at the other drivers.
> 
> I guess I just volunteered myself to do this. Just needs plenty of
> acks from driver maintainers I suppose.

Yeah consistently using drm_mode_get_hv_timing from
crtc_state->requested_mode seems like the right thing to do. Otherwise
there's a driver bug lurking somewhere ...
-Daniel

> 
> > 
> > > 
> > > Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > 
> > > > ---
> > > >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
> > > >  1 file changed, 13 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > > index 515b67783a41..efa41c086198 100644
> > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > > @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
> > > >  int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> > > >  				      struct drm_plane_state *state)
> > > >  {
> > > > +	struct drm_crtc_state *crtc_state = NULL;
> > > >  	struct drm_framebuffer *new_fb = state->fb;
> > > > -	struct drm_rect clip = {
> > > > -		.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 clip = {};
> > > >  	int ret;
> > > >  
> > > > -	ret = drm_plane_helper_check_state(state, &clip,
> > > > -					    DRM_PLANE_HELPER_NO_SCALING,
> > > > -					    DRM_PLANE_HELPER_NO_SCALING,
> > > > -					    false, true);
> > > > +	if (state->crtc)
> > > > +		crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
> > > >  
> > > > +	if (crtc_state && crtc_state->enable) {
> > > > +		clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > > > +		clip.y2 = crtc_state->adjusted_mode.vdisplay;
> > > > +	}
> > > > +
> > > > +	ret = drm_plane_helper_check_state(state, &clip,
> > > > +					   DRM_PLANE_HELPER_NO_SCALING,
> > > > +					   DRM_PLANE_HELPER_NO_SCALING,
> > > > +					   false, true);
> > > >  
> > > >  	if (!ret && new_fb) {
> > > >  		struct drm_crtc *crtc = state->crtc;
> > > > -- 
> > > > 2.13.6
> > > > 
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > 
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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

Re: [Intel-gfx] [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping

[Laurent Pinchart]

Hi Ville,

On Monday, 6 November 2017 20:04:38 EET Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 03:19:30PM +0200, Ville Syrjälä wrote:
> > On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> >> On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> 
> >>> Try to fix the code to actually clip the plane to the crtc bounds
> >>> instead of the user provided crtc coordinates (which would be a no-op
> >>> since those are exactly the coordinates before clipping).
> >>> 
> >>> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> >>> Cc: Sinclair Yeh <syeh@vmware.com>
> >>> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> 
> >> I kinda wonder whether we shouldn't push this down into the helper ...

I think that would be nice.

> > Would require quite going through all drivers making sure they are
> > happy with using the adjusted_mode.crtc_ timings. I think most drivers
> > use the other adjusted_mode timings currently, and some even use the
> > user mode timings (which could be an actual bug).
> 
> Let me take that back. What we want is to clip to the user mode
> timings. Stereo 3D needs the special treatment from
> drm_mode_get_hv_timing(). I'm getting confused by all these
> different timings we have all over the place.
> 
> I think for i915 all we would need is to change the double wide/dual
> link adjustent for pipe_src_w to simply reject odd widths instead.
> That would guarantee that the user mode timings match the pipe_src_w/h
> 100%.
> 
> For the other driver we'd need to figure out why they're using
> adjusted_mode timings for clipping. I guess that's just a mistake,
> which I repeated here with vmwgfx after getting confused by looking
> at the other drivers.

I suppose it's a mix of cargo-cult and the fact that adjusted_mode hints that 
it contains the timings that should be applied to the hardware. That's why I 
use it in rcar-du.

Are drivers allowed to adjust the hdisplay and vdisplay values though ? I 
thought unsupported values there had to be rejected with an error at atomic 
check time, not adjusted.

> I guess I just volunteered myself to do this. Just needs plenty of
> acks from driver maintainers I suppose.

I'll review and test the rcar-du patch :-)

-- 
Regards,

Laurent Pinchart

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

[PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_plane_helper_check_state() is supposed to do things the atomic way,
so it should not be inspecting crtc->enabled. Rather we should be
looking at crtc_state->enable.

We have a slight complication due to drm_plane_helper_check_update()
reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
we'll have to pass the crtc_state in manally and construct a fake
crtc_state in drm_plane_helper_check_update().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c            |  2 +-
 drivers/gpu/drm/arm/malidp_planes.c         |  3 +-
 drivers/gpu/drm/drm_plane_helper.c          | 52 +++++++++++++++++------------
 drivers/gpu/drm/drm_simple_kms_helper.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c        |  2 ++
 drivers/gpu/drm/imx/ipuv3-plane.c           |  2 +-
 drivers/gpu/drm/mediatek/mtk_drm_plane.c    |  2 +-
 drivers/gpu/drm/meson/meson_plane.c         |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  4 +--
 drivers/gpu/drm/nouveau/nv50_display.c      |  6 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  2 +-
 drivers/gpu/drm/tegra/dc.c                  |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  2 +-
 drivers/gpu/drm/zte/zx_plane.c              |  4 +--
 include/drm/drm_plane_helper.h              |  3 +-
 15 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 72b22b805412..14721723fa8a 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    false, true);
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 94e7e3fa3408..492d99dd55d4 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
 
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   0, INT_MAX, true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 759ed93f4ba8..adb8d94484f2 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
 
 /**
  * drm_plane_helper_check_state() - Check plane state for validity
- * @state: plane state to check
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
  * @clip: integer clipping coordinates
  * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
  * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
@@ -120,35 +121,38 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
  * RETURNS:
  * Zero if update appears valid, error code on failure
  */
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+				 const struct drm_crtc_state *crtc_state,
 				 const struct drm_rect *clip,
 				 int min_scale,
 				 int max_scale,
 				 bool can_position,
 				 bool can_update_disabled)
 {
-	struct drm_crtc *crtc = state->crtc;
-	struct drm_framebuffer *fb = state->fb;
-	struct drm_rect *src = &state->src;
-	struct drm_rect *dst = &state->dst;
-	unsigned int rotation = state->rotation;
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_rect *src = &plane_state->src;
+	struct drm_rect *dst = &plane_state->dst;
+	unsigned int rotation = plane_state->rotation;
 	int hscale, vscale;
 
-	*src = drm_plane_state_src(state);
-	*dst = drm_plane_state_dest(state);
+	WARN_ON(!crtc_state != !plane_state->crtc);
+	WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
+
+	*src = drm_plane_state_src(plane_state);
+	*dst = drm_plane_state_dest(plane_state);
 
 	if (!fb) {
-		state->visible = false;
+		plane_state->visible = false;
 		return 0;
 	}
 
 	/* crtc should only be NULL when disabling (i.e., !fb) */
-	if (WARN_ON(!crtc)) {
-		state->visible = false;
+	if (WARN_ON(!plane_state->crtc)) {
+		plane_state->visible = false;
 		return 0;
 	}
 
-	if (!crtc->enabled && !can_update_disabled) {
+	if (!crtc_state->enable && !can_update_disabled) {
 		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
 		return -EINVAL;
 	}
@@ -160,16 +164,16 @@ int drm_plane_helper_check_state(struct drm_plane_state *state,
 	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
 	if (hscale < 0 || vscale < 0) {
 		DRM_DEBUG_KMS("Invalid scaling of plane\n");
-		drm_rect_debug_print("src: ", &state->src, true);
-		drm_rect_debug_print("dst: ", &state->dst, false);
+		drm_rect_debug_print("src: ", &plane_state->src, true);
+		drm_rect_debug_print("dst: ", &plane_state->dst, false);
 		return -ERANGE;
 	}
 
-	state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
 
 	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
 
-	if (!state->visible)
+	if (!plane_state->visible)
 		/*
 		 * Plane isn't visible; some drivers can handle this
 		 * so we just return success here.  Drivers that can't
@@ -230,7 +234,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
 				  bool can_update_disabled,
 				  bool *visible)
 {
-	struct drm_plane_state state = {
+	struct drm_plane_state plane_state = {
 		.plane = plane,
 		.crtc = crtc,
 		.fb = fb,
@@ -245,18 +249,22 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
 		.rotation = rotation,
 		.visible = *visible,
 	};
+	struct drm_crtc_state crtc_state = {
+		.crtc = crtc,
+		.enable = crtc->enabled,
+	};
 	int ret;
 
-	ret = drm_plane_helper_check_state(&state, clip,
+	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
 					   min_scale, max_scale,
 					   can_position,
 					   can_update_disabled);
 	if (ret)
 		return ret;
 
-	*src = state.src;
-	*dst = state.dst;
-	*visible = state.visible;
+	*src = plane_state.src;
+	*dst = plane_state.dst;
+	*visible = plane_state.visible;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index dc9fd109de14..d428c805025c 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(plane_state, &clip,
+	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..4a459726b36b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 	int ret;
 
 	ret = drm_plane_helper_check_state(&plane_state->base,
+					   &crtc_state->base,
 					   &plane_state->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
@@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
 	}
 
 	ret = drm_plane_helper_check_state(&state->base,
+					   &crtc_state->base,
 					   &state->clip,
 					   min_scale, max_scale,
 					   can_position, true);
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 247c60e6bed2..51b23ac175e3 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	clip.y1 = 0;
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   can_position, true);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 6f121891430f..7ebb33657704 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,7 +111,7 @@ 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_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    true, true);
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 17e96fa47868..2a00b720c371 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->mode.hdisplay;
 	clip.y2 = crtc_state->mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    true, true);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 4b22ac3413a1..1c194a9453d9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
 	min_scale = FRAC_16_16(1, 8);
 	max_scale = FRAC_16_16(8, 1);
 
-	ret = drm_plane_helper_check_state(state, &clip, min_scale,
-					   max_scale, true, true);
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   min_scale, max_scale, true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index e4751f92b342..7f11a88d0593 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 {
 	int ret;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+					   &asyw->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   true, true);
@@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 	if (!fb->format->depth)
 		return -EINVAL;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+					   &asyw->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 19128b4dea54..36d0f101e30d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   min_scale, max_scale,
 					   true, true);
 	if (ret)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 24a5ef4f5bb8..e95d9e2a7f6c 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
 	clip.y2 = crtc_state->mode.vdisplay;
 
 	/* Check plane state for visibility and calculate clipping bounds */
-	err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
-					   true, true);
+	err = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   0, INT_MAX, true, true);
 	if (err < 0)
 		return err;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index efa41c086198..d95e7b1c3b11 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 		clip.y2 = crtc_state->adjusted_mode.vdisplay;
 	}
 
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index 18e763493264..ee0002529b8f 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, &clip,
+	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					    min_scale, max_scale,
 					    true, true);
 }
@@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, &clip,
+	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    false, true);
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 7c8a00ceadb7..41b8309b0a57 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,7 +38,8 @@
  */
 #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
 
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+				 const struct drm_crtc_state *crtc_state,
 				 const struct drm_rect *clip,
 				 int min_scale, int max_scale,
 				 bool can_position,
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_plane_helper_check_state() is supposed to do things the atomic way,
so it should not be inspecting crtc->enabled. Rather we should be
looking at crtc_state->enable.

We have a slight complication due to drm_plane_helper_check_update()
reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
we'll have to pass the crtc_state in manally and construct a fake
crtc_state in drm_plane_helper_check_update().

v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c            |  2 +-
 drivers/gpu/drm/arm/malidp_planes.c         |  3 +-
 drivers/gpu/drm/drm_plane_helper.c          | 51 ++++++++++++++++-------------
 drivers/gpu/drm/drm_simple_kms_helper.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c        |  2 ++
 drivers/gpu/drm/imx/ipuv3-plane.c           |  2 +-
 drivers/gpu/drm/mediatek/mtk_drm_plane.c    |  2 +-
 drivers/gpu/drm/meson/meson_plane.c         |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  4 +--
 drivers/gpu/drm/nouveau/nv50_display.c      |  6 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  2 +-
 drivers/gpu/drm/tegra/dc.c                  |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  2 +-
 drivers/gpu/drm/zte/zx_plane.c              |  4 +--
 include/drm/drm_plane_helper.h              |  3 +-
 15 files changed, 52 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 72b22b805412..14721723fa8a 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    false, true);
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 94e7e3fa3408..492d99dd55d4 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
 
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   0, INT_MAX, true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 759ed93f4ba8..eef0ffcd7ed5 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
 
 /**
  * drm_plane_helper_check_state() - Check plane state for validity
- * @state: plane state to check
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
  * @clip: integer clipping coordinates
  * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
  * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
@@ -120,35 +121,37 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
  * RETURNS:
  * Zero if update appears valid, error code on failure
  */
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+				 const struct drm_crtc_state *crtc_state,
 				 const struct drm_rect *clip,
 				 int min_scale,
 				 int max_scale,
 				 bool can_position,
 				 bool can_update_disabled)
 {
-	struct drm_crtc *crtc = state->crtc;
-	struct drm_framebuffer *fb = state->fb;
-	struct drm_rect *src = &state->src;
-	struct drm_rect *dst = &state->dst;
-	unsigned int rotation = state->rotation;
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_rect *src = &plane_state->src;
+	struct drm_rect *dst = &plane_state->dst;
+	unsigned int rotation = plane_state->rotation;
 	int hscale, vscale;
 
-	*src = drm_plane_state_src(state);
-	*dst = drm_plane_state_dest(state);
+	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
+
+	*src = drm_plane_state_src(plane_state);
+	*dst = drm_plane_state_dest(plane_state);
 
 	if (!fb) {
-		state->visible = false;
+		plane_state->visible = false;
 		return 0;
 	}
 
 	/* crtc should only be NULL when disabling (i.e., !fb) */
-	if (WARN_ON(!crtc)) {
-		state->visible = false;
+	if (WARN_ON(!plane_state->crtc)) {
+		plane_state->visible = false;
 		return 0;
 	}
 
-	if (!crtc->enabled && !can_update_disabled) {
+	if (!crtc_state->enable && !can_update_disabled) {
 		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
 		return -EINVAL;
 	}
@@ -160,16 +163,16 @@ int drm_plane_helper_check_state(struct drm_plane_state *state,
 	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
 	if (hscale < 0 || vscale < 0) {
 		DRM_DEBUG_KMS("Invalid scaling of plane\n");
-		drm_rect_debug_print("src: ", &state->src, true);
-		drm_rect_debug_print("dst: ", &state->dst, false);
+		drm_rect_debug_print("src: ", &plane_state->src, true);
+		drm_rect_debug_print("dst: ", &plane_state->dst, false);
 		return -ERANGE;
 	}
 
-	state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
 
 	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
 
-	if (!state->visible)
+	if (!plane_state->visible)
 		/*
 		 * Plane isn't visible; some drivers can handle this
 		 * so we just return success here.  Drivers that can't
@@ -230,7 +233,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
 				  bool can_update_disabled,
 				  bool *visible)
 {
-	struct drm_plane_state state = {
+	struct drm_plane_state plane_state = {
 		.plane = plane,
 		.crtc = crtc,
 		.fb = fb,
@@ -245,18 +248,22 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
 		.rotation = rotation,
 		.visible = *visible,
 	};
+	struct drm_crtc_state crtc_state = {
+		.crtc = crtc,
+		.enable = crtc->enabled,
+	};
 	int ret;
 
-	ret = drm_plane_helper_check_state(&state, clip,
+	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
 					   min_scale, max_scale,
 					   can_position,
 					   can_update_disabled);
 	if (ret)
 		return ret;
 
-	*src = state.src;
-	*dst = state.dst;
-	*visible = state.visible;
+	*src = plane_state.src;
+	*dst = plane_state.dst;
+	*visible = plane_state.visible;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index dc9fd109de14..d428c805025c 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(plane_state, &clip,
+	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..4a459726b36b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 	int ret;
 
 	ret = drm_plane_helper_check_state(&plane_state->base,
+					   &crtc_state->base,
 					   &plane_state->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
@@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
 	}
 
 	ret = drm_plane_helper_check_state(&state->base,
+					   &crtc_state->base,
 					   &state->clip,
 					   min_scale, max_scale,
 					   can_position, true);
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 247c60e6bed2..51b23ac175e3 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	clip.y1 = 0;
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   can_position, true);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 6f121891430f..7ebb33657704 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,7 +111,7 @@ 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_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    true, true);
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 17e96fa47868..2a00b720c371 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->mode.hdisplay;
 	clip.y2 = crtc_state->mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    true, true);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 4b22ac3413a1..1c194a9453d9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
 	min_scale = FRAC_16_16(1, 8);
 	max_scale = FRAC_16_16(8, 1);
 
-	ret = drm_plane_helper_check_state(state, &clip, min_scale,
-					   max_scale, true, true);
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   min_scale, max_scale, true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index e4751f92b342..7f11a88d0593 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 {
 	int ret;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+					   &asyw->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   true, true);
@@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 	if (!fb->format->depth)
 		return -EINVAL;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+					   &asyw->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 19128b4dea54..36d0f101e30d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   min_scale, max_scale,
 					   true, true);
 	if (ret)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 24a5ef4f5bb8..e95d9e2a7f6c 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
 	clip.y2 = crtc_state->mode.vdisplay;
 
 	/* Check plane state for visibility and calculate clipping bounds */
-	err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
-					   true, true);
+	err = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   0, INT_MAX, true, true);
 	if (err < 0)
 		return err;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index efa41c086198..d95e7b1c3b11 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 		clip.y2 = crtc_state->adjusted_mode.vdisplay;
 	}
 
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index 18e763493264..ee0002529b8f 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, &clip,
+	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					    min_scale, max_scale,
 					    true, true);
 }
@@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, &clip,
+	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    false, true);
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 7c8a00ceadb7..41b8309b0a57 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,7 +38,8 @@
  */
 #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
 
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+				 const struct drm_crtc_state *crtc_state,
 				 const struct drm_rect *clip,
 				 int min_scale, int max_scale,
 				 bool can_position,
-- 
2.13.6

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

Re: [PATCH v2 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()

[Daniel Vetter]

On Wed, Nov 01, 2017 at 10:15:58PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> drm_plane_helper_check_state() is supposed to do things the atomic way,
> so it should not be inspecting crtc->enabled. Rather we should be
> looking at crtc_state->enable.

Oh dear, how often are those legacy state thingies going to hurt us :-/

> We have a slight complication due to drm_plane_helper_check_update()
> reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
> we'll have to pass the crtc_state in manally and construct a fake
> crtc_state in drm_plane_helper_check_update().
> 
> v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I guess even more reasons to drop the clip rectangle and just look at
crtc_state?

Anyway, this looks good to me.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/arm/hdlcd_crtc.c            |  2 +-
>  drivers/gpu/drm/arm/malidp_planes.c         |  3 +-
>  drivers/gpu/drm/drm_plane_helper.c          | 51 ++++++++++++++++-------------
>  drivers/gpu/drm/drm_simple_kms_helper.c     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c        |  2 ++
>  drivers/gpu/drm/imx/ipuv3-plane.c           |  2 +-
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c    |  2 +-
>  drivers/gpu/drm/meson/meson_plane.c         |  2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  4 +--
>  drivers/gpu/drm/nouveau/nv50_display.c      |  6 ++--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  2 +-
>  drivers/gpu/drm/tegra/dc.c                  |  4 +--
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  2 +-
>  drivers/gpu/drm/zte/zx_plane.c              |  4 +--
>  include/drm/drm_plane_helper.h              |  3 +-
>  15 files changed, 52 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 72b22b805412..14721723fa8a 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	return drm_plane_helper_check_state(state, &clip,
> +	return drm_plane_helper_check_state(state, crtc_state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    false, true);
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index 94e7e3fa3408..492d99dd55d4 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
>  
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> -	ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> +					   0, INT_MAX, true, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index 759ed93f4ba8..eef0ffcd7ed5 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>  
>  /**
>   * drm_plane_helper_check_state() - Check plane state for validity
> - * @state: plane state to check
> + * @plane_state: plane state to check
> + * @crtc_state: crtc state to check
>   * @clip: integer clipping coordinates
>   * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
>   * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> @@ -120,35 +121,37 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>   * RETURNS:
>   * Zero if update appears valid, error code on failure
>   */
> -int drm_plane_helper_check_state(struct drm_plane_state *state,
> +int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> +				 const struct drm_crtc_state *crtc_state,
>  				 const struct drm_rect *clip,
>  				 int min_scale,
>  				 int max_scale,
>  				 bool can_position,
>  				 bool can_update_disabled)
>  {
> -	struct drm_crtc *crtc = state->crtc;
> -	struct drm_framebuffer *fb = state->fb;
> -	struct drm_rect *src = &state->src;
> -	struct drm_rect *dst = &state->dst;
> -	unsigned int rotation = state->rotation;
> +	struct drm_framebuffer *fb = plane_state->fb;
> +	struct drm_rect *src = &plane_state->src;
> +	struct drm_rect *dst = &plane_state->dst;
> +	unsigned int rotation = plane_state->rotation;
>  	int hscale, vscale;
>  
> -	*src = drm_plane_state_src(state);
> -	*dst = drm_plane_state_dest(state);
> +	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> +
> +	*src = drm_plane_state_src(plane_state);
> +	*dst = drm_plane_state_dest(plane_state);
>  
>  	if (!fb) {
> -		state->visible = false;
> +		plane_state->visible = false;
>  		return 0;
>  	}
>  
>  	/* crtc should only be NULL when disabling (i.e., !fb) */
> -	if (WARN_ON(!crtc)) {
> -		state->visible = false;
> +	if (WARN_ON(!plane_state->crtc)) {
> +		plane_state->visible = false;
>  		return 0;
>  	}
>  
> -	if (!crtc->enabled && !can_update_disabled) {
> +	if (!crtc_state->enable && !can_update_disabled) {
>  		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
>  		return -EINVAL;
>  	}
> @@ -160,16 +163,16 @@ int drm_plane_helper_check_state(struct drm_plane_state *state,
>  	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
>  	if (hscale < 0 || vscale < 0) {
>  		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> -		drm_rect_debug_print("src: ", &state->src, true);
> -		drm_rect_debug_print("dst: ", &state->dst, false);
> +		drm_rect_debug_print("src: ", &plane_state->src, true);
> +		drm_rect_debug_print("dst: ", &plane_state->dst, false);
>  		return -ERANGE;
>  	}
>  
> -	state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> +	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
>  
>  	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
>  
> -	if (!state->visible)
> +	if (!plane_state->visible)
>  		/*
>  		 * Plane isn't visible; some drivers can handle this
>  		 * so we just return success here.  Drivers that can't
> @@ -230,7 +233,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
>  				  bool can_update_disabled,
>  				  bool *visible)
>  {
> -	struct drm_plane_state state = {
> +	struct drm_plane_state plane_state = {
>  		.plane = plane,
>  		.crtc = crtc,
>  		.fb = fb,
> @@ -245,18 +248,22 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
>  		.rotation = rotation,
>  		.visible = *visible,
>  	};
> +	struct drm_crtc_state crtc_state = {
> +		.crtc = crtc,
> +		.enable = crtc->enabled,
> +	};
>  	int ret;
>  
> -	ret = drm_plane_helper_check_state(&state, clip,
> +	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
>  					   min_scale, max_scale,
>  					   can_position,
>  					   can_update_disabled);
>  	if (ret)
>  		return ret;
>  
> -	*src = state.src;
> -	*dst = state.dst;
> -	*visible = state.visible;
> +	*src = plane_state.src;
> +	*dst = plane_state.dst;
> +	*visible = plane_state.visible;
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index dc9fd109de14..d428c805025c 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	ret = drm_plane_helper_check_state(plane_state, &clip,
> +	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   false, true);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 737de251d0f8..4a459726b36b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
>  	int ret;
>  
>  	ret = drm_plane_helper_check_state(&plane_state->base,
> +					   &crtc_state->base,
>  					   &plane_state->clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
> @@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
>  	}
>  
>  	ret = drm_plane_helper_check_state(&state->base,
> +					   &crtc_state->base,
>  					   &state->clip,
>  					   min_scale, max_scale,
>  					   can_position, true);
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 247c60e6bed2..51b23ac175e3 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>  	clip.y1 = 0;
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> -	ret = drm_plane_helper_check_state(state, &clip,
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   can_position, true);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 6f121891430f..7ebb33657704 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -111,7 +111,7 @@ 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_state(state, &clip,
> +	return drm_plane_helper_check_state(state, crtc_state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    true, true);
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 17e96fa47868..2a00b720c371 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->mode.hdisplay;
>  	clip.y2 = crtc_state->mode.vdisplay;
>  
> -	return drm_plane_helper_check_state(state, &clip,
> +	return drm_plane_helper_check_state(state, crtc_state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    true, true);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index 4b22ac3413a1..1c194a9453d9 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
>  	min_scale = FRAC_16_16(1, 8);
>  	max_scale = FRAC_16_16(8, 1);
>  
> -	ret = drm_plane_helper_check_state(state, &clip, min_scale,
> -					   max_scale, true, true);
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> +					   min_scale, max_scale, true, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index e4751f92b342..7f11a88d0593 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
>  {
>  	int ret;
>  
> -	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
> +	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> +					   &asyw->clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   true, true);
> @@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
>  	if (!fb->format->depth)
>  		return -EINVAL;
>  
> -	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
> +	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> +					   &asyw->clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   false, true);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 19128b4dea54..36d0f101e30d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	ret = drm_plane_helper_check_state(state, &clip,
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
>  					   min_scale, max_scale,
>  					   true, true);
>  	if (ret)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 24a5ef4f5bb8..e95d9e2a7f6c 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
>  	clip.y2 = crtc_state->mode.vdisplay;
>  
>  	/* Check plane state for visibility and calculate clipping bounds */
> -	err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
> -					   true, true);
> +	err = drm_plane_helper_check_state(state, crtc_state, &clip,
> +					   0, INT_MAX, true, true);
>  	if (err < 0)
>  		return err;
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index efa41c086198..d95e7b1c3b11 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
>  		clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  	}
>  
> -	ret = drm_plane_helper_check_state(state, &clip,
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   false, true);
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index 18e763493264..ee0002529b8f 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	return drm_plane_helper_check_state(plane_state, &clip,
> +	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
>  					    min_scale, max_scale,
>  					    true, true);
>  }
> @@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	return drm_plane_helper_check_state(plane_state, &clip,
> +	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    false, true);
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 7c8a00ceadb7..41b8309b0a57 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -38,7 +38,8 @@
>   */
>  #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
>  
> -int drm_plane_helper_check_state(struct drm_plane_state *state,
> +int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> +				 const struct drm_crtc_state *crtc_state,
>  				 const struct drm_rect *clip,
>  				 int min_scale, int max_scale,
>  				 bool can_position,
> -- 
> 2.13.6
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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

Re: [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()

[Laurent Pinchart]

Hi Ville,

Thank you for the patch.

On Wednesday, 1 November 2017 20:29:19 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> drm_plane_helper_check_state() is supposed to do things the atomic way,
> so it should not be inspecting crtc->enabled. Rather we should be
> looking at crtc_state->enable.
> 
> We have a slight complication due to drm_plane_helper_check_update()
> reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
> we'll have to pass the crtc_state in manally and construct a fake
> crtc_state in drm_plane_helper_check_update().

The only user of drm_plane_helper_check_update() apart from vmwgfx which you 
converted in patch 2/5 is the armada driver. It would be nice to convert it to 
atomic modesetting and drop this function :-) At the very least, I'd add a 
comment to drm_plane_helper_check_update() that this change should be reverted 
when the function is dropped, otherwise we'll carry the CRTC state argument 
for a long time without realizing it's not needed anymore.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/arm/hdlcd_crtc.c            |  2 +-
>  drivers/gpu/drm/arm/malidp_planes.c         |  3 +-
>  drivers/gpu/drm/drm_plane_helper.c          | 52 ++++++++++++++------------
>  drivers/gpu/drm/drm_simple_kms_helper.c     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c        |  2 ++
>  drivers/gpu/drm/imx/ipuv3-plane.c           |  2 +-
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c    |  2 +-
>  drivers/gpu/drm/meson/meson_plane.c         |  2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  4 +--
>  drivers/gpu/drm/nouveau/nv50_display.c      |  6 ++--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  2 +-
>  drivers/gpu/drm/tegra/dc.c                  |  4 +--
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  2 +-
>  drivers/gpu/drm/zte/zx_plane.c              |  4 +--
>  include/drm/drm_plane_helper.h              |  3 +-
>  15 files changed, 53 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c
> b/drivers/gpu/drm/arm/hdlcd_crtc.c index 72b22b805412..14721723fa8a 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	return drm_plane_helper_check_state(state, &clip,
> +	return drm_plane_helper_check_state(state, crtc_state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    false, true);
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> b/drivers/gpu/drm/arm/malidp_planes.c index 94e7e3fa3408..492d99dd55d4
> 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane
> *mp,
> 
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> -	ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> +					   0, INT_MAX, true, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/drm_plane_helper.c
> b/drivers/gpu/drm/drm_plane_helper.c index 759ed93f4ba8..adb8d94484f2
> 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc
> *crtc,
> 
>  /**
>   * drm_plane_helper_check_state() - Check plane state for validity
> - * @state: plane state to check
> + * @plane_state: plane state to check
> + * @crtc_state: crtc state to check
>   * @clip: integer clipping coordinates
>   * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
>   * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> @@ -120,35 +121,38 @@ static int get_connectors_for_crtc(struct drm_crtc
> *crtc, * RETURNS:
>   * Zero if update appears valid, error code on failure
>   */
> -int drm_plane_helper_check_state(struct drm_plane_state *state,
> +int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> +				 const struct drm_crtc_state *crtc_state,
>  				 const struct drm_rect *clip,
>  				 int min_scale,
>  				 int max_scale,
>  				 bool can_position,
>  				 bool can_update_disabled)
>  {
> -	struct drm_crtc *crtc = state->crtc;
> -	struct drm_framebuffer *fb = state->fb;
> -	struct drm_rect *src = &state->src;
> -	struct drm_rect *dst = &state->dst;
> -	unsigned int rotation = state->rotation;
> +	struct drm_framebuffer *fb = plane_state->fb;
> +	struct drm_rect *src = &plane_state->src;
> +	struct drm_rect *dst = &plane_state->dst;
> +	unsigned int rotation = plane_state->rotation;
>  	int hscale, vscale;
> 
> -	*src = drm_plane_state_src(state);
> -	*dst = drm_plane_state_dest(state);
> +	WARN_ON(!crtc_state != !plane_state->crtc);
> +	WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
> +
> +	*src = drm_plane_state_src(plane_state);
> +	*dst = drm_plane_state_dest(plane_state);
> 
>  	if (!fb) {
> -		state->visible = false;
> +		plane_state->visible = false;
>  		return 0;
>  	}
> 
>  	/* crtc should only be NULL when disabling (i.e., !fb) */
> -	if (WARN_ON(!crtc)) {
> -		state->visible = false;
> +	if (WARN_ON(!plane_state->crtc)) {
> +		plane_state->visible = false;
>  		return 0;
>  	}
> 
> -	if (!crtc->enabled && !can_update_disabled) {
> +	if (!crtc_state->enable && !can_update_disabled) {
>  		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
>  		return -EINVAL;
>  	}
> @@ -160,16 +164,16 @@ int drm_plane_helper_check_state(struct
> drm_plane_state *state, vscale = drm_rect_calc_vscale(src, dst, min_scale,
> max_scale);
>  	if (hscale < 0 || vscale < 0) {
>  		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> -		drm_rect_debug_print("src: ", &state->src, true);
> -		drm_rect_debug_print("dst: ", &state->dst, false);
> +		drm_rect_debug_print("src: ", &plane_state->src, true);
> +		drm_rect_debug_print("dst: ", &plane_state->dst, false);
>  		return -ERANGE;
>  	}
> 
> -	state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> +	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale,
> vscale);
> 
>  	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> 
> -	if (!state->visible)
> +	if (!plane_state->visible)
>  		/*
>  		 * Plane isn't visible; some drivers can handle this
>  		 * so we just return success here.  Drivers that can't
> @@ -230,7 +234,7 @@ int drm_plane_helper_check_update(struct drm_plane
> *plane, bool can_update_disabled,
>  				  bool *visible)
>  {
> -	struct drm_plane_state state = {
> +	struct drm_plane_state plane_state = {
>  		.plane = plane,
>  		.crtc = crtc,
>  		.fb = fb,
> @@ -245,18 +249,22 @@ int drm_plane_helper_check_update(struct drm_plane
> *plane, .rotation = rotation,
>  		.visible = *visible,
>  	};
> +	struct drm_crtc_state crtc_state = {
> +		.crtc = crtc,
> +		.enable = crtc->enabled,
> +	};
>  	int ret;
> 
> -	ret = drm_plane_helper_check_state(&state, clip,
> +	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
>  					   min_scale, max_scale,
>  					   can_position,
>  					   can_update_disabled);
>  	if (ret)
>  		return ret;
> 
> -	*src = state.src;
> -	*dst = state.dst;
> -	*visible = state.visible;
> +	*src = plane_state.src;
> +	*dst = plane_state.dst;
> +	*visible = plane_state.visible;
> 
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
> b/drivers/gpu/drm/drm_simple_kms_helper.c index dc9fd109de14..d428c805025c
> 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct
> drm_plane *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	ret = drm_plane_helper_check_state(plane_state, &clip,
> +	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   false, true);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 737de251d0f8..4a459726b36b
> 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state
> *crtc_state, int ret;
> 
>  	ret = drm_plane_helper_check_state(&plane_state->base,
> +					   &crtc_state->base,
>  					   &plane_state->clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
> @@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
> }
> 
>  	ret = drm_plane_helper_check_state(&state->base,
> +					   &crtc_state->base,
>  					   &state->clip,
>  					   min_scale, max_scale,
>  					   can_position, true);
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
> b/drivers/gpu/drm/imx/ipuv3-plane.c index 247c60e6bed2..51b23ac175e3 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane
> *plane, clip.y1 = 0;
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> -	ret = drm_plane_helper_check_state(state, &clip,
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   can_position, true);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 6f121891430f..7ebb33657704
> 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -111,7 +111,7 @@ 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_state(state, &clip,
> +	return drm_plane_helper_check_state(state, crtc_state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    true, true);
> diff --git a/drivers/gpu/drm/meson/meson_plane.c
> b/drivers/gpu/drm/meson/meson_plane.c index 17e96fa47868..2a00b720c371
> 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->mode.hdisplay;
>  	clip.y2 = crtc_state->mode.vdisplay;
> 
> -	return drm_plane_helper_check_state(state, &clip,
> +	return drm_plane_helper_check_state(state, crtc_state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    true, true);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index
> 4b22ac3413a1..1c194a9453d9 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct
> drm_crtc_state *crtc_state, min_scale = FRAC_16_16(1, 8);
>  	max_scale = FRAC_16_16(8, 1);
> 
> -	ret = drm_plane_helper_check_state(state, &clip, min_scale,
> -					   max_scale, true, true);
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> +					   min_scale, max_scale, true, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
> b/drivers/gpu/drm/nouveau/nv50_display.c index e4751f92b342..7f11a88d0593
> 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw, {
>  	int ret;
> 
> -	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
> +	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> +					   &asyw->clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   true, true);
> @@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw, if (!fb->format->depth)
>  		return -EINVAL;
> 
> -	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
> +	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> +					   &asyw->clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   false, true);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index
> 19128b4dea54..36d0f101e30d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	ret = drm_plane_helper_check_state(state, &clip,
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
>  					   min_scale, max_scale,
>  					   true, true);
>  	if (ret)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 24a5ef4f5bb8..e95d9e2a7f6c 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane
> *plane, clip.y2 = crtc_state->mode.vdisplay;
> 
>  	/* Check plane state for visibility and calculate clipping bounds */
> -	err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
> -					   true, true);
> +	err = drm_plane_helper_check_state(state, crtc_state, &clip,
> +					   0, INT_MAX, true, true);
>  	if (err < 0)
>  		return err;
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index efa41c086198..d95e7b1c3b11
> 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
> *plane, clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  	}
> 
> -	ret = drm_plane_helper_check_state(state, &clip,
> +	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   DRM_PLANE_HELPER_NO_SCALING,
>  					   false, true);
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index 18e763493264..ee0002529b8f 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	return drm_plane_helper_check_state(plane_state, &clip,
> +	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
>  					    min_scale, max_scale,
>  					    true, true);
>  }
> @@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	return drm_plane_helper_check_state(plane_state, &clip,
> +	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    false, true);
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 7c8a00ceadb7..41b8309b0a57 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -38,7 +38,8 @@
>   */
>  #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
> 
> -int drm_plane_helper_check_state(struct drm_plane_state *state,
> +int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> +				 const struct drm_crtc_state *crtc_state,
>  				 const struct drm_rect *clip,
>  				 int min_scale, int max_scale,
>  				 bool can_position,

-- 
Regards,

Laurent Pinchart

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

[PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_plane_helper_check_update() isn't a transitional helper, so let's
rename it to drm_atomic_helper_check_plane_state() and move it into
drm_atomic_helper.c.

Cc: Daniel Vetter <daniel@ffwll.ch>
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c            |   8 +--
 drivers/gpu/drm/arm/malidp_planes.c         |   4 +-
 drivers/gpu/drm/drm_atomic_helper.c         |  95 +++++++++++++++++++++++++
 drivers/gpu/drm/drm_plane_helper.c          | 103 ++--------------------------
 drivers/gpu/drm/drm_simple_kms_helper.c     |   9 +--
 drivers/gpu/drm/i915/intel_display.c        |  22 +++---
 drivers/gpu/drm/imx/ipuv3-plane.c           |   8 +--
 drivers/gpu/drm/mediatek/mtk_drm_plane.c    |   8 +--
 drivers/gpu/drm/meson/meson_plane.c         |   8 +--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
 drivers/gpu/drm/nouveau/nv50_display.c      |  20 +++---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
 drivers/gpu/drm/tegra/dc.c                  |   4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |   8 +--
 drivers/gpu/drm/zte/zx_plane.c              |  15 ++--
 include/drm/drm_atomic_helper.h             |   7 ++
 include/drm/drm_plane_helper.h              |   6 --
 17 files changed, 170 insertions(+), 166 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 14721723fa8a..63511a3bbf6c 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,10 +252,10 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, crtc_state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    false, true);
+	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   false, true);
 }
 
 static void hdlcd_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 492d99dd55d4..72a07950167e 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,8 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
 
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   0, INT_MAX, true, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  0, INT_MAX, true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 71d712f1b56a..7595ad8ad2f3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -696,6 +696,101 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
 
 /**
+ * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
+ * @clip: integer clipping coordinates
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ * @can_position: is it legal to position the plane such that it
+ *                doesn't cover the entire crtc?  This will generally
+ *                only be false for primary planes.
+ * @can_update_disabled: can the plane be updated while the crtc
+ *                       is disabled?
+ *
+ * Checks that a desired plane update is valid, and updates various
+ * bits of derived state (clipped coordinates etc.). Drivers that provide
+ * their own plane handling rather than helper-provided implementations may
+ * still wish to call this function to avoid duplication of error checking
+ * code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+					const struct drm_crtc_state *crtc_state,
+					const struct drm_rect *clip,
+					int min_scale,
+					int max_scale,
+					bool can_position,
+					bool can_update_disabled)
+{
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_rect *src = &plane_state->src;
+	struct drm_rect *dst = &plane_state->dst;
+	unsigned int rotation = plane_state->rotation;
+	int hscale, vscale;
+
+	WARN_ON(!crtc_state != !plane_state->crtc);
+	WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
+
+	*src = drm_plane_state_src(plane_state);
+	*dst = drm_plane_state_dest(plane_state);
+
+	if (!fb) {
+		plane_state->visible = false;
+		return 0;
+	}
+
+	/* crtc should only be NULL when disabling (i.e., !fb) */
+	if (WARN_ON(!plane_state->crtc)) {
+		plane_state->visible = false;
+		return 0;
+	}
+
+	if (!crtc_state->enable && !can_update_disabled) {
+		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
+		return -EINVAL;
+	}
+
+	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
+
+	/* Check scaling */
+	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
+	if (hscale < 0 || vscale < 0) {
+		DRM_DEBUG_KMS("Invalid scaling of plane\n");
+		drm_rect_debug_print("src: ", &plane_state->src, true);
+		drm_rect_debug_print("dst: ", &plane_state->dst, false);
+		return -ERANGE;
+	}
+
+	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+
+	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
+
+	if (!plane_state->visible)
+		/*
+		 * Plane isn't visible; some drivers can handle this
+		 * so we just return success here.  Drivers that can't
+		 * (including those that use the primary plane helper's
+		 * update function) will return an error from their
+		 * update_plane handler.
+		 */
+		return 0;
+
+	if (!can_position && !drm_rect_equals(dst, clip)) {
+		DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
+		drm_rect_debug_print("dst: ", dst, false);
+		drm_rect_debug_print("clip: ", clip, false);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
+
+/**
  * drm_atomic_helper_check_planes - validate state object for planes changes
  * @dev: DRM device
  * @state: the driver state object
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index adb8d94484f2..f1be8cd4e387 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -100,101 +100,6 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
 }
 
 /**
- * drm_plane_helper_check_state() - Check plane state for validity
- * @plane_state: plane state to check
- * @crtc_state: crtc state to check
- * @clip: integer clipping coordinates
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
- * @can_position: is it legal to position the plane such that it
- *                doesn't cover the entire crtc?  This will generally
- *                only be false for primary planes.
- * @can_update_disabled: can the plane be updated while the crtc
- *                       is disabled?
- *
- * Checks that a desired plane update is valid, and updates various
- * bits of derived state (clipped coordinates etc.). Drivers that provide
- * their own plane handling rather than helper-provided implementations may
- * still wish to call this function to avoid duplication of error checking
- * code.
- *
- * RETURNS:
- * Zero if update appears valid, error code on failure
- */
-int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
-				 const struct drm_crtc_state *crtc_state,
-				 const struct drm_rect *clip,
-				 int min_scale,
-				 int max_scale,
-				 bool can_position,
-				 bool can_update_disabled)
-{
-	struct drm_framebuffer *fb = plane_state->fb;
-	struct drm_rect *src = &plane_state->src;
-	struct drm_rect *dst = &plane_state->dst;
-	unsigned int rotation = plane_state->rotation;
-	int hscale, vscale;
-
-	WARN_ON(!crtc_state != !plane_state->crtc);
-	WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
-
-	*src = drm_plane_state_src(plane_state);
-	*dst = drm_plane_state_dest(plane_state);
-
-	if (!fb) {
-		plane_state->visible = false;
-		return 0;
-	}
-
-	/* crtc should only be NULL when disabling (i.e., !fb) */
-	if (WARN_ON(!plane_state->crtc)) {
-		plane_state->visible = false;
-		return 0;
-	}
-
-	if (!crtc_state->enable && !can_update_disabled) {
-		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
-		return -EINVAL;
-	}
-
-	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
-
-	/* Check scaling */
-	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
-	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
-	if (hscale < 0 || vscale < 0) {
-		DRM_DEBUG_KMS("Invalid scaling of plane\n");
-		drm_rect_debug_print("src: ", &plane_state->src, true);
-		drm_rect_debug_print("dst: ", &plane_state->dst, false);
-		return -ERANGE;
-	}
-
-	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
-
-	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
-
-	if (!plane_state->visible)
-		/*
-		 * Plane isn't visible; some drivers can handle this
-		 * so we just return success here.  Drivers that can't
-		 * (including those that use the primary plane helper's
-		 * update function) will return an error from their
-		 * update_plane handler.
-		 */
-		return 0;
-
-	if (!can_position && !drm_rect_equals(dst, clip)) {
-		DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
-		drm_rect_debug_print("dst: ", dst, false);
-		drm_rect_debug_print("clip: ", clip, false);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(drm_plane_helper_check_state);
-
-/**
  * drm_plane_helper_check_update() - Check plane update for validity
  * @plane: plane object to update
  * @crtc: owning CRTC of owning plane
@@ -255,10 +160,10 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
 	};
 	int ret;
 
-	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
-					   min_scale, max_scale,
-					   can_position,
-					   can_update_disabled);
+	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
+						  clip, min_scale, max_scale,
+						  can_position,
+						  can_update_disabled);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index d428c805025c..9f3b1c94802b 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,10 +103,11 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   false, true);
+	ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+						  &clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  false, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4a459726b36b..e0ff599d2090 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9316,12 +9316,12 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 	u32 offset;
 	int ret;
 
-	ret = drm_plane_helper_check_state(&plane_state->base,
-					   &crtc_state->base,
-					   &plane_state->clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   true, true);
+	ret = drm_atomic_helper_check_plane_state(&plane_state->base,
+						  &crtc_state->base,
+						  &plane_state->clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  true, true);
 	if (ret)
 		return ret;
 
@@ -12808,11 +12808,11 @@ intel_check_primary_plane(struct intel_plane *plane,
 		can_position = true;
 	}
 
-	ret = drm_plane_helper_check_state(&state->base,
-					   &crtc_state->base,
-					   &state->clip,
-					   min_scale, max_scale,
-					   can_position, true);
+	ret = drm_atomic_helper_check_plane_state(&state->base,
+						  &crtc_state->base,
+						  &state->clip,
+						  min_scale, max_scale,
+						  can_position, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 51b23ac175e3..5a67daedcf4d 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,10 +342,10 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	clip.y1 = 0;
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   can_position, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  can_position, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 7ebb33657704..5ef898b93d8d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,10 +111,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_state(state, crtc_state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    true, true);
+	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   true, true);
 }
 
 static void mtk_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 2a00b720c371..d0a6ac8390f3 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,10 +61,10 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->mode.hdisplay;
 	clip.y2 = crtc_state->mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, crtc_state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    true, true);
+	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   true, true);
 }
 
 /* Takes a fixed 16.16 number and converts it to integer. */
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 1c194a9453d9..df3360acacca 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,9 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
 	min_scale = FRAC_16_16(1, 8);
 	max_scale = FRAC_16_16(8, 1);
 
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   min_scale, max_scale, true, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  min_scale, max_scale,
+						  true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 7f11a88d0593..8ab0fac3544d 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,11 +1143,11 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 {
 	int ret;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
-					   &asyw->clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   true, true);
+	ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
+						  &asyw->clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  true, true);
 	asyh->curs.visible = asyw->state.visible;
 	if (ret || !asyh->curs.visible)
 		return ret;
@@ -1433,11 +1433,11 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 	if (!fb->format->depth)
 		return -EINVAL;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
-					   &asyw->clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   false, true);
+	ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
+						  &asyw->clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  false, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 36d0f101e30d..ba7505292b78 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,9 +659,9 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   min_scale, max_scale,
-					   true, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  min_scale, max_scale,
+						  true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index e95d9e2a7f6c..fc70351b9017 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
 	clip.y2 = crtc_state->mode.vdisplay;
 
 	/* Check plane state for visibility and calculate clipping bounds */
-	err = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   0, INT_MAX, true, true);
+	err = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  0, INT_MAX, true, true);
 	if (err < 0)
 		return err;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index d95e7b1c3b11..a2a93d7e2a04 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,10 +454,10 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 		clip.y2 = crtc_state->adjusted_mode.vdisplay;
 	}
 
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   false, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  false, true);
 
 	if (!ret && new_fb) {
 		struct drm_crtc *crtc = state->crtc;
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index ee0002529b8f..68fd2e2dc78a 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,9 +80,9 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
-					    min_scale, max_scale,
-					    true, true);
+	return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+						   &clip, min_scale, max_scale,
+						   true, true);
 }
 
 static int zx_vl_get_fmt(uint32_t format)
@@ -315,10 +315,11 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    false, true);
+	return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+						   &clip,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   false, true);
 }
 
 static int zx_gl_get_fmt(uint32_t format)
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index d2b56cc657e9..4842ee9485ce 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -38,6 +38,13 @@ struct drm_private_state;
 
 int drm_atomic_helper_check_modeset(struct drm_device *dev,
 				struct drm_atomic_state *state);
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+					const struct drm_crtc_state *crtc_state,
+					const struct drm_rect *clip,
+					int min_scale,
+					int max_scale,
+					bool can_position,
+					bool can_update_disabled);
 int drm_atomic_helper_check_planes(struct drm_device *dev,
 			       struct drm_atomic_state *state);
 int drm_atomic_helper_check(struct drm_device *dev,
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 41b8309b0a57..8aa49c0ecd4d 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,12 +38,6 @@
  */
 #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
 
-int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
-				 const struct drm_crtc_state *crtc_state,
-				 const struct drm_rect *clip,
-				 int min_scale, int max_scale,
-				 bool can_position,
-				 bool can_update_disabled);
 int drm_plane_helper_check_update(struct drm_plane *plane,
 				  struct drm_crtc *crtc,
 				  struct drm_framebuffer *fb,
-- 
2.13.6

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

[PATCH v2 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_plane_helper_check_update() isn't a transitional helper, so let's
rename it to drm_atomic_helper_check_plane_state() and move it into
drm_atomic_helper.c.

v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc

Cc: Daniel Vetter <daniel@ffwll.ch>
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c            |   8 +--
 drivers/gpu/drm/arm/malidp_planes.c         |   4 +-
 drivers/gpu/drm/drm_atomic_helper.c         |  94 +++++++++++++++++++++++++
 drivers/gpu/drm/drm_plane_helper.c          | 102 ++--------------------------
 drivers/gpu/drm/drm_simple_kms_helper.c     |   9 +--
 drivers/gpu/drm/i915/intel_display.c        |  22 +++---
 drivers/gpu/drm/imx/ipuv3-plane.c           |   8 +--
 drivers/gpu/drm/mediatek/mtk_drm_plane.c    |   8 +--
 drivers/gpu/drm/meson/meson_plane.c         |   8 +--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
 drivers/gpu/drm/nouveau/nv50_display.c      |  20 +++---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
 drivers/gpu/drm/tegra/dc.c                  |   4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |   8 +--
 drivers/gpu/drm/zte/zx_plane.c              |  15 ++--
 include/drm/drm_atomic_helper.h             |   7 ++
 include/drm/drm_plane_helper.h              |   6 --
 17 files changed, 169 insertions(+), 165 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 14721723fa8a..63511a3bbf6c 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,10 +252,10 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, crtc_state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    false, true);
+	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   false, true);
 }
 
 static void hdlcd_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 492d99dd55d4..72a07950167e 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,8 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
 
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   0, INT_MAX, true, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  0, INT_MAX, true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 71d712f1b56a..a381913c894d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -696,6 +696,100 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
 
 /**
+ * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
+ * @clip: integer clipping coordinates
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ * @can_position: is it legal to position the plane such that it
+ *                doesn't cover the entire crtc?  This will generally
+ *                only be false for primary planes.
+ * @can_update_disabled: can the plane be updated while the crtc
+ *                       is disabled?
+ *
+ * Checks that a desired plane update is valid, and updates various
+ * bits of derived state (clipped coordinates etc.). Drivers that provide
+ * their own plane handling rather than helper-provided implementations may
+ * still wish to call this function to avoid duplication of error checking
+ * code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+					const struct drm_crtc_state *crtc_state,
+					const struct drm_rect *clip,
+					int min_scale,
+					int max_scale,
+					bool can_position,
+					bool can_update_disabled)
+{
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_rect *src = &plane_state->src;
+	struct drm_rect *dst = &plane_state->dst;
+	unsigned int rotation = plane_state->rotation;
+	int hscale, vscale;
+
+	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
+
+	*src = drm_plane_state_src(plane_state);
+	*dst = drm_plane_state_dest(plane_state);
+
+	if (!fb) {
+		plane_state->visible = false;
+		return 0;
+	}
+
+	/* crtc should only be NULL when disabling (i.e., !fb) */
+	if (WARN_ON(!plane_state->crtc)) {
+		plane_state->visible = false;
+		return 0;
+	}
+
+	if (!crtc_state->enable && !can_update_disabled) {
+		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
+		return -EINVAL;
+	}
+
+	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
+
+	/* Check scaling */
+	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
+	if (hscale < 0 || vscale < 0) {
+		DRM_DEBUG_KMS("Invalid scaling of plane\n");
+		drm_rect_debug_print("src: ", &plane_state->src, true);
+		drm_rect_debug_print("dst: ", &plane_state->dst, false);
+		return -ERANGE;
+	}
+
+	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+
+	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
+
+	if (!plane_state->visible)
+		/*
+		 * Plane isn't visible; some drivers can handle this
+		 * so we just return success here.  Drivers that can't
+		 * (including those that use the primary plane helper's
+		 * update function) will return an error from their
+		 * update_plane handler.
+		 */
+		return 0;
+
+	if (!can_position && !drm_rect_equals(dst, clip)) {
+		DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
+		drm_rect_debug_print("dst: ", dst, false);
+		drm_rect_debug_print("clip: ", clip, false);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
+
+/**
  * drm_atomic_helper_check_planes - validate state object for planes changes
  * @dev: DRM device
  * @state: the driver state object
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index eef0ffcd7ed5..f1be8cd4e387 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -100,100 +100,6 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
 }
 
 /**
- * drm_plane_helper_check_state() - Check plane state for validity
- * @plane_state: plane state to check
- * @crtc_state: crtc state to check
- * @clip: integer clipping coordinates
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
- * @can_position: is it legal to position the plane such that it
- *                doesn't cover the entire crtc?  This will generally
- *                only be false for primary planes.
- * @can_update_disabled: can the plane be updated while the crtc
- *                       is disabled?
- *
- * Checks that a desired plane update is valid, and updates various
- * bits of derived state (clipped coordinates etc.). Drivers that provide
- * their own plane handling rather than helper-provided implementations may
- * still wish to call this function to avoid duplication of error checking
- * code.
- *
- * RETURNS:
- * Zero if update appears valid, error code on failure
- */
-int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
-				 const struct drm_crtc_state *crtc_state,
-				 const struct drm_rect *clip,
-				 int min_scale,
-				 int max_scale,
-				 bool can_position,
-				 bool can_update_disabled)
-{
-	struct drm_framebuffer *fb = plane_state->fb;
-	struct drm_rect *src = &plane_state->src;
-	struct drm_rect *dst = &plane_state->dst;
-	unsigned int rotation = plane_state->rotation;
-	int hscale, vscale;
-
-	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
-
-	*src = drm_plane_state_src(plane_state);
-	*dst = drm_plane_state_dest(plane_state);
-
-	if (!fb) {
-		plane_state->visible = false;
-		return 0;
-	}
-
-	/* crtc should only be NULL when disabling (i.e., !fb) */
-	if (WARN_ON(!plane_state->crtc)) {
-		plane_state->visible = false;
-		return 0;
-	}
-
-	if (!crtc_state->enable && !can_update_disabled) {
-		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
-		return -EINVAL;
-	}
-
-	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
-
-	/* Check scaling */
-	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
-	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
-	if (hscale < 0 || vscale < 0) {
-		DRM_DEBUG_KMS("Invalid scaling of plane\n");
-		drm_rect_debug_print("src: ", &plane_state->src, true);
-		drm_rect_debug_print("dst: ", &plane_state->dst, false);
-		return -ERANGE;
-	}
-
-	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
-
-	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
-
-	if (!plane_state->visible)
-		/*
-		 * Plane isn't visible; some drivers can handle this
-		 * so we just return success here.  Drivers that can't
-		 * (including those that use the primary plane helper's
-		 * update function) will return an error from their
-		 * update_plane handler.
-		 */
-		return 0;
-
-	if (!can_position && !drm_rect_equals(dst, clip)) {
-		DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
-		drm_rect_debug_print("dst: ", dst, false);
-		drm_rect_debug_print("clip: ", clip, false);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(drm_plane_helper_check_state);
-
-/**
  * drm_plane_helper_check_update() - Check plane update for validity
  * @plane: plane object to update
  * @crtc: owning CRTC of owning plane
@@ -254,10 +160,10 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
 	};
 	int ret;
 
-	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
-					   min_scale, max_scale,
-					   can_position,
-					   can_update_disabled);
+	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
+						  clip, min_scale, max_scale,
+						  can_position,
+						  can_update_disabled);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index d428c805025c..9f3b1c94802b 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,10 +103,11 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   false, true);
+	ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+						  &clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  false, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4a459726b36b..e0ff599d2090 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9316,12 +9316,12 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 	u32 offset;
 	int ret;
 
-	ret = drm_plane_helper_check_state(&plane_state->base,
-					   &crtc_state->base,
-					   &plane_state->clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   true, true);
+	ret = drm_atomic_helper_check_plane_state(&plane_state->base,
+						  &crtc_state->base,
+						  &plane_state->clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  true, true);
 	if (ret)
 		return ret;
 
@@ -12808,11 +12808,11 @@ intel_check_primary_plane(struct intel_plane *plane,
 		can_position = true;
 	}
 
-	ret = drm_plane_helper_check_state(&state->base,
-					   &crtc_state->base,
-					   &state->clip,
-					   min_scale, max_scale,
-					   can_position, true);
+	ret = drm_atomic_helper_check_plane_state(&state->base,
+						  &crtc_state->base,
+						  &state->clip,
+						  min_scale, max_scale,
+						  can_position, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 51b23ac175e3..5a67daedcf4d 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,10 +342,10 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	clip.y1 = 0;
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   can_position, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  can_position, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 7ebb33657704..5ef898b93d8d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,10 +111,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_state(state, crtc_state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    true, true);
+	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   true, true);
 }
 
 static void mtk_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 2a00b720c371..d0a6ac8390f3 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,10 +61,10 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->mode.hdisplay;
 	clip.y2 = crtc_state->mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, crtc_state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    true, true);
+	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   true, true);
 }
 
 /* Takes a fixed 16.16 number and converts it to integer. */
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 1c194a9453d9..df3360acacca 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,9 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
 	min_scale = FRAC_16_16(1, 8);
 	max_scale = FRAC_16_16(8, 1);
 
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   min_scale, max_scale, true, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  min_scale, max_scale,
+						  true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 7f11a88d0593..8ab0fac3544d 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,11 +1143,11 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 {
 	int ret;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
-					   &asyw->clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   true, true);
+	ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
+						  &asyw->clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  true, true);
 	asyh->curs.visible = asyw->state.visible;
 	if (ret || !asyh->curs.visible)
 		return ret;
@@ -1433,11 +1433,11 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 	if (!fb->format->depth)
 		return -EINVAL;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
-					   &asyw->clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   false, true);
+	ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
+						  &asyw->clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  false, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 36d0f101e30d..ba7505292b78 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,9 +659,9 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   min_scale, max_scale,
-					   true, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  min_scale, max_scale,
+						  true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index e95d9e2a7f6c..fc70351b9017 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
 	clip.y2 = crtc_state->mode.vdisplay;
 
 	/* Check plane state for visibility and calculate clipping bounds */
-	err = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   0, INT_MAX, true, true);
+	err = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  0, INT_MAX, true, true);
 	if (err < 0)
 		return err;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index d95e7b1c3b11..a2a93d7e2a04 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,10 +454,10 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 		clip.y2 = crtc_state->adjusted_mode.vdisplay;
 	}
 
-	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   false, true);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  false, true);
 
 	if (!ret && new_fb) {
 		struct drm_crtc *crtc = state->crtc;
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index ee0002529b8f..68fd2e2dc78a 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,9 +80,9 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
-					    min_scale, max_scale,
-					    true, true);
+	return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+						   &clip, min_scale, max_scale,
+						   true, true);
 }
 
 static int zx_vl_get_fmt(uint32_t format)
@@ -315,10 +315,11 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    false, true);
+	return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+						   &clip,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   DRM_PLANE_HELPER_NO_SCALING,
+						   false, true);
 }
 
 static int zx_gl_get_fmt(uint32_t format)
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index d2b56cc657e9..4842ee9485ce 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -38,6 +38,13 @@ struct drm_private_state;
 
 int drm_atomic_helper_check_modeset(struct drm_device *dev,
 				struct drm_atomic_state *state);
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+					const struct drm_crtc_state *crtc_state,
+					const struct drm_rect *clip,
+					int min_scale,
+					int max_scale,
+					bool can_position,
+					bool can_update_disabled);
 int drm_atomic_helper_check_planes(struct drm_device *dev,
 			       struct drm_atomic_state *state);
 int drm_atomic_helper_check(struct drm_device *dev,
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 41b8309b0a57..8aa49c0ecd4d 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,12 +38,6 @@
  */
 #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
 
-int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
-				 const struct drm_crtc_state *crtc_state,
-				 const struct drm_rect *clip,
-				 int min_scale, int max_scale,
-				 bool can_position,
-				 bool can_update_disabled);
 int drm_plane_helper_check_update(struct drm_plane *plane,
 				  struct drm_crtc *crtc,
 				  struct drm_framebuffer *fb,
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c

[Daniel Vetter]

On Wed, Nov 01, 2017 at 10:16:19PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> drm_plane_helper_check_update() isn't a transitional helper, so let's
> rename it to drm_atomic_helper_check_plane_state() and move it into
> drm_atomic_helper.c.
> 
> v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I guess I should finally unlazy and split up drm_atomic_helper.c into the
different parts, it's getting a bit huge. But after your stuff has landed.

Before pushing pls make sure vmwgfx has been tested, and ofc that our CI
approves too.

Cheers, Daniel

> ---
>  drivers/gpu/drm/arm/hdlcd_crtc.c            |   8 +--
>  drivers/gpu/drm/arm/malidp_planes.c         |   4 +-
>  drivers/gpu/drm/drm_atomic_helper.c         |  94 +++++++++++++++++++++++++
>  drivers/gpu/drm/drm_plane_helper.c          | 102 ++--------------------------
>  drivers/gpu/drm/drm_simple_kms_helper.c     |   9 +--
>  drivers/gpu/drm/i915/intel_display.c        |  22 +++---
>  drivers/gpu/drm/imx/ipuv3-plane.c           |   8 +--
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c    |   8 +--
>  drivers/gpu/drm/meson/meson_plane.c         |   8 +--
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
>  drivers/gpu/drm/nouveau/nv50_display.c      |  20 +++---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
>  drivers/gpu/drm/tegra/dc.c                  |   4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |   8 +--
>  drivers/gpu/drm/zte/zx_plane.c              |  15 ++--
>  include/drm/drm_atomic_helper.h             |   7 ++
>  include/drm/drm_plane_helper.h              |   6 --
>  17 files changed, 169 insertions(+), 165 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 14721723fa8a..63511a3bbf6c 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -252,10 +252,10 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	return drm_plane_helper_check_state(state, crtc_state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    false, true);
> +	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   false, true);
>  }
>  
>  static void hdlcd_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index 492d99dd55d4..72a07950167e 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -150,8 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
>  
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   0, INT_MAX, true, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  0, INT_MAX, true, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 71d712f1b56a..a381913c894d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -696,6 +696,100 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
>  
>  /**
> + * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * @plane_state: plane state to check
> + * @crtc_state: crtc state to check
> + * @clip: integer clipping coordinates
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + * @can_position: is it legal to position the plane such that it
> + *                doesn't cover the entire crtc?  This will generally
> + *                only be false for primary planes.
> + * @can_update_disabled: can the plane be updated while the crtc
> + *                       is disabled?
> + *
> + * Checks that a desired plane update is valid, and updates various
> + * bits of derived state (clipped coordinates etc.). Drivers that provide
> + * their own plane handling rather than helper-provided implementations may
> + * still wish to call this function to avoid duplication of error checking
> + * code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> +					const struct drm_crtc_state *crtc_state,
> +					const struct drm_rect *clip,
> +					int min_scale,
> +					int max_scale,
> +					bool can_position,
> +					bool can_update_disabled)
> +{
> +	struct drm_framebuffer *fb = plane_state->fb;
> +	struct drm_rect *src = &plane_state->src;
> +	struct drm_rect *dst = &plane_state->dst;
> +	unsigned int rotation = plane_state->rotation;
> +	int hscale, vscale;
> +
> +	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> +
> +	*src = drm_plane_state_src(plane_state);
> +	*dst = drm_plane_state_dest(plane_state);
> +
> +	if (!fb) {
> +		plane_state->visible = false;
> +		return 0;
> +	}
> +
> +	/* crtc should only be NULL when disabling (i.e., !fb) */
> +	if (WARN_ON(!plane_state->crtc)) {
> +		plane_state->visible = false;
> +		return 0;
> +	}
> +
> +	if (!crtc_state->enable && !can_update_disabled) {
> +		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> +		return -EINVAL;
> +	}
> +
> +	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> +
> +	/* Check scaling */
> +	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> +	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> +	if (hscale < 0 || vscale < 0) {
> +		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> +		drm_rect_debug_print("src: ", &plane_state->src, true);
> +		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> +		return -ERANGE;
> +	}
> +
> +	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> +
> +	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> +
> +	if (!plane_state->visible)
> +		/*
> +		 * Plane isn't visible; some drivers can handle this
> +		 * so we just return success here.  Drivers that can't
> +		 * (including those that use the primary plane helper's
> +		 * update function) will return an error from their
> +		 * update_plane handler.
> +		 */
> +		return 0;
> +
> +	if (!can_position && !drm_rect_equals(dst, clip)) {
> +		DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> +		drm_rect_debug_print("dst: ", dst, false);
> +		drm_rect_debug_print("clip: ", clip, false);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
> +
> +/**
>   * drm_atomic_helper_check_planes - validate state object for planes changes
>   * @dev: DRM device
>   * @state: the driver state object
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index eef0ffcd7ed5..f1be8cd4e387 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -100,100 +100,6 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>  }
>  
>  /**
> - * drm_plane_helper_check_state() - Check plane state for validity
> - * @plane_state: plane state to check
> - * @crtc_state: crtc state to check
> - * @clip: integer clipping coordinates
> - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> - * @can_position: is it legal to position the plane such that it
> - *                doesn't cover the entire crtc?  This will generally
> - *                only be false for primary planes.
> - * @can_update_disabled: can the plane be updated while the crtc
> - *                       is disabled?
> - *
> - * Checks that a desired plane update is valid, and updates various
> - * bits of derived state (clipped coordinates etc.). Drivers that provide
> - * their own plane handling rather than helper-provided implementations may
> - * still wish to call this function to avoid duplication of error checking
> - * code.
> - *
> - * RETURNS:
> - * Zero if update appears valid, error code on failure
> - */
> -int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> -				 const struct drm_crtc_state *crtc_state,
> -				 const struct drm_rect *clip,
> -				 int min_scale,
> -				 int max_scale,
> -				 bool can_position,
> -				 bool can_update_disabled)
> -{
> -	struct drm_framebuffer *fb = plane_state->fb;
> -	struct drm_rect *src = &plane_state->src;
> -	struct drm_rect *dst = &plane_state->dst;
> -	unsigned int rotation = plane_state->rotation;
> -	int hscale, vscale;
> -
> -	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> -
> -	*src = drm_plane_state_src(plane_state);
> -	*dst = drm_plane_state_dest(plane_state);
> -
> -	if (!fb) {
> -		plane_state->visible = false;
> -		return 0;
> -	}
> -
> -	/* crtc should only be NULL when disabling (i.e., !fb) */
> -	if (WARN_ON(!plane_state->crtc)) {
> -		plane_state->visible = false;
> -		return 0;
> -	}
> -
> -	if (!crtc_state->enable && !can_update_disabled) {
> -		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> -		return -EINVAL;
> -	}
> -
> -	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> -
> -	/* Check scaling */
> -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> -	if (hscale < 0 || vscale < 0) {
> -		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> -		drm_rect_debug_print("src: ", &plane_state->src, true);
> -		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> -		return -ERANGE;
> -	}
> -
> -	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> -
> -	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> -
> -	if (!plane_state->visible)
> -		/*
> -		 * Plane isn't visible; some drivers can handle this
> -		 * so we just return success here.  Drivers that can't
> -		 * (including those that use the primary plane helper's
> -		 * update function) will return an error from their
> -		 * update_plane handler.
> -		 */
> -		return 0;
> -
> -	if (!can_position && !drm_rect_equals(dst, clip)) {
> -		DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> -		drm_rect_debug_print("dst: ", dst, false);
> -		drm_rect_debug_print("clip: ", clip, false);
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(drm_plane_helper_check_state);
> -
> -/**
>   * drm_plane_helper_check_update() - Check plane update for validity
>   * @plane: plane object to update
>   * @crtc: owning CRTC of owning plane
> @@ -254,10 +160,10 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
>  	};
>  	int ret;
>  
> -	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
> -					   min_scale, max_scale,
> -					   can_position,
> -					   can_update_disabled);
> +	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> +						  clip, min_scale, max_scale,
> +						  can_position,
> +						  can_update_disabled);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index d428c805025c..9f3b1c94802b 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -103,10 +103,11 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   false, true);
> +	ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> +						  &clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  false, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4a459726b36b..e0ff599d2090 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9316,12 +9316,12 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
>  	u32 offset;
>  	int ret;
>  
> -	ret = drm_plane_helper_check_state(&plane_state->base,
> -					   &crtc_state->base,
> -					   &plane_state->clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   true, true);
> +	ret = drm_atomic_helper_check_plane_state(&plane_state->base,
> +						  &crtc_state->base,
> +						  &plane_state->clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  true, true);
>  	if (ret)
>  		return ret;
>  
> @@ -12808,11 +12808,11 @@ intel_check_primary_plane(struct intel_plane *plane,
>  		can_position = true;
>  	}
>  
> -	ret = drm_plane_helper_check_state(&state->base,
> -					   &crtc_state->base,
> -					   &state->clip,
> -					   min_scale, max_scale,
> -					   can_position, true);
> +	ret = drm_atomic_helper_check_plane_state(&state->base,
> +						  &crtc_state->base,
> +						  &state->clip,
> +						  min_scale, max_scale,
> +						  can_position, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 51b23ac175e3..5a67daedcf4d 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -342,10 +342,10 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>  	clip.y1 = 0;
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   can_position, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  can_position, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 7ebb33657704..5ef898b93d8d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -111,10 +111,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_state(state, crtc_state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    true, true);
> +	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   true, true);
>  }
>  
>  static void mtk_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 2a00b720c371..d0a6ac8390f3 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -61,10 +61,10 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->mode.hdisplay;
>  	clip.y2 = crtc_state->mode.vdisplay;
>  
> -	return drm_plane_helper_check_state(state, crtc_state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    true, true);
> +	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   true, true);
>  }
>  
>  /* Takes a fixed 16.16 number and converts it to integer. */
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index 1c194a9453d9..df3360acacca 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -348,8 +348,9 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
>  	min_scale = FRAC_16_16(1, 8);
>  	max_scale = FRAC_16_16(8, 1);
>  
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   min_scale, max_scale, true, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  min_scale, max_scale,
> +						  true, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 7f11a88d0593..8ab0fac3544d 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1143,11 +1143,11 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
>  {
>  	int ret;
>  
> -	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> -					   &asyw->clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   true, true);
> +	ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
> +						  &asyw->clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  true, true);
>  	asyh->curs.visible = asyw->state.visible;
>  	if (ret || !asyh->curs.visible)
>  		return ret;
> @@ -1433,11 +1433,11 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
>  	if (!fb->format->depth)
>  		return -EINVAL;
>  
> -	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> -					   &asyw->clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   false, true);
> +	ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
> +						  &asyw->clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  false, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 36d0f101e30d..ba7505292b78 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -659,9 +659,9 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   min_scale, max_scale,
> -					   true, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  min_scale, max_scale,
> +						  true, true);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index e95d9e2a7f6c..fc70351b9017 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
>  	clip.y2 = crtc_state->mode.vdisplay;
>  
>  	/* Check plane state for visibility and calculate clipping bounds */
> -	err = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   0, INT_MAX, true, true);
> +	err = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  0, INT_MAX, true, true);
>  	if (err < 0)
>  		return err;
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index d95e7b1c3b11..a2a93d7e2a04 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -454,10 +454,10 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
>  		clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  	}
>  
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   false, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  false, true);
>  
>  	if (!ret && new_fb) {
>  		struct drm_crtc *crtc = state->crtc;
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index ee0002529b8f..68fd2e2dc78a 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -80,9 +80,9 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> -					    min_scale, max_scale,
> -					    true, true);
> +	return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> +						   &clip, min_scale, max_scale,
> +						   true, true);
>  }
>  
>  static int zx_vl_get_fmt(uint32_t format)
> @@ -315,10 +315,11 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  
> -	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    false, true);
> +	return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> +						   &clip,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   false, true);
>  }
>  
>  static int zx_gl_get_fmt(uint32_t format)
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d2b56cc657e9..4842ee9485ce 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -38,6 +38,13 @@ struct drm_private_state;
>  
>  int drm_atomic_helper_check_modeset(struct drm_device *dev,
>  				struct drm_atomic_state *state);
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> +					const struct drm_crtc_state *crtc_state,
> +					const struct drm_rect *clip,
> +					int min_scale,
> +					int max_scale,
> +					bool can_position,
> +					bool can_update_disabled);
>  int drm_atomic_helper_check_planes(struct drm_device *dev,
>  			       struct drm_atomic_state *state);
>  int drm_atomic_helper_check(struct drm_device *dev,
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 41b8309b0a57..8aa49c0ecd4d 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -38,12 +38,6 @@
>   */
>  #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
>  
> -int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> -				 const struct drm_crtc_state *crtc_state,
> -				 const struct drm_rect *clip,
> -				 int min_scale, int max_scale,
> -				 bool can_position,
> -				 bool can_update_disabled);
>  int drm_plane_helper_check_update(struct drm_plane *plane,
>  				  struct drm_crtc *crtc,
>  				  struct drm_framebuffer *fb,
> -- 
> 2.13.6
> 

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

Re: [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c

[Laurent Pinchart]

Hi Ville,

Thank you for the patch.

On Wednesday, 1 November 2017 20:29:20 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> drm_plane_helper_check_update() isn't a transitional helper, so let's
> rename it to drm_atomic_helper_check_plane_state() and move it into
> drm_atomic_helper.c.
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I would move this patch before 4/5 to make it easier to revert 4/5 (for the 
reason explaining as a reply to that patch).

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/arm/hdlcd_crtc.c            |   8 +--
>  drivers/gpu/drm/arm/malidp_planes.c         |   4 +-
>  drivers/gpu/drm/drm_atomic_helper.c         |  95 +++++++++++++++++++++++++
>  drivers/gpu/drm/drm_plane_helper.c          | 103 +-----------------------
>  drivers/gpu/drm/drm_simple_kms_helper.c     |   9 +--
>  drivers/gpu/drm/i915/intel_display.c        |  22 +++---
>  drivers/gpu/drm/imx/ipuv3-plane.c           |   8 +--
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c    |   8 +--
>  drivers/gpu/drm/meson/meson_plane.c         |   8 +--
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
>  drivers/gpu/drm/nouveau/nv50_display.c      |  20 +++---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
>  drivers/gpu/drm/tegra/dc.c                  |   4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |   8 +--
>  drivers/gpu/drm/zte/zx_plane.c              |  15 ++--
>  include/drm/drm_atomic_helper.h             |   7 ++
>  include/drm/drm_plane_helper.h              |   6 --
>  17 files changed, 170 insertions(+), 166 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c
> b/drivers/gpu/drm/arm/hdlcd_crtc.c index 14721723fa8a..63511a3bbf6c 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -252,10 +252,10 @@ static int hdlcd_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	return drm_plane_helper_check_state(state, crtc_state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    false, true);
> +	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   false, true);
>  }
> 
>  static void hdlcd_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> b/drivers/gpu/drm/arm/malidp_planes.c index 492d99dd55d4..72a07950167e
> 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -150,8 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane
> *mp,
> 
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   0, INT_MAX, true, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  0, INT_MAX, true, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index 71d712f1b56a..7595ad8ad2f3
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -696,6 +696,101 @@ drm_atomic_helper_check_modeset(struct drm_device
> *dev, EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
> 
>  /**
> + * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * @plane_state: plane state to check
> + * @crtc_state: crtc state to check
> + * @clip: integer clipping coordinates
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + * @can_position: is it legal to position the plane such that it
> + *                doesn't cover the entire crtc?  This will generally
> + *                only be false for primary planes.
> + * @can_update_disabled: can the plane be updated while the crtc
> + *                       is disabled?
> + *
> + * Checks that a desired plane update is valid, and updates various
> + * bits of derived state (clipped coordinates etc.). Drivers that provide
> + * their own plane handling rather than helper-provided implementations may
> + * still wish to call this function to avoid duplication of error checking
> + * code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state
> *plane_state, +					const struct drm_crtc_state *crtc_state,
> +					const struct drm_rect *clip,
> +					int min_scale,
> +					int max_scale,
> +					bool can_position,
> +					bool can_update_disabled)
> +{
> +	struct drm_framebuffer *fb = plane_state->fb;
> +	struct drm_rect *src = &plane_state->src;
> +	struct drm_rect *dst = &plane_state->dst;
> +	unsigned int rotation = plane_state->rotation;
> +	int hscale, vscale;
> +
> +	WARN_ON(!crtc_state != !plane_state->crtc);
> +	WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
> +
> +	*src = drm_plane_state_src(plane_state);
> +	*dst = drm_plane_state_dest(plane_state);
> +
> +	if (!fb) {
> +		plane_state->visible = false;
> +		return 0;
> +	}
> +
> +	/* crtc should only be NULL when disabling (i.e., !fb) */
> +	if (WARN_ON(!plane_state->crtc)) {
> +		plane_state->visible = false;
> +		return 0;
> +	}
> +
> +	if (!crtc_state->enable && !can_update_disabled) {
> +		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> +		return -EINVAL;
> +	}
> +
> +	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> +
> +	/* Check scaling */
> +	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> +	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> +	if (hscale < 0 || vscale < 0) {
> +		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> +		drm_rect_debug_print("src: ", &plane_state->src, true);
> +		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> +		return -ERANGE;
> +	}
> +
> +	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale,
> vscale); +
> +	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> +
> +	if (!plane_state->visible)
> +		/*
> +		 * Plane isn't visible; some drivers can handle this
> +		 * so we just return success here.  Drivers that can't
> +		 * (including those that use the primary plane helper's
> +		 * update function) will return an error from their
> +		 * update_plane handler.
> +		 */
> +		return 0;
> +
> +	if (!can_position && !drm_rect_equals(dst, clip)) {
> +		DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> +		drm_rect_debug_print("dst: ", dst, false);
> +		drm_rect_debug_print("clip: ", clip, false);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
> +
> +/**
>   * drm_atomic_helper_check_planes - validate state object for planes
> changes * @dev: DRM device
>   * @state: the driver state object
> diff --git a/drivers/gpu/drm/drm_plane_helper.c
> b/drivers/gpu/drm/drm_plane_helper.c index adb8d94484f2..f1be8cd4e387
> 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -100,101 +100,6 @@ static int get_connectors_for_crtc(struct drm_crtc
> *crtc, }
> 
>  /**
> - * drm_plane_helper_check_state() - Check plane state for validity
> - * @plane_state: plane state to check
> - * @crtc_state: crtc state to check
> - * @clip: integer clipping coordinates
> - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> - * @can_position: is it legal to position the plane such that it
> - *                doesn't cover the entire crtc?  This will generally
> - *                only be false for primary planes.
> - * @can_update_disabled: can the plane be updated while the crtc
> - *                       is disabled?
> - *
> - * Checks that a desired plane update is valid, and updates various
> - * bits of derived state (clipped coordinates etc.). Drivers that provide
> - * their own plane handling rather than helper-provided implementations may
> - * still wish to call this function to avoid duplication of error checking
> - * code.
> - *
> - * RETURNS:
> - * Zero if update appears valid, error code on failure
> - */
> -int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> -				 const struct drm_crtc_state *crtc_state,
> -				 const struct drm_rect *clip,
> -				 int min_scale,
> -				 int max_scale,
> -				 bool can_position,
> -				 bool can_update_disabled)
> -{
> -	struct drm_framebuffer *fb = plane_state->fb;
> -	struct drm_rect *src = &plane_state->src;
> -	struct drm_rect *dst = &plane_state->dst;
> -	unsigned int rotation = plane_state->rotation;
> -	int hscale, vscale;
> -
> -	WARN_ON(!crtc_state != !plane_state->crtc);
> -	WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
> -
> -	*src = drm_plane_state_src(plane_state);
> -	*dst = drm_plane_state_dest(plane_state);
> -
> -	if (!fb) {
> -		plane_state->visible = false;
> -		return 0;
> -	}
> -
> -	/* crtc should only be NULL when disabling (i.e., !fb) */
> -	if (WARN_ON(!plane_state->crtc)) {
> -		plane_state->visible = false;
> -		return 0;
> -	}
> -
> -	if (!crtc_state->enable && !can_update_disabled) {
> -		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> -		return -EINVAL;
> -	}
> -
> -	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> -
> -	/* Check scaling */
> -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> -	if (hscale < 0 || vscale < 0) {
> -		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> -		drm_rect_debug_print("src: ", &plane_state->src, true);
> -		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> -		return -ERANGE;
> -	}
> -
> -	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale,
> vscale); -
> -	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> -
> -	if (!plane_state->visible)
> -		/*
> -		 * Plane isn't visible; some drivers can handle this
> -		 * so we just return success here.  Drivers that can't
> -		 * (including those that use the primary plane helper's
> -		 * update function) will return an error from their
> -		 * update_plane handler.
> -		 */
> -		return 0;
> -
> -	if (!can_position && !drm_rect_equals(dst, clip)) {
> -		DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> -		drm_rect_debug_print("dst: ", dst, false);
> -		drm_rect_debug_print("clip: ", clip, false);
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(drm_plane_helper_check_state);
> -
> -/**
>   * drm_plane_helper_check_update() - Check plane update for validity
>   * @plane: plane object to update
>   * @crtc: owning CRTC of owning plane
> @@ -255,10 +160,10 @@ int drm_plane_helper_check_update(struct drm_plane
> *plane, };
>  	int ret;
> 
> -	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
> -					   min_scale, max_scale,
> -					   can_position,
> -					   can_update_disabled);
> +	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> +						  clip, min_scale, max_scale,
> +						  can_position,
> +						  can_update_disabled);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
> b/drivers/gpu/drm/drm_simple_kms_helper.c index d428c805025c..9f3b1c94802b
> 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -103,10 +103,11 @@ static int drm_simple_kms_plane_atomic_check(struct
> drm_plane *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   false, true);
> +	ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> +						  &clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  false, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 4a459726b36b..e0ff599d2090
> 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9316,12 +9316,12 @@ static int intel_check_cursor(struct
> intel_crtc_state *crtc_state, u32 offset;
>  	int ret;
> 
> -	ret = drm_plane_helper_check_state(&plane_state->base,
> -					   &crtc_state->base,
> -					   &plane_state->clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   true, true);
> +	ret = drm_atomic_helper_check_plane_state(&plane_state->base,
> +						  &crtc_state->base,
> +						  &plane_state->clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  true, true);
>  	if (ret)
>  		return ret;
> 
> @@ -12808,11 +12808,11 @@ intel_check_primary_plane(struct intel_plane
> *plane, can_position = true;
>  	}
> 
> -	ret = drm_plane_helper_check_state(&state->base,
> -					   &crtc_state->base,
> -					   &state->clip,
> -					   min_scale, max_scale,
> -					   can_position, true);
> +	ret = drm_atomic_helper_check_plane_state(&state->base,
> +						  &crtc_state->base,
> +						  &state->clip,
> +						  min_scale, max_scale,
> +						  can_position, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
> b/drivers/gpu/drm/imx/ipuv3-plane.c index 51b23ac175e3..5a67daedcf4d 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -342,10 +342,10 @@ static int ipu_plane_atomic_check(struct drm_plane
> *plane, clip.y1 = 0;
>  	clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   can_position, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  can_position, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 7ebb33657704..5ef898b93d8d
> 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -111,10 +111,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_state(state, crtc_state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    true, true);
> +	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   true, true);
>  }
> 
>  static void mtk_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/meson/meson_plane.c
> b/drivers/gpu/drm/meson/meson_plane.c index 2a00b720c371..d0a6ac8390f3
> 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -61,10 +61,10 @@ static int meson_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->mode.hdisplay;
>  	clip.y2 = crtc_state->mode.vdisplay;
> 
> -	return drm_plane_helper_check_state(state, crtc_state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    true, true);
> +	return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   true, true);
>  }
> 
>  /* Takes a fixed 16.16 number and converts it to integer. */
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index
> 1c194a9453d9..df3360acacca 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -348,8 +348,9 @@ static int mdp5_plane_atomic_check_with_state(struct
> drm_crtc_state *crtc_state, min_scale = FRAC_16_16(1, 8);
>  	max_scale = FRAC_16_16(8, 1);
> 
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   min_scale, max_scale, true, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  min_scale, max_scale,
> +						  true, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
> b/drivers/gpu/drm/nouveau/nv50_display.c index 7f11a88d0593..8ab0fac3544d
> 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1143,11 +1143,11 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw, {
>  	int ret;
> 
> -	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> -					   &asyw->clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   true, true);
> +	ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
> +						  &asyw->clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  true, true);
>  	asyh->curs.visible = asyw->state.visible;
>  	if (ret || !asyh->curs.visible)
>  		return ret;
> @@ -1433,11 +1433,11 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw, if (!fb->format->depth)
>  		return -EINVAL;
> 
> -	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> -					   &asyw->clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   false, true);
> +	ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
> +						  &asyw->clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  false, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index
> 36d0f101e30d..ba7505292b78 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -659,9 +659,9 @@ static int vop_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   min_scale, max_scale,
> -					   true, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  min_scale, max_scale,
> +						  true, true);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index e95d9e2a7f6c..fc70351b9017 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane
> *plane, clip.y2 = crtc_state->mode.vdisplay;
> 
>  	/* Check plane state for visibility and calculate clipping bounds */
> -	err = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   0, INT_MAX, true, true);
> +	err = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  0, INT_MAX, true, true);
>  	if (err < 0)
>  		return err;
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index d95e7b1c3b11..a2a93d7e2a04
> 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -454,10 +454,10 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
> *plane, clip.y2 = crtc_state->adjusted_mode.vdisplay;
>  	}
> 
> -	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   DRM_PLANE_HELPER_NO_SCALING,
> -					   false, true);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  false, true);
> 
>  	if (!ret && new_fb) {
>  		struct drm_crtc *crtc = state->crtc;
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index ee0002529b8f..68fd2e2dc78a 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -80,9 +80,9 @@ static int zx_vl_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> -					    min_scale, max_scale,
> -					    true, true);
> +	return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> +						   &clip, min_scale, max_scale,
> +						   true, true);
>  }
> 
>  static int zx_vl_get_fmt(uint32_t format)
> @@ -315,10 +315,11 @@ static int zx_gl_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
>  	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> 
> -	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    DRM_PLANE_HELPER_NO_SCALING,
> -					    false, true);
> +	return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> +						   &clip,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   DRM_PLANE_HELPER_NO_SCALING,
> +						   false, true);
>  }
> 
>  static int zx_gl_get_fmt(uint32_t format)
> diff --git a/include/drm/drm_atomic_helper.h
> b/include/drm/drm_atomic_helper.h index d2b56cc657e9..4842ee9485ce 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -38,6 +38,13 @@ struct drm_private_state;
> 
>  int drm_atomic_helper_check_modeset(struct drm_device *dev,
>  				struct drm_atomic_state *state);
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state
> *plane_state, +					const struct drm_crtc_state *crtc_state,
> +					const struct drm_rect *clip,
> +					int min_scale,
> +					int max_scale,
> +					bool can_position,
> +					bool can_update_disabled);
>  int drm_atomic_helper_check_planes(struct drm_device *dev,
>  			       struct drm_atomic_state *state);
>  int drm_atomic_helper_check(struct drm_device *dev,
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 41b8309b0a57..8aa49c0ecd4d 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -38,12 +38,6 @@
>   */
>  #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
> 
> -int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> -				 const struct drm_crtc_state *crtc_state,
> -				 const struct drm_rect *clip,
> -				 int min_scale, int max_scale,
> -				 bool can_position,
> -				 bool can_update_disabled);
>  int drm_plane_helper_check_update(struct drm_plane *plane,
>  				  struct drm_crtc *crtc,
>  				  struct drm_framebuffer *fb,


-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Sinclair Yeh]

Sorry this took so long.

The vmwgfx part:  Reviewed-by: Sinclair Yeh <syeh@vmware.com>

I've done some testing and the vmwgfx part looks good.  Has Daniel
already taken these or should I put them in my next request?

Sinclair

On Wed, Nov 01, 2017 at 08:29:15PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> While trawling the tree I spotted some issues with the way vmwgfx
> uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
> Do note that I haven't actually tested the resulting code at all,
> but it does build at least.
> 
> And while touching that general area I took up Daniel's suggestion from
> long ago that drm_plane_helper_check_state() should be renamed and
> relocated to better reflect its status.
> 
> Here's a branch with the entire series:
> git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff
> 
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> 
> Ville Syrjälä (5):
>   drm/vmwgfx: Remove bogus crtc coords vs fb size check
>   drm/vmwgfx: Use drm_plane_helper_check_state()
>   drm/vmwgfx: Try to fix plane clipping
>   drm: Check crtc_state->enable rather than crtc->enabled in
>     drm_plane_helper_check_state()
>   drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
> 
>  drivers/gpu/drm/arm/hdlcd_crtc.c            |   8 +-
>  drivers/gpu/drm/arm/malidp_planes.c         |   3 +-
>  drivers/gpu/drm/drm_atomic_helper.c         |  95 ++++++++++++++++++++++++
>  drivers/gpu/drm/drm_plane_helper.c          | 111 +++-------------------------
>  drivers/gpu/drm/drm_simple_kms_helper.c     |   9 ++-
>  drivers/gpu/drm/i915/intel_display.c        |  20 ++---
>  drivers/gpu/drm/imx/ipuv3-plane.c           |   8 +-
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c    |   8 +-
>  drivers/gpu/drm/meson/meson_plane.c         |   8 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
>  drivers/gpu/drm/nouveau/nv50_display.c      |  18 +++--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
>  drivers/gpu/drm/tegra/dc.c                  |   4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  40 ++++------
>  drivers/gpu/drm/zte/zx_plane.c              |  15 ++--
>  include/drm/drm_atomic_helper.h             |   7 ++
>  include/drm/drm_plane_helper.h              |   5 --
>  17 files changed, 187 insertions(+), 183 deletions(-)
> 
> -- 
> 2.13.6
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Ville Syrjälä]

On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> Sorry this took so long.

No worries.

> 
> The vmwgfx part:  Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> 
> I've done some testing and the vmwgfx part looks good.  Has Daniel
> already taken these or should I put them in my next request?

You can take them, or I can push them to drm-misc-next. Whatever
works best for you.

And I'll want to revisit this topic soonish and move the clip
handling into the helper as discussed with Daniel. But that can
wait a bit until we get this round merged somewhere.

> 
> Sinclair
> 
> On Wed, Nov 01, 2017 at 08:29:15PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > While trawling the tree I spotted some issues with the way vmwgfx
> > uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
> > Do note that I haven't actually tested the resulting code at all,
> > but it does build at least.
> > 
> > And while touching that general area I took up Daniel's suggestion from
> > long ago that drm_plane_helper_check_state() should be renamed and
> > relocated to better reflect its status.
> > 
> > Here's a branch with the entire series:
> > git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff
> > 
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Sinclair Yeh <syeh@vmware.com>
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > 
> > Ville Syrjälä (5):
> >   drm/vmwgfx: Remove bogus crtc coords vs fb size check
> >   drm/vmwgfx: Use drm_plane_helper_check_state()
> >   drm/vmwgfx: Try to fix plane clipping
> >   drm: Check crtc_state->enable rather than crtc->enabled in
> >     drm_plane_helper_check_state()
> >   drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
> > 
> >  drivers/gpu/drm/arm/hdlcd_crtc.c            |   8 +-
> >  drivers/gpu/drm/arm/malidp_planes.c         |   3 +-
> >  drivers/gpu/drm/drm_atomic_helper.c         |  95 ++++++++++++++++++++++++
> >  drivers/gpu/drm/drm_plane_helper.c          | 111 +++-------------------------
> >  drivers/gpu/drm/drm_simple_kms_helper.c     |   9 ++-
> >  drivers/gpu/drm/i915/intel_display.c        |  20 ++---
> >  drivers/gpu/drm/imx/ipuv3-plane.c           |   8 +-
> >  drivers/gpu/drm/mediatek/mtk_drm_plane.c    |   8 +-
> >  drivers/gpu/drm/meson/meson_plane.c         |   8 +-
> >  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
> >  drivers/gpu/drm/nouveau/nv50_display.c      |  18 +++--
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
> >  drivers/gpu/drm/tegra/dc.c                  |   4 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  40 ++++------
> >  drivers/gpu/drm/zte/zx_plane.c              |  15 ++--
> >  include/drm/drm_atomic_helper.h             |   7 ++
> >  include/drm/drm_plane_helper.h              |   5 --
> >  17 files changed, 187 insertions(+), 183 deletions(-)
> > 
> > -- 
> > 2.13.6
> > 

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Daniel Vetter]

On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> > Sorry this took so long.
> 
> No worries.
> 
> > 
> > The vmwgfx part:  Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> > 
> > I've done some testing and the vmwgfx part looks good.  Has Daniel
> > already taken these or should I put them in my next request?
> 
> You can take them, or I can push them to drm-misc-next. Whatever
> works best for you.
> 
> And I'll want to revisit this topic soonish and move the clip
> handling into the helper as discussed with Daniel. But that can
> wait a bit until we get this round merged somewhere.

Because we're still in the merge window I think it's probably best if we
push the entire series in through drm-misc. Tree-coordination in the merge
window is always a bit a pain.
-Daniel

> 
> > 
> > Sinclair
> > 
> > On Wed, Nov 01, 2017 at 08:29:15PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > While trawling the tree I spotted some issues with the way vmwgfx
> > > uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
> > > Do note that I haven't actually tested the resulting code at all,
> > > but it does build at least.
> > > 
> > > And while touching that general area I took up Daniel's suggestion from
> > > long ago that drm_plane_helper_check_state() should be renamed and
> > > relocated to better reflect its status.
> > > 
> > > Here's a branch with the entire series:
> > > git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff
> > > 
> > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > 
> > > Ville Syrjälä (5):
> > >   drm/vmwgfx: Remove bogus crtc coords vs fb size check
> > >   drm/vmwgfx: Use drm_plane_helper_check_state()
> > >   drm/vmwgfx: Try to fix plane clipping
> > >   drm: Check crtc_state->enable rather than crtc->enabled in
> > >     drm_plane_helper_check_state()
> > >   drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
> > > 
> > >  drivers/gpu/drm/arm/hdlcd_crtc.c            |   8 +-
> > >  drivers/gpu/drm/arm/malidp_planes.c         |   3 +-
> > >  drivers/gpu/drm/drm_atomic_helper.c         |  95 ++++++++++++++++++++++++
> > >  drivers/gpu/drm/drm_plane_helper.c          | 111 +++-------------------------
> > >  drivers/gpu/drm/drm_simple_kms_helper.c     |   9 ++-
> > >  drivers/gpu/drm/i915/intel_display.c        |  20 ++---
> > >  drivers/gpu/drm/imx/ipuv3-plane.c           |   8 +-
> > >  drivers/gpu/drm/mediatek/mtk_drm_plane.c    |   8 +-
> > >  drivers/gpu/drm/meson/meson_plane.c         |   8 +-
> > >  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |   5 +-
> > >  drivers/gpu/drm/nouveau/nv50_display.c      |  18 +++--
> > >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   6 +-
> > >  drivers/gpu/drm/tegra/dc.c                  |   4 +-
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  40 ++++------
> > >  drivers/gpu/drm/zte/zx_plane.c              |  15 ++--
> > >  include/drm/drm_atomic_helper.h             |   7 ++
> > >  include/drm/drm_plane_helper.h              |   5 --
> > >  17 files changed, 187 insertions(+), 183 deletions(-)
> > > 
> > > -- 
> > > 2.13.6
> > > 
> 
> -- 
> Ville Syrjälä
> Intel OTC

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

Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Sinclair Yeh]

On Mon, Nov 20, 2017 at 08:34:50AM +0100, Daniel Vetter wrote:
> On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> > On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> > > Sorry this took so long.
> > 
> > No worries.
> > 
> > > 
> > > The vmwgfx part:  Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> > > 
> > > I've done some testing and the vmwgfx part looks good.  Has Daniel
> > > already taken these or should I put them in my next request?
> > 
> > You can take them, or I can push them to drm-misc-next. Whatever
> > works best for you.
> > 
> > And I'll want to revisit this topic soonish and move the clip
> > handling into the helper as discussed with Daniel. But that can
> > wait a bit until we get this round merged somewhere.
> 
> Because we're still in the merge window I think it's probably best if we
> push the entire series in through drm-misc. Tree-coordination in the merge
> window is always a bit a pain.
> -Daniel

Ok, so I'll leave this series to you then.

thanks,

Sinclair

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Ville Syrjälä]

On Mon, Nov 20, 2017 at 09:32:56AM -0800, Sinclair Yeh wrote:
> On Mon, Nov 20, 2017 at 08:34:50AM +0100, Daniel Vetter wrote:
> > On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> > > On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> > > > Sorry this took so long.
> > > 
> > > No worries.
> > > 
> > > > 
> > > > The vmwgfx part:  Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> > > > 
> > > > I've done some testing and the vmwgfx part looks good.  Has Daniel
> > > > already taken these or should I put them in my next request?
> > > 
> > > You can take them, or I can push them to drm-misc-next. Whatever
> > > works best for you.
> > > 
> > > And I'll want to revisit this topic soonish and move the clip
> > > handling into the helper as discussed with Daniel. But that can
> > > wait a bit until we get this round merged somewhere.
> > 
> > Because we're still in the merge window I think it's probably best if we
> > push the entire series in through drm-misc. Tree-coordination in the merge
> > window is always a bit a pain.
> > -Daniel
> 
> Ok, so I'll leave this series to you then.

All right. Series pushed to drm-misc-next. Thanks for the reviews.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff

[Laurent Pinchart]

Hi Ville,

On Monday, 20 November 2017 21:36:46 EET Ville Syrjälä wrote:
> On Mon, Nov 20, 2017 at 09:32:56AM -0800, Sinclair Yeh wrote:
> > On Mon, Nov 20, 2017 at 08:34:50AM +0100, Daniel Vetter wrote:
> >> On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> >>> On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> >>> > Sorry this took so long.
> >>> 
> >>> No worries.
> >>> 
> >>>> The vmwgfx part:  Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> >>>> 
> >>>> I've done some testing and the vmwgfx part looks good.  Has Daniel
> >>>> already taken these or should I put them in my next request?
> >>> 
> >>> You can take them, or I can push them to drm-misc-next. Whatever
> >>> works best for you.
> >>> 
> >>> And I'll want to revisit this topic soonish and move the clip
> >>> handling into the helper as discussed with Daniel. But that can
> >>> wait a bit until we get this round merged somewhere.
> >> 
> >> Because we're still in the merge window I think it's probably best if we
> >> push the entire series in through drm-misc. Tree-coordination in the
> >> merge window is always a bit a pain.
> > 
> > Ok, so I'll leave this series to you then.
> 
> All right. Series pushed to drm-misc-next. Thanks for the reviews.

And I now realize my review comments aren't very useful (apart possibly the 
one about adding a comment to the drm_plane_helper_check_update() function, 
feel free to submit a patch for that if you think it's useful) as the series 
has been merged already :-/ I should really start reading mails in the reverse 
chronological order.

-- 
Regards,

Laurent Pinchart

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