> -----Original Message-----
> From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Pankaj
> Bharadiya
> Sent: Monday, August 3, 2020 10:00 AM
> To: jani.nik...@linux.intel.com; dan...@ffwll.ch; 
> intel-...@lists.freedesktop.org;
> dri-devel@lists.freedesktop.org; ville.syrj...@linux.intel.com;
> dani...@collabora.com; Lattannavar, Sameer <sameer.lattanna...@intel.com>;
> Maarten Lankhorst <maarten.lankho...@linux.intel.com>; Maxime Ripard
> <mrip...@kernel.org>; Thomas Zimmermann <tzimmerm...@suse.de>; David
> Airlie <airl...@linux.ie>
> Subject: [Intel-gfx] [PATCH v5 1/5] drm: Introduce plane and CRTC scaling 
> filter
> properties
> 
> Introduce per-plane and per-CRTC scaling filter properties to allow userspace 
> to
> select the driver's default scaling filter or
> Nearest-neighbor(NN) filter for upscaling operations on CRTC and plane.
> 
> Drivers can set up this property for a plane by calling
> drm_plane_create_scaling_filter() and for a CRTC by calling
> drm_crtc_create_scaling_filter().
> 
> NN filter works by filling in the missing color values in the upscaled image 
> with
> that of the coordinate-mapped nearest source pixel value.
> 
> NN filter for integer multiple scaling can be particularly useful for for 
> pixel art
> games that rely on sharp, blocky images to deliver their distinctive look.
> 
> changes since v3:
> * Refactor code, add new function for common code (Ville) changes since v2:
> * Create per-plane and per-CRTC scaling filter property (Ville) changes since 
> v1:
> * None
> changes since RFC:
> * Add separate properties for plane and CRTC (Ville)
> 
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharad...@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c   |  8 +++
>  drivers/gpu/drm/drm_crtc.c          | 48 +++++++++++++++
>  drivers/gpu/drm/drm_crtc_internal.h |  3 +
>  drivers/gpu/drm/drm_plane.c         | 90 +++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h              | 16 +++++
>  include/drm/drm_plane.h             | 21 +++++++
>  6 files changed, 186 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 25c269bc4681..ef82009035e6 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -469,6 +469,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
> *crtc,
>                       return -EFAULT;
> 
>               set_out_fence_for_crtc(state->state, crtc, fence_ptr);
> +     } else if (property == crtc->scaling_filter_property) {
> +             state->scaling_filter = val;
>       } else if (crtc->funcs->atomic_set_property) {
>               return crtc->funcs->atomic_set_property(crtc, state, property,
> val);
>       } else {
> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>               *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>       else if (property == config->prop_out_fence_ptr)
>               *val = 0;
> +     else if (property == crtc->scaling_filter_property)
> +             *val = state->scaling_filter;
>       else if (crtc->funcs->atomic_get_property)
>               return crtc->funcs->atomic_get_property(crtc, state, property,
> val);
>       else
> @@ -585,6 +589,8 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
>                                       sizeof(struct drm_rect),
>                                       &replaced);
>               return ret;
> +     } else if (property == plane->scaling_filter_property) {
> +             state->scaling_filter = val;
>       } else if (plane->funcs->atomic_set_property) {
>               return plane->funcs->atomic_set_property(plane, state,
>                               property, val);
> @@ -643,6 +649,8 @@ drm_atomic_plane_get_property(struct drm_plane
> *plane,
>       } else if (property == config->prop_fb_damage_clips) {
>               *val = (state->fb_damage_clips) ?
>                       state->fb_damage_clips->base.id : 0;
> +     } else if (property == plane->scaling_filter_property) {
> +             *val = state->scaling_filter;
>       } else if (plane->funcs->atomic_get_property) {
>               return plane->funcs->atomic_get_property(plane, state,
> property, val);
>       } else {
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index
> 283bcc4362ca..70f5cd9704ba 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -776,3 +776,51 @@ int drm_mode_crtc_set_obj_prop(struct
> drm_mode_object *obj,
> 
>       return ret;
>  }
> +
> +/**
> + * DOC: CRTC scaling filter property
> + *
> + * SCALING_FILTER:
> + *
> + *   Indicates scaling filter to be used for CRTC scaler
> + *
> + *   The value of this property can be one of the following:
> + *   Default:
> + *           Driver's default scaling filter
> + *   Nearest Neighbor:
> + *           Nearest Neighbor scaling filter
> + *
> + * Drivers can set up this property for a CRTC by calling
> + * drm_crtc_create_scaling_filter_property
> + */
> +
> +/**
> + * drm_crtc_create_scaling_filter_property - create a new scaling
> +filter
> + * property
> + *
> + * @crtc: drm CRTC
> + * @supported_filters: bitmask of supported scaling filters, must include
> + *                  BIT(DRM_SCALING_FILTER_DEFAULT).
> + *
> + * This function lets driver to enable the scaling filter property on a
> +given
> + * CRTC.
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
> +                                         unsigned int supported_filters) {
> +     struct drm_property *prop =
> +             drm_create_scaling_filter_prop(crtc->dev, supported_filters);
> +
> +     if (IS_ERR(prop))
> +             return PTR_ERR(prop);
> +
> +     drm_object_attach_property(&crtc->base, prop,
> +                                DRM_SCALING_FILTER_DEFAULT);
> +     crtc->scaling_filter_property = prop;
> +
> +     return 0;
> +}
> +EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h
> b/drivers/gpu/drm/drm_crtc_internal.h
> index da96b2f64d7e..54d4cf1233e9 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -72,6 +72,9 @@ int drm_crtc_force_disable(struct drm_crtc *crtc);
> 
>  struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc);
> 
> +struct drm_property *
> +drm_create_scaling_filter_prop(struct drm_device *dev,
> +                            unsigned int supported_filters);
>  /* IOCTLs */
>  int drm_mode_getcrtc(struct drm_device *dev,
>                    void *data, struct drm_file *file_priv); diff --git
> a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index
> b7b90b3a2e38..de3888572684 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1231,3 +1231,93 @@ int drm_mode_page_flip_ioctl(struct drm_device
> *dev,
> 
>       return ret;
>  }
> +
> +struct drm_property *
> +drm_create_scaling_filter_prop(struct drm_device *dev,
> +                            unsigned int supported_filters) {

This property is used generically by crtc and plane. Can we move this function 
outside the plane
file to a more generic location. We could use drm_mode_config.c for instance.

Other than this, it looks good to me.
Reviewed-by: Uma Shankar <uma.shan...@intel.com>

> +     struct drm_property *prop;
> +     static const struct drm_prop_enum_list props[] = {
> +             { DRM_SCALING_FILTER_DEFAULT, "Default" },
> +             { DRM_SCALING_FILTER_NEAREST_NEIGHBOR, "Nearest
> Neighbor" },
> +     };
> +     unsigned int valid_mode_mask = BIT(DRM_SCALING_FILTER_DEFAULT) |
> +
> BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR);
> +     int i;
> +
> +     if (WARN_ON((supported_filters & ~valid_mode_mask) ||
> +                 ((supported_filters & BIT(DRM_SCALING_FILTER_DEFAULT)) ==
> 0)))
> +             return ERR_PTR(-EINVAL);
> +
> +     prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
> +                                "SCALING_FILTER",
> +                                hweight32(supported_filters));
> +     if (!prop)
> +             return ERR_PTR(-ENOMEM);
> +
> +     for (i = 0; i < ARRAY_SIZE(props); i++) {
> +             int ret;
> +
> +             if (!(BIT(props[i].type) & supported_filters))
> +                     continue;
> +
> +             ret = drm_property_add_enum(prop, props[i].type,
> +                                         props[i].name);
> +
> +             if (ret) {
> +                     drm_property_destroy(dev, prop);
> +
> +                     return ERR_PTR(ret);
> +             }
> +     }
> +
> +     return prop;
> +}
> +
> +/**
> + * DOC: Plane scaling filter property
> + *
> + * SCALING_FILTER:
> + *
> + *   Indicates scaling filter to be used for plane scaler
> + *
> + *   The value of this property can be one of the following:
> + *   Default:
> + *           Driver's default scaling filter
> + *   Nearest Neighbor:
> + *           Nearest Neighbor scaling filter
> + *
> + * Drivers can set up this property for a plane by calling
> + * drm_plane_create_scaling_filter_property
> + */
> +
> +/**
> + * drm_plane_create_scaling_filter_property - create a new scaling
> +filter
> + * property
> + *
> + * @plane: drm plane
> + * @supported_filters: bitmask of supported scaling filters, must include
> + *                  BIT(DRM_SCALING_FILTER_DEFAULT).
> + *
> + * This function lets driver to enable the scaling filter property on a
> +given
> + * plane.
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
> +                                          unsigned int supported_filters) {
> +     struct drm_property *prop =
> +             drm_create_scaling_filter_prop(plane->dev, supported_filters);
> +
> +     if (IS_ERR(prop))
> +             return PTR_ERR(prop);
> +
> +     drm_object_attach_property(&plane->base, prop,
> +                                DRM_SCALING_FILTER_DEFAULT);
> +     plane->scaling_filter_property = prop;
> +
> +     return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_scaling_filter_property);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> 59b51a09cae6..ba839e5e357d 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -324,6 +324,13 @@ struct drm_crtc_state {
>        */
>       bool self_refresh_active;
> 
> +     /**
> +      * @scaling_filter:
> +      *
> +      * Scaling filter to be applied
> +      */
> +     enum drm_scaling_filter scaling_filter;
> +
>       /**
>        * @event:
>        *
> @@ -1083,6 +1090,12 @@ struct drm_crtc {
>       /** @properties: property tracking for this CRTC */
>       struct drm_object_properties properties;
> 
> +     /**
> +      * @scaling_filter_property: property to apply a particular filter while
> +      * scaling.
> +      */
> +     struct drm_property *scaling_filter_property;
> +
>       /**
>        * @state:
>        *
> @@ -1266,4 +1279,7 @@ static inline struct drm_crtc *drm_crtc_find(struct
> drm_device *dev,  #define drm_for_each_crtc(crtc, dev) \
>       list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
> 
> +int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
> +                                         unsigned int supported_filters);
> +
>  #endif /* __DRM_CRTC_H__ */
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
> 3f396d94afe4..1d82b264e5e4 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -35,6 +35,11 @@ struct drm_crtc;
>  struct drm_printer;
>  struct drm_modeset_acquire_ctx;
> 
> +enum drm_scaling_filter {
> +     DRM_SCALING_FILTER_DEFAULT,
> +     DRM_SCALING_FILTER_NEAREST_NEIGHBOR,
> +};
> +
>  /**
>   * struct drm_plane_state - mutable plane state
>   *
> @@ -214,6 +219,13 @@ struct drm_plane_state {
>        */
>       bool visible;
> 
> +     /**
> +      * @scaling_filter:
> +      *
> +      * Scaling filter to be applied
> +      */
> +     enum drm_scaling_filter scaling_filter;
> +
>       /**
>        * @commit: Tracks the pending commit to prevent use-after-free
> conditions,
>        * and for async plane updates.
> @@ -724,6 +736,12 @@ struct drm_plane {
>        * See drm_plane_create_color_properties().
>        */
>       struct drm_property *color_range_property;
> +
> +     /**
> +      * @scaling_filter_property: property to apply a particular filter while
> +      * scaling.
> +      */
> +     struct drm_property *scaling_filter_property;
>  };
> 
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base) @@ -862,4
> +880,7 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state)
>                                       state->fb_damage_clips->data : NULL);  }
> 
> +int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
> +                                          unsigned int supported_filters);
> +
>  #endif
> --
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to