Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1692911 > unrolled thread

[PATCH v2 0/4] drm/sun4i: Fix a register access bug

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/4] drm/sun4i: Fix a register access bug Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-20 15:10 +0200
    [PATCH v2 2/4] drm/sun4i: Use the runtime_pm commit_tail variant Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-20 15:10 +0200
    [PATCH v2 3/4] drm/sun4i: engine: Add commit_poll function Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-20 15:10 +0200
    [PATCH v2 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-20 15:10 +0200
      Re: [PATCH v2 1/4] drm/atomic: implement  drm_atomic_helper_commit_tail for runtime_pm users Daniel Vetter <daniel@ffwll.ch> - 2017-07-20 20:50 +0200
        Re: [PATCH v2 1/4] drm/atomic: implement  drm_atomic_helper_commit_tail for runtime_pm users Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-25 09:00 +0200
          Re: [PATCH v2 1/4] drm/atomic: implement  drm_atomic_helper_commit_tail for runtime_pm users Daniel Vetter <daniel@ffwll.ch> - 2017-07-25 10:10 +0200
    [PATCH v2 4/4] drm/sun4i: make sure we don't have a commit pending Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-20 15:10 +0200

#1692911 — [PATCH v2 0/4] drm/sun4i: Fix a register access bug

The Allwinner backend has a commit bit in order to push the new
configuration to the actual hardware. We've always been using that bit.

However, we also should poll for that bit to clear, which we don't.
Accessing any register while a commit is pending is forbidden, and will for
example show a symptom of reading another, random, register.

If you get this during a read/modify/write cycle, this will result in
random register corruption, which are pretty bad.

This can be shown using the following program (while the backend is
active):
http://code.bulix.org/gdl44p-161437?raw

Fortunately for us, this is not really likely to happen. The window where
it can happen is quite thin, and it only happens during a modeset, since
it's the only time we commit some new configuration.

Unfortunately for us, QT does a ridiculous amount of modeset, and will just
hit that window after a while, creating a distorded (since the register we
read/modify/write also has scaling attributes) or with weird colors (since
it also has a invertion of red and blue components). And might fix itself
later on by reading another random register with proper values for these
fields.

Let me know what you think,
Maxime

Changes from v1:
  - Renamed the function drm_atomic_helper_commit_tail_rpm
  - Dropped the changes in the rcar-du driver
  - Enhanced the documentation based on Daniel's comments

Maxime Ripard (4):
  drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users
  drm/sun4i: Use the runtime_pm commit_tail variant
  drm/sun4i: engine: Add commit_poll function
  drm/sun4i: make sure we don't have a commit pending

 drivers/gpu/drm/drm_atomic_helper.c        | 49 +++++++++++++++--------
 drivers/gpu/drm/exynos/exynos_drm_fb.c     | 27 +-------------
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 21 +----------
 drivers/gpu/drm/sun4i/sun4i_backend.c      | 14 +++++++-
 drivers/gpu/drm/sun4i/sun4i_crtc.c         |  3 +-
 drivers/gpu/drm/sun4i/sun4i_framebuffer.c  |  6 +++-
 drivers/gpu/drm/sun4i/sunxi_engine.h       | 14 +++++++-
 include/drm/drm_atomic_helper.h            |  1 +-
 8 files changed, 74 insertions(+), 61 deletions(-)

base-commit: 5771a8c08880cdca3bfb4a3fc6d309d6bba20877
-- 
git-series 0.9.1

[toc] | [next] | [standalone]

#1692912 — [PATCH v2 2/4] drm/sun4i: Use the runtime_pm commit_tail variant

The backend (planes) commit can only happen when the TCON (CRTC) is
enabled, which is not guaranteed with the default commit_tail helper.

Let's use the runtime_pm version that is designed specifically to deal with
that case.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
index 9872e0fc03b0..53ab61665390 100644
--- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
@@ -10,6 +10,7 @@
  * the License, or (at your option) any later version.
  */
 
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drmP.h>
@@ -31,6 +32,10 @@ static const struct drm_mode_config_funcs sun4i_de_mode_config_funcs = {
 	.fb_create		= drm_fb_cma_create,
 };
 
+static struct drm_mode_config_helper_funcs sun4i_de_mode_config_helpers = {
+	.atomic_commit_tail	= drm_atomic_helper_commit_tail_rpm,
+};
+
 struct drm_fbdev_cma *sun4i_framebuffer_init(struct drm_device *drm)
 {
 	drm_mode_config_reset(drm);
@@ -39,6 +44,7 @@ struct drm_fbdev_cma *sun4i_framebuffer_init(struct drm_device *drm)
 	drm->mode_config.max_height = 8192;
 
 	drm->mode_config.funcs = &sun4i_de_mode_config_funcs;
+	drm->mode_config.helper_private = &sun4i_de_mode_config_helpers;
 
 	return drm_fbdev_cma_init(drm, 32, drm->mode_config.num_connector);
 }
-- 
git-series 0.9.1

[toc] | [prev] | [next] | [standalone]

#1692915 — [PATCH v2 3/4] drm/sun4i: engine: Add commit_poll function

On the earlier Allwinner chips, with the first iteration of the display
engine, the backend commit bit needs to be polled before making any
register access to the backend.

Add an operation for that, that will be called in atomic_begin in order to
be sure to have that bit cleared before we do any modifications.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_crtc.c   |  2 ++
 drivers/gpu/drm/sun4i/sunxi_engine.h | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c
index f8c70439d1e2..2eba1d8639d8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -45,6 +45,8 @@ static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc,
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		crtc->state->event = NULL;
 	 }
+
+	WARN_ON(sunxi_engine_commit_poll(engine));
 }
 
 static void sun4i_crtc_atomic_flush(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/sun4i/sunxi_engine.h b/drivers/gpu/drm/sun4i/sunxi_engine.h
index 4cb70ae65c79..6618e182613c 100644
--- a/drivers/gpu/drm/sun4i/sunxi_engine.h
+++ b/drivers/gpu/drm/sun4i/sunxi_engine.h
@@ -17,6 +17,7 @@ struct sunxi_engine;
 
 struct sunxi_engine_ops {
 	void (*commit)(struct sunxi_engine *engine);
+	int (*commit_poll)(struct sunxi_engine *engine);
 	struct drm_plane **(*layers_init)(struct drm_device *drm,
 					  struct sunxi_engine *engine);
 
@@ -55,6 +56,19 @@ sunxi_engine_commit(struct sunxi_engine *engine)
 }
 
 /**
+ * sunxi_engine_commit_poll() - wait for all changes to be committed
+ * @engine:	pointer to the engine
+ */
+static inline int
+sunxi_engine_commit_poll(struct sunxi_engine *engine)
+{
+	if (engine->ops && engine->ops->commit_poll)
+		return engine->ops->commit_poll(engine);
+
+	return 0;
+}
+
+/**
  * sunxi_engine_layers_init() - Create planes (layers) for the engine
  * @drm:	pointer to the drm_device for which planes will be created
  * @engine:	pointer to the engine
-- 
git-series 0.9.1

[toc] | [prev] | [next] | [standalone]

#1692917 — [PATCH v2 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

The current drm_atomic_helper_commit_tail helper works only if the CRTC is
accessible, and documents an alternative implementation that is supposed to
be used if that happens.

That implementation is then duplicated by some drivers. Instead of
documenting it, let's implement an helper that all the relevant users can
use directly.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/drm_atomic_helper.c        | 49 +++++++++++++++--------
 drivers/gpu/drm/exynos/exynos_drm_fb.c     | 27 +-------------
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 21 +----------
 include/drm/drm_atomic_helper.h            |  1 +-
 4 files changed, 37 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 86d3093c6c9b..d06a65ed3a57 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1245,23 +1245,13 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
  * @old_state: atomic state object with old state structures
  *
  * This is the default implementation for the
- * &drm_mode_config_helper_funcs.atomic_commit_tail hook.
+ * &drm_mode_config_helper_funcs.atomic_commit_tail hook, for drivers
+ * that do not support runtime_pm or do not need the CRTC to be
+ * enabled to perform a commit. Otherwise, see
+ * drm_atomic_helper_commit_tail_rpm().
  *
  * Note that the default ordering of how the various stages are called is to
- * match the legacy modeset helper library closest. One peculiarity of that is
- * that it doesn't mesh well with runtime PM at all.
- *
- * For drivers supporting runtime PM the recommended sequence is instead ::
- *
- *     drm_atomic_helper_commit_modeset_disables(dev, old_state);
- *
- *     drm_atomic_helper_commit_modeset_enables(dev, old_state);
- *
- *     drm_atomic_helper_commit_planes(dev, old_state,
- *                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
- *
- * for committing the atomic update to hardware.  See the kerneldoc entries for
- * these three functions for more details.
+ * match the legacy modeset helper library closest.
  */
 void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state)
 {
@@ -1281,6 +1271,35 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state)
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit_tail);
 
+/**
+ * drm_atomic_helper_commit_tail_rpm - commit atomic update to hardware
+ * @old_state: new modeset state to be committed
+ *
+ * This is an alternative implementation for the
+ * &drm_mode_config_helper_funcs.atomic_commit_tail hook, for drivers
+ * that support runtime_pm or need the CRTC to be enabled to perform a
+ * commit. Otherwise, one should use the default implementation
+ * drm_atomic_helper_commit_tail().
+ */
+void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
+{
+	struct drm_device *dev = old_state->dev;
+
+	drm_atomic_helper_commit_modeset_disables(dev, old_state);
+
+	drm_atomic_helper_commit_modeset_enables(dev, old_state);
+
+	drm_atomic_helper_commit_planes(dev, old_state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
+
+	drm_atomic_helper_commit_hw_done(old_state);
+
+	drm_atomic_helper_wait_for_vblanks(dev, old_state);
+
+	drm_atomic_helper_cleanup_planes(dev, old_state);
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit_tail_rpm);
+
 static void commit_tail(struct drm_atomic_state *old_state)
 {
 	struct drm_device *dev = old_state->dev;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index d48fd7c918f8..ed1a648d518c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -187,33 +187,8 @@ dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer *fb, int index)
 	return exynos_fb->dma_addr[index];
 }
 
-static void exynos_drm_atomic_commit_tail(struct drm_atomic_state *state)
-{
-	struct drm_device *dev = state->dev;
-
-	drm_atomic_helper_commit_modeset_disables(dev, state);
-
-	drm_atomic_helper_commit_modeset_enables(dev, state);
-
-	/*
-	 * Exynos can't update planes with CRTCs and encoders disabled,
-	 * its updates routines, specially for FIMD, requires the clocks
-	 * to be enabled. So it is necessary to handle the modeset operations
-	 * *before* the commit_planes() step, this way it will always
-	 * have the relevant clocks enabled to perform the update.
-	 */
-	drm_atomic_helper_commit_planes(dev, state,
-					DRM_PLANE_COMMIT_ACTIVE_ONLY);
-
-	drm_atomic_helper_commit_hw_done(state);
-
-	drm_atomic_helper_wait_for_vblanks(dev, state);
-
-	drm_atomic_helper_cleanup_planes(dev, state);
-}
-
 static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
-	.atomic_commit_tail = exynos_drm_atomic_commit_tail,
+	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
 };
 
 static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 81f9548672b0..35ad386c401e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -174,27 +174,8 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev)
 		drm_fb_helper_hotplug_event(fb_helper);
 }
 
-static void
-rockchip_atomic_commit_tail(struct drm_atomic_state *state)
-{
-	struct drm_device *dev = state->dev;
-
-	drm_atomic_helper_commit_modeset_disables(dev, state);
-
-	drm_atomic_helper_commit_modeset_enables(dev, state);
-
-	drm_atomic_helper_commit_planes(dev, state,
-					DRM_PLANE_COMMIT_ACTIVE_ONLY);
-
-	drm_atomic_helper_commit_hw_done(state);
-
-	drm_atomic_helper_wait_for_vblanks(dev, state);
-
-	drm_atomic_helper_cleanup_planes(dev, state);
-}
-
 static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = {
-	.atomic_commit_tail = rockchip_atomic_commit_tail,
+	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
 };
 
 static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index f0a8678ae98e..af4aaff9ec0b 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -41,6 +41,7 @@ int drm_atomic_helper_check_planes(struct drm_device *dev,
 int drm_atomic_helper_check(struct drm_device *dev,
 			    struct drm_atomic_state *state);
 void drm_atomic_helper_commit_tail(struct drm_atomic_state *state);
+void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *state);
 int drm_atomic_helper_commit(struct drm_device *dev,
 			     struct drm_atomic_state *state,
 			     bool nonblock);
-- 
git-series 0.9.1

[toc] | [prev] | [next] | [standalone]

#1693170 — Re: [PATCH v2 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

On Thu, Jul 20, 2017 at 03:01:16PM +0200, Maxime Ripard wrote:
> The current drm_atomic_helper_commit_tail helper works only if the CRTC is
> accessible, and documents an alternative implementation that is supposed to
> be used if that happens.
> 
> That implementation is then duplicated by some drivers. Instead of
> documenting it, let's implement an helper that all the relevant users can
> use directly.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

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

Should I throw this into drm-misc, or do you want to merge this through
some driver tree?
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c        | 49 +++++++++++++++--------
>  drivers/gpu/drm/exynos/exynos_drm_fb.c     | 27 +-------------
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 21 +----------
>  include/drm/drm_atomic_helper.h            |  1 +-
>  4 files changed, 37 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 86d3093c6c9b..d06a65ed3a57 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1245,23 +1245,13 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
>   * @old_state: atomic state object with old state structures
>   *
>   * This is the default implementation for the
> - * &drm_mode_config_helper_funcs.atomic_commit_tail hook.
> + * &drm_mode_config_helper_funcs.atomic_commit_tail hook, for drivers
> + * that do not support runtime_pm or do not need the CRTC to be
> + * enabled to perform a commit. Otherwise, see
> + * drm_atomic_helper_commit_tail_rpm().
>   *
>   * Note that the default ordering of how the various stages are called is to
> - * match the legacy modeset helper library closest. One peculiarity of that is
> - * that it doesn't mesh well with runtime PM at all.
> - *
> - * For drivers supporting runtime PM the recommended sequence is instead ::
> - *
> - *     drm_atomic_helper_commit_modeset_disables(dev, old_state);
> - *
> - *     drm_atomic_helper_commit_modeset_enables(dev, old_state);
> - *
> - *     drm_atomic_helper_commit_planes(dev, old_state,
> - *                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
> - *
> - * for committing the atomic update to hardware.  See the kerneldoc entries for
> - * these three functions for more details.
> + * match the legacy modeset helper library closest.
>   */
>  void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state)
>  {
> @@ -1281,6 +1271,35 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state)
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_commit_tail);
>  
> +/**
> + * drm_atomic_helper_commit_tail_rpm - commit atomic update to hardware
> + * @old_state: new modeset state to be committed
> + *
> + * This is an alternative implementation for the
> + * &drm_mode_config_helper_funcs.atomic_commit_tail hook, for drivers
> + * that support runtime_pm or need the CRTC to be enabled to perform a
> + * commit. Otherwise, one should use the default implementation
> + * drm_atomic_helper_commit_tail().
> + */
> +void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
> +{
> +	struct drm_device *dev = old_state->dev;
> +
> +	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> +
> +	drm_atomic_helper_commit_modeset_enables(dev, old_state);
> +
> +	drm_atomic_helper_commit_planes(dev, old_state,
> +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
> +
> +	drm_atomic_helper_commit_hw_done(old_state);
> +
> +	drm_atomic_helper_wait_for_vblanks(dev, old_state);
> +
> +	drm_atomic_helper_cleanup_planes(dev, old_state);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_commit_tail_rpm);
> +
>  static void commit_tail(struct drm_atomic_state *old_state)
>  {
>  	struct drm_device *dev = old_state->dev;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index d48fd7c918f8..ed1a648d518c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -187,33 +187,8 @@ dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer *fb, int index)
>  	return exynos_fb->dma_addr[index];
>  }
>  
> -static void exynos_drm_atomic_commit_tail(struct drm_atomic_state *state)
> -{
> -	struct drm_device *dev = state->dev;
> -
> -	drm_atomic_helper_commit_modeset_disables(dev, state);
> -
> -	drm_atomic_helper_commit_modeset_enables(dev, state);
> -
> -	/*
> -	 * Exynos can't update planes with CRTCs and encoders disabled,
> -	 * its updates routines, specially for FIMD, requires the clocks
> -	 * to be enabled. So it is necessary to handle the modeset operations
> -	 * *before* the commit_planes() step, this way it will always
> -	 * have the relevant clocks enabled to perform the update.
> -	 */
> -	drm_atomic_helper_commit_planes(dev, state,
> -					DRM_PLANE_COMMIT_ACTIVE_ONLY);
> -
> -	drm_atomic_helper_commit_hw_done(state);
> -
> -	drm_atomic_helper_wait_for_vblanks(dev, state);
> -
> -	drm_atomic_helper_cleanup_planes(dev, state);
> -}
> -
>  static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
> -	.atomic_commit_tail = exynos_drm_atomic_commit_tail,
> +	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
>  };
>  
>  static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 81f9548672b0..35ad386c401e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -174,27 +174,8 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev)
>  		drm_fb_helper_hotplug_event(fb_helper);
>  }
>  
> -static void
> -rockchip_atomic_commit_tail(struct drm_atomic_state *state)
> -{
> -	struct drm_device *dev = state->dev;
> -
> -	drm_atomic_helper_commit_modeset_disables(dev, state);
> -
> -	drm_atomic_helper_commit_modeset_enables(dev, state);
> -
> -	drm_atomic_helper_commit_planes(dev, state,
> -					DRM_PLANE_COMMIT_ACTIVE_ONLY);
> -
> -	drm_atomic_helper_commit_hw_done(state);
> -
> -	drm_atomic_helper_wait_for_vblanks(dev, state);
> -
> -	drm_atomic_helper_cleanup_planes(dev, state);
> -}
> -
>  static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = {
> -	.atomic_commit_tail = rockchip_atomic_commit_tail,
> +	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
>  };
>  
>  static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index f0a8678ae98e..af4aaff9ec0b 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -41,6 +41,7 @@ int drm_atomic_helper_check_planes(struct drm_device *dev,
>  int drm_atomic_helper_check(struct drm_device *dev,
>  			    struct drm_atomic_state *state);
>  void drm_atomic_helper_commit_tail(struct drm_atomic_state *state);
> +void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *state);
>  int drm_atomic_helper_commit(struct drm_device *dev,
>  			     struct drm_atomic_state *state,
>  			     bool nonblock);
> -- 
> git-series 0.9.1
> _______________________________________________
> 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

[toc] | [prev] | [next] | [standalone]

#1695420 — Re: [PATCH v2 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Multipart message — attachments visible in raw view] — view raw

Hi Daniel,

On Thu, Jul 20, 2017 at 08:46:28PM +0200, Daniel Vetter wrote:
> On Thu, Jul 20, 2017 at 03:01:16PM +0200, Maxime Ripard wrote:
> > The current drm_atomic_helper_commit_tail helper works only if the CRTC is
> > accessible, and documents an alternative implementation that is supposed to
> > be used if that happens.
> > 
> > That implementation is then duplicated by some drivers. Instead of
> > documenting it, let's implement an helper that all the relevant users can
> > use directly.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Should I throw this into drm-misc, or do you want to merge this through
> some driver tree?

Yes, you can put them in drm-misc, it will the be the easiest option.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1695475 — Re: [PATCH v2 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

On Tue, Jul 25, 2017 at 08:57:34AM +0200, Maxime Ripard wrote:
> Hi Daniel,
> 
> On Thu, Jul 20, 2017 at 08:46:28PM +0200, Daniel Vetter wrote:
> > On Thu, Jul 20, 2017 at 03:01:16PM +0200, Maxime Ripard wrote:
> > > The current drm_atomic_helper_commit_tail helper works only if the CRTC is
> > > accessible, and documents an alternative implementation that is supposed to
> > > be used if that happens.
> > > 
> > > That implementation is then duplicated by some drivers. Instead of
> > > documenting it, let's implement an helper that all the relevant users can
> > > use directly.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > Should I throw this into drm-misc, or do you want to merge this through
> > some driver tree?
> 
> Yes, you can put them in drm-misc, it will the be the easiest option.

Merged, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1692920 — [PATCH v2 4/4] drm/sun4i: make sure we don't have a commit pending

In the earlier display engine designs, any register access while a commit
is pending is forbidden.

One of the symptoms is that reading a register will return another, random,
register value which can lead to register corruptions if we ever do a
read/modify/write cycle.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 14 ++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_crtc.c    |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index cf480218daa5..ce1f40f7511e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -67,6 +67,19 @@ static void sun4i_backend_commit(struct sunxi_engine *engine)
 		     SUN4I_BACKEND_REGBUFFCTL_LOADCTL);
 }
 
+static int sun4i_backend_commit_poll(struct sunxi_engine *engine)
+{
+	u32 val;
+
+	DRM_DEBUG_DRIVER("Polling for the commit to end\n");
+
+	return regmap_read_poll_timeout(engine->regs,
+					SUN4I_BACKEND_REGBUFFCTL_REG,
+					val,
+					!(val & SUN4I_BACKEND_REGBUFFCTL_LOADCTL),
+					100, 50000);
+}
+
 void sun4i_backend_layer_enable(struct sun4i_backend *backend,
 				int layer, bool enable)
 {
@@ -330,6 +343,7 @@ static int sun4i_backend_of_get_id(struct device_node *node)
 
 static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
 	.commit				= sun4i_backend_commit,
+	.commit_poll			= sun4i_backend_commit_poll,
 	.layers_init			= sun4i_layers_init,
 	.apply_color_correction		= sun4i_backend_apply_color_correction,
 	.disable_color_correction	= sun4i_backend_disable_color_correction,
diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c
index 2eba1d8639d8..31550493fa1d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -34,6 +34,7 @@ static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc,
 				    struct drm_crtc_state *old_state)
 {
 	struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
+	struct sunxi_engine *engine = scrtc->engine;
 	struct drm_device *dev = crtc->dev;
 	unsigned long flags;
 
-- 
git-series 0.9.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web