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) {
} else if (property == config->degamma_lut_property) { ret = drm_atomic_replace_property_blob_from_id(dev, &state->degamma_lut,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)
else if (crtc->funcs->atomic_get_property) return crtc->funcs->atomic_get_property(crtc, state, property, val); else*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.