Re: [RFC][PATCH 1/5] drm: Introduce scaling filter property

[Ville Syrjälä]

On Mon, Mar 16, 2020 at 09:31:32AM +0100, Daniel Vetter wrote:
> On Tue, Mar 10, 2020 at 06:01:06PM +0200, Ville Syrjälä wrote:
> > On Tue, Feb 25, 2020 at 12:35:41PM +0530, Pankaj Bharadiya wrote:
> > > Introduce new scaling filter property to allow userspace to select
> > > the driver's default scaling filter or Nearest-neighbor(NN) filter
> > > for upscaling operations on crtc/plane.
> > > 
> > > Drivers can set up this property for a plane by calling
> > > drm_plane_enable_scaling_filter() and for a CRTC by calling
> > > drm_crtc_enable_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.
> > > 
> > > Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@xxxxxxxxx>
> > > Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_uapi.c |  8 +++++++
> > >  drivers/gpu/drm/drm_crtc.c        | 16 ++++++++++++++
> > >  drivers/gpu/drm/drm_mode_config.c | 13 ++++++++++++
> > >  drivers/gpu/drm/drm_plane.c       | 35 +++++++++++++++++++++++++++++++
> > >  include/drm/drm_crtc.h            | 10 +++++++++
> > >  include/drm/drm_mode_config.h     |  6 ++++++
> > >  include/drm/drm_plane.h           | 14 +++++++++++++
> > >  7 files changed, 102 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index a1e5e262bae2..4e3c1f3176e4 100644
> > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> > >  		return ret;
> > >  	} else if (property == config->prop_vrr_enabled) {
> > >  		state->vrr_enabled = val;
> > > +	} else if (property == config->scaling_filter_property) {
> > > +		state->scaling_filter = val;
> > 
> > I think we want a per-plane/per-crtc prop for this. If we start adding
> > more filters we are surely going to need different sets for different hw
> > blocks.
> 
> In the past we've only done that once we have a demonstrated need. Usually
> the patch to move the property to a per-object location isn't a lot of
> churn.

Seems silly to not do it from the start when we already know there is
hardware out there that has different capabilities per hw block.

-- 
Ville Syrjälä
Intel