Re: [PATCH 03/14] drm: clear plane damage during full modeset

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Deepak Rawat <drawat@vmware.com>
Cc: thellstrom@vmware.com, linux-graphics-maintainer@vmware.com,
	lukasz.spintzyk@displaylink.com, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 03/14] drm: clear plane damage during full modeset
Date: Thu, 6 Sep 2018 15:02:32 +0300	[thread overview]
Message-ID: <20180906120232.GY5565@intel.com> (raw)
In-Reply-To: <20180905233901.2321-4-drawat@vmware.com>

On Wed, Sep 05, 2018 at 04:38:50PM -0700, Deepak Rawat wrote:
> Plane damage is irrelevant when full modeset happens so clear the damage
> blob property(If set by user-space). With damage blob cleared damage
> helper iterator will return full plane src as damage clip.
> 
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>  include/drm/drm_damage_helper.h     | 10 ++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index be83e2763c18..e06d2d5d582f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -31,6 +31,7 @@
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_writeback.h>
> +#include <drm/drm_damage_helper.h>
>  #include <linux/dma-fence.h>
>  
>  #include "drm_crtc_helper_internal.h"
> @@ -88,6 +89,9 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
>  			return;
>  
>  		crtc_state->planes_changed = true;
> +
> +		if (drm_atomic_crtc_needs_modeset(crtc_state))
> +			drm_plane_clear_damage(plane_state);

So if we sometimes magically clear it, where do we reinitialize it with
the user provided damage for the next update?

>  	}
>  }
>  
> diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
> index f1282b459a4f..1f988f7fdd72 100644
> --- a/include/drm/drm_damage_helper.h
> +++ b/include/drm/drm_damage_helper.h
> @@ -71,6 +71,16 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state)
>  				   state->fb_damage_clips->data : NULL);
>  }
>  
> +/**
> + * drm_plane_clear_damage - clears damage blob in a plane state
> + * @state: Plane state
> + */
> +static inline void drm_plane_clear_damage(struct drm_plane_state *state)
> +{
> +	drm_property_blob_put(state->fb_damage_clips);
> +	state->fb_damage_clips = NULL;

Ah. So you're trying to clear out the user provided damage here. That
doesn't really seem like sane sematics to me. Either the user provided
damage should stick always instead of just sometimes, or it should 
always be cleared once the update is done (ie. could clear in
duplicate_state() in that case).

> +}
> +
>  void drm_plane_enable_fb_damage_clips(struct drm_plane *plane);
>  int
>  drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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