Blurry outputs during upscaling the video buffer, is a generic problem of graphics industry. One of the major reason behind this blurriness is the interpolation of pixel values used by most of the upscaling hardwares.
Nearest-neighbor is a scaling mode, which works by filling in the missing color values in the upscaled image with that of the coordinate-mapped nearest source pixel value.
Nearest-neighbor can produce (almost) non-blurry scaling outputs when the scaling ratio is complete integer. For example: - input buffer resolution: 1280x720(HD) - output buffer resolution: 3840x2160(UHD/4K) - scaling ratio (h) = 3840/1280 = 3 - scaling ratio (v) = 2160/720 = 3
In such scenarios, we should try to pick Nearest-neighbor as scaling method when possible.
Many gaming communities have been asking for integer-mode scaling support, some links and background: https://software.intel.com/en-us/articles/integer-scaling-support-on-intel-g... http://tanalin.com/en/articles/lossless-scaling/ https://community.amd.com/thread/209107 https://www.nvidia.com/en-us/geforce/forums/game-ready-drivers/13/1002/featu...
This patch series adds support for programmable scaling filters, which can help a user to pick and enables NN scaling on Intel display hardware (ICL onwards). There is also one option to pick NN only when the scaling ratios are integer, to achieve Integer scaling, without(alsmost) any side effets.
there was an RFC series published for the same, which can be seen here: https://patchwork.freedesktop.org/series/66175/
Shashank Sharma (3): drm: Introduce scaling filter mode property drm/i915: Add support for scaling filters drm/i915: Handle nearest-neighbor scaling filter
drivers/gpu/drm/drm_atomic_uapi.c | 4 + drivers/gpu/drm/i915/display/intel_display.c | 161 +++++++++++++++++- .../drm/i915/display/intel_display_types.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 31 ++++ include/drm/drm_crtc.h | 26 +++ include/drm/drm_mode_config.h | 6 + 6 files changed, 230 insertions(+), 1 deletion(-)
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 shashank.sharma@intel.com --- 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, &state->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 *)&val); @@ -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;
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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hey Daniel,
-----Original Message----- From: Daniel Vetter daniel.vetter@ffwll.ch On Behalf Of Daniel Vetter Sent: Tuesday, October 22, 2019 3:39 PM To: Sharma, Shashank shashank.sharma@intel.com Cc: dri-devel@lists.freedesktop.org; intel-gfx@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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Tue, Oct 22, 2019 at 10:12:29AM +0000, Sharma, Shashank wrote:
Hey Daniel,
-----Original Message----- From: Daniel Vetter daniel.vetter@ffwll.ch On Behalf Of Daniel Vetter Sent: Tuesday, October 22, 2019 3:39 PM To: Sharma, Shashank shashank.sharma@intel.com Cc: dri-devel@lists.freedesktop.org; intel-gfx@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 shashank.sharma@intel.com
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.
On Tue, 22 Oct 2019, Daniel Vetter daniel@ffwll.ch 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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
On Tue, 22 Oct 2019, Daniel Vetter daniel@ffwll.ch 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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Jani Nikula, Intel Open Source Graphics Center
On Thu, 24 Oct 2019, Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
On Tue, 22 Oct 2019, Daniel Vetter daniel@ffwll.ch 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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Jani Nikula, Intel Open Source Graphics Center
On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
On Thu, 24 Oct 2019, Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
On Tue, 22 Oct 2019, Daniel Vetter daniel@ffwll.ch 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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Jani Nikula, Intel Open Source Graphics Center
-- Jani Nikula, Intel Open Source Graphics Center
On Thu, 24 Oct 2019, Daniel Vetter daniel@ffwll.ch wrote:
On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
On Thu, 24 Oct 2019, Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
On Tue, 22 Oct 2019, Daniel Vetter daniel@ffwll.ch 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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Jani Nikula, Intel Open Source Graphics Center
-- Jani Nikula, Intel Open Source Graphics Center
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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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
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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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
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 shashank.sharma@intel.com
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;@@ -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;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 *)&val);
@@ -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
On Tue, Oct 22, 2019 at 02:06:22PM +0000, 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 shashank.sharma@intel.com
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;@@ -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;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.
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 shashank.sharma@intel.com
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;@@ -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;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 *)&val);
@@ -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 --- 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
- Shashank
On Tue, 22 Oct 2019 20:48:02 +0530 "Sharma, Shashank" shashank.sharma@intel.com 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 shashank.sharma@intel.com
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
On Wed, Oct 23, 2019 at 10:34:05AM +0300, Pekka Paalanen wrote:
On Tue, 22 Oct 2019 20:48:02 +0530 "Sharma, Shashank" shashank.sharma@intel.com 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 shashank.sharma@intel.com
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.
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 shashank.sharma@intel.com
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;@@ -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;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 *)&val); @@ -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
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 shashank.sharma@intel.com
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;@@ -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;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 *)&val);
@@ -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
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 shashank.sharma@intel.com
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;@@ -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;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 *)&val);
@@ -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.
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 shashank.sharma@intel.com
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, &state->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); elsediff --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 *)&val); @@ -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.
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 shashank.sharma@intel.com
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, &state->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); elsediff --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 *)&val); @@ -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 filterA 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.
Sorry about that notice, not intentional :-(
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 shashank.sharma@intel.com
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, &state->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); elsediff --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 *)&val);
@@ -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 filterA 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 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.
This patch does the following: - Creates the CRTC property for scaling filter mode (for GEN11 and +). - Applies the chosen filter value while enabling the panel fitter. - Adds CRTC state readouts and comparisons.
Signed-off-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/i915/display/intel_display.c | 59 +++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 1a533ccdb54f..21993f9fd2ae 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5615,11 +5615,35 @@ static void skylake_scaler_disable(struct intel_crtc *crtc) skl_detach_scaler(crtc, i); }
+static u32 +icelake_get_scaler_filter(const struct intel_crtc_state *crtc_state) +{ + const struct drm_crtc_state *state = &crtc_state->base; + + switch (state->scaling_filter) { + case DRM_SCALING_FILTER_BILINEAR: + return PS_FILTER_BILINEAR; + case DRM_SCALING_FILTER_EDGE_ENHANCE: + return PS_FILTER_EDGE_ENHANCE; + case DRM_SCALING_FILTER_NN: + case DRM_SCALING_FILTER_NN_IS_ONLY: + return PS_FILTER_PROGRAMMED; + case DRM_SCALING_FILTER_INVALID: + DRM_ERROR("Ignoring invalid scaler filter mode\n"); + return PS_FILTER_MEDIUM; + case DRM_SCALING_FILTER_DEFAULT: + case DRM_SCALING_FILTER_MEDIUM: + default: + return PS_FILTER_MEDIUM; + } +} + static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); enum pipe pipe = crtc->pipe; + u32 scaler_filter; const struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
@@ -5640,9 +5664,11 @@ static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state) uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false); uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
+ scaler_filter = icelake_get_scaler_filter(crtc_state); + id = scaler_state->scaler_id; I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN | - PS_FILTER_MEDIUM | scaler_state->scalers[id].mode); + scaler_filter | scaler_state->scalers[id].mode); I915_WRITE_FW(SKL_PS_VPHASE(pipe, id), PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase)); I915_WRITE_FW(SKL_PS_HPHASE(pipe, id), @@ -12192,6 +12218,10 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, pipe_config->scaler_state.scaler_users, pipe_config->scaler_state.scaler_id);
+ if (INTEL_GEN(dev_priv) >= 11) + DRM_DEBUG_KMS("scaling_filter: %d\n", + pipe_config->base.scaling_filter); + if (HAS_GMCH(dev_priv)) DRM_DEBUG_KMS("gmch pfit: control: 0x%08x, ratios: 0x%08x, lvds border: 0x%08x\n", pipe_config->gmch_pfit.control, @@ -12858,6 +12888,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, }
PIPE_CONF_CHECK_I(scaler_state.scaler_id); + PIPE_CONF_CHECK_I(base.scaling_filter); PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
PIPE_CONF_CHECK_X(gamma_mode); @@ -14996,6 +15027,29 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, return ERR_PTR(ret); }
+static void icl_create_scaler_filter_property(struct intel_crtc *crtc, + struct intel_crtc_state *crtc_state) +{ + struct drm_property *prop; + struct drm_device *dev = crtc->base.dev; + struct drm_mode_config *mode_config = &dev->mode_config; + u8 size; + + if (mode_config->scaling_filter_property) + return; + + size = ARRAY_SIZE(drm_scaling_filter_enum_list); + prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE, + "SCALING_FILTERS", + drm_scaling_filter_enum_list, size); + if (!prop) { + DRM_ERROR("Failed to create scaling filter property\n"); + return; + } + + dev->mode_config.scaling_filter_property = prop; +} + static void intel_crtc_init_scalers(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state) { @@ -15016,6 +15070,9 @@ static void intel_crtc_init_scalers(struct intel_crtc *crtc, }
scaler_state->scaler_id = -1; + + if (INTEL_GEN(dev_priv) >= 11) + icl_create_scaler_filter_property(crtc, crtc_state); }
#define INTEL_CRTC_FUNCS \
This patch adds support to handle nearest-neighbor(NN) filter option for the panel fitter / pipe scaler.
Nearest-neighbor filter, when applied for integer upscaling ratios, can produce sharp and blurless outputs. This is pretty useful while upscaling the old games to higher resolution displays.
To enable NN: - we need to set the scaler filter select value to "programmed" - and then we need to program specific values to the scaler data registers.
Naturally, this patch also handles the Integer scaling scenarios. If the user selects scaler filter value as "DRM_SCALING_FILTER_NN_IS_ONLY", this means user wants to enable NN filter only when the upscaling ratios are complete integer.
PS: There are two 80 char warnings in this patch, which is left deliberately for the better readibility of register definision and to maintain the existing formatting of the file. Maintainers can suggest.
Signed-off-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/i915/display/intel_display.c | 102 ++++++++++++++++++ .../drm/i915/display/intel_display_types.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 31 ++++++ 3 files changed, 136 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 21993f9fd2ae..b882e9886fde 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5411,6 +5411,17 @@ u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_cosited) #define SKL_MIN_YUV_420_SRC_W 16 #define SKL_MIN_YUV_420_SRC_H 16
+static inline bool +scaling_ratio_is_integer(int src_w, int dst_w, int src_h, int dst_h) +{ + /* Integer mode scaling is applicable only for upscaling scenarios */ + if (dst_w < src_w || dst_h < src_h) + return false; + if (dst_w % src_w == 0 && dst_h % src_h == 0) + return true; + return false; +} + static int skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, unsigned int scaler_user, int *scaler_id, @@ -5445,6 +5456,15 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, return -EINVAL; }
+ /* + * If we are upscaling, and the scaling ratios are integer, we can + * pick nearest-neighbour method in HW for scaling, which produces + * blurless outputs in such scenarios. + */ + if (INTEL_GEN(dev_priv) >= 11 && + scaling_ratio_is_integer(src_w, dst_w, src_h, dst_h)) + scaler_state->integer_scaling = true; + /* * if plane is being disabled or scaler is no more required or force detach * - free scaler binded to this plane/crtc @@ -5615,6 +5635,86 @@ static void skylake_scaler_disable(struct intel_crtc *crtc) skl_detach_scaler(crtc, i); }
+static void +icl_setup_nearest_neighbor_filter(const struct intel_crtc_state *crtc_state) +{ + int count; + int phase; + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + int scaler_id = crtc_state->scaler_state.scaler_id; + enum pipe pipe = crtc->pipe; + + /* + * To setup nearest-neighbor integer scaling mode: + * Write 60 dwords: represnting 119 coefficients. + * + * Seven basic Coefficients are named from An......Gn. + * Value of every D'th coefficent must be 1, all others to be 0. + * + * 17 such phases of 7 such coefficients = 119 coefficients. + * Arrange these 119 coefficients in 60 dwords, 2 coefficient + * per dword, in the sequence shown below: + * + *+------------+--------------+ + *| B0 | A0 | + *+---------------------------+ + *| D0 = 1 | C0 | + *+---------------------------+ + *| F0 | E0 | + *+---------------------------+ + *| A1 | G0 | + *+---------------------------+ + *| C1 | B1 | + *+---------------------------+ + *| E1 | D1 = 1 | + *+---------------------------+ + *| ..... | ..... | + *+---------------------------+ + *| ...... | ...... | + *+---------------------------+ + *| Res | G16 | + *+------------+--------------+ + * + */ + + for (phase = 0; phase < 17; phase++) { + for (count = 0; count < 7; count++) { + u32 val = 0; + + /* Every D'th entry needs to be 1 */ + if (count == 3) + (phase % 2) ? (val = 1) : (val = (1 << 16)); + + I915_WRITE_FW(SKL_PS_COEF_INDEX_SET0(pipe, scaler_id), + phase * 17 + count); + I915_WRITE_FW(SKL_PS_COEF_DATA_SET0(pipe, scaler_id), + val); + + I915_WRITE_FW(SKL_PS_COEF_INDEX_SET1(pipe, scaler_id), + phase * 17 + count); + I915_WRITE_FW(SKL_PS_COEF_DATA_SET1(pipe, scaler_id), + val); + } + } +} + +static u32 icl_program_pfit_filter(const struct intel_crtc_state *crtc_state) +{ + const struct drm_crtc_state *state = &crtc_state->base; + const struct intel_crtc_scaler_state *scaler_state = + &crtc_state->scaler_state; + + if (state->scaling_filter == DRM_SCALING_FILTER_NN_IS_ONLY && + !scaler_state->integer_scaling) { + DRM_DEBUG_DRIVER("Scaling ratio not integer, not picking NN\n"); + return PS_FILTER_MEDIUM; + } + + icl_setup_nearest_neighbor_filter(crtc_state); + return PS_FILTER_PROGRAMMED; +} + static u32 icelake_get_scaler_filter(const struct intel_crtc_state *crtc_state) { @@ -5665,6 +5765,8 @@ static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state) uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
scaler_filter = icelake_get_scaler_filter(crtc_state); + if (scaler_filter == PS_FILTER_PROGRAMMED) + scaler_filter = icl_program_pfit_filter(crtc_state);
id = scaler_state->scaler_id; I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN | diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 40390d855815..8da499031bc2 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -620,6 +620,9 @@ struct intel_crtc_scaler_state {
/* scaler used by crtc for panel fitting purpose */ int scaler_id; + + /* indicate if scaling ratio is integer, to apply scaling filters */ + bool integer_scaling; };
/* drm_mode->private_flags */ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 1dc067fc57ab..f1ed5aa96ffd 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7114,6 +7114,7 @@ enum { #define PS_PLANE_SEL(plane) (((plane) + 1) << 25) #define PS_FILTER_MASK (3 << 23) #define PS_FILTER_MEDIUM (0 << 23) +#define PS_FILTER_PROGRAMMED (1 << 23) #define PS_FILTER_EDGE_ENHANCE (2 << 23) #define PS_FILTER_BILINEAR (3 << 23) #define PS_VERT3TAP (1 << 21) @@ -7190,6 +7191,24 @@ enum { #define _PS_ECC_STAT_2B 0x68AD0 #define _PS_ECC_STAT_1C 0x691D0
+#define _PS_COEF_SET0_INDEX_1A 0x68198 +#define _PS_COEF_SET0_INDEX_2A 0x68298 +#define _PS_COEF_SET0_INDEX_1B 0x68998 +#define _PS_COEF_SET0_INDEX_2B 0x68A98 +#define _PS_COEF_SET1_INDEX_1A 0x681A0 +#define _PS_COEF_SET1_INDEX_2A 0x682A0 +#define _PS_COEF_SET1_INDEX_1B 0x689A0 +#define _PS_COEF_SET1_INDEX_2B 0x68AA0 + +#define _PS_COEF_SET0_DATA_1A 0x6819C +#define _PS_COEF_SET0_DATA_2A 0x6829C +#define _PS_COEF_SET0_DATA_1B 0x6899C +#define _PS_COEF_SET0_DATA_2B 0x68A9C +#define _PS_COEF_SET1_DATA_1A 0x681A4 +#define _PS_COEF_SET1_DATA_2A 0x682A4 +#define _PS_COEF_SET1_DATA_1B 0x689A4 +#define _PS_COEF_SET1_DATA_2B 0x68AA4 + #define _ID(id, a, b) _PICK_EVEN(id, a, b) #define SKL_PS_CTRL(pipe, id) _MMIO_PIPE(pipe, \ _ID(id, _PS_1A_CTRL, _PS_2A_CTRL), \ @@ -7218,6 +7237,18 @@ enum { #define SKL_PS_ECC_STAT(pipe, id) _MMIO_PIPE(pipe, \ _ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A), \ _ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B)) +#define SKL_PS_COEF_DATA_SET0(pipe, id) _MMIO_PIPE(pipe, \ + _ID(id, _PS_COEF_SET0_DATA_1A, _PS_COEF_SET0_DATA_2A), \ + _ID(id, _PS_COEF_SET0_DATA_1B, _PS_COEF_SET0_DATA_1B)) +#define SKL_PS_COEF_DATA_SET1(pipe, id) _MMIO_PIPE(pipe, \ + _ID(id, _PS_COEF_SET1_DATA_1A, _PS_COEF_SET1_DATA_2A), \ + _ID(id, _PS_COEF_SET1_DATA_1B, _PS_COEF_SET1_DATA_1B)) +#define SKL_PS_COEF_INDEX_SET0(pipe, id) _MMIO_PIPE(pipe, \ + _ID(id, _PS_COEF_SET0_INDEX_1A, _PS_COEF_SET0_INDEX_2A), \ + _ID(id, _PS_COEF_SET0_INDEX_1B, _PS_COEF_SET0_INDEX_1B)) +#define SKL_PS_COEF_INDEX_SET1(pipe, id) _MMIO_PIPE(pipe, \ + _ID(id, _PS_COEF_SET1_INDEX_1A, _PS_COEF_SET1_INDEX_2A), \ + _ID(id, _PS_COEF_SET1_INDEX_1B, _PS_COEF_SET1_INDEX_1B))
/* legacy palette */ #define _LGC_PALETTE_A 0x4a000
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Harry Wentland -
Jani Nikula -
Mihail Atanassov -
Pekka Paalanen -
Sharma, Shashank -
Shashank Sharma
-
Ville Syrjälä