Op 21-09-18 om 18:40 schreef Matt Roper: > On Fri, Sep 21, 2018 at 04:44:37PM +0200, Maarten Lankhorst wrote: >> On skylake we can switch to a high quality scaler mode when only 1 out >> of 2 scalers are used, but on GLK and later bit 28 has a different >> meaning. Don't set it, and make clear the distinction between >> SKL and later PS values. >> >> Changes since v1: >> - Add missing break statement. >> >> Signed-off-by: Maarten Lankhorst <[email protected]> >> Reviewed-by: Matt Roper <[email protected]> #v1 > R-b for v2 as well. > >> --- >> drivers/gpu/drm/i915/i915_reg.h | 7 +- >> drivers/gpu/drm/i915/intel_atomic.c | 109 +++++++++++++++------------ >> drivers/gpu/drm/i915/intel_display.c | 2 +- >> 3 files changed, 67 insertions(+), 51 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 4948b352bf4c..e7e6ca7f9665 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -6850,11 +6850,12 @@ enum { >> #define _PS_2B_CTRL 0x68A80 >> #define _PS_1C_CTRL 0x69180 >> #define PS_SCALER_EN (1 << 31) >> -#define PS_SCALER_MODE_MASK (3 << 28) >> -#define PS_SCALER_MODE_DYN (0 << 28) >> -#define PS_SCALER_MODE_HQ (1 << 28) >> +#define SKL_PS_SCALER_MODE_MASK (3 << 28) >> +#define SKL_PS_SCALER_MODE_DYN (0 << 28) >> +#define SKL_PS_SCALER_MODE_HQ (1 << 28) >> #define SKL_PS_SCALER_MODE_NV12 (2 << 28) >> #define PS_SCALER_MODE_PLANAR (1 << 29) >> +#define PS_SCALER_MODE_PACKED (0 << 29) >> #define PS_PLANE_SEL_MASK (7 << 25) >> #define PS_PLANE_SEL(plane) (((plane) + 1) << 25) >> #define PS_FILTER_MASK (3 << 23) >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c >> b/drivers/gpu/drm/i915/intel_atomic.c >> index b04952bacf77..c23a6ec40f60 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic.c >> +++ b/drivers/gpu/drm/i915/intel_atomic.c >> @@ -203,6 +203,63 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, >> drm_atomic_helper_crtc_destroy_state(crtc, state); >> } >> >> +static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state >> *scaler_state, >> + int num_scalers_need, struct intel_crtc >> *intel_crtc, >> + const char *name, int idx, >> + struct intel_plane_state *plane_state, >> + int *scaler_id) >> +{ >> + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); >> + int j; >> + u32 mode; >> + >> + if (*scaler_id < 0) { >> + /* find a free scaler */ >> + for (j = 0; j < intel_crtc->num_scalers; j++) { >> + if (scaler_state->scalers[j].in_use) >> + continue; >> + >> + *scaler_id = j; >> + scaler_state->scalers[*scaler_id].in_use = 1; >> + break; >> + } >> + } >> + >> + if (WARN(*scaler_id < 0, "Cannot find scaler for %s:%d\n", name, idx)) >> + return; >> + >> + /* set scaler mode */ >> + if (plane_state && plane_state->base.fb && >> + plane_state->base.fb->format->is_yuv && >> + plane_state->base.fb->format->num_planes > 1) { >> + if (INTEL_GEN(dev_priv) == 9 && >> + !IS_GEMINILAKE(dev_priv)) >> + mode = SKL_PS_SCALER_MODE_NV12; >> + else >> + mode = PS_SCALER_MODE_PLANAR; >> + >> + } else if (INTEL_GEN(dev_priv) > 9 || IS_GEMINILAKE(dev_priv)) { >> + mode = PS_SCALER_MODE_PACKED; >> + } else if (num_scalers_need == 1 && intel_crtc->num_scalers > 1) { >> + /* >> + * when only 1 scaler is in use on a pipe with 2 scalers >> + * scaler 0 operates in high quality (HQ) mode. >> + * In this case use scaler 0 to take advantage of HQ mode >> + */ >> + scaler_state->scalers[*scaler_id].in_use = 0; >> + *scaler_id = 0; >> + scaler_state->scalers[0].in_use = 1; >> + mode = SKL_PS_SCALER_MODE_HQ; >> + } else { >> + mode = SKL_PS_SCALER_MODE_DYN; >> + } >> + >> + DRM_DEBUG_KMS("Attached scaler id %u.%u to %s:%d\n", >> + intel_crtc->pipe, *scaler_id, name, idx); >> + scaler_state->scalers[*scaler_id].mode = mode; >> +} >> + >> + >> /** >> * intel_atomic_setup_scalers() - setup scalers for crtc per staged requests >> * @dev_priv: i915 device >> @@ -232,7 +289,7 @@ int intel_atomic_setup_scalers(struct drm_i915_private >> *dev_priv, >> struct drm_atomic_state *drm_state = crtc_state->base.state; >> struct intel_atomic_state *intel_state = >> to_intel_atomic_state(drm_state); >> int num_scalers_need; >> - int i, j; >> + int i; >> >> num_scalers_need = hweight32(scaler_state->scaler_users); >> >> @@ -304,59 +361,17 @@ int intel_atomic_setup_scalers(struct drm_i915_private >> *dev_priv, >> idx = plane->base.id; >> >> /* plane on different crtc cannot be a scaler user of >> this crtc */ >> - if (WARN_ON(intel_plane->pipe != intel_crtc->pipe)) { >> + if (WARN_ON(intel_plane->pipe != intel_crtc->pipe)) >> continue; >> - } >> >> plane_state = >> intel_atomic_get_new_plane_state(intel_state, >> >> intel_plane); >> scaler_id = &plane_state->scaler_id; >> } >> >> - if (*scaler_id < 0) { >> - /* find a free scaler */ >> - for (j = 0; j < intel_crtc->num_scalers; j++) { >> - if (!scaler_state->scalers[j].in_use) { >> - scaler_state->scalers[j].in_use = 1; >> - *scaler_id = j; >> - DRM_DEBUG_KMS("Attached scaler id %u.%u >> to %s:%d\n", >> - intel_crtc->pipe, *scaler_id, >> name, idx); >> - break; >> - } >> - } >> - } >> - >> - if (WARN_ON(*scaler_id < 0)) { >> - DRM_DEBUG_KMS("Cannot find scaler for %s:%d\n", name, >> idx); >> - continue; >> - } >> - >> - /* set scaler mode */ >> - if ((INTEL_GEN(dev_priv) >= 9) && >> - plane_state && plane_state->base.fb && >> - plane_state->base.fb->format->format == >> - DRM_FORMAT_NV12) { >> - if (INTEL_GEN(dev_priv) == 9 && >> - !IS_GEMINILAKE(dev_priv) && >> - !IS_SKYLAKE(dev_priv)) >> - scaler_state->scalers[*scaler_id].mode = >> - SKL_PS_SCALER_MODE_NV12; >> - else >> - scaler_state->scalers[*scaler_id].mode = >> - PS_SCALER_MODE_PLANAR; >> - } else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) >> { >> - /* >> - * when only 1 scaler is in use on either pipe A or B, >> - * scaler 0 operates in high quality (HQ) mode. >> - * In this case use scaler 0 to take advantage of HQ >> mode >> - */ >> - *scaler_id = 0; >> - scaler_state->scalers[0].in_use = 1; >> - scaler_state->scalers[0].mode = PS_SCALER_MODE_HQ; >> - scaler_state->scalers[1].in_use = 0; >> - } else { >> - scaler_state->scalers[*scaler_id].mode = >> PS_SCALER_MODE_DYN; >> - } >> + intel_atomic_setup_scaler(scaler_state, num_scalers_need, >> + intel_crtc, name, idx, >> + plane_state, scaler_id); >> } >> >> return 0; >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index 02374185b993..931898013506 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -13963,7 +13963,7 @@ static void intel_crtc_init_scalers(struct >> intel_crtc *crtc, >> struct intel_scaler *scaler = &scaler_state->scalers[i]; >> >> scaler->in_use = 0; >> - scaler->mode = PS_SCALER_MODE_DYN; >> + scaler->mode = 0; >> } >> >> scaler_state->scaler_id = -1; >> -- >> 2.18.0 >> Thanks, pushed the series. :)
~Maarten _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
