| Message ID | 20241016053626.2850384-1-arun.r.murthy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display: plane property for async supported modifiers | expand |
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. > 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
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
On Wed, Oct 16, 2024 at 04:54:09PM +0300, Ville Syrjälä wrote: > 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 Although we should probably pass the format to that as well... > > > > 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
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Wednesday, October 16, 2024 7:31 PM > To: Murthy, Arun R <arun.r.murthy@intel.com> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; dri- > devel@lists.freedesktop.org > Subject: Re: [PATCH] drm/i915/display: plane property for async supported > modifiers > > On Wed, Oct 16, 2024 at 04:54:09PM +0300, Ville Syrjälä wrote: > > 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#no > > > > te_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 > > Although we should probably pass the format to that as well... > The blob points to the struct with elements, modifier and formats. The main intention of this patch to get the i915 plane private property. The reason for not having this as drm property is we are the only user for this and no other vendor has this restriction. Thanks and Regards, Arun R Murthy --------------------
> > Subject: Re: [PATCH] drm/i915/display: plane property for async > > supported modifiers > > > > On Wed, Oct 16, 2024 at 04:54:09PM +0300, Ville Syrjälä wrote: > > > 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# > > > > > no > > > > > te_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 > > > > Although we should probably pass the format to that as well... > > > The blob points to the struct with elements, modifier and formats. > The main intention of this patch to get the i915 plane private property. > The reason for not having this as drm property is we are the only user for this > and no other vendor has this restriction. > Any comments on this? Thanks and Regards, Arun R Murthy ---------------------
On Fri, Oct 25, 2024 at 04:28:18AM +0000, Murthy, Arun R wrote: > > > Subject: Re: [PATCH] drm/i915/display: plane property for async > > > supported modifiers > > > > > > On Wed, Oct 16, 2024 at 04:54:09PM +0300, Ville Syrjälä wrote: > > > > 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# > > > > > > no > > > > > > te_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 > > > > > > Although we should probably pass the format to that as well... > > > > > The blob points to the struct with elements, modifier and formats. > > The main intention of this patch to get the i915 plane private property. > > The reason for not having this as drm property is we are the only user for this > > and no other vendor has this restriction. > > > > Any comments on this? Seeinag as the patch does nothing so there is nothing to comment beyond what I already said.
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);
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 ++++++++++++++++++- 3 files changed, 58 insertions(+), 1 deletion(-)