Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property
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
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
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
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
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: [PATCH 1/3] drm: Introduce scaling filter mode property
On Wed, Oct 23, 2019 at 10:34:05AM +0300, Pekka Paalanen wrote:
> On Tue, 22 Oct 2019 20:48:02 +0530
> "Sharma, Shashank" wrote:
>
> > Hello Ville,
> >
> > Thanks for the comments, mine inline.
> >
> >
> > On 10/22/2019 5:50 PM, Ville Syrjälä 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
> > >> ---
> > >> 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/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,
> > > Please use real words.
> > Yes, I saw that coming. It was getting difficult with the 80 char stuff,
> > it was even more difficult while using it :). But let me see how better
> > can I do here.
> > >> +DRM_SCALING_FILTER_NN_IS_ONLY,
> > > Not a big fan of this. I'd just add the explicit nearest filter
> > > and leave the decision whether to use it to userspace.
> > Agree, that's also one option. I was thinking to make it convenient for
> > userspace, For example if a compositor just want to checkout NN only
> > when its beneficial (like old gaming scenarios) but doesn't have enough
> > information (or intent), it can leave it to kernel too. But I agree,
> > this can cause corner cases. Let's discuss and see if we need it at all,
> > as you mentioned.
>
> Hi,
>
> how could the kernel have more information or knowledge of intent than
> a display server? I cannot see how, because everything the kernel gets
> comes through the display server.
The only exception is due to that annoying scaling mode property.
Currently userspace just tells the kernel "center vs. fullscreen vs.
aspect" so in theory only the kernel knows the actual output size
(though userspace should be able to calculate it as well).
But maybe we can just avoid that little issue by also exposing the
"TV" margin properties on local panels, at which point userspace
can be more explicit about what it wants.
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
On Tue, 22 Oct 2019 20:48:02 +0530
"Sharma, Shashank" wrote:
> Hello Ville,
>
> Thanks for the comments, mine inline.
>
>
> On 10/22/2019 5:50 PM, Ville Syrjälä 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
> >> ---
> >> 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/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,
> > Please use real words.
> Yes, I saw that coming. It was getting difficult with the 80 char stuff,
> it was even more difficult while using it :). But let me see how better
> can I do here.
> >> + DRM_SCALING_FILTER_NN_IS_ONLY,
> > Not a big fan of this. I'd just add the explicit nearest filter
> > and leave the decision whether to use it to userspace.
> Agree, that's also one option. I was thinking to make it convenient for
> userspace, For example if a compositor just want to checkout NN only
> when its beneficial (like old gaming scenarios) but doesn't have enough
> information (or intent), it can leave it to kernel too. But I agree,
> this can cause corner cases. Let's discuss and see if we need it at all,
> as you mentioned.
Hi,
how could the kernel have more information or knowledge of intent than
a display server? I cannot see how, because everything the kernel gets
comes through the display server.
I agree with Ville here.
Thanks,
pq
pgpQhimUfi5wd.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
On Tue, Oct 22, 2019 at 02:06:22PM +, Harry Wentland wrote:
>
>
> On 2019-10-22 8:20 a.m., Ville Syrjälä 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
> >> ---
> >> 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,
> >
> > Please use real words.
> >
> >> + DRM_SCALING_FILTER_NN_IS_ONLY,
> >
> > Not a big fan of this. I'd just add the explicit nearest filter
> > and leave the decision whether to use it to userspace.
> >
> >> + DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> + DRM_SCALING_FILTER_INVALID,
> >
> > That invalid enum value seems entirely pointless.
> >
> > This set of filters is pretty arbitrary. It doesn't even cover all
> > Intel hw. I would probably just leave it at "default+linear+nearest"
> > initially. Exposing more vague hw specific choices needs more though,
> > and I'd prefer not to spend those brain cells until a real use case
> > emerges.
> >
>
> FWIW, AMD HW allows us to specify a number of horizontal and vertical
> taps for scaling. Number of taps are limited by our linebuffer size. The
> default is 4 in each dimension but you could have 2 v_taps and 4 h_taps
> if your're running a large horizontal resolution on some ASICs.
>
> I'm not sure it makes sense to define filters here that aren't used. It
> sounds like default and nearest neighbour would suffice for now in order
> to support integer scaling.
Yeah, even linear is somewhat questionable since we don't have clear
need for it. Although given that it is well defined it's much less
of a problem than a bunch of the other proposed filters.
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
On Tue, Oct 22, 2019 at 08:51:20PM +0530, Sharma, Shashank wrote:
>
> On 10/22/2019 5:56 PM, Ville Syrjälä 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
> >> ---
> >> 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;
> > This needs to per-crtc/plane.
>
> I am not getting this, why ? It's not different than any other CRTC
> property like gamma/CSC etc, where we just attach them to corresponding
> CRTC, isn't it ?
Different crtcs/planes can have different capabilities, and so not all
of them may want to expose all the possible filters.
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
Hello Harry,
Thanks for your comments, please find mine inline.
On 10/22/2019 7:36 PM, Harry Wentland wrote:
On 2019-10-22 8:20 a.m., Ville Syrjälä 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
---
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,
Please use real words.
+ DRM_SCALING_FILTER_NN_IS_ONLY,
Not a big fan of this. I'd just add the explicit nearest filter
and leave the decision whether to use it to userspace.
+ DRM_SCALING_FILTER_EDGE_ENHANCE,
+ DRM_SCALING_FILTER_INVALID,
That invalid enum value seems entirely pointless.
This set of filters is pretty arbitrary. It doesn't even cover all
Intel hw. I would probably just leave it at "default+linear+nearest"
initially. Exposing more vague hw specific choices needs more though,
and I'd prefer not to spend those brain cells until a real use case
emerges.
FWIW, AMD HW allows us to specify a number of horizontal and vertical
taps for scaling. Number of taps are limited by our linebuffer size. The
default is 4 in each dimension but you could have 2 v_taps and 4 h_taps
if your're running a large horizontal resolution on some ASICs.
I'm not sure it makes sense to define filters here that aren't used. It
sounds like default and nearest neighbour would suffice for now in order
to support integer scaling.
Agree, this seems to be a consistent feedback from the community, I will
probably start with a smaller set of most popular/used ones.
- Shashank
Harry
+};
+
+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;
We have an enum so should use it. The documentation should probably
point out that this applies to full crtc scaling only, not plane
scaling.
+
/**
* @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 {
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
Hello Mihail,
Thanks for your review, my comments inline.
On 10/22/2019 6:56 PM, Mihail Atanassov wrote:
Hi Shashank,
On Tuesday, 22 October 2019 10:59:44 BST 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
---
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,
This one looks to be missing a stringified version below.
Oh yes, it did. Thanks for pointing this out.
Just wanted
to jump in early and suggest dropping it from the scaling filter enum.
Edge enhancement is orthogonal to the mode used for scaling, at least
on komeda HW, so we wouldn't be able to make the best use of the
property in its current form.
Yes, Ville also suggested similar, I guess we can start with the smaller
set.
+ 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
A scaling filter option can also be useful for scaling individual
planes, do you have any plans to extend the property's applications
in that direction?
Yes, the second stage of development should contain the plane level
filtering too, but as you know, that would be a complex thing to handle,
as planes apply filter pre-blending. We need to discus that (in a
parallel thread :)) how should that look like.
- Shashank
+ * while scaling in panel fitter mode.
+ */
+ struct drm_property *scaling_filter_property;
+
/* cursor size */
uint32_t cursor_width, cursor_height;
--
Mihail
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
On 10/22/2019 5:56 PM, Ville Syrjälä 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
---
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;
This needs to per-crtc/plane.
I am not getting this, why ? It's not different than any other CRTC
property like gamma/CSC etc, where we just attach them to corresponding
CRTC, isn't it ?
- Shashank
+
/* cursor size */
uint32_t cursor_width, cursor_height;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
Hello Ville,
Thanks for the comments, mine inline.
On 10/22/2019 5:50 PM, Ville Syrjälä 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
---
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,
Please use real words.
Yes, I saw that coming. It was getting difficult with the 80 char stuff,
it was even more difficult while using it :). But let me see how better
can I do here.
+ DRM_SCALING_FILTER_NN_IS_ONLY,
Not a big fan of this. I'd just add the explicit nearest filter
and leave the decision whether to use it to userspace.
Agree, that's also one option. I was thinking to make it convenient for
userspace, For example if a compositor just want to checkout NN only
when its beneficial (like old gaming scenarios) but doesn't have enough
information (or intent), it can leave it to kernel too. But I agree,
this can cause corner cases. Let's discuss and see if we need it at all,
as you mentioned.
+ DRM_SCALING_FILTER_EDGE_ENHANCE,
+ DRM_SCALING_FILTER_INVALID,
That invalid enum value seems entirely pointless.
I was thinking about loops and all, but yeah, we can remove it.
This set of filters is pretty arbitrary. It doesn't even cover all
Intel hw. I would probably just leave it at "default+linear+nearest"
initially. Exposing more vague hw specific choices needs more though,
and I'd prefer not to spend those brain cells until a real use case
emerges.
Sure, lets start with smaller set.
+};
+
+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;
We have an enum so should use it.
Got it.
The documentation should probably
point out that this applies to full crtc scaling only, not plane
scaling.
The comment is actually with the property, Here I think its clear that
it's for CRTC scaling, as its a part of CRTC state.
+
/**
* @async_flip:
*
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 3bcbe30339f0..efd6fd55770f 100644
---
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
On 2019-10-22 8:20 a.m., Ville Syrjälä 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
>> ---
>> 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,
>
> Please use real words.
>
>> +DRM_SCALING_FILTER_NN_IS_ONLY,
>
> Not a big fan of this. I'd just add the explicit nearest filter
> and leave the decision whether to use it to userspace.
>
>> +DRM_SCALING_FILTER_EDGE_ENHANCE,
>> +DRM_SCALING_FILTER_INVALID,
>
> That invalid enum value seems entirely pointless.
>
> This set of filters is pretty arbitrary. It doesn't even cover all
> Intel hw. I would probably just leave it at "default+linear+nearest"
> initially. Exposing more vague hw specific choices needs more though,
> and I'd prefer not to spend those brain cells until a real use case
> emerges.
>
FWIW, AMD HW allows us to specify a number of horizontal and vertical
taps for scaling. Number of taps are limited by our linebuffer size. The
default is 4 in each dimension but you could have 2 v_taps and 4 h_taps
if your're running a large horizontal resolution on some ASICs.
I'm not sure it makes sense to define filters here that aren't used. It
sounds like default and nearest neighbour would suffice for now in order
to support integer scaling.
Harry
>> +};
>> +
>> +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;
>
> We have an enum so should use it. The documentation should probably
> point out that this applies to full crtc scaling only, not plane
> scaling.
>
>> +
>> /**
>> * @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;
>>
>> +/**
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
On Tuesday, 22 October 2019 14:26:38 BST Mihail Atanassov wrote:
> Hi Shashank,
>
> On Tuesday, 22 October 2019 10:59:44 BST 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
> > ---
> > 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,
>
> This one looks to be missing a stringified version below. Just wanted
> to jump in early and suggest dropping it from the scaling filter enum.
> Edge enhancement is orthogonal to the mode used for scaling, at least
> on komeda HW, so we wouldn't be able to make the best use of the
> property in its current form.
>
> > + 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
>
> A scaling filter option can also be useful for scaling individual
> planes, do you have any plans to extend the property's applications
> in that direction?
>
> > + * while scaling in panel fitter mode.
> > + */
> > + struct drm_property *scaling_filter_property;
> > +
> > /* cursor size */
> > uint32_t cursor_width, cursor_height;
> >
> >
>
>
> --
> Mihail
>
>
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
>
Re: [PATCH 1/3] drm: Introduce scaling filter mode property
Hi Shashank,
On Tuesday, 22 October 2019 10:59:44 BST 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
> ---
> 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,
This one looks to be missing a stringified version below. Just wanted
to jump in early and suggest dropping it from the scaling filter enum.
Edge enhancement is orthogonal to the mode used for scaling, at least
on komeda HW, so we wouldn't be able to make the best use of the
property in its current form.
> + 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
A scaling filter option can also be useful for scaling individual
planes, do you have any plans to extend the property's applications
in that direction?
> + * while scaling in panel fitter mode.
> + */
> + struct drm_property *scaling_filter_property;
> +
> /* cursor size */
> uint32_t cursor_width, cursor_height;
>
>
--
Mihail
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
Re: [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
> ---
> 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;
This needs to per-crtc/plane.
> +
> /* cursor size */
> uint32_t cursor_width, cursor_height;
>
> --
> 2.17.1
--
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
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: [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
> ---
> 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,
Please use real words.
> + DRM_SCALING_FILTER_NN_IS_ONLY,
Not a big fan of this. I'd just add the explicit nearest filter
and leave the decision whether to use it to userspace.
> + DRM_SCALING_FILTER_EDGE_ENHANCE,
> + DRM_SCALING_FILTER_INVALID,
That invalid enum value seems entirely pointless.
This set of filters is pretty arbitrary. It doesn't even cover all
Intel hw. I would probably just leave it at "default+linear+nearest"
initially. Exposing more vague hw specific choices needs more though,
and I'd prefer not to spend those brain cells until a real use case
emerges.
> +};
> +
> +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;
We have an enum so should use it. The documentation should probably
point out that this applies to full crtc scaling only, not plane
scaling.
> +
> /**
>* @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
--
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
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
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
[PATCH 1/3] drm: Introduce scaling filter mode property
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
---
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
