Re: [PATCH] drm/i915/display: plane property for async supported modifiers

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Oct 16, 2024 at 04:30:19PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 16, 2024 at 11:06:26AM +0530, Arun R Murthy wrote:
> > Create a i915 private plane property for sharing the async supported
> > modifiers to the user.
> > UMD related discussion requesting the same
> > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29618#note_2487123
> > 
> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > ---
> >  .../gpu/drm/i915/display/intel_atomic_plane.c |  6 +++
> >  .../drm/i915/display/intel_display_types.h    |  4 ++
> >  .../drm/i915/display/skl_universal_plane.c    | 49 ++++++++++++++++++-
> 
> This whole thing belongs in the drm core.

And I don't even see an actual implementation of anything here.
Why did you even post this when it doesn't do anything?

Anyways, thinking about how we might actually implement this,
we can probably leverage
https://patchwork.freedesktop.org/patch/619047/?series=139807&rev=3

> >  3 files changed, 58 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > index b7e462075ded..ef41b50cc765 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -117,6 +117,9 @@ intel_plane_duplicate_state(struct drm_plane *plane)
> >  	intel_state->dpt_vma = NULL;
> >  	intel_state->flags = 0;
> >  
> > +	if (intel_state->async_sup_modifiers)
> > +		drm_property_blob_get(intel_state->async_sup_modifiers);
> > +
> >  	/* add reference to fb */
> >  	if (intel_state->hw.fb)
> >  		drm_framebuffer_get(intel_state->hw.fb);
> > @@ -141,6 +144,9 @@ intel_plane_destroy_state(struct drm_plane *plane,
> >  	drm_WARN_ON(plane->dev, plane_state->ggtt_vma);
> >  	drm_WARN_ON(plane->dev, plane_state->dpt_vma);
> >  
> > +	if (plane_state->async_sup_modifiers)
> > +		drm_property_blob_put(plane_state->async_sup_modifiers);
> > +
> >  	__drm_atomic_helper_plane_destroy_state(&plane_state->uapi);
> >  	if (plane_state->hw.fb)
> >  		drm_framebuffer_put(plane_state->hw.fb);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 2bb1fa64da2f..a5a301ca521a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -683,6 +683,8 @@ struct intel_plane_state {
> >  	u64 ccval;
> >  
> >  	const char *no_fbc_reason;
> > +
> > +	struct drm_property_blob *async_sup_modifiers;
> >  };
> >  
> >  struct intel_initial_plane_config {
> > @@ -1435,6 +1437,8 @@ struct intel_plane {
> >  
> >  	struct intel_fbc *fbc;
> >  
> > +	struct drm_property *async_modifiers_property;
> > +
> >  	/*
> >  	 * NOTE: Do not place new plane state fields here (e.g., when adding
> >  	 * new plane properties).  New runtime state should now be placed in
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 9557b08ca2e2..6790bdf00c8f 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -2383,6 +2383,29 @@ static bool icl_plane_format_mod_supported(struct drm_plane *_plane,
> >  	}
> >  }
> >  
> > +static int intel_plane_get_property(struct drm_plane *plane,
> > +				    const struct drm_plane_state *state,
> > +				    struct drm_property *property,
> > +				    uint64_t *val)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(plane->dev);
> > +	const struct intel_plane_state *intel_plane_state =
> > +		to_intel_plane_state(state);
> > +	struct intel_plane *intel_plane = to_intel_plane(plane);
> > +
> > +	if (property == intel_plane->async_modifiers_property) {
> > +		*val = intel_plane_state->async_sup_modifiers ?
> > +			intel_plane_state->async_sup_modifiers->base.id : 0;
> > +	} else {
> > +		drm_err(&i915->drm,
> > +			"Unknown property [PROP:%d:%s]\n",
> > +			property->base.id, property->name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static bool tgl_plane_format_mod_supported(struct drm_plane *_plane,
> >  					   u32 format, u64 modifier)
> >  {
> > @@ -2442,6 +2465,7 @@ static const struct drm_plane_funcs skl_plane_funcs = {
> >  	.atomic_duplicate_state = intel_plane_duplicate_state,
> >  	.atomic_destroy_state = intel_plane_destroy_state,
> >  	.format_mod_supported = skl_plane_format_mod_supported,
> > +	.atomic_get_property = intel_plane_get_property,
> >  };
> >  
> >  static const struct drm_plane_funcs icl_plane_funcs = {
> > @@ -2451,6 +2475,7 @@ static const struct drm_plane_funcs icl_plane_funcs = {
> >  	.atomic_duplicate_state = intel_plane_duplicate_state,
> >  	.atomic_destroy_state = intel_plane_destroy_state,
> >  	.format_mod_supported = icl_plane_format_mod_supported,
> > +	.atomic_get_property = intel_plane_get_property,
> >  };
> >  
> >  static const struct drm_plane_funcs tgl_plane_funcs = {
> > @@ -2460,6 +2485,7 @@ static const struct drm_plane_funcs tgl_plane_funcs = {
> >  	.atomic_duplicate_state = intel_plane_duplicate_state,
> >  	.atomic_destroy_state = intel_plane_destroy_state,
> >  	.format_mod_supported = tgl_plane_format_mod_supported,
> > +	.atomic_get_property = intel_plane_get_property,
> >  };
> >  
> >  static void
> > @@ -2549,6 +2575,25 @@ static u8 skl_get_plane_caps(struct drm_i915_private *i915,
> >  	return caps;
> >  }
> >  
> > +static void intel_plane_attach_async_modifiers_property(struct intel_plane *intel_plane)
> > +{
> > +	struct drm_plane *plane = &intel_plane->base;
> > +	struct drm_device *dev = plane->dev;
> > +	struct drm_property *prop;
> > +
> > +	prop = intel_plane->async_modifiers_property;
> > +	if (!prop) {
> > +		prop = drm_property_create(dev, DRM_MODE_PROP_BLOB | DRM_MODE_PROP_ATOMIC,
> > +					   "Async Supported Modifiers", 0);
> > +		if (!prop)
> > +			return;
> > +
> > +		intel_plane->async_modifiers_property = prop;
> > +	}
> > +
> > +	drm_object_attach_property(&plane->base, prop, 0);
> > +}
> > +
> >  struct intel_plane *
> >  skl_universal_plane_create(struct drm_i915_private *dev_priv,
> >  			   enum pipe pipe, enum plane_id plane_id)
> > @@ -2694,10 +2739,12 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
> >  	if (DISPLAY_VER(dev_priv) >= 12)
> >  		drm_plane_enable_fb_damage_clips(&plane->base);
> >  
> > -	if (DISPLAY_VER(dev_priv) >= 11)
> > +	if (DISPLAY_VER(dev_priv) >= 11) {
> >  		drm_plane_create_scaling_filter_property(&plane->base,
> >  						BIT(DRM_SCALING_FILTER_DEFAULT) |
> >  						BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR));
> > +		intel_plane_attach_async_modifiers_property(plane);
> > +	}
> >  
> >  	intel_plane_helper_add(plane);
> >  
> > -- 
> > 2.25.1
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel