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

[Pekka Paalanen]

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?


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__ */

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240308/c1ced82a/attachment.sig>