[PATCH v3 0/3] drm/imx: Add active plane reconfiguration support

[PATCH v3 0/3] drm/imx: Add active plane reconfiguration support

[Liu Ying]

This patch adds active plane reconfiguration support for imx-drm.
This may fixes some mode setting failure issues which were introduced
by imx-drm atomic conversion patch set.  The main idea is to disable the
plane in question in CRTC's disable operation and then the drm atomic
core will enable it again automatically.

Daniel,

As we discussed on irc, using disable_planes_on_crtc() is a bit invasive.
So, this set implements the similar logic in imx-drm CRTC's ->atomic_disable
callback.

v2->v3:
* Disable all appropriate affected planes(when necessary) in CRTC's
  ->atomic_disable callback, but not in each plane's ->atomic_update callback,
  as suggested by Daniel Vetter.
* +Cc Lucas Stach, as he tested the patch v2.

v1->v2:
* Do not reject reconfiguring an active overlay plane.

Liu Ying (3):
  drm/atomic-helper: Add atomic_disable CRTC helper callback
  drm/imx: ipuv3-crtc: Use the callback ->atomic_disable instead of
    ->disable
  drm/imx: Add active plane reconfiguration support

 drivers/gpu/drm/drm_atomic_helper.c      |  2 ++
 drivers/gpu/drm/imx/imx-drm-core.c       | 26 +++++++++++++++++++++++++-
 drivers/gpu/drm/imx/ipuv3-crtc.c         | 31 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/imx/ipuv3-plane.c        | 21 ++++++++++++++-------
 include/drm/drm_modeset_helper_vtables.h | 14 ++++++++++++++
 5 files changed, 84 insertions(+), 10 deletions(-)

-- 
2.7.4

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

[PATCH v3 1/3] drm/atomic-helper: Add atomic_disable CRTC helper callback

[Liu Ying]

Some display controllers need plane(s) to be disabled together with
the relevant CRTC, e.g., the IPUv3 display controller for imx-drm.
This patch adds atomic_disable CRTC helper callback so that
old_crtc_state(as a parameter of the callback) could be used
to get all appropriate active plane(s) of the old CRTC state for
disable operation.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v3:
* Newly introduced in v3.

 drivers/gpu/drm/drm_atomic_helper.c      |  2 ++
 include/drm/drm_modeset_helper_vtables.h | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9abe0a2..254bdde 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -749,6 +749,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 		/* Right function depends upon target state. */
 		if (crtc->state->enable && funcs->prepare)
 			funcs->prepare(crtc);
+		else if (funcs->atomic_disable)
+			 funcs->atomic_disable(crtc, old_crtc_state);
 		else if (funcs->disable)
 			funcs->disable(crtc);
 		else
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 686feec..16fd918 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -391,6 +391,20 @@ struct drm_crtc_helper_funcs {
 	 */
 	void (*atomic_flush)(struct drm_crtc *crtc,
 			     struct drm_crtc_state *old_crtc_state);
+
+	/**
+	 * @atomic_disable:
+	 *
+	 * This callback should be used to disable the CRTC. It is called
+	 * after all encoders connected to this CRTC have been shut off
+	 * already using their own ->disable hook.
+	 *
+	 * This hook is used only by atomic helpers. Atomic drivers don't
+	 * need to implement it if there's no need to disable anything at the
+	 * CRTC level.
+	 */
+	void (*atomic_disable)(struct drm_crtc *crtc,
+			       struct drm_crtc_state *old_crtc_state);
 };
 
 /**
-- 
2.7.4

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

[PATCH v3 2/3] drm/imx: ipuv3-crtc: Use the callback ->atomic_disable instead of ->disable

[Liu Ying]

Now that the drm atomic core supports the callback ->atomic_disable,
we may replace the legacy one ->disable with it.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v3:
* Newly introduced in v3.

 drivers/gpu/drm/imx/ipuv3-crtc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 6e1dc90..83c46bd 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -60,7 +60,8 @@ static void ipu_crtc_enable(struct drm_crtc *crtc)
 	ipu_di_enable(ipu_crtc->di);
 }
 
-static void ipu_crtc_disable(struct drm_crtc *crtc)
+static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
+				    struct drm_crtc_state *old_crtc_state)
 {
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
 	struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
@@ -241,7 +242,7 @@ static const struct drm_crtc_helper_funcs ipu_helper_funcs = {
 	.mode_set_nofb = ipu_crtc_mode_set_nofb,
 	.atomic_check = ipu_crtc_atomic_check,
 	.atomic_begin = ipu_crtc_atomic_begin,
-	.disable = ipu_crtc_disable,
+	.atomic_disable = ipu_crtc_atomic_disable,
 	.enable = ipu_crtc_enable,
 };
 
-- 
2.7.4

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

[PATCH v3 3/3] drm/imx: Add active plane reconfiguration support

[Liu Ying]

We don't support configuring active plane on-the-fly for imx-drm.
The relevant CRTC should be disabled before the plane configuration.
Of course, the plane itself should be disabled as well.

This patch adds active plane reconfiguration support by forcing CRTC
mode change in plane's ->atomic_check callback and adding disable
operation for all appropriate active planes(when necessary) in CRTC's
->atomic_disable callback.  The atomic core would reenabled the
affected CRTC and planes at atomic commit stage.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Peter Senna Tschudin <peter.senna@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
v2->v3:
* Disable all appropriate affected planes(when necessary) in CRTC's
  ->atomic_disable callback, but not in each plane's ->atomic_update callback,
  as suggested by Daniel Vetter.
* +Cc Lucas Stach, as he tested the patch v2.

v1->v2:
* Do not reject reconfiguring an active overlay plane.

 drivers/gpu/drm/imx/imx-drm-core.c | 26 +++++++++++++++++++++++++-
 drivers/gpu/drm/imx/ipuv3-crtc.c   | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/imx/ipuv3-plane.c  | 21 ++++++++++++++-------
 3 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 438bac8..d61b048 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -146,10 +146,34 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
 	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
 }
 
+static int imx_drm_atomic_check(struct drm_device *dev,
+				struct drm_atomic_state *state)
+{
+	int ret;
+
+	ret = drm_atomic_helper_check_modeset(dev, state);
+	if (ret)
+		return ret;
+
+	ret = drm_atomic_helper_check_planes(dev, state);
+	if (ret)
+		return ret;
+
+	/*
+	 * Check modeset again in case crtc_state->mode_changed is
+	 * updated in plane's ->atomic_check callback.
+	 */
+	ret = drm_atomic_helper_check_modeset(dev, state);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
 static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
 	.fb_create = drm_fb_cma_create,
 	.output_poll_changed = imx_drm_output_poll_changed,
-	.atomic_check = drm_atomic_helper_check,
+	.atomic_check = imx_drm_atomic_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
 
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 83c46bd..0779eed 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -76,6 +76,32 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
 		crtc->state->event = NULL;
 	}
 	spin_unlock_irq(&crtc->dev->event_lock);
+
+	/*
+	 * The relevant plane's ->atomic_check callback may set
+	 * crtc_state->mode_changed to be true when the active
+	 * plane needs to be reconfigured.  In this case and only
+	 * this case, active_changed is false - we disable all the
+	 * appropriate active planes here.
+	 */
+	if (!crtc->state->active_changed) {
+		struct drm_plane *plane;
+
+		drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) {
+			const struct drm_plane_helper_funcs *plane_funcs =
+				plane->helper_private;
+
+			/*
+			 * Filter out the plane which is explicitly required
+			 * to be disabled by the user via atomic commit
+			 * so that it won't be accidentally disabled twice.
+			 */
+			if (!plane->state->crtc)
+				continue;
+
+			plane_funcs->atomic_disable(plane, NULL);
+		}
+	}
 }
 
 static void imx_drm_crtc_reset(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 4ad67d0..6063ebe 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -319,13 +319,16 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 
 	/*
-	 * since we cannot touch active IDMAC channels, we do not support
-	 * resizing the enabled plane or changing its format
+	 * We support resizing active plane or changing its format by
+	 * forcing CRTC mode change in plane's ->atomic_check callback
+	 * and disabling all appropriate active planes in CRTC's
+	 * ->atomic_disable callback.  The planes will be reenabled in
+	 * plane's ->atomic_update callback.
 	 */
 	if (old_fb && (state->src_w != old_state->src_w ||
 			      state->src_h != old_state->src_h ||
 			      fb->pixel_format != old_fb->pixel_format))
-		return -EINVAL;
+		crtc_state->mode_changed = true;
 
 	eba = drm_plane_state_to_eba(state);
 
@@ -336,7 +339,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 
 	if (old_fb && fb->pitches[0] != old_fb->pitches[0])
-		return -EINVAL;
+		crtc_state->mode_changed = true;
 
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_YUV420:
@@ -372,7 +375,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 			return -EINVAL;
 
 		if (old_fb && old_fb->pitches[1] != fb->pitches[1])
-			return -EINVAL;
+			crtc_state->mode_changed = true;
 	}
 
 	return 0;
@@ -392,8 +395,12 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 	enum ipu_color_space ics;
 
 	if (old_state->fb) {
-		ipu_plane_atomic_set_base(ipu_plane, old_state);
-		return;
+		struct drm_crtc_state *crtc_state = state->crtc->state;
+
+		if (!crtc_state->mode_changed) {
+			ipu_plane_atomic_set_base(ipu_plane, old_state);
+			return;
+		}
 	}
 
 	switch (ipu_plane->dp_flow) {
-- 
2.7.4

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

Re: [PATCH v3 1/3] drm/atomic-helper: Add atomic_disable CRTC helper callback

[Daniel Vetter]

On Fri, Aug 19, 2016 at 05:36:57PM +0800, Liu Ying wrote:
> Some display controllers need plane(s) to be disabled together with
> the relevant CRTC, e.g., the IPUv3 display controller for imx-drm.
> This patch adds atomic_disable CRTC helper callback so that
> old_crtc_state(as a parameter of the callback) could be used
> to get all appropriate active plane(s) of the old CRTC state for
> disable operation.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Peter Senna Tschudin <peter.senna@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Liu Ying <gnuiyl@gmail.com>
> ---
> v3:
> * Newly introduced in v3.
> 
>  drivers/gpu/drm/drm_atomic_helper.c      |  2 ++
>  include/drm/drm_modeset_helper_vtables.h | 14 ++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 9abe0a2..254bdde 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -749,6 +749,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  		/* Right function depends upon target state. */
>  		if (crtc->state->enable && funcs->prepare)
>  			funcs->prepare(crtc);
> +		else if (funcs->atomic_disable)
> +			 funcs->atomic_disable(crtc, old_crtc_state);
>  		else if (funcs->disable)
>  			funcs->disable(crtc);
>  		else
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 686feec..16fd918 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -391,6 +391,20 @@ struct drm_crtc_helper_funcs {
>  	 */
>  	void (*atomic_flush)(struct drm_crtc *crtc,
>  			     struct drm_crtc_state *old_crtc_state);
> +
> +	/**
> +	 * @atomic_disable:
> +	 *
> +	 * This callback should be used to disable the CRTC. It is called
> +	 * after all encoders connected to this CRTC have been shut off
> +	 * already using their own ->disable hook.
> +	 *
> +	 * This hook is used only by atomic helpers. Atomic drivers don't
> +	 * need to implement it if there's no need to disable anything at the
> +	 * CRTC level.

kernel-doc is a bit lacking compared to the one for @disable. And I think
on top of that it should explain better the difference with @disable (and
@disable should reference @atomic_disable too).
-Daniel

> +	 */
> +	void (*atomic_disable)(struct drm_crtc *crtc,
> +			       struct drm_crtc_state *old_crtc_state);
>  };
>  
>  /**
> -- 
> 2.7.4
> 

-- 
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 v3 3/3] drm/imx: Add active plane reconfiguration support

[Daniel Vetter]

On Fri, Aug 19, 2016 at 05:36:59PM +0800, Liu Ying wrote:
> We don't support configuring active plane on-the-fly for imx-drm.
> The relevant CRTC should be disabled before the plane configuration.
> Of course, the plane itself should be disabled as well.
> 
> This patch adds active plane reconfiguration support by forcing CRTC
> mode change in plane's ->atomic_check callback and adding disable
> operation for all appropriate active planes(when necessary) in CRTC's
> ->atomic_disable callback.  The atomic core would reenabled the
> affected CRTC and planes at atomic commit stage.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Peter Senna Tschudin <peter.senna@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Liu Ying <gnuiyl@gmail.com>
> ---
> v2->v3:
> * Disable all appropriate affected planes(when necessary) in CRTC's
>   ->atomic_disable callback, but not in each plane's ->atomic_update callback,
>   as suggested by Daniel Vetter.
> * +Cc Lucas Stach, as he tested the patch v2.
> 
> v1->v2:
> * Do not reject reconfiguring an active overlay plane.
> 
>  drivers/gpu/drm/imx/imx-drm-core.c | 26 +++++++++++++++++++++++++-
>  drivers/gpu/drm/imx/ipuv3-crtc.c   | 26 ++++++++++++++++++++++++++
>  drivers/gpu/drm/imx/ipuv3-plane.c  | 21 ++++++++++++++-------
>  3 files changed, 65 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> index 438bac8..d61b048 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -146,10 +146,34 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
>  	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
>  }
>  
> +static int imx_drm_atomic_check(struct drm_device *dev,
> +				struct drm_atomic_state *state)
> +{
> +	int ret;
> +
> +	ret = drm_atomic_helper_check_modeset(dev, state);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_atomic_helper_check_planes(dev, state);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Check modeset again in case crtc_state->mode_changed is
> +	 * updated in plane's ->atomic_check callback.
> +	 */
> +	ret = drm_atomic_helper_check_modeset(dev, state);
> +	if (ret)
> +		return ret;
> +
> +	return ret;
> +}
> +
>  static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
>  	.fb_create = drm_fb_cma_create,
>  	.output_poll_changed = imx_drm_output_poll_changed,
> -	.atomic_check = drm_atomic_helper_check,
> +	.atomic_check = imx_drm_atomic_check,
>  	.atomic_commit = drm_atomic_helper_commit,
>  };
>  
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 83c46bd..0779eed 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -76,6 +76,32 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
>  		crtc->state->event = NULL;
>  	}
>  	spin_unlock_irq(&crtc->dev->event_lock);
> +
> +	/*
> +	 * The relevant plane's ->atomic_check callback may set
> +	 * crtc_state->mode_changed to be true when the active
> +	 * plane needs to be reconfigured.  In this case and only
> +	 * this case, active_changed is false - we disable all the
> +	 * appropriate active planes here.
> +	 */
> +	if (!crtc->state->active_changed) {
> +		struct drm_plane *plane;
> +
> +		drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) {
> +			const struct drm_plane_helper_funcs *plane_funcs =
> +				plane->helper_private;
> +
> +			/*
> +			 * Filter out the plane which is explicitly required
> +			 * to be disabled by the user via atomic commit
> +			 * so that it won't be accidentally disabled twice.
> +			 */
> +			if (!plane->state->crtc)
> +				continue;

I think the better place would be to do the filtering in commit_planes(),
not here. And would be great to include your patch to fix up
disable_planes_on_crtc(), too.
-Daniel

> +
> +			plane_funcs->atomic_disable(plane, NULL);
> +		}
> +	}
>  }
>  
>  static void imx_drm_crtc_reset(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 4ad67d0..6063ebe 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -319,13 +319,16 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>  		return -EINVAL;
>  
>  	/*
> -	 * since we cannot touch active IDMAC channels, we do not support
> -	 * resizing the enabled plane or changing its format
> +	 * We support resizing active plane or changing its format by
> +	 * forcing CRTC mode change in plane's ->atomic_check callback
> +	 * and disabling all appropriate active planes in CRTC's
> +	 * ->atomic_disable callback.  The planes will be reenabled in
> +	 * plane's ->atomic_update callback.
>  	 */
>  	if (old_fb && (state->src_w != old_state->src_w ||
>  			      state->src_h != old_state->src_h ||
>  			      fb->pixel_format != old_fb->pixel_format))
> -		return -EINVAL;
> +		crtc_state->mode_changed = true;
>  
>  	eba = drm_plane_state_to_eba(state);
>  
> @@ -336,7 +339,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>  		return -EINVAL;
>  
>  	if (old_fb && fb->pitches[0] != old_fb->pitches[0])
> -		return -EINVAL;
> +		crtc_state->mode_changed = true;
>  
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_YUV420:
> @@ -372,7 +375,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>  			return -EINVAL;
>  
>  		if (old_fb && old_fb->pitches[1] != fb->pitches[1])
> -			return -EINVAL;
> +			crtc_state->mode_changed = true;
>  	}
>  
>  	return 0;
> @@ -392,8 +395,12 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
>  	enum ipu_color_space ics;
>  
>  	if (old_state->fb) {
> -		ipu_plane_atomic_set_base(ipu_plane, old_state);
> -		return;
> +		struct drm_crtc_state *crtc_state = state->crtc->state;
> +
> +		if (!crtc_state->mode_changed) {
> +			ipu_plane_atomic_set_base(ipu_plane, old_state);
> +			return;
> +		}
>  	}
>  
>  	switch (ipu_plane->dp_flow) {
> -- 
> 2.7.4
> 

-- 
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 v3 1/3] drm/atomic-helper: Add atomic_disable CRTC helper callback

[Ying Liu]

On Mon, Aug 22, 2016 at 3:01 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Aug 19, 2016 at 05:36:57PM +0800, Liu Ying wrote:
>> Some display controllers need plane(s) to be disabled together with
>> the relevant CRTC, e.g., the IPUv3 display controller for imx-drm.
>> This patch adds atomic_disable CRTC helper callback so that
>> old_crtc_state(as a parameter of the callback) could be used
>> to get all appropriate active plane(s) of the old CRTC state for
>> disable operation.
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Peter Senna Tschudin <peter.senna@gmail.com>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Signed-off-by: Liu Ying <gnuiyl@gmail.com>
>> ---
>> v3:
>> * Newly introduced in v3.
>>
>>  drivers/gpu/drm/drm_atomic_helper.c      |  2 ++
>>  include/drm/drm_modeset_helper_vtables.h | 14 ++++++++++++++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 9abe0a2..254bdde 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -749,6 +749,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>>               /* Right function depends upon target state. */
>>               if (crtc->state->enable && funcs->prepare)
>>                       funcs->prepare(crtc);
>> +             else if (funcs->atomic_disable)
>> +                      funcs->atomic_disable(crtc, old_crtc_state);
>>               else if (funcs->disable)
>>                       funcs->disable(crtc);
>>               else
>> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
>> index 686feec..16fd918 100644
>> --- a/include/drm/drm_modeset_helper_vtables.h
>> +++ b/include/drm/drm_modeset_helper_vtables.h
>> @@ -391,6 +391,20 @@ struct drm_crtc_helper_funcs {
>>        */
>>       void (*atomic_flush)(struct drm_crtc *crtc,
>>                            struct drm_crtc_state *old_crtc_state);
>> +
>> +     /**
>> +      * @atomic_disable:
>> +      *
>> +      * This callback should be used to disable the CRTC. It is called
>> +      * after all encoders connected to this CRTC have been shut off
>> +      * already using their own ->disable hook.
>> +      *
>> +      * This hook is used only by atomic helpers. Atomic drivers don't
>> +      * need to implement it if there's no need to disable anything at the
>> +      * CRTC level.
>
> kernel-doc is a bit lacking compared to the one for @disable. And I think
> on top of that it should explain better the difference with @disable (and
> @disable should reference @atomic_disable too).

Will improve the kernel-doc in the next version.  Thanks.

Regards,
Liu Ying

> -Daniel
>
>> +      */
>> +     void (*atomic_disable)(struct drm_crtc *crtc,
>> +                            struct drm_crtc_state *old_crtc_state);
>>  };
>>
>>  /**
>> --
>> 2.7.4
>>
>
> --
> 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 v3 3/3] drm/imx: Add active plane reconfiguration support

[Ying Liu]

Hi Daniel,

On Mon, Aug 22, 2016 at 3:20 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Aug 19, 2016 at 05:36:59PM +0800, Liu Ying wrote:
>> We don't support configuring active plane on-the-fly for imx-drm.
>> The relevant CRTC should be disabled before the plane configuration.
>> Of course, the plane itself should be disabled as well.
>>
>> This patch adds active plane reconfiguration support by forcing CRTC
>> mode change in plane's ->atomic_check callback and adding disable
>> operation for all appropriate active planes(when necessary) in CRTC's
>> ->atomic_disable callback.  The atomic core would reenabled the
>> affected CRTC and planes at atomic commit stage.
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Peter Senna Tschudin <peter.senna@gmail.com>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Signed-off-by: Liu Ying <gnuiyl@gmail.com>
>> ---
>> v2->v3:
>> * Disable all appropriate affected planes(when necessary) in CRTC's
>>   ->atomic_disable callback, but not in each plane's ->atomic_update callback,
>>   as suggested by Daniel Vetter.
>> * +Cc Lucas Stach, as he tested the patch v2.
>>
>> v1->v2:
>> * Do not reject reconfiguring an active overlay plane.
>>
>>  drivers/gpu/drm/imx/imx-drm-core.c | 26 +++++++++++++++++++++++++-
>>  drivers/gpu/drm/imx/ipuv3-crtc.c   | 26 ++++++++++++++++++++++++++
>>  drivers/gpu/drm/imx/ipuv3-plane.c  | 21 ++++++++++++++-------
>>  3 files changed, 65 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
>> index 438bac8..d61b048 100644
>> --- a/drivers/gpu/drm/imx/imx-drm-core.c
>> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
>> @@ -146,10 +146,34 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
>>       drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
>>  }
>>
>> +static int imx_drm_atomic_check(struct drm_device *dev,
>> +                             struct drm_atomic_state *state)
>> +{
>> +     int ret;
>> +
>> +     ret = drm_atomic_helper_check_modeset(dev, state);
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = drm_atomic_helper_check_planes(dev, state);
>> +     if (ret)
>> +             return ret;
>> +
>> +     /*
>> +      * Check modeset again in case crtc_state->mode_changed is
>> +      * updated in plane's ->atomic_check callback.
>> +      */
>> +     ret = drm_atomic_helper_check_modeset(dev, state);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return ret;
>> +}
>> +
>>  static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
>>       .fb_create = drm_fb_cma_create,
>>       .output_poll_changed = imx_drm_output_poll_changed,
>> -     .atomic_check = drm_atomic_helper_check,
>> +     .atomic_check = imx_drm_atomic_check,
>>       .atomic_commit = drm_atomic_helper_commit,
>>  };
>>
>> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
>> index 83c46bd..0779eed 100644
>> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
>> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
>> @@ -76,6 +76,32 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
>>               crtc->state->event = NULL;
>>       }
>>       spin_unlock_irq(&crtc->dev->event_lock);
>> +
>> +     /*
>> +      * The relevant plane's ->atomic_check callback may set
>> +      * crtc_state->mode_changed to be true when the active
>> +      * plane needs to be reconfigured.  In this case and only
>> +      * this case, active_changed is false - we disable all the
>> +      * appropriate active planes here.
>> +      */
>> +     if (!crtc->state->active_changed) {
>> +             struct drm_plane *plane;
>> +
>> +             drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) {
>> +                     const struct drm_plane_helper_funcs *plane_funcs =
>> +                             plane->helper_private;
>> +
>> +                     /*
>> +                      * Filter out the plane which is explicitly required
>> +                      * to be disabled by the user via atomic commit
>> +                      * so that it won't be accidentally disabled twice.
>> +                      */
>> +                     if (!plane->state->crtc)
>> +                             continue;
>
> I think the better place would be to do the filtering in commit_planes(),
> not here. And would be great to include your patch to fix up
> disable_planes_on_crtc(), too.

Do you mean we can do the filtering by checking (!plane->state->crtc)
right before plane_funcs->atomic_disable in commit_planes()?
Won't it filter out all the apples?
commit_planes() doesn't know whether the driver calls
disable_planes_on_crtc() or not.

imo, doing the filtering in crtc_funcs->atomic_disable is sane.

Regards,
Liu Ying


> -Daniel
>
>> +
>> +                     plane_funcs->atomic_disable(plane, NULL);
>> +             }
>> +     }
>>  }
>>
>>  static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
>> index 4ad67d0..6063ebe 100644
>> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
>> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
>> @@ -319,13 +319,16 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>>               return -EINVAL;
>>
>>       /*
>> -      * since we cannot touch active IDMAC channels, we do not support
>> -      * resizing the enabled plane or changing its format
>> +      * We support resizing active plane or changing its format by
>> +      * forcing CRTC mode change in plane's ->atomic_check callback
>> +      * and disabling all appropriate active planes in CRTC's
>> +      * ->atomic_disable callback.  The planes will be reenabled in
>> +      * plane's ->atomic_update callback.
>>        */
>>       if (old_fb && (state->src_w != old_state->src_w ||
>>                             state->src_h != old_state->src_h ||
>>                             fb->pixel_format != old_fb->pixel_format))
>> -             return -EINVAL;
>> +             crtc_state->mode_changed = true;
>>
>>       eba = drm_plane_state_to_eba(state);
>>
>> @@ -336,7 +339,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>>               return -EINVAL;
>>
>>       if (old_fb && fb->pitches[0] != old_fb->pitches[0])
>> -             return -EINVAL;
>> +             crtc_state->mode_changed = true;
>>
>>       switch (fb->pixel_format) {
>>       case DRM_FORMAT_YUV420:
>> @@ -372,7 +375,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
>>                       return -EINVAL;
>>
>>               if (old_fb && old_fb->pitches[1] != fb->pitches[1])
>> -                     return -EINVAL;
>> +                     crtc_state->mode_changed = true;
>>       }
>>
>>       return 0;
>> @@ -392,8 +395,12 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
>>       enum ipu_color_space ics;
>>
>>       if (old_state->fb) {
>> -             ipu_plane_atomic_set_base(ipu_plane, old_state);
>> -             return;
>> +             struct drm_crtc_state *crtc_state = state->crtc->state;
>> +
>> +             if (!crtc_state->mode_changed) {
>> +                     ipu_plane_atomic_set_base(ipu_plane, old_state);
>> +                     return;
>> +             }
>>       }
>>
>>       switch (ipu_plane->dp_flow) {
>> --
>> 2.7.4
>>
>
> --
> 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 v3 3/3] drm/imx: Add active plane reconfiguration support

[Daniel Vetter]

On Mon, Aug 22, 2016 at 9:43 AM, Ying Liu <gnuiyl@gmail.com> wrote:
>>> +
>>> +     /*
>>> +      * The relevant plane's ->atomic_check callback may set
>>> +      * crtc_state->mode_changed to be true when the active
>>> +      * plane needs to be reconfigured.  In this case and only
>>> +      * this case, active_changed is false - we disable all the
>>> +      * appropriate active planes here.
>>> +      */
>>> +     if (!crtc->state->active_changed) {
>>> +             struct drm_plane *plane;
>>> +
>>> +             drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) {
>>> +                     const struct drm_plane_helper_funcs *plane_funcs =
>>> +                             plane->helper_private;
>>> +
>>> +                     /*
>>> +                      * Filter out the plane which is explicitly required
>>> +                      * to be disabled by the user via atomic commit
>>> +                      * so that it won't be accidentally disabled twice.
>>> +                      */
>>> +                     if (!plane->state->crtc)
>>> +                             continue;
>>
>> I think the better place would be to do the filtering in commit_planes(),
>> not here. And would be great to include your patch to fix up
>> disable_planes_on_crtc(), too.
>
> Do you mean we can do the filtering by checking (!plane->state->crtc)
> right before plane_funcs->atomic_disable in commit_planes()?
> Won't it filter out all the apples?
> commit_planes() doesn't know whether the driver calls
> disable_planes_on_crtc() or not.

You need to filter on needs_modeset(crtc_state), and it needs to be
optional like active_only, using a flag.

> imo, doing the filtering in crtc_funcs->atomic_disable is sane.

It is not sane in general, since usually you need to call
disable_planes_on_crtc to avoid upsetting your hardware. And for that
use-case filtering in disable will lead to hung hardware. Which really
means that you need to filter in commit_planes.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/3] drm/atomic-helper: Add atomic_disable CRTC helper callback

[Daniel Vetter]

On Mon, Aug 22, 2016 at 9:37 AM, Ying Liu <gnuiyl@gmail.com> wrote:
>> kernel-doc is a bit lacking compared to the one for @disable. And I think
>> on top of that it should explain better the difference with @disable (and
>> @disable should reference @atomic_disable too).
>
> Will improve the kernel-doc in the next version.  Thanks.

For consistency pls start out with a copy of the exact text for
@disable (minus the note which is only relevant for legacy crtc
helpers ofc).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/3] drm/imx: Add active plane reconfiguration support

[Ying Liu]

On Mon, Aug 22, 2016 at 3:53 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Aug 22, 2016 at 9:43 AM, Ying Liu <gnuiyl@gmail.com> wrote:
>>>> +
>>>> +     /*
>>>> +      * The relevant plane's ->atomic_check callback may set
>>>> +      * crtc_state->mode_changed to be true when the active
>>>> +      * plane needs to be reconfigured.  In this case and only
>>>> +      * this case, active_changed is false - we disable all the
>>>> +      * appropriate active planes here.
>>>> +      */
>>>> +     if (!crtc->state->active_changed) {
>>>> +             struct drm_plane *plane;
>>>> +
>>>> +             drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) {
>>>> +                     const struct drm_plane_helper_funcs *plane_funcs =
>>>> +                             plane->helper_private;
>>>> +
>>>> +                     /*
>>>> +                      * Filter out the plane which is explicitly required
>>>> +                      * to be disabled by the user via atomic commit
>>>> +                      * so that it won't be accidentally disabled twice.
>>>> +                      */
>>>> +                     if (!plane->state->crtc)
>>>> +                             continue;
>>>
>>> I think the better place would be to do the filtering in commit_planes(),
>>> not here. And would be great to include your patch to fix up
>>> disable_planes_on_crtc(), too.
>>
>> Do you mean we can do the filtering by checking (!plane->state->crtc)
>> right before plane_funcs->atomic_disable in commit_planes()?
>> Won't it filter out all the apples?
>> commit_planes() doesn't know whether the driver calls
>> disable_planes_on_crtc() or not.
>
> You need to filter on needs_modeset(crtc_state), and it needs to be
> optional like active_only, using a flag.

Then, IIUC, the flag will be CRTC specific and be dynamically
changeable. I'm not clear about the way to implement this.

>
>> imo, doing the filtering in crtc_funcs->atomic_disable is sane.
>
> It is not sane in general, since usually you need to call
> disable_planes_on_crtc to avoid upsetting your hardware. And for that

Calling disable_planes_on_crtc() in crtc_funcs->atomic_disable is
a bit over-kill and will cause the issue of disabling plane twice, unless
the filtering is done in disable_planes_on_crtc() (which was rejected
by you on irc) or in commit_planes()?(which I'm not clear about
the way to implement, as I mentioned before).  So I chose to do the
filtering in the imx-drm specific crtc_funcs->atomic_disable function.

> use-case filtering in disable will lead to hung hardware. Which really

Hung hardware due to filtering? How?

> means that you need to filter in commit_planes.

Really don't understand the rational for filtering in commit_planes...
Did I miss anything?

Regards,
Liu Ying

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/3] drm/imx: Add active plane reconfiguration support

[Daniel Vetter]

On Mon, Aug 22, 2016 at 04:23:36PM +0800, Ying Liu wrote:
> On Mon, Aug 22, 2016 at 3:53 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Aug 22, 2016 at 9:43 AM, Ying Liu <gnuiyl@gmail.com> wrote:
> >>>> +
> >>>> +     /*
> >>>> +      * The relevant plane's ->atomic_check callback may set
> >>>> +      * crtc_state->mode_changed to be true when the active
> >>>> +      * plane needs to be reconfigured.  In this case and only
> >>>> +      * this case, active_changed is false - we disable all the
> >>>> +      * appropriate active planes here.
> >>>> +      */
> >>>> +     if (!crtc->state->active_changed) {
> >>>> +             struct drm_plane *plane;
> >>>> +
> >>>> +             drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) {
> >>>> +                     const struct drm_plane_helper_funcs *plane_funcs =
> >>>> +                             plane->helper_private;
> >>>> +
> >>>> +                     /*
> >>>> +                      * Filter out the plane which is explicitly required
> >>>> +                      * to be disabled by the user via atomic commit
> >>>> +                      * so that it won't be accidentally disabled twice.
> >>>> +                      */
> >>>> +                     if (!plane->state->crtc)
> >>>> +                             continue;
> >>>
> >>> I think the better place would be to do the filtering in commit_planes(),
> >>> not here. And would be great to include your patch to fix up
> >>> disable_planes_on_crtc(), too.
> >>
> >> Do you mean we can do the filtering by checking (!plane->state->crtc)
> >> right before plane_funcs->atomic_disable in commit_planes()?
> >> Won't it filter out all the apples?
> >> commit_planes() doesn't know whether the driver calls
> >> disable_planes_on_crtc() or not.
> >
> > You need to filter on needs_modeset(crtc_state), and it needs to be
> > optional like active_only, using a flag.
> 
> Then, IIUC, the flag will be CRTC specific and be dynamically
> changeable. I'm not clear about the way to implement this.
> 
> >
> >> imo, doing the filtering in crtc_funcs->atomic_disable is sane.
> >
> > It is not sane in general, since usually you need to call
> > disable_planes_on_crtc to avoid upsetting your hardware. And for that
> 
> Calling disable_planes_on_crtc() in crtc_funcs->atomic_disable is
> a bit over-kill and will cause the issue of disabling plane twice, unless
> the filtering is done in disable_planes_on_crtc() (which was rejected
> by you on irc) or in commit_planes()?(which I'm not clear about
> the way to implement, as I mentioned before).  So I chose to do the
> filtering in the imx-drm specific crtc_funcs->atomic_disable function.

For filtering in commit_planes I think the best plane is to
- add a new flag NO_DISABLE_AFTER_MODESET
- which does exactly what it says on the tin: If the commmit_planes helper
  would call plane_funcs->atomic_disable, but
  drm_atomic_crtc_needs_modeset() for that crtc is true then you skip the
  ->atomic_disable call.

This assumes that all disables have already been committed when disabling
the crtc. For a lot of hardware it only makes sense to set both this flag
and ACTIVE_ONLY, but there's probably some fun hardware out there where
this is not the case (yours seems to be one example).

Now it's not pretty to have 2 boolean arguments for the same function, so
we also need to convert that into a bitflag field. End result:

#define COMMIT_ACTIVE_ONLY		BIT(0)
#define	COMMIT_NO_DISABLE_AFTER_MODESET	BIT(1)

Ofc kernel-doc should explain what the flags are for and when to use them.

void drm_atomic_helper_commit_planes(struct drm_device *dev,
				     struct drm_atomic_state *state,
				     unsigned flags);

A bit more work, but I think overall worth it.

> > use-case filtering in disable will lead to hung hardware. Which really
> 
> Hung hardware due to filtering? How?

If you shut down the crtc and keep the planes running the hw dies. Yup,
this happens.

> > means that you need to filter in commit_planes.
> 
> Really don't understand the rational for filtering in commit_planes...
> Did I miss anything?

See above, it might work for your driver, but it definitely restricts the
usefulness of the helpers in general. And helpers which are only useful
for 1 driver aren't useful.

I hope this explains my idea a bit better, otherwise probably best if you
ping me on irc.
-Daniel
-- 
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