[PATCH 1/7] drm/arc: Stop consulting plane->fb

[PATCH 1/7] drm/arc: Stop consulting plane->fb

[Ville Syrjala]

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

We want to stop using plane->fb with atomic driver, so stop looking at
it.

I have no idea what this code is trying to achieve. There is no
corresponding check in the enable path. Also since
arc_pgu_set_pxl_fmt() will anyway oops if there is no fb I'm going
to assuming that I can just remove the check entirely. There seems
to be a general shortage of .atomic_check() in this driver...

Cc: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 16903dc7fe0d..c3349b8fb58b 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -136,9 +136,6 @@ static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
 {
 	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
 
-	if (!crtc->primary->fb)
-		return;
-
 	clk_disable_unprepare(arcpgu->clk);
 	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
 			      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) &
-- 
2.16.1

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

[PATCH 2/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()

[Ville Syrjala]

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

Instead of looking at plane->fb let's look at the proper new
plane state.

Not that the code makes a ton of sense. It's only going through the
crtcs in the atomic state, so assuming not all of them are included
we're not even calculating the total bandwidth here. Also we're
not considering whether each crtc is actually enabled or not.

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

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 6728c6247b4b..a2a796b4cc23 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1536,9 +1536,13 @@ vmw_kms_atomic_check_modeset(struct drm_device *dev,
 		unsigned long requested_bb_mem = 0;
 
 		if (dev_priv->active_display_unit == vmw_du_screen_target) {
-			if (crtc->primary->fb) {
-				int cpp = crtc->primary->fb->pitches[0] /
-					  crtc->primary->fb->width;
+			struct drm_plane *plane = crtc->primary;
+			struct drm_plane_state *plane_state;
+
+			plane_state = drm_atomic_get_new_plane_state(state, plane);
+
+			if (plane_state && plane_state->fb) {
+				int cpp = plane_state->fb->format->cpp[0];
 
 				requested_bb_mem += crtc->mode.hdisplay * cpp *
 						    crtc->mode.vdisplay;
-- 
2.16.1

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

[PATCH 3/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()

[Ville Syrjala]

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

Instead of plane->fb (which we're going to deprecate for atomic drivers)
we need to look at plane->state->fb. The maze of code leading to
vmw_kms_helper_dirty() wasn't particularly clear, but my analysis
concluded that the calls originating from vmw_*_primary_plane_atomic_update()
all pass in the crtc which means we'll never end up in this branch
of the function. All other callers use drm_modeset_lock_all() somewhere
higher up, which means accessing plane->state is safe. We'll toss in
a lockdep assert to catch wrongdoers.

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

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index a2a796b4cc23..5a824125c231 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2326,9 +2326,18 @@ int vmw_kms_helper_dirty(struct vmw_private *dev_priv,
 	} else {
 		list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list,
 				    head) {
-			if (crtc->primary->fb != &framebuffer->base)
-				continue;
-			units[num_units++] = vmw_crtc_to_du(crtc);
+			struct drm_plane *plane = crtc->primary;
+
+			/*
+			 * vmw_*_primary_plane_atomic_update() pass in the crtc,
+			 * and so don't end up here. All other callers use
+			 * drm_modeset_lock_all(), hence we can access the
+			 * plane state safely.
+			 */
+			lockdep_assert_held(&plane->mutex);
+
+			if (plane->state->fb != &framebuffer->base)
+				units[num_units++] = vmw_crtc_to_du(crtc);
 		}
 	}
 
-- 
2.16.1

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

[PATCH 4/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()

[Ville Syrjala]

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

The only caller of vmw_kms_update_implicit_fb() is the page_flip
hook which itself gets called with the plane mutex already held.
Hence we can look at plane->state safely. Toss in a lockdep assert
to make the situation more clear.

Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 5a824125c231..a93d290b0f35 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2811,14 +2811,17 @@ void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv,
 				struct drm_crtc *crtc)
 {
 	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
+	struct drm_plane *plane = crtc->primary;
 	struct vmw_framebuffer *vfb;
 
+	lockdep_assert_held(&plane->mutex);
+
 	mutex_lock(&dev_priv->global_kms_state_mutex);
 
 	if (!du->is_implicit)
 		goto out_unlock;
 
-	vfb = vmw_framebuffer_to_vfb(crtc->primary->fb);
+	vfb = vmw_framebuffer_to_vfb(plane->state->fb);
 	WARN_ON_ONCE(dev_priv->num_implicit != 1 &&
 		     dev_priv->implicit_fb != vfb);
 
-- 
2.16.1

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

[PATCH 5/7] drm/vmwgfx: Stop updating plane->fb

[Ville Syrjala]

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

We want to get rid of plane->fb on atomic drivers. Stop setting it.

Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 --
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 648f8127f65a..bbd3f19b1a0b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -525,8 +525,6 @@ vmw_sou_primary_plane_atomic_update(struct drm_plane *plane,
 		 */
 		if (ret != 0)
 			DRM_ERROR("Failed to update screen.\n");
-
-		crtc->primary->fb = plane->state->fb;
 	} else {
 		/*
 		 * When disabling a plane, CRTC and FB should always be NULL
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 67331f01ef32..90445bc590cb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1285,8 +1285,6 @@ vmw_stdu_primary_plane_atomic_update(struct drm_plane *plane,
 							 1, 1, NULL, crtc);
 		if (ret)
 			DRM_ERROR("Failed to update STDU.\n");
-
-		crtc->primary->fb = plane->state->fb;
 	} else {
 		crtc = old_state->crtc;
 		stdu = vmw_crtc_to_stdu(crtc);
-- 
2.16.1

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

[PATCH 6/7] drm/vmwgfx: Stop using plane->fb in atomic_enable()

[Ville Syrjala]

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

Instead of looking at the (soon to be deprecated) plane->fb we'll
examing plane->state->fb instead. We can do this because
vmw_du_crtc_atomic_check() prevents us from enabling a crtc
without the primary plane also being enabled.

Due to that same reason, I'm actually not sure what the checks here are
for NULL fb. If we can't enable the crtc without an enabled plane
we should always have an fb. But I'll leave that for someone else
to figure out.

Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 90445bc590cb..152e96cb1c01 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -414,6 +414,7 @@ static void vmw_stdu_crtc_helper_prepare(struct drm_crtc *crtc)
 static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
 					struct drm_crtc_state *old_state)
 {
+	struct drm_plane_state *plane_state = crtc->primary->state;
 	struct vmw_private *dev_priv;
 	struct vmw_screen_target_display_unit *stdu;
 	struct vmw_framebuffer *vfb;
@@ -422,7 +423,7 @@ static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	stdu     = vmw_crtc_to_stdu(crtc);
 	dev_priv = vmw_priv(crtc->dev);
-	fb       = crtc->primary->fb;
+	fb       = plane_state->fb;
 
 	vfb = (fb) ? vmw_framebuffer_to_vfb(fb) : NULL;
 
-- 
2.16.1

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

[PATCH 7/7] drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc

[Ville Syrjala]

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

plane->fb/old_fb/crtc should no longer be used by atomic
drivers. Stop messing about with them.

TODO: Squash with the core/helper patch?

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

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index 2582ffd36bb5..3c5935f3d49e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -439,8 +439,6 @@ static int vmw_fb_compute_depth(struct fb_var_screeninfo *var,
 static int vmwgfx_set_config_internal(struct drm_mode_set *set)
 {
 	struct drm_crtc *crtc = set->crtc;
-	struct drm_framebuffer *fb;
-	struct drm_crtc *tmp;
 	struct drm_modeset_acquire_ctx *ctx;
 	struct drm_device *dev = set->crtc->dev;
 	int ret;
@@ -448,29 +446,7 @@ static int vmwgfx_set_config_internal(struct drm_mode_set *set)
 	ctx = dev->mode_config.acquire_ctx;
 
 restart:
-	/*
-	 * NOTE: ->set_config can also disable other crtcs (if we steal all
-	 * connectors from it), hence we need to refcount the fbs across all
-	 * crtcs. Atomic modeset will have saner semantics ...
-	 */
-	drm_for_each_crtc(tmp, dev)
-		tmp->primary->old_fb = tmp->primary->fb;
-
-	fb = set->fb;
-
 	ret = crtc->funcs->set_config(set, ctx);
-	if (ret == 0) {
-		crtc->primary->crtc = crtc;
-		crtc->primary->fb = fb;
-	}
-
-	drm_for_each_crtc(tmp, dev) {
-		if (tmp->primary->fb)
-			drm_framebuffer_get(tmp->primary->fb);
-		if (tmp->primary->old_fb)
-			drm_framebuffer_put(tmp->primary->old_fb);
-		tmp->primary->old_fb = NULL;
-	}
 
 	if (ret == -EDEADLK) {
 		dev->mode_config.acquire_ctx = NULL;
-- 
2.16.1

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/arc: Stop consulting plane->fb

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/arc: Stop consulting plane->fb
URL   : https://patchwork.freedesktop.org/series/41230/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b94876325cd5 drm/arc: Stop consulting plane->fb
8c083114e1a4 drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()
cf25abb66c8d drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()
-:12: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#12: 
concluded that the calls originating from vmw_*_primary_plane_atomic_update()

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
90290f06a0cd drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()
69073dfedd60 drm/vmwgfx: Stop updating plane->fb
192117f681f1 drm/vmwgfx: Stop using plane->fb in atomic_enable()
ac7ba740a34d drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc

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

Re: [PATCH 1/7] drm/arc: Stop consulting plane->fb

[Daniel Vetter]

On Thu, Apr 05, 2018 at 10:50:29PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We want to stop using plane->fb with atomic driver, so stop looking at
> it.
> 
> I have no idea what this code is trying to achieve. There is no
> corresponding check in the enable path. Also since
> arc_pgu_set_pxl_fmt() will anyway oops if there is no fb I'm going
> to assuming that I can just remove the check entirely. There seems
> to be a general shortage of .atomic_check() in this driver...

I think arcpgu is the perfect example of a small driver that _really_
wants to use drm_simple_display_pipe_helper. Which would address the
outright lack of any and all atomic_check code (beyond basic mode
validation through mode_valid).

I also just noticed that it still has a bunch of the legacy hooks set,
e.g. mode_set, mode_set_base are all no longer used in atomic.

I think the code won't be able to oops, since if there's no fb, we don't
enable the plane (and it happily allows that), so should be all
non-oopsing. Even with this check here removed.

Ofc the hw might get pissed at us in this case, but I can't tell that.
Like I said, conversion to drm_simple_display_pipe_helper is probably the
way to go.

Anyway, this patch here looks good, if you adjust the commit message to
explain why it can't oops:

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

> 
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/arc/arcpgu_crtc.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index 16903dc7fe0d..c3349b8fb58b 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -136,9 +136,6 @@ static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
>  {
>  	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
>  
> -	if (!crtc->primary->fb)
> -		return;
> -
>  	clk_disable_unprepare(arcpgu->clk);
>  	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
>  			      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) &
> -- 
> 2.16.1
> 

-- 
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/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()

[Deepak Singh Rawat]

> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Instead of looking at plane->fb let's look at the proper new
> plane state.
> 
> Not that the code makes a ton of sense. It's only going through the
> crtcs in the atomic state, so assuming not all of them are included
> we're not even calculating the total bandwidth here. Also we're
> not considering whether each crtc is actually enabled or not.
> 
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 6728c6247b4b..a2a796b4cc23 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1536,9 +1536,13 @@ vmw_kms_atomic_check_modeset(struct
> drm_device *dev,
>  		unsigned long requested_bb_mem = 0;
> 
>  		if (dev_priv->active_display_unit ==
> vmw_du_screen_target) {
> -			if (crtc->primary->fb) {
> -				int cpp = crtc->primary->fb->pitches[0] /
> -					  crtc->primary->fb->width;
> +			struct drm_plane *plane = crtc->primary;
> +			struct drm_plane_state *plane_state;
> +
> +			plane_state =
> drm_atomic_get_new_plane_state(state, plane);
> +
> +			if (plane_state && plane_state->fb) {
> +				int cpp = plane_state->fb->format->cpp[0];

Hi Ville,

Thanks for the patch, recently I have done some refactoring of this code area
which is no yet sent to dri-devel. But the refactored code eliminated the need
to look the fb

https://cgit.freedesktop.org/mesa/vmwgfx/commit/?id=c54cdb6549b7d1c04ff61e639fc0e6de0dcc1ed6

There is still some future work to be done in this area. 

> 
>  				requested_bb_mem += crtc->mode.hdisplay
> * cpp *
>  						    crtc->mode.vdisplay;
> --
> 2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()

[Thomas Hellstrom]

On 04/05/2018 09:50 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Instead of plane->fb (which we're going to deprecate for atomic drivers)
> we need to look at plane->state->fb. The maze of code leading to
> vmw_kms_helper_dirty() wasn't particularly clear, but my analysis
> concluded that the calls originating from vmw_*_primary_plane_atomic_update()
> all pass in the crtc which means we'll never end up in this branch
> of the function. All other callers use drm_modeset_lock_all() somewhere
> higher up, which means accessing plane->state is safe. We'll toss in
> a lockdep assert to catch wrongdoers.
>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index a2a796b4cc23..5a824125c231 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2326,9 +2326,18 @@ int vmw_kms_helper_dirty(struct vmw_private *dev_priv,
>   	} else {
>   		list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list,
>   				    head) {
> -			if (crtc->primary->fb != &framebuffer->base)
> -				continue;
> -			units[num_units++] = vmw_crtc_to_du(crtc);
> +			struct drm_plane *plane = crtc->primary;
> +
> +			/*
> +			 * vmw_*_primary_plane_atomic_update() pass in the crtc,
> +			 * and so don't end up here. All other callers use
> +			 * drm_modeset_lock_all(), hence we can access the
> +			 * plane state safely.
> +			 */
> +			lockdep_assert_held(&plane->mutex);
> +
I think we can remove the comment (it's a helper, so current users may 
not be future users),
but the lockdep assert should be OK.
> +			if (plane->state->fb != &framebuffer->base)
> +				units[num_units++] = vmw_crtc_to_du(crtc);

This doesn't seem to do what the original code did...

>   		}
>   	}
>   

/Thomas


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

Re: [PATCH 1/7] drm/arc: Stop consulting plane->fb

[Ville Syrjälä]

On Thu, Apr 05, 2018 at 10:08:57PM +0200, Daniel Vetter wrote:
> On Thu, Apr 05, 2018 at 10:50:29PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We want to stop using plane->fb with atomic driver, so stop looking at
> > it.
> > 
> > I have no idea what this code is trying to achieve. There is no
> > corresponding check in the enable path. Also since
> > arc_pgu_set_pxl_fmt() will anyway oops if there is no fb I'm going
> > to assuming that I can just remove the check entirely. There seems
> > to be a general shortage of .atomic_check() in this driver...
> 
> I think arcpgu is the perfect example of a small driver that _really_
> wants to use drm_simple_display_pipe_helper. Which would address the
> outright lack of any and all atomic_check code (beyond basic mode
> validation through mode_valid).
> 
> I also just noticed that it still has a bunch of the legacy hooks set,
> e.g. mode_set, mode_set_base are all no longer used in atomic.
> 
> I think the code won't be able to oops, since if there's no fb, we don't
> enable the plane (and it happily allows that), so should be all
> non-oopsing. Even with this check here removed.

arc_pgu_set_pxl_fmt() gets called from the .mode_set_nofb() hook
which I assume doesn't care about planes at all. So to me it looks like
it'll definitely oops.

> 
> Ofc the hw might get pissed at us in this case, but I can't tell that.
> Like I said, conversion to drm_simple_display_pipe_helper is probably the
> way to go.
> 
> Anyway, this patch here looks good, if you adjust the commit message to
> explain why it can't oops:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > 
> > Cc: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/arc/arcpgu_crtc.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > index 16903dc7fe0d..c3349b8fb58b 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > @@ -136,9 +136,6 @@ static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
> >  {
> >  	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> >  
> > -	if (!crtc->primary->fb)
> > -		return;
> > -
> >  	clk_disable_unprepare(arcpgu->clk);
> >  	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
> >  			      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) &
> > -- 
> > 2.16.1
> > 
> 
> -- 
> 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

✓ Fi.CI.BAT: success for series starting with [1/7] drm/arc: Stop consulting plane->fb

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/arc: Stop consulting plane->fb
URL   : https://patchwork.freedesktop.org/series/41230/
State : success

== Summary ==

Series 41230v1 series starting with [1/7] drm/arc: Stop consulting plane->fb
https://patchwork.freedesktop.org/api/1.0/series/41230/revisions/1/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-cnl-y3) fdo#104951
Test prime_vgem:
        Subgroup basic-fence-flip:
                fail       -> PASS       (fi-ilk-650) fdo#104008

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:432s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:441s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:380s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:540s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:515s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:515s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:526s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:509s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:410s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:559s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:509s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:589s
fi-elk-e7500     total:285  pass:226  dwarn:0   dfail:0   fail:0   skip:59  time:424s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:313s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:488s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:404s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:423s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:474s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:430s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:471s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:464s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:508s
fi-pnv-d510      total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:663s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:442s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:534s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:499s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:504s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:432s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:448s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:580s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:408s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:256  dwarn:3   dfail:0   fail:0   skip:26  time:529s

fcaf73c13c14d6bfd64c4f37089bf5437fb32221 drm-tip: 2018y-04m-05d-15h-53m-18s UTC integration manifest
ac7ba740a34d drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc
192117f681f1 drm/vmwgfx: Stop using plane->fb in atomic_enable()
69073dfedd60 drm/vmwgfx: Stop updating plane->fb
90290f06a0cd drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()
cf25abb66c8d drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()
8c083114e1a4 drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()
b94876325cd5 drm/arc: Stop consulting plane->fb

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8605/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()

[Ville Syrjälä]

On Thu, Apr 05, 2018 at 10:15:57PM +0200, Thomas Hellstrom wrote:
> On 04/05/2018 09:50 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Instead of plane->fb (which we're going to deprecate for atomic drivers)
> > we need to look at plane->state->fb. The maze of code leading to
> > vmw_kms_helper_dirty() wasn't particularly clear, but my analysis
> > concluded that the calls originating from vmw_*_primary_plane_atomic_update()
> > all pass in the crtc which means we'll never end up in this branch
> > of the function. All other callers use drm_modeset_lock_all() somewhere
> > higher up, which means accessing plane->state is safe. We'll toss in
> > a lockdep assert to catch wrongdoers.
> >
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Cc: Sinclair Yeh <syeh@vmware.com>
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 15 ++++++++++++---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index a2a796b4cc23..5a824125c231 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -2326,9 +2326,18 @@ int vmw_kms_helper_dirty(struct vmw_private *dev_priv,
> >   	} else {
> >   		list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list,
> >   				    head) {
> > -			if (crtc->primary->fb != &framebuffer->base)
> > -				continue;
> > -			units[num_units++] = vmw_crtc_to_du(crtc);
> > +			struct drm_plane *plane = crtc->primary;
> > +
> > +			/*
> > +			 * vmw_*_primary_plane_atomic_update() pass in the crtc,
> > +			 * and so don't end up here. All other callers use
> > +			 * drm_modeset_lock_all(), hence we can access the
> > +			 * plane state safely.
> > +			 */
> > +			lockdep_assert_held(&plane->mutex);
> > +
> I think we can remove the comment (it's a helper, so current users may 
> not be future users),
> but the lockdep assert should be OK.

OK.

> > +			if (plane->state->fb != &framebuffer->base)
> > +				units[num_units++] = vmw_crtc_to_du(crtc);
> 
> This doesn't seem to do what the original code did...

Whoops. Good catch.

> 
> >   		}
> >   	}
> >   
> 
> /Thomas
> 

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

Re: [PATCH 2/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()

[Ville Syrjälä]

On Thu, Apr 05, 2018 at 08:15:05PM +0000, Deepak Singh Rawat wrote:
> 
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Instead of looking at plane->fb let's look at the proper new
> > plane state.
> > 
> > Not that the code makes a ton of sense. It's only going through the
> > crtcs in the atomic state, so assuming not all of them are included
> > we're not even calculating the total bandwidth here. Also we're
> > not considering whether each crtc is actually enabled or not.
> > 
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Cc: Sinclair Yeh <syeh@vmware.com>
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 6728c6247b4b..a2a796b4cc23 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -1536,9 +1536,13 @@ vmw_kms_atomic_check_modeset(struct
> > drm_device *dev,
> >  		unsigned long requested_bb_mem = 0;
> > 
> >  		if (dev_priv->active_display_unit ==
> > vmw_du_screen_target) {
> > -			if (crtc->primary->fb) {
> > -				int cpp = crtc->primary->fb->pitches[0] /
> > -					  crtc->primary->fb->width;
> > +			struct drm_plane *plane = crtc->primary;
> > +			struct drm_plane_state *plane_state;
> > +
> > +			plane_state =
> > drm_atomic_get_new_plane_state(state, plane);
> > +
> > +			if (plane_state && plane_state->fb) {
> > +				int cpp = plane_state->fb->format->cpp[0];
> 
> Hi Ville,
> 
> Thanks for the patch, recently I have done some refactoring of this code area
> which is no yet sent to dri-devel. But the refactored code eliminated the need
> to look the fb
> 
> https://cgit.freedesktop.org/mesa/vmwgfx/commit/?id=c54cdb6549b7d1c04ff61e639fc0e6de0dcc1ed6

Hmm. What's the timelike for landing that stuff?

A cursory glance tells me we should just change the current code with
s/cpp/4/ and it should still be fine?

BTW the drm_for_each_crtc(crtc, dev) loop in there doesn't look entirely
kosher. It's potentially going to access crtc->state w/o holding the lock.

> 
> There is still some future work to be done in this area. 
> 
> > 
> >  				requested_bb_mem += crtc->mode.hdisplay
> > * cpp *
> >  						    crtc->mode.vdisplay;
> > --
> > 2.16.1

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

[PATCH v2 3/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()

[Ville Syrjala]

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

Instead of plane->fb (which we're going to deprecate for atomic drivers)
we need to look at plane->state->fb. The maze of code leading to
vmw_kms_helper_dirty() wasn't particularly clear, but my analysis
concluded that the calls originating from vmw_*_primary_plane_atomic_update()
all pass in the crtc which means we'll never end up in this branch
of the function. All other callers use drm_modeset_lock_all() somewhere
higher up, which means accessing plane->state is safe. We'll toss in
a lockdep assert to catch wrongdoers.

v2: Drop the comment and make the code do what it did before (Thomas)

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

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index a2a796b4cc23..73484ad1ceeb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2326,9 +2326,12 @@ int vmw_kms_helper_dirty(struct vmw_private *dev_priv,
 	} else {
 		list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list,
 				    head) {
-			if (crtc->primary->fb != &framebuffer->base)
-				continue;
-			units[num_units++] = vmw_crtc_to_du(crtc);
+			struct drm_plane *plane = crtc->primary;
+
+			lockdep_assert_held(&plane->mutex);
+
+			if (plane->state->fb == &framebuffer->base)
+				units[num_units++] = vmw_crtc_to_du(crtc);
 		}
 	}
 
-- 
2.16.1

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

RE: [PATCH 2/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()

[Deepak Singh Rawat]

> 
> On Thu, Apr 05, 2018 at 08:15:05PM +0000, Deepak Singh Rawat wrote:
> >
> > >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Instead of looking at plane->fb let's look at the proper new
> > > plane state.
> > >
> > > Not that the code makes a ton of sense. It's only going through the
> > > crtcs in the atomic state, so assuming not all of them are included
> > > we're not even calculating the total bandwidth here. Also we're
> > > not considering whether each crtc is actually enabled or not.
> > >
> > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > index 6728c6247b4b..a2a796b4cc23 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > @@ -1536,9 +1536,13 @@ vmw_kms_atomic_check_modeset(struct
> > > drm_device *dev,
> > >  		unsigned long requested_bb_mem = 0;
> > >
> > >  		if (dev_priv->active_display_unit ==
> > > vmw_du_screen_target) {
> > > -			if (crtc->primary->fb) {
> > > -				int cpp = crtc->primary->fb->pitches[0] /
> > > -					  crtc->primary->fb->width;
> > > +			struct drm_plane *plane = crtc->primary;
> > > +			struct drm_plane_state *plane_state;
> > > +
> > > +			plane_state =
> > > drm_atomic_get_new_plane_state(state, plane);
> > > +
> > > +			if (plane_state && plane_state->fb) {
> > > +				int cpp = plane_state->fb->format->cpp[0];
> >
> > Hi Ville,
> >
> > Thanks for the patch, recently I have done some refactoring of this code
> area
> > which is no yet sent to dri-devel. But the refactored code eliminated the
> need
> > to look the fb
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__cgit.freedesktop.org_mesa_vmwgfx_commit_-3Fid-
> 3Dc54cdb6549b7d1c04ff61e639fc0e6de0dcc1ed6&d=DwIDAw&c=uilaK90D4T
> OVoH58JNXRgQ&r=zOOG28inJK0762SxAf-
> cyZdStnd2NQpRu98lJP2iYGw&m=lrHQqnjNpBdFNzU0f4b3rLTtaYp0VRzCgZztJ
> ew0kz0&s=mz1Kt2NO_HIpgVtQ9xHvKRQLGXx2HSqY8xt0oiEpGWg&e=
> 
> Hmm. What's the timelike for landing that stuff?

I am not sure, I still think there is more work here to clean this up. I guess for now
we can have your patch to take care of avoiding plane->fb.

> 
> A cursory glance tells me we should just change the current code with
> s/cpp/4/ and it should still be fine?

Yes replacing cpp with 4 is fine as that is what virtual hardware always look for.

> 
> BTW the drm_for_each_crtc(crtc, dev) loop in there doesn't look entirely
> kosher. It's potentially going to access crtc->state w/o holding the lock.

Thanks for the suggestion, I will look into this.

> 
> >
> > There is still some future work to be done in this area.
> >
> > >
> > >  				requested_bb_mem += crtc->mode.hdisplay
> > > * cpp *
> > >  						    crtc->mode.vdisplay;
> > > --
> > > 2.16.1
> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/7] drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc

[Daniel Vetter]

On Thu, Apr 05, 2018 at 10:50:35PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> plane->fb/old_fb/crtc should no longer be used by atomic
> drivers. Stop messing about with them.
> 
> TODO: Squash with the core/helper patch?

Not possible, because the core setconfig one does no longer work with
atomic drivers ever since I've thrown away the magic backoff. Since then
we have the rule that for any atomic driver, you need a real atomic commit
(including passing the acquire_ctx down to that very call to
drm_atomic_commit).

See the patch that introduced this copypasta for full details plus how to
fix it up properly:

commit 3bacf4361cd07f988a13de78d672928606df24ad
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Apr 6 22:02:56 2017 +0200

    drm/vmwgfx: Fix fbdev emulation using legacy functions
    
    I've broken this by removing the backoff handling from the
    set_config2atomic helper in
    
    commit 38b6441e4e75c0b319cfe4d9364c1059fc1e3c2b
    Author: Daniel Vetter <daniel.vetter@ffwll.ch>
    Date:   Wed Mar 22 22:50:58 2017 +0100
    
        drm/atomic-helper: Remove the backoff hack from set_config
    
    Fixing this properly would mean we get to wire the acquire_ctx all the
    way through vmwgfx fbdev code, and doing the same was tricky for the
    shared fbdev layer. Probably much better to look into refactoring the
    entire code to use the helpers, but since that's not a viable
    long-term solution fix the issue by open-coding a vmwgfx version of
    set_config, that does the legacy backoff dance internally.
    
    Note: Just compile-tested. The idea is to take
    drm_mode_set_config_internal(), remove the "is this a legacy driver"
    check, and whack the drm_atomic_legacy_backoff trickery at the end.
    Since drm_atomic_legacy_backoff is for atomic commits only we need to
    open-code it.
    
    Cc: Thomas Hellstrom <thellstrom@vmware.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
    Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
    Signed-off-by: Sean Paul <seanpaul@chromium.org>
    Link: http://patchwork.freedesktop.org/patch/msgid/20170406200256.26040-1-daniel.vetter@ffwll.ch

I scrolled through your vmwgfx patches, and will try to look at them maybe
next week. Too complicated for this late, and tomorrow I'm ooo.
-Daniel

> 
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 24 ------------------------
>  1 file changed, 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> index 2582ffd36bb5..3c5935f3d49e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> @@ -439,8 +439,6 @@ static int vmw_fb_compute_depth(struct fb_var_screeninfo *var,
>  static int vmwgfx_set_config_internal(struct drm_mode_set *set)
>  {
>  	struct drm_crtc *crtc = set->crtc;
> -	struct drm_framebuffer *fb;
> -	struct drm_crtc *tmp;
>  	struct drm_modeset_acquire_ctx *ctx;
>  	struct drm_device *dev = set->crtc->dev;
>  	int ret;
> @@ -448,29 +446,7 @@ static int vmwgfx_set_config_internal(struct drm_mode_set *set)
>  	ctx = dev->mode_config.acquire_ctx;
>  
>  restart:
> -	/*
> -	 * NOTE: ->set_config can also disable other crtcs (if we steal all
> -	 * connectors from it), hence we need to refcount the fbs across all
> -	 * crtcs. Atomic modeset will have saner semantics ...
> -	 */
> -	drm_for_each_crtc(tmp, dev)
> -		tmp->primary->old_fb = tmp->primary->fb;
> -
> -	fb = set->fb;
> -
>  	ret = crtc->funcs->set_config(set, ctx);
> -	if (ret == 0) {
> -		crtc->primary->crtc = crtc;
> -		crtc->primary->fb = fb;
> -	}
> -
> -	drm_for_each_crtc(tmp, dev) {
> -		if (tmp->primary->fb)
> -			drm_framebuffer_get(tmp->primary->fb);
> -		if (tmp->primary->old_fb)
> -			drm_framebuffer_put(tmp->primary->old_fb);
> -		tmp->primary->old_fb = NULL;
> -	}
>  
>  	if (ret == -EDEADLK) {
>  		dev->mode_config.acquire_ctx = NULL;
> -- 
> 2.16.1
> 

-- 
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/7] drm/arc: Stop consulting plane->fb

[Daniel Vetter]

On Thu, Apr 05, 2018 at 11:19:44PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 05, 2018 at 10:08:57PM +0200, Daniel Vetter wrote:
> > On Thu, Apr 05, 2018 at 10:50:29PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We want to stop using plane->fb with atomic driver, so stop looking at
> > > it.
> > > 
> > > I have no idea what this code is trying to achieve. There is no
> > > corresponding check in the enable path. Also since
> > > arc_pgu_set_pxl_fmt() will anyway oops if there is no fb I'm going
> > > to assuming that I can just remove the check entirely. There seems
> > > to be a general shortage of .atomic_check() in this driver...
> > 
> > I think arcpgu is the perfect example of a small driver that _really_
> > wants to use drm_simple_display_pipe_helper. Which would address the
> > outright lack of any and all atomic_check code (beyond basic mode
> > validation through mode_valid).
> > 
> > I also just noticed that it still has a bunch of the legacy hooks set,
> > e.g. mode_set, mode_set_base are all no longer used in atomic.
> > 
> > I think the code won't be able to oops, since if there's no fb, we don't
> > enable the plane (and it happily allows that), so should be all
> > non-oopsing. Even with this check here removed.
> 
> arc_pgu_set_pxl_fmt() gets called from the .mode_set_nofb() hook
> which I assume doesn't care about planes at all. So to me it looks like
> it'll definitely oops.

Indeed, I was blind. Commit message looks good.

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

> 
> > 
> > Ofc the hw might get pissed at us in this case, but I can't tell that.
> > Like I said, conversion to drm_simple_display_pipe_helper is probably the
> > way to go.
> > 
> > Anyway, this patch here looks good, if you adjust the commit message to
> > explain why it can't oops:
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > > 
> > > Cc: Alexey Brodkin <abrodkin@synopsys.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/arc/arcpgu_crtc.c | 3 ---
> > >  1 file changed, 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > > index 16903dc7fe0d..c3349b8fb58b 100644
> > > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> > > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > > @@ -136,9 +136,6 @@ static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
> > >  {
> > >  	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> > >  
> > > -	if (!crtc->primary->fb)
> > > -		return;
> > > -
> > >  	clk_disable_unprepare(arcpgu->clk);
> > >  	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
> > >  			      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) &
> > > -- 
> > > 2.16.1
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> 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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/arc: Stop consulting plane->fb (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/arc: Stop consulting plane->fb (rev2)
URL   : https://patchwork.freedesktop.org/series/41230/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
72b80067880c drm/arc: Stop consulting plane->fb
-:22: WARNING:BAD_SIGN_OFF: Duplicate signature
#22: 
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

total: 0 errors, 1 warnings, 0 checks, 9 lines checked
488cc436263f drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()
6cdbeabec8d8 drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()
-:12: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#12: 
concluded that the calls originating from vmw_*_primary_plane_atomic_update()

total: 0 errors, 1 warnings, 0 checks, 15 lines checked
416b0dd8d5bc drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()
91ef12040af0 drm/vmwgfx: Stop updating plane->fb
fbcdb617ee01 drm/vmwgfx: Stop using plane->fb in atomic_enable()
d1d669671e9b drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc

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

✓ Fi.CI.BAT: success for series starting with [1/7] drm/arc: Stop consulting plane->fb (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/arc: Stop consulting plane->fb (rev2)
URL   : https://patchwork.freedesktop.org/series/41230/
State : success

== Summary ==

Series 41230v2 series starting with [1/7] drm/arc: Stop consulting plane->fb
https://patchwork.freedesktop.org/api/1.0/series/41230/revisions/2/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-cnl-y3) fdo#104951
Test prime_vgem:
        Subgroup basic-fence-flip:
                fail       -> PASS       (fi-ilk-650) fdo#104008

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:430s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:443s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:381s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:539s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:520s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:511s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:525s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:509s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:561s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:511s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:580s
fi-elk-e7500     total:285  pass:226  dwarn:0   dfail:0   fail:0   skip:59  time:422s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:312s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:485s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:407s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:472s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:431s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:471s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:465s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-pnv-d510      total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:678s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:439s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:532s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:492s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:432s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:445s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:564s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:398s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:256  dwarn:3   dfail:0   fail:0   skip:26  time:531s

fcaf73c13c14d6bfd64c4f37089bf5437fb32221 drm-tip: 2018y-04m-05d-15h-53m-18s UTC integration manifest
d1d669671e9b drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc
fbcdb617ee01 drm/vmwgfx: Stop using plane->fb in atomic_enable()
91ef12040af0 drm/vmwgfx: Stop updating plane->fb
416b0dd8d5bc drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()
6cdbeabec8d8 drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()
488cc436263f drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()
72b80067880c drm/arc: Stop consulting plane->fb

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8606/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/7] drm/arc: Stop consulting plane->fb

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/arc: Stop consulting plane->fb
URL   : https://patchwork.freedesktop.org/series/41230/
State : success

== Summary ==

---- Possible new issues:

Test kms_frontbuffer_tracking:
        Subgroup fbcpsr-1p-primscrn-shrfb-msflip-blt:
                fail       -> SKIP       (shard-snb)

---- Known issues:

Test kms_flip:
        Subgroup 2x-flip-vs-expired-vblank:
                fail       -> PASS       (shard-hsw) fdo#102887
        Subgroup dpms-vs-vblank-race-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#103060
        Subgroup plain-flip-ts-check:
                fail       -> PASS       (shard-hsw) fdo#100368
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-apl) fdo#99912
Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252 +1

fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-apl        total:2680 pass:1835 dwarn:1   dfail:0   fail:7   skip:836 time:12708s
shard-hsw        total:2680 pass:1784 dwarn:1   dfail:0   fail:3   skip:891 time:11431s
shard-snb        total:2680 pass:1376 dwarn:1   dfail:0   fail:4   skip:1299 time:6948s
Blacklisted hosts:
shard-kbl        total:1944 pass:1407 dwarn:12  dfail:0   fail:5   skip:519 time:6332s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8605/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for series starting with [1/7] drm/arc: Stop consulting plane->fb (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/arc: Stop consulting plane->fb (rev2)
URL   : https://patchwork.freedesktop.org/series/41230/
State : warning

== Summary ==

---- Possible new issues:

Test kms_flip:
        Subgroup 2x-flip-vs-modeset:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_frontbuffer_tracking:
        Subgroup fbcpsr-1p-primscrn-shrfb-msflip-blt:
                fail       -> SKIP       (shard-snb)

---- Known issues:

Test kms_flip:
        Subgroup 2x-flip-vs-expired-vblank:
                fail       -> PASS       (shard-hsw) fdo#102887 +1
        Subgroup 2x-plain-flip-ts-check:
                pass       -> FAIL       (shard-hsw) fdo#100368 +1
        Subgroup modeset-vs-vblank-race:
                pass       -> FAIL       (shard-hsw) fdo#103060
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-apl) fdo#99912
Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252

fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-apl        total:2680 pass:1835 dwarn:1   dfail:0   fail:7   skip:836 time:12652s
shard-hsw        total:2680 pass:1783 dwarn:2   dfail:0   fail:3   skip:891 time:11339s
shard-snb        total:2680 pass:1375 dwarn:1   dfail:0   fail:5   skip:1299 time:6880s
Blacklisted hosts:
shard-kbl        total:1945 pass:1424 dwarn:1   dfail:0   fail:3   skip:516 time:6732s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8606/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.IGT: warning for series starting with [1/7] drm/arc: Stop consulting plane->fb (rev2)

[Saarinen, Jani]

HI,

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Patchwork
> Sent: perjantai 6. huhtikuuta 2018 1.51
> To: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] ✗ Fi.CI.IGT: warning for series starting with [1/7] drm/arc:
> Stop consulting plane->fb (rev2)
> 
> == Series Details ==
> 
> Series: series starting with [1/7] drm/arc: Stop consulting plane->fb (rev2)
> URL   : https://patchwork.freedesktop.org/series/41230/
> State : warning
> 
> == Summary ==
> 
> ---- Possible new issues:
> 
> Test kms_flip:
>         Subgroup 2x-flip-vs-modeset:
>                 pass       -> DMESG-WARN (shard-hsw)
> Test kms_frontbuffer_tracking:
>         Subgroup fbcpsr-1p-primscrn-shrfb-msflip-blt:
>                 fail       -> SKIP       (shard-snb)
> 
> ---- Known issues:
> 
> Test kms_flip:
>         Subgroup 2x-flip-vs-expired-vblank:
>                 fail       -> PASS       (shard-hsw) fdo#102887 +1
>         Subgroup 2x-plain-flip-ts-check:
>                 pass       -> FAIL       (shard-hsw) fdo#100368 +1
>         Subgroup modeset-vs-vblank-race:
>                 pass       -> FAIL       (shard-hsw) fdo#103060
> Test kms_setmode:
>         Subgroup basic:
>                 pass       -> FAIL       (shard-apl) fdo#99912
> Test perf:
>         Subgroup blocking:
>                 fail       -> PASS       (shard-hsw) fdo#102252
> 
> fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
> fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
> fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
> fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
> fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
> 
> shard-apl        total:2680 pass:1835 dwarn:1   dfail:0   fail:7   skip:836
> time:12652s
> shard-hsw        total:2680 pass:1783 dwarn:2   dfail:0   fail:3   skip:891
> time:11339s
> shard-snb        total:2680 pass:1375 dwarn:1   dfail:0   fail:5   skip:1299
> time:6880s
> Blacklisted hosts:
> shard-kbl        total:1945 pass:1424 dwarn:1   dfail:0   fail:3   skip:516
> time:6732s
For some reason not totally success run for kbl. 
Execution stops on   0.00 igt@pm_rpm@system-suspend-execbuf incomplete 
Tomi, Ville, any idea? 

> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-
> tip/Patchwork_8606/shards.html
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/arc: Stop consulting plane->fb

[Alexey Brodkin]

Hi Ville,

On Thu, 2018-04-05 at 22:50 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We want to stop using plane->fb with atomic driver, so stop looking at
> it.
> 
> I have no idea what this code is trying to achieve. There is no
> corresponding check in the enable path. Also since
> arc_pgu_set_pxl_fmt() will anyway oops if there is no fb I'm going
> to assuming that I can just remove the check entirely. There seems
> to be a general shortage of .atomic_check() in this driver...
> 
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Acked-by: Alexey Brodkin <abrodkin@synopys.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9105 bytes --]

Hi Ville,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on next-20180405]
[cannot apply to v4.16]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-arc-Stop-consulting-plane-fb/20180406-155056
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: x86_64-randconfig-x010-201813 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from arch/x86/include/asm/bug.h:83:0,
                    from include/linux/bug.h:5,
                    from include/linux/debug_locks.h:7,
                    from include/linux/lockdep.h:28,
                    from include/linux/spinlock_types.h:18,
                    from include/linux/mutex.h:16,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/cdev.h:5,
                    from include/drm/drmP.h:36,
                    from drivers/gpu/drm/vmwgfx/vmwgfx_kms.h:31,
                    from drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:28:
   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c: In function 'vmw_kms_helper_dirty':
>> include/linux/lockdep.h:347:52: error: 'struct drm_modeset_lock' has no member named 'dep_map'
    #define lockdep_is_held(lock)  lock_is_held(&(lock)->dep_map)
                                                       ^
   include/asm-generic/bug.h:112:25: note: in definition of macro 'WARN_ON'
     int __ret_warn_on = !!(condition);    \
                            ^~~~~~~~~
>> include/linux/lockdep.h:373:27: note: in expansion of macro 'lockdep_is_held'
      WARN_ON(debug_locks && !lockdep_is_held(l)); \
                              ^~~~~~~~~~~~~~~
>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:2337:4: note: in expansion of macro 'lockdep_assert_held'
       lockdep_assert_held(&plane->mutex);
       ^~~~~~~~~~~~~~~~~~~
--
   In file included from arch/x86/include/asm/bug.h:83:0,
                    from include/linux/bug.h:5,
                    from include/linux/debug_locks.h:7,
                    from include/linux/lockdep.h:28,
                    from include/linux/spinlock_types.h:18,
                    from include/linux/mutex.h:16,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/cdev.h:5,
                    from include/drm/drmP.h:36,
                    from drivers/gpu//drm/vmwgfx/vmwgfx_kms.h:31,
                    from drivers/gpu//drm/vmwgfx/vmwgfx_kms.c:28:
   drivers/gpu//drm/vmwgfx/vmwgfx_kms.c: In function 'vmw_kms_helper_dirty':
>> include/linux/lockdep.h:347:52: error: 'struct drm_modeset_lock' has no member named 'dep_map'
    #define lockdep_is_held(lock)  lock_is_held(&(lock)->dep_map)
                                                       ^
   include/asm-generic/bug.h:112:25: note: in definition of macro 'WARN_ON'
     int __ret_warn_on = !!(condition);    \
                            ^~~~~~~~~
>> include/linux/lockdep.h:373:27: note: in expansion of macro 'lockdep_is_held'
      WARN_ON(debug_locks && !lockdep_is_held(l)); \
                              ^~~~~~~~~~~~~~~
   drivers/gpu//drm/vmwgfx/vmwgfx_kms.c:2337:4: note: in expansion of macro 'lockdep_assert_held'
       lockdep_assert_held(&plane->mutex);
       ^~~~~~~~~~~~~~~~~~~

vim +/lockdep_assert_held +2337 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c

  2289	
  2290	/**
  2291	 * vmw_kms_helper_dirty - Helper to build commands and perform actions based
  2292	 * on a set of cliprects and a set of display units.
  2293	 *
  2294	 * @dev_priv: Pointer to a device private structure.
  2295	 * @framebuffer: Pointer to the framebuffer on which to perform the actions.
  2296	 * @clips: A set of struct drm_clip_rect. Either this os @vclips must be NULL.
  2297	 * Cliprects are given in framebuffer coordinates.
  2298	 * @vclips: A set of struct drm_vmw_rect cliprects. Either this or @clips must
  2299	 * be NULL. Cliprects are given in source coordinates.
  2300	 * @dest_x: X coordinate offset for the crtc / destination clip rects.
  2301	 * @dest_y: Y coordinate offset for the crtc / destination clip rects.
  2302	 * @num_clips: Number of cliprects in the @clips or @vclips array.
  2303	 * @increment: Integer with which to increment the clip counter when looping.
  2304	 * Used to skip a predetermined number of clip rects.
  2305	 * @dirty: Closure structure. See the description of struct vmw_kms_dirty.
  2306	 */
  2307	int vmw_kms_helper_dirty(struct vmw_private *dev_priv,
  2308				 struct vmw_framebuffer *framebuffer,
  2309				 const struct drm_clip_rect *clips,
  2310				 const struct drm_vmw_rect *vclips,
  2311				 s32 dest_x, s32 dest_y,
  2312				 int num_clips,
  2313				 int increment,
  2314				 struct vmw_kms_dirty *dirty)
  2315	{
  2316		struct vmw_display_unit *units[VMWGFX_NUM_DISPLAY_UNITS];
  2317		struct drm_crtc *crtc;
  2318		u32 num_units = 0;
  2319		u32 i, k;
  2320	
  2321		dirty->dev_priv = dev_priv;
  2322	
  2323		/* If crtc is passed, no need to iterate over other display units */
  2324		if (dirty->crtc) {
  2325			units[num_units++] = vmw_crtc_to_du(dirty->crtc);
  2326		} else {
  2327			list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list,
  2328					    head) {
  2329				struct drm_plane *plane = crtc->primary;
  2330	
  2331				/*
  2332				 * vmw_*_primary_plane_atomic_update() pass in the crtc,
  2333				 * and so don't end up here. All other callers use
  2334				 * drm_modeset_lock_all(), hence we can access the
  2335				 * plane state safely.
  2336				 */
> 2337				lockdep_assert_held(&plane->mutex);
  2338	
  2339				if (plane->state->fb != &framebuffer->base)
  2340					units[num_units++] = vmw_crtc_to_du(crtc);
  2341			}
  2342		}
  2343	
  2344		for (k = 0; k < num_units; k++) {
  2345			struct vmw_display_unit *unit = units[k];
  2346			s32 crtc_x = unit->crtc.x;
  2347			s32 crtc_y = unit->crtc.y;
  2348			s32 crtc_width = unit->crtc.mode.hdisplay;
  2349			s32 crtc_height = unit->crtc.mode.vdisplay;
  2350			const struct drm_clip_rect *clips_ptr = clips;
  2351			const struct drm_vmw_rect *vclips_ptr = vclips;
  2352	
  2353			dirty->unit = unit;
  2354			if (dirty->fifo_reserve_size > 0) {
  2355				dirty->cmd = vmw_fifo_reserve(dev_priv,
  2356							      dirty->fifo_reserve_size);
  2357				if (!dirty->cmd) {
  2358					DRM_ERROR("Couldn't reserve fifo space "
  2359						  "for dirty blits.\n");
  2360					return -ENOMEM;
  2361				}
  2362				memset(dirty->cmd, 0, dirty->fifo_reserve_size);
  2363			}
  2364			dirty->num_hits = 0;
  2365			for (i = 0; i < num_clips; i++, clips_ptr += increment,
  2366			       vclips_ptr += increment) {
  2367				s32 clip_left;
  2368				s32 clip_top;
  2369	
  2370				/*
  2371				 * Select clip array type. Note that integer type
  2372				 * in @clips is unsigned short, whereas in @vclips
  2373				 * it's 32-bit.
  2374				 */
  2375				if (clips) {
  2376					dirty->fb_x = (s32) clips_ptr->x1;
  2377					dirty->fb_y = (s32) clips_ptr->y1;
  2378					dirty->unit_x2 = (s32) clips_ptr->x2 + dest_x -
  2379						crtc_x;
  2380					dirty->unit_y2 = (s32) clips_ptr->y2 + dest_y -
  2381						crtc_y;
  2382				} else {
  2383					dirty->fb_x = vclips_ptr->x;
  2384					dirty->fb_y = vclips_ptr->y;
  2385					dirty->unit_x2 = dirty->fb_x + vclips_ptr->w +
  2386						dest_x - crtc_x;
  2387					dirty->unit_y2 = dirty->fb_y + vclips_ptr->h +
  2388						dest_y - crtc_y;
  2389				}
  2390	
  2391				dirty->unit_x1 = dirty->fb_x + dest_x - crtc_x;
  2392				dirty->unit_y1 = dirty->fb_y + dest_y - crtc_y;
  2393	
  2394				/* Skip this clip if it's outside the crtc region */
  2395				if (dirty->unit_x1 >= crtc_width ||
  2396				    dirty->unit_y1 >= crtc_height ||
  2397				    dirty->unit_x2 <= 0 || dirty->unit_y2 <= 0)
  2398					continue;
  2399	
  2400				/* Clip right and bottom to crtc limits */
  2401				dirty->unit_x2 = min_t(s32, dirty->unit_x2,
  2402						       crtc_width);
  2403				dirty->unit_y2 = min_t(s32, dirty->unit_y2,
  2404						       crtc_height);
  2405	
  2406				/* Clip left and top to crtc limits */
  2407				clip_left = min_t(s32, dirty->unit_x1, 0);
  2408				clip_top = min_t(s32, dirty->unit_y1, 0);
  2409				dirty->unit_x1 -= clip_left;
  2410				dirty->unit_y1 -= clip_top;
  2411				dirty->fb_x -= clip_left;
  2412				dirty->fb_y -= clip_top;
  2413	
  2414				dirty->clip(dirty);
  2415			}
  2416	
  2417			dirty->fifo_commit(dirty);
  2418		}
  2419	
  2420		return 0;
  2421	}
  2422	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32835 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 4/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()

[Deepak Singh Rawat]

Reviewed-by: Deepak Rawat <drawat@vmware.com>

> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The only caller of vmw_kms_update_implicit_fb() is the page_flip
> hook which itself gets called with the plane mutex already held.
> Hence we can look at plane->state safely. Toss in a lockdep assert
> to make the situation more clear.
> 
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 5a824125c231..a93d290b0f35 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2811,14 +2811,17 @@ void vmw_kms_update_implicit_fb(struct
> vmw_private *dev_priv,
>  				struct drm_crtc *crtc)
>  {
>  	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
> +	struct drm_plane *plane = crtc->primary;
>  	struct vmw_framebuffer *vfb;
> 
> +	lockdep_assert_held(&plane->mutex);
> +
>  	mutex_lock(&dev_priv->global_kms_state_mutex);
> 
>  	if (!du->is_implicit)
>  		goto out_unlock;
> 
> -	vfb = vmw_framebuffer_to_vfb(crtc->primary->fb);
> +	vfb = vmw_framebuffer_to_vfb(plane->state->fb);
>  	WARN_ON_ONCE(dev_priv->num_implicit != 1 &&
>  		     dev_priv->implicit_fb != vfb);
> 
> --
> 2.16.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/vmwgfx: Stop using plane->fb in atomic_enable()

[Deepak Singh Rawat]

> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Instead of looking at the (soon to be deprecated) plane->fb we'll
> examing plane->state->fb instead. We can do this because
> vmw_du_crtc_atomic_check() prevents us from enabling a crtc
> without the primary plane also being enabled.
> 
> Due to that same reason, I'm actually not sure what the checks here are
> for NULL fb. If we can't enable the crtc without an enabled plane
> we should always have an fb. But I'll leave that for someone else
> to figure out.

Hi Ville,

AFAIK the NULL check is set or clear the implicit framebuffer property
which is specific to vmwgfx and for current hardware version is disabled.
I have this future TODO work item to get rid of implicit placement property
or at least make it read only.

I still don’t have complete understanding of atomic state but this patch looks
good to me.

Reviewed-by: Deepak Rawat <drawat@vmware.com>

> 
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 90445bc590cb..152e96cb1c01 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -414,6 +414,7 @@ static void vmw_stdu_crtc_helper_prepare(struct
> drm_crtc *crtc)
>  static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
>  					struct drm_crtc_state *old_state)
>  {
> +	struct drm_plane_state *plane_state = crtc->primary->state;
>  	struct vmw_private *dev_priv;
>  	struct vmw_screen_target_display_unit *stdu;
>  	struct vmw_framebuffer *vfb;
> @@ -422,7 +423,7 @@ static void vmw_stdu_crtc_atomic_enable(struct
> drm_crtc *crtc,
> 
>  	stdu     = vmw_crtc_to_stdu(crtc);
>  	dev_priv = vmw_priv(crtc->dev);
> -	fb       = crtc->primary->fb;
> +	fb       = plane_state->fb;
> 
>  	vfb = (fb) ? vmw_framebuffer_to_vfb(fb) : NULL;
> 
> --
> 2.16.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/vmwgfx: Stop updating plane->fb

[Deepak Singh Rawat]

This makes sense once we got rid of plane->fb

Will this go to drm-next? Could you please CC
me so that I can do some testing myself. Thanks.

Reviewed-by: Deepak Rawat <drawat@vmware.com>


> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We want to get rid of plane->fb on atomic drivers. Stop setting it.
> 
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 --
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index 648f8127f65a..bbd3f19b1a0b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -525,8 +525,6 @@ vmw_sou_primary_plane_atomic_update(struct
> drm_plane *plane,
>  		 */
>  		if (ret != 0)
>  			DRM_ERROR("Failed to update screen.\n");
> -
> -		crtc->primary->fb = plane->state->fb;
>  	} else {
>  		/*
>  		 * When disabling a plane, CRTC and FB should always be
> NULL
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 67331f01ef32..90445bc590cb 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1285,8 +1285,6 @@ vmw_stdu_primary_plane_atomic_update(struct
> drm_plane *plane,
>  							 1, 1, NULL, crtc);
>  		if (ret)
>  			DRM_ERROR("Failed to update STDU.\n");
> -
> -		crtc->primary->fb = plane->state->fb;
>  	} else {
>  		crtc = old_state->crtc;
>  		stdu = vmw_crtc_to_stdu(crtc);
> --
> 2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/vmwgfx: Stop updating plane->fb

[Ville Syrjälä]

On Fri, Apr 06, 2018 at 07:14:51PM +0000, Deepak Singh Rawat wrote:
> This makes sense once we got rid of plane->fb
> 
> Will this go to drm-next?

The plan is to push to drm-misc-next once we get all
the ducks in a row.

> Could you please CC
> me so that I can do some testing myself. Thanks.

Here's a branch if you want a head start:
git://github.com/vsyrjala/linux.git plane_fb_crtc_nuke_2

I'd definitely appreciate some testing of this stuff. Wouldn't
want to break you stuff accidentally.

> 
> Reviewed-by: Deepak Rawat <drawat@vmware.com>
> 
> 
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We want to get rid of plane->fb on atomic drivers. Stop setting it.
> > 
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Cc: Sinclair Yeh <syeh@vmware.com>
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 --
> >  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 --
> >  2 files changed, 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > index 648f8127f65a..bbd3f19b1a0b 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > @@ -525,8 +525,6 @@ vmw_sou_primary_plane_atomic_update(struct
> > drm_plane *plane,
> >  		 */
> >  		if (ret != 0)
> >  			DRM_ERROR("Failed to update screen.\n");
> > -
> > -		crtc->primary->fb = plane->state->fb;
> >  	} else {
> >  		/*
> >  		 * When disabling a plane, CRTC and FB should always be
> > NULL
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > index 67331f01ef32..90445bc590cb 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > @@ -1285,8 +1285,6 @@ vmw_stdu_primary_plane_atomic_update(struct
> > drm_plane *plane,
> >  							 1, 1, NULL, crtc);
> >  		if (ret)
> >  			DRM_ERROR("Failed to update STDU.\n");
> > -
> > -		crtc->primary->fb = plane->state->fb;
> >  	} else {
> >  		crtc = old_state->crtc;
> >  		stdu = vmw_crtc_to_stdu(crtc);
> > --
> > 2.16.1

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

Re: [PATCH 5/7] drm/vmwgfx: Stop updating plane->fb

[Ville Syrjälä]

On Fri, Apr 06, 2018 at 10:35:00PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 06, 2018 at 07:14:51PM +0000, Deepak Singh Rawat wrote:
> > This makes sense once we got rid of plane->fb
> > 
> > Will this go to drm-next?
> 
> The plan is to push to drm-misc-next once we get all
> the ducks in a row.
> 
> > Could you please CC
> > me so that I can do some testing myself. Thanks.
> 
> Here's a branch if you want a head start:
> git://github.com/vsyrjala/linux.git plane_fb_crtc_nuke_2
> 
> I'd definitely appreciate some testing of this stuff. Wouldn't
> want to break you stuff accidentally.

Did we get anywhere with testing this? I'd like to land the remaining 
bits, but I'd feel much safer doing that if it was tested.

> 
> > 
> > Reviewed-by: Deepak Rawat <drawat@vmware.com>
> > 
> > 
> > > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We want to get rid of plane->fb on atomic drivers. Stop setting it.
> > > 
> > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 --
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 --
> > >  2 files changed, 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > > index 648f8127f65a..bbd3f19b1a0b 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > > @@ -525,8 +525,6 @@ vmw_sou_primary_plane_atomic_update(struct
> > > drm_plane *plane,
> > >  		 */
> > >  		if (ret != 0)
> > >  			DRM_ERROR("Failed to update screen.\n");
> > > -
> > > -		crtc->primary->fb = plane->state->fb;
> > >  	} else {
> > >  		/*
> > >  		 * When disabling a plane, CRTC and FB should always be
> > > NULL
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > > index 67331f01ef32..90445bc590cb 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > > @@ -1285,8 +1285,6 @@ vmw_stdu_primary_plane_atomic_update(struct
> > > drm_plane *plane,
> > >  							 1, 1, NULL, crtc);
> > >  		if (ret)
> > >  			DRM_ERROR("Failed to update STDU.\n");
> > > -
> > > -		crtc->primary->fb = plane->state->fb;
> > >  	} else {
> > >  		crtc = old_state->crtc;
> > >  		stdu = vmw_crtc_to_stdu(crtc);
> > > --
> > > 2.16.1
> 
> -- 
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/vmwgfx: Stop updating plane->fb

[Deepak Singh Rawat]

> 
> On Fri, Apr 06, 2018 at 10:35:00PM +0300, Ville Syrjälä wrote:
> > On Fri, Apr 06, 2018 at 07:14:51PM +0000, Deepak Singh Rawat wrote:
> > > This makes sense once we got rid of plane->fb
> > >
> > > Will this go to drm-next?
> >
> > The plan is to push to drm-misc-next once we get all
> > the ducks in a row.
> >
> > > Could you please CC
> > > me so that I can do some testing myself. Thanks.
> >
> > Here's a branch if you want a head start:
> > git://github.com/vsyrjala/linux.git plane_fb_crtc_nuke_2
> >
> > I'd definitely appreciate some testing of this stuff. Wouldn't
> > want to break you stuff accidentally.
> 
> Did we get anywhere with testing this? I'd like to land the remaining
> bits, but I'd feel much safer doing that if it was tested.

Hi Ville,

I did some basic mode-setting testing by taking your patches to
vmwgfx private branch and things seems to work fine.

Thanks,
Deepak

> 
> >
> > >
> > > Reviewed-by: Deepak Rawat <drawat@vmware.com>
> > >
> > >
> > > >
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > We want to get rid of plane->fb on atomic drivers. Stop setting it.
> > > >
> > > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 --
> > > >  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 --
> > > >  2 files changed, 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > > > b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > > > index 648f8127f65a..bbd3f19b1a0b 100644
> > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > > > @@ -525,8 +525,6 @@
> vmw_sou_primary_plane_atomic_update(struct
> > > > drm_plane *plane,
> > > >  		 */
> > > >  		if (ret != 0)
> > > >  			DRM_ERROR("Failed to update screen.\n");
> > > > -
> > > > -		crtc->primary->fb = plane->state->fb;
> > > >  	} else {
> > > >  		/*
> > > >  		 * When disabling a plane, CRTC and FB should always be
> > > > NULL
> > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > > > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > > > index 67331f01ef32..90445bc590cb 100644
> > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > > > @@ -1285,8 +1285,6 @@
> vmw_stdu_primary_plane_atomic_update(struct
> > > > drm_plane *plane,
> > > >  							 1, 1, NULL, crtc);
> > > >  		if (ret)
> > > >  			DRM_ERROR("Failed to update STDU.\n");
> > > > -
> > > > -		crtc->primary->fb = plane->state->fb;
> > > >  	} else {
> > > >  		crtc = old_state->crtc;
> > > >  		stdu = vmw_crtc_to_stdu(crtc);
> > > > --
> > > > 2.16.1
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lists.freedesktop.org_mailman_listinfo_intel-
> 2Dgfx&d=DwIDAw&c=uilaK90D4TOVoH58JNXRgQ&r=zOOG28inJK0762SxAf-
> cyZdStnd2NQpRu98lJP2iYGw&m=3J7W8_yE3JhMDcN3FfZN8bWZON61wueSY
> YfSGxPNHVE&s=TqYFqV1NCzCnakZHMWVyJ9k42n0CUm5Kcl9xW2Cdvz4&e=
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx