Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

2019-10-24 Thread Jani Nikula
On Thu, 24 Oct 2019, Daniel Vetter  wrote:
> On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
>> On Thu, 24 Oct 2019, Daniel Vetter  wrote:
>> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> >> On Tue, 22 Oct 2019, Daniel Vetter  wrote:
>> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> >> This patch adds a scaling filter mode porperty
>> >> >> to allow:
>> >> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> >> - A userspace to pick a desired effect while scaling.
>> >> >> 
>> >> >> This option will be particularly useful in the scenarios where
>> >> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >> >> 
>> >> >> There was a RFC patch series published, to discus the request to enable
>> >> >> Integer mode scaling by some of the gaming communities, which can be
>> >> >> found here:
>> >> >> https://patchwork.freedesktop.org/series/66175/
>> >> >> 
>> >> >> Signed-off-by: Shashank Sharma 
>> >> >
>> >> > Two things:
>> >> >
>> >> > - needs real property docs for this in the kms property chapter
>> >> > - would be good if we could somehow subsume the panel fitter prop into
>> >> >   this? Or at least document possible interactions with that.
>> >> >
>> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> >> > pointing at the userspace patch series/merge request directly to the 
>> >> > patch
>> >> > that adds the new uapi.
>> >> 
>> >> I think Link: should only be used for referencing the patch that became
>> >> the commit?
>> >
>> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
>> > herd here. But yeah maybe we want something else.
>> >> 
>> >> References: perhaps?
>> >
>> > Or Userspace: to make it real obvious?
>> 
>> Works for me, as long as we don't conflate Link. (Maybe Link: should've
>> been Patch: or Submission: or whatever.)
>
> The Powers That Be (kernel summit) decided that it's Link for reference to
> a http link of an archive of some sort that contains the msg-id.

Oh totally aware of that. But hopefully they also assert that Link is
not also for something else.

> Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
> lore.k.o. They'll backfill the entire archive too. Imo still better we
> point at patchwork, since that's where hopefully the CI result hangs out.

Thanks, appreciated. Agreed on patchwork.

BR,
Jani.


> -Daniel
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> > -Daniel
>> >
>> >> 
>> >> >
>> >> > Cheers, Daniel
>> >> >> ---
>> >> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 
>> >> >>  include/drm/drm_crtc.h| 26 ++
>> >> >>  include/drm/drm_mode_config.h |  6 ++
>> >> >>  3 files changed, 36 insertions(+)
>> >> >> 
>> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
>> >> >> b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> index 0d466d3b0809..883329453a86 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;
>> >> >>} else if (property == config->degamma_lut_property) {
>> >> >>ret = drm_atomic_replace_property_blob_from_id(dev,
>> >> >>>degamma_lut,
>> >> >> @@ -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 == config->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
>> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> >> --- a/include/drm/drm_crtc.h
>> >> >> +++ b/include/drm/drm_crtc.h
>> >> >> @@ -58,6 +58,25 @@ struct device_node;
>> >> >>  struct dma_fence;
>> >> >>  struct edid;
>> >> >>  
>> >> >> +enum drm_scaling_filters {
>> >> >> +  DRM_SCALING_FILTER_DEFAULT,
>> >> >> +  DRM_SCALING_FILTER_MEDIUM,
>> >> >> +  DRM_SCALING_FILTER_BILINEAR,
>> >> >> +  DRM_SCALING_FILTER_NN,
>> >> >> +  DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> >> +  DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> >> +  DRM_SCALING_FILTER_INVALID,
>> >> >> +};
>> >> >> +
>> >> >> +static const struct 

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

2019-10-24 Thread Daniel Vetter
On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
> On Thu, 24 Oct 2019, Daniel Vetter  wrote:
> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
> >> On Tue, 22 Oct 2019, Daniel Vetter  wrote:
> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> >> This patch adds a scaling filter mode porperty
> >> >> to allow:
> >> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> >> - A userspace to pick a desired effect while scaling.
> >> >> 
> >> >> This option will be particularly useful in the scenarios where
> >> >> Integer mode scaling is possible, and a UI client wants to pick
> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >> >> 
> >> >> There was a RFC patch series published, to discus the request to enable
> >> >> Integer mode scaling by some of the gaming communities, which can be
> >> >> found here:
> >> >> https://patchwork.freedesktop.org/series/66175/
> >> >> 
> >> >> Signed-off-by: Shashank Sharma 
> >> >
> >> > Two things:
> >> >
> >> > - needs real property docs for this in the kms property chapter
> >> > - would be good if we could somehow subsume the panel fitter prop into
> >> >   this? Or at least document possible interactions with that.
> >> >
> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
> >> > pointing at the userspace patch series/merge request directly to the 
> >> > patch
> >> > that adds the new uapi.
> >> 
> >> I think Link: should only be used for referencing the patch that became
> >> the commit?
> >
> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
> > herd here. But yeah maybe we want something else.
> >> 
> >> References: perhaps?
> >
> > Or Userspace: to make it real obvious?
> 
> Works for me, as long as we don't conflate Link. (Maybe Link: should've
> been Patch: or Submission: or whatever.)

The Powers That Be (kernel summit) decided that it's Link for reference to
a http link of an archive of some sort that contains the msg-id.

Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
lore.k.o. They'll backfill the entire archive too. Imo still better we
point at patchwork, since that's where hopefully the CI result hangs out.
-Daniel

> 
> BR,
> Jani.
> 
> 
> > -Daniel
> >
> >> 
> >> >
> >> > Cheers, Daniel
> >> >> ---
> >> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 
> >> >>  include/drm/drm_crtc.h| 26 ++
> >> >>  include/drm/drm_mode_config.h |  6 ++
> >> >>  3 files changed, 36 insertions(+)
> >> >> 
> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> >> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> index 0d466d3b0809..883329453a86 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;
> >> >> } else if (property == config->degamma_lut_property) {
> >> >> ret = drm_atomic_replace_property_blob_from_id(dev,
> >> >> >degamma_lut,
> >> >> @@ -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 == config->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
> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> >> --- a/include/drm/drm_crtc.h
> >> >> +++ b/include/drm/drm_crtc.h
> >> >> @@ -58,6 +58,25 @@ struct device_node;
> >> >>  struct dma_fence;
> >> >>  struct edid;
> >> >>  
> >> >> +enum drm_scaling_filters {
> >> >> +   DRM_SCALING_FILTER_DEFAULT,
> >> >> +   DRM_SCALING_FILTER_MEDIUM,
> >> >> +   DRM_SCALING_FILTER_BILINEAR,
> >> >> +   DRM_SCALING_FILTER_NN,
> >> >> +   DRM_SCALING_FILTER_NN_IS_ONLY,
> >> >> +   DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> >> +   DRM_SCALING_FILTER_INVALID,
> >> >> +};
> >> >> +
> >> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] 
> >> >> = {
> >> >> +   { DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> >> +   { DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> >> +   { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> >> +   { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> >> >> 

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

2019-10-24 Thread Jani Nikula
On Thu, 24 Oct 2019, Daniel Vetter  wrote:
> On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> On Tue, 22 Oct 2019, Daniel Vetter  wrote:
>> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> This patch adds a scaling filter mode porperty
>> >> to allow:
>> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> - A userspace to pick a desired effect while scaling.
>> >> 
>> >> This option will be particularly useful in the scenarios where
>> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >> 
>> >> There was a RFC patch series published, to discus the request to enable
>> >> Integer mode scaling by some of the gaming communities, which can be
>> >> found here:
>> >> https://patchwork.freedesktop.org/series/66175/
>> >> 
>> >> Signed-off-by: Shashank Sharma 
>> >
>> > Two things:
>> >
>> > - needs real property docs for this in the kms property chapter
>> > - would be good if we could somehow subsume the panel fitter prop into
>> >   this? Or at least document possible interactions with that.
>> >
>> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> > pointing at the userspace patch series/merge request directly to the patch
>> > that adds the new uapi.
>> 
>> I think Link: should only be used for referencing the patch that became
>> the commit?
>
> Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
> herd here. But yeah maybe we want something else.
>> 
>> References: perhaps?
>
> Or Userspace: to make it real obvious?

Works for me, as long as we don't conflate Link. (Maybe Link: should've
been Patch: or Submission: or whatever.)

BR,
Jani.


> -Daniel
>
>> 
>> >
>> > Cheers, Daniel
>> >> ---
>> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 
>> >>  include/drm/drm_crtc.h| 26 ++
>> >>  include/drm/drm_mode_config.h |  6 ++
>> >>  3 files changed, 36 insertions(+)
>> >> 
>> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
>> >> b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> index 0d466d3b0809..883329453a86 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;
>> >>   } else if (property == config->degamma_lut_property) {
>> >>   ret = drm_atomic_replace_property_blob_from_id(dev,
>> >>   >degamma_lut,
>> >> @@ -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 == config->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
>> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> --- a/include/drm/drm_crtc.h
>> >> +++ b/include/drm/drm_crtc.h
>> >> @@ -58,6 +58,25 @@ struct device_node;
>> >>  struct dma_fence;
>> >>  struct edid;
>> >>  
>> >> +enum drm_scaling_filters {
>> >> + DRM_SCALING_FILTER_DEFAULT,
>> >> + DRM_SCALING_FILTER_MEDIUM,
>> >> + DRM_SCALING_FILTER_BILINEAR,
>> >> + DRM_SCALING_FILTER_NN,
>> >> + DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> + DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> + DRM_SCALING_FILTER_INVALID,
>> >> +};
>> >> +
>> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> >> + { DRM_SCALING_FILTER_DEFAULT, "Default" },
>> >> + { DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> >> + { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> >> + { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> >> + { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> >> + { DRM_SCALING_FILTER_INVALID, "Invalid" },
>> >> +};
>> >> +
>> >>  static inline int64_t U642I64(uint64_t val)
>> >>  {
>> >>   return (int64_t)*((int64_t *));
>> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>> >>*/
>> >>   u32 target_vblank;
>> >>  
>> >> + /**
>> >> +  * @scaling_filter:
>> >> +  *
>> >> +  * Scaling filter mode to be applied
>> >> +  */
>> >> + u32 scaling_filter;
>> >> +
>> >>   /**
>> >>* @async_flip:
>> >>*
>> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> >> index 3bcbe30339f0..efd6fd55770f 100644
>> >> --- a/include/drm/drm_mode_config.h
>> >> +++ b/include/drm/drm_mode_config.h
>> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
>> >>*/
>> >>   struct drm_property 

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

2019-10-24 Thread Daniel Vetter
On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
> On Tue, 22 Oct 2019, Daniel Vetter  wrote:
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> This patch adds a scaling filter mode porperty
> >> to allow:
> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> - A userspace to pick a desired effect while scaling.
> >> 
> >> This option will be particularly useful in the scenarios where
> >> Integer mode scaling is possible, and a UI client wants to pick
> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >> 
> >> There was a RFC patch series published, to discus the request to enable
> >> Integer mode scaling by some of the gaming communities, which can be
> >> found here:
> >> https://patchwork.freedesktop.org/series/66175/
> >> 
> >> Signed-off-by: Shashank Sharma 
> >
> > Two things:
> >
> > - needs real property docs for this in the kms property chapter
> > - would be good if we could somehow subsume the panel fitter prop into
> >   this? Or at least document possible interactions with that.
> >
> > Plus userspace ofc, recommended best practices is to add a Link: tag
> > pointing at the userspace patch series/merge request directly to the patch
> > that adds the new uapi.
> 
> I think Link: should only be used for referencing the patch that became
> the commit?

Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
herd here. But yeah maybe we want something else.
> 
> References: perhaps?

Or Userspace: to make it real obvious?
-Daniel

> 
> >
> > Cheers, Daniel
> >> ---
> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 
> >>  include/drm/drm_crtc.h| 26 ++
> >>  include/drm/drm_mode_config.h |  6 ++
> >>  3 files changed, 36 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 0d466d3b0809..883329453a86 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;
> >>} else if (property == config->degamma_lut_property) {
> >>ret = drm_atomic_replace_property_blob_from_id(dev,
> >>>degamma_lut,
> >> @@ -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 == config->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
> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -58,6 +58,25 @@ struct device_node;
> >>  struct dma_fence;
> >>  struct edid;
> >>  
> >> +enum drm_scaling_filters {
> >> +  DRM_SCALING_FILTER_DEFAULT,
> >> +  DRM_SCALING_FILTER_MEDIUM,
> >> +  DRM_SCALING_FILTER_BILINEAR,
> >> +  DRM_SCALING_FILTER_NN,
> >> +  DRM_SCALING_FILTER_NN_IS_ONLY,
> >> +  DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> +  DRM_SCALING_FILTER_INVALID,
> >> +};
> >> +
> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> >> +  { DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> +  { DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> +  { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> +  { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> >> +  { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> >> +  { DRM_SCALING_FILTER_INVALID, "Invalid" },
> >> +};
> >> +
> >>  static inline int64_t U642I64(uint64_t val)
> >>  {
> >>return (int64_t)*((int64_t *));
> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >> */
> >>u32 target_vblank;
> >>  
> >> +  /**
> >> +   * @scaling_filter:
> >> +   *
> >> +   * Scaling filter mode to be applied
> >> +   */
> >> +  u32 scaling_filter;
> >> +
> >>/**
> >> * @async_flip:
> >> *
> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> >> index 3bcbe30339f0..efd6fd55770f 100644
> >> --- a/include/drm/drm_mode_config.h
> >> +++ b/include/drm/drm_mode_config.h
> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
> >> */
> >>struct drm_property *modifiers_property;
> >>  
> >> +  /**
> >> +   * @scaling_filter_property: CRTC property to apply a particular filter
> >> +   * while scaling in panel fitter mode.
> >> +   */
> >> +  struct drm_property *scaling_filter_property;
> >> +
> >>/* cursor 

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

2019-10-23 Thread Jani Nikula
On Tue, 22 Oct 2019, Daniel Vetter  wrote:
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> This patch adds a scaling filter mode porperty
>> to allow:
>> - A driver/HW to showcase it's scaling filter capabilities.
>> - A userspace to pick a desired effect while scaling.
>> 
>> This option will be particularly useful in the scenarios where
>> Integer mode scaling is possible, and a UI client wants to pick
>> filters like Nearest-neighbor applied for non-blurry outputs.
>> 
>> There was a RFC patch series published, to discus the request to enable
>> Integer mode scaling by some of the gaming communities, which can be
>> found here:
>> https://patchwork.freedesktop.org/series/66175/
>> 
>> Signed-off-by: Shashank Sharma 
>
> Two things:
>
> - needs real property docs for this in the kms property chapter
> - would be good if we could somehow subsume the panel fitter prop into
>   this? Or at least document possible interactions with that.
>
> Plus userspace ofc, recommended best practices is to add a Link: tag
> pointing at the userspace patch series/merge request directly to the patch
> that adds the new uapi.

I think Link: should only be used for referencing the patch that became
the commit?

References: perhaps?

>
> Cheers, Daniel
>> ---
>>  drivers/gpu/drm/drm_atomic_uapi.c |  4 
>>  include/drm/drm_crtc.h| 26 ++
>>  include/drm/drm_mode_config.h |  6 ++
>>  3 files changed, 36 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
>> b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 0d466d3b0809..883329453a86 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;
>>  } else if (property == config->degamma_lut_property) {
>>  ret = drm_atomic_replace_property_blob_from_id(dev,
>>  >degamma_lut,
>> @@ -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 == config->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
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 5e9b15a0e8c5..94c5509474a8 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -58,6 +58,25 @@ struct device_node;
>>  struct dma_fence;
>>  struct edid;
>>  
>> +enum drm_scaling_filters {
>> +DRM_SCALING_FILTER_DEFAULT,
>> +DRM_SCALING_FILTER_MEDIUM,
>> +DRM_SCALING_FILTER_BILINEAR,
>> +DRM_SCALING_FILTER_NN,
>> +DRM_SCALING_FILTER_NN_IS_ONLY,
>> +DRM_SCALING_FILTER_EDGE_ENHANCE,
>> +DRM_SCALING_FILTER_INVALID,
>> +};
>> +
>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> +{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> +{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> +{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> +{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> +{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> +{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> +};
>> +
>>  static inline int64_t U642I64(uint64_t val)
>>  {
>>  return (int64_t)*((int64_t *));
>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>   */
>>  u32 target_vblank;
>>  
>> +/**
>> + * @scaling_filter:
>> + *
>> + * Scaling filter mode to be applied
>> + */
>> +u32 scaling_filter;
>> +
>>  /**
>>   * @async_flip:
>>   *
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 3bcbe30339f0..efd6fd55770f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>   */
>>  struct drm_property *modifiers_property;
>>  
>> +/**
>> + * @scaling_filter_property: CRTC property to apply a particular filter
>> + * while scaling in panel fitter mode.
>> + */
>> +struct drm_property *scaling_filter_property;
>> +
>>  /* cursor size */
>>  uint32_t cursor_width, cursor_height;
>>  
>> -- 
>> 2.17.1
>> 
>> ___
>> Intel-gfx mailing list
>> intel-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

2019-10-22 Thread Ville Syrjälä
On Tue, Oct 22, 2019 at 10:12:29AM +, Sharma, Shashank wrote:
> Hey Daniel, 
> 
> > -Original Message-
> > From: Daniel Vetter  On Behalf Of Daniel Vetter
> > Sent: Tuesday, October 22, 2019 3:39 PM
> > To: Sharma, Shashank 
> > Cc: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode 
> > property
> > 
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> > > This patch adds a scaling filter mode porperty to allow:
> > > - A driver/HW to showcase it's scaling filter capabilities.
> > > - A userspace to pick a desired effect while scaling.
> > >
> > > This option will be particularly useful in the scenarios where Integer
> > > mode scaling is possible, and a UI client wants to pick filters like
> > > Nearest-neighbor applied for non-blurry outputs.
> > >
> > > There was a RFC patch series published, to discus the request to
> > > enable Integer mode scaling by some of the gaming communities, which
> > > can be found here:
> > > https://patchwork.freedesktop.org/series/66175/
> > >
> > > Signed-off-by: Shashank Sharma 
> > 
> > Two things:
> > 
> > - needs real property docs for this in the kms property chapter
> Got it, 
> > - would be good if we could somehow subsume the panel fitter prop into
> >   this? Or at least document possible interactions with that.
> > 
> Sure, let me see what can I do here. 

The scaling mode prop has nothing really to do with the filter being
used. The scaling mode prop just provides a bad mechanism for
configuring the destination coordinates of the scaler.

Trying to shoehorn these things into one prop would be a mistake IMO.

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

2019-10-22 Thread Sharma, Shashank
Hey Daniel, 

> -Original Message-
> From: Daniel Vetter  On Behalf Of Daniel Vetter
> Sent: Tuesday, October 22, 2019 3:39 PM
> To: Sharma, Shashank 
> Cc: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode 
> property
> 
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> > This patch adds a scaling filter mode porperty to allow:
> > - A driver/HW to showcase it's scaling filter capabilities.
> > - A userspace to pick a desired effect while scaling.
> >
> > This option will be particularly useful in the scenarios where Integer
> > mode scaling is possible, and a UI client wants to pick filters like
> > Nearest-neighbor applied for non-blurry outputs.
> >
> > There was a RFC patch series published, to discus the request to
> > enable Integer mode scaling by some of the gaming communities, which
> > can be found here:
> > https://patchwork.freedesktop.org/series/66175/
> >
> > Signed-off-by: Shashank Sharma 
> 
> Two things:
> 
> - needs real property docs for this in the kms property chapter
Got it, 
> - would be good if we could somehow subsume the panel fitter prop into
>   this? Or at least document possible interactions with that.
> 
Sure, let me see what can I do here. 
> Plus userspace ofc, recommended best practices is to add a Link: tag pointing 
> at the
> userspace patch series/merge request directly to the patch that adds the new 
> uapi.
> 

Yep, WIP, will soon drop a link here. 

- Shashank
> Cheers, Daniel
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 
> >  include/drm/drm_crtc.h| 26 ++
> >  include/drm/drm_mode_config.h |  6 ++
> >  3 files changed, 36 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 0d466d3b0809..883329453a86 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;
> > } else if (property == config->degamma_lut_property) {
> > ret = drm_atomic_replace_property_blob_from_id(dev,
> > >degamma_lut,
> > @@ -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 == config->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
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> > 5e9b15a0e8c5..94c5509474a8 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -58,6 +58,25 @@ struct device_node;  struct dma_fence;  struct
> > edid;
> >
> > +enum drm_scaling_filters {
> > +   DRM_SCALING_FILTER_DEFAULT,
> > +   DRM_SCALING_FILTER_MEDIUM,
> > +   DRM_SCALING_FILTER_BILINEAR,
> > +   DRM_SCALING_FILTER_NN,
> > +   DRM_SCALING_FILTER_NN_IS_ONLY,
> > +   DRM_SCALING_FILTER_EDGE_ENHANCE,
> > +   DRM_SCALING_FILTER_INVALID,
> > +};
> > +
> > +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> > +   { DRM_SCALING_FILTER_DEFAULT, "Default" },
> > +   { DRM_SCALING_FILTER_MEDIUM, "Medium" },
> > +   { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> > +   { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> > +   { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> > +   { DRM_SCALING_FILTER_INVALID, "Invalid" }, };
> > +
> >  static inline int64_t U642I64(uint64_t val)  {
> > return (int64_t)*((int64_t *));
> > @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >  */
> > u32 target_vblank;
> >
> > +   /**
> > +* @scaling_filter:
> > +*
> > +* Scaling filter mode to be applied
> > +*/
> > +   u32 scaling_filter;
> > +
>

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

2019-10-22 Thread Daniel Vetter
On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> This patch adds a scaling filter mode porperty
> to allow:
> - A driver/HW to showcase it's scaling filter capabilities.
> - A userspace to pick a desired effect while scaling.
> 
> This option will be particularly useful in the scenarios where
> Integer mode scaling is possible, and a UI client wants to pick
> filters like Nearest-neighbor applied for non-blurry outputs.
> 
> There was a RFC patch series published, to discus the request to enable
> Integer mode scaling by some of the gaming communities, which can be
> found here:
> https://patchwork.freedesktop.org/series/66175/
> 
> Signed-off-by: Shashank Sharma 

Two things:

- needs real property docs for this in the kms property chapter
- would be good if we could somehow subsume the panel fitter prop into
  this? Or at least document possible interactions with that.

Plus userspace ofc, recommended best practices is to add a Link: tag
pointing at the userspace patch series/merge request directly to the patch
that adds the new uapi.

Cheers, Daniel
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 
>  include/drm/drm_crtc.h| 26 ++
>  include/drm/drm_mode_config.h |  6 ++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 0d466d3b0809..883329453a86 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;
>   } else if (property == config->degamma_lut_property) {
>   ret = drm_atomic_replace_property_blob_from_id(dev,
>   >degamma_lut,
> @@ -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 == config->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
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5e9b15a0e8c5..94c5509474a8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -58,6 +58,25 @@ struct device_node;
>  struct dma_fence;
>  struct edid;
>  
> +enum drm_scaling_filters {
> + DRM_SCALING_FILTER_DEFAULT,
> + DRM_SCALING_FILTER_MEDIUM,
> + DRM_SCALING_FILTER_BILINEAR,
> + DRM_SCALING_FILTER_NN,
> + DRM_SCALING_FILTER_NN_IS_ONLY,
> + DRM_SCALING_FILTER_EDGE_ENHANCE,
> + DRM_SCALING_FILTER_INVALID,
> +};
> +
> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> + { DRM_SCALING_FILTER_DEFAULT, "Default" },
> + { DRM_SCALING_FILTER_MEDIUM, "Medium" },
> + { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> + { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> + { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> + { DRM_SCALING_FILTER_INVALID, "Invalid" },
> +};
> +
>  static inline int64_t U642I64(uint64_t val)
>  {
>   return (int64_t)*((int64_t *));
> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>*/
>   u32 target_vblank;
>  
> + /**
> +  * @scaling_filter:
> +  *
> +  * Scaling filter mode to be applied
> +  */
> + u32 scaling_filter;
> +
>   /**
>* @async_flip:
>*
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 3bcbe30339f0..efd6fd55770f 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -914,6 +914,12 @@ struct drm_mode_config {
>*/
>   struct drm_property *modifiers_property;
>  
> + /**
> +  * @scaling_filter_property: CRTC property to apply a particular filter
> +  * while scaling in panel fitter mode.
> +  */
> + struct drm_property *scaling_filter_property;
> +
>   /* cursor size */
>   uint32_t cursor_width, cursor_height;
>  
> -- 
> 2.17.1
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel