[PATCH 1/5] drm: Introduce sharpness mode property

[Garg, Nemesa]

> -----Original Message-----
> From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf Of Pekka
> Paalanen
> Sent: Friday, March 8, 2024 2:18 PM
> To: Garg, Nemesa <nemesa.garg at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
> Subject: Re: [PATCH 1/5] drm: Introduce sharpness mode property
> 
> On Thu,  7 Mar 2024 14:02:33 +0530
> Nemesa Garg <nemesa.garg at intel.com> wrote:
> 
> > This allows the user to set the intensity so as to get the sharpness
> > effect.
> >
> > It is useful in scenario when the output is blurry and user want to
> > sharpen the pixels.
> >
> > Signed-off-by: Nemesa Garg <nemesa.garg at intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >  drivers/gpu/drm/drm_crtc.c        | 17 +++++++++++++++++
> >  include/drm/drm_crtc.h            | 17 +++++++++++++++++
> >  3 files changed, 38 insertions(+)
> 
> Hi,
> 
> the UAPI documentation is completely missing. This cannot be discussed until the
> UAPI contract is drafted. It needs to end up in the appropriate "Properties"
> section under https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#kms-
> properties
> when documentation is built.
> 
> I also do not see any of the previous review comments being addressed that I
> recall.
> 
> The typo "sharpeness" is very common in these patches, and it is even in the UAPI
> as the property name. Also doc comments use different spelling than actual code.
> And sometimes you use even "sharpening"
> instead of "sharpness". Which one is it?

Thank you for pointing it out. I'll change it to sharpness to make it consistent everywhere and will add document in next revision. It seems I have missed some queries I will try to address it.

Thanks,
Nemesa

> 
> 
> Thanks,
> pq
> 
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 29d4940188d4..773873726b87 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -417,6 +417,8 @@ static int drm_atomic_crtc_set_property(struct
> drm_crtc *crtc,
> >  		set_out_fence_for_crtc(state->state, crtc, fence_ptr);
> >  	} else if (property == crtc->scaling_filter_property) {
> >  		state->scaling_filter = val;
> > +	} else if (property == crtc->sharpening_strength_prop) {
> > +		state->sharpeness_strength = val;
> >  	} else if (crtc->funcs->atomic_set_property) {
> >  		return crtc->funcs->atomic_set_property(crtc, state, property,
> val);
> >  	} else {
> > @@ -454,6 +456,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >  		*val = 0;
> >  	else if (property == crtc->scaling_filter_property)
> >  		*val = state->scaling_filter;
> > +	else if (property == crtc->sharpening_strength_prop)
> > +		*val = state->sharpeness_strength;
> >  	else if (crtc->funcs->atomic_get_property)
> >  		return crtc->funcs->atomic_get_property(crtc, state, property,
> val);
> >  	else {
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index cb90e70d85e8..d01ab76a719f 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -955,3 +955,20 @@ int drm_crtc_create_scaling_filter_property(struct
> drm_crtc *crtc,
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
> > +
> > +int drm_crtc_create_sharpening_strength_property(struct drm_crtc
> > +*crtc) {
> > +	struct drm_device *dev = crtc->dev;
> > +
> > +	struct drm_property *prop =
> > +		drm_property_create_range(dev, 0, "SHARPENESS_STRENGTH",
> 0, 255);
> > +
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	crtc->sharpening_strength_prop = prop;
> > +	drm_object_attach_property(&crtc->base, prop, 0);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_crtc_create_sharpening_strength_property);
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> > 8b48a1974da3..241514fc3eea 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -317,6 +317,16 @@ struct drm_crtc_state {
> >  	 */
> >  	enum drm_scaling_filter scaling_filter;
> >
> > +	/**
> > +	 * @sharpness_strength
> > +	 *
> > +	 * Used by the user to set the sharpness intensity.
> > +	 * The value ranges from 0-255.
> > +	 * Any value greater than 0 means enabling the featuring
> > +	 * along with setting the value for sharpness.
> > +	 */
> > +	u8 sharpeness_strength;
> > +
> >  	/**
> >  	 * @event:
> >  	 *
> > @@ -1088,6 +1098,12 @@ struct drm_crtc {
> >  	 */
> >  	struct drm_property *scaling_filter_property;
> >
> > +	/**
> > +	 * @sharpening_strength_prop: property to apply
> > +	 * the intensity of the sharpness requested.
> > +	 */
> > +	struct drm_property *sharpening_strength_prop;
> > +
> >  	/**
> >  	 * @state:
> >  	 *
> > @@ -1324,4 +1340,5 @@ static inline struct drm_crtc
> > *drm_crtc_find(struct drm_device *dev,  int
> drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
> >  					    unsigned int supported_filters);
> >
> > +int drm_crtc_create_sharpening_strength_property(struct drm_crtc
> > +*crtc);
> >  #endif /* __DRM_CRTC_H__ */