| Message ID | 20181018115134.9061-6-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gen11: Add support for the NV12 format. | expand |
On Thu, Oct 18, 2018 at 01:51:31PM +0200, Maarten Lankhorst wrote: > The first 3 planes (primary, sprite 0 and 1) have a dedicated chroma > upsampler to upscale YUV420 to YUV444 and the scaler should only be > used for upscaling. Because of this we shouldn't program the scalers > in planar mode if NV12 and the chroma upsampler are used. Instead > program the scalers like on normal planes. > > Sprite 2 and 3 have no dedicated scaler, and need to program the > selected Y plane in the scaler mode. > > Changes since v1: > - Make the comment less confusing. > Changes since v2: > - Fix checkpatch warning (Matt) > - gen10- -> Pre-gen11 (Ville) > - PS_SCALER_MODE_PACKED -> PS_SCALER_MODE_NORMAL. (Matt) > - Add comment about scaler mode in intel_atomic_setup_scaler(). (Matt) > - Rename need_scaling to need_scaler. (Matt) > - Move the crtc need_scaling check to skl_update_scaler_crtc(). > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> I like it. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++- > drivers/gpu/drm/i915/intel_atomic.c | 16 +++++++++--- > drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++-------------- > drivers/gpu/drm/i915/intel_drv.h | 8 ++++++ > drivers/gpu/drm/i915/intel_sprite.c | 3 ++- > 5 files changed, 44 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 81f1c601987d..8a81e7c2fe25 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6819,7 +6819,7 @@ enum { > #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_SCALER_MODE_NORMAL (0 << 29) > #define PS_PLANE_SEL_MASK (7 << 25) > #define PS_PLANE_SEL(plane) (((plane) + 1) << 25) > #define PS_FILTER_MASK (3 << 23) > @@ -6836,6 +6836,8 @@ enum { > #define PS_VADAPT_MODE_LEAST_ADAPT (0 << 5) > #define PS_VADAPT_MODE_MOD_ADAPT (1 << 5) > #define PS_VADAPT_MODE_MOST_ADAPT (3 << 5) > +#define PS_PLANE_Y_SEL_MASK (7 << 5) > +#define PS_PLANE_Y_SEL(plane) (((plane) + 1) << 5) > > #define _PS_PWR_GATE_1A 0x68160 > #define _PS_PWR_GATE_2A 0x68260 > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index 20bfc89c652c..7238ab615548 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -233,13 +233,23 @@ static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_sta > 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)) > + !IS_GEMINILAKE(dev_priv)) { > mode = SKL_PS_SCALER_MODE_NV12; > - else > + } else if (icl_is_hdr_plane(to_intel_plane(plane_state->base.plane))) { > + /* > + * On gen11+'s HDR planes we only use the scaler for > + * scaling. They have a dedicated chroma upsampler, so > + * we don't need the scaler to upsample the UV plane. > + */ > + mode = PS_SCALER_MODE_NORMAL; > + } else { > mode = PS_SCALER_MODE_PLANAR; > > + if (plane_state->linked_plane) > + mode |= PS_PLANE_Y_SEL(plane_state->linked_plane->id); > + } > } else if (INTEL_GEN(dev_priv) > 9 || IS_GEMINILAKE(dev_priv)) { > - mode = PS_SCALER_MODE_PACKED; > + mode = PS_SCALER_MODE_NORMAL; > } else if (num_scalers_need == 1 && intel_crtc->num_scalers > 1) { > /* > * when only 1 scaler is in use on a pipe with 2 scalers > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index cbb3fb1d5ad4..5997097177e9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4836,8 +4836,7 @@ static int > skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > unsigned int scaler_user, int *scaler_id, > int src_w, int src_h, int dst_w, int dst_h, > - bool plane_scaler_check, > - uint32_t pixel_format) > + const struct drm_format_info *format, bool need_scaler) > { > struct intel_crtc_scaler_state *scaler_state = > &crtc_state->scaler_state; > @@ -4846,22 +4845,14 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); > const struct drm_display_mode *adjusted_mode = > &crtc_state->base.adjusted_mode; > - int need_scaling; > > /* > * Src coordinates are already rotated by 270 degrees for > * the 90/270 degree plane rotation cases (to match the > * GTT mapping), hence no need to account for rotation here. > */ > - need_scaling = src_w != dst_w || src_h != dst_h; > - > - if (plane_scaler_check) > - if (pixel_format == DRM_FORMAT_NV12) > - need_scaling = true; > - > - if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 && > - scaler_user == SKL_CRTC_INDEX) > - need_scaling = true; > + if (src_w != dst_w || src_h != dst_h) > + need_scaler = true; > > /* > * Scaling/fitting not supported in IF-ID mode in GEN9+ > @@ -4870,7 +4861,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > * for NV12. > */ > if (INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable && > - need_scaling && adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) { > + need_scaler && adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) { > DRM_DEBUG_KMS("Pipe/Plane scaling not supported with IF-ID mode\n"); > return -EINVAL; > } > @@ -4885,7 +4876,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > * update to free the scaler is done in plane/panel-fit programming. > * For this purpose crtc/plane_state->scaler_id isn't reset here. > */ > - if (force_detach || !need_scaling) { > + if (force_detach || !need_scaler) { > if (*scaler_id >= 0) { > scaler_state->scaler_users &= ~(1 << scaler_user); > scaler_state->scalers[*scaler_id].in_use = 0; > @@ -4899,7 +4890,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > return 0; > } > > - if (plane_scaler_check && pixel_format == DRM_FORMAT_NV12 && > + if (format && format->format == DRM_FORMAT_NV12 && > (src_h < SKL_MIN_YUV_420_SRC_H || src_w < SKL_MIN_YUV_420_SRC_W)) { > DRM_DEBUG_KMS("NV12: src dimensions not met\n"); > return -EINVAL; > @@ -4942,12 +4933,16 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > int skl_update_scaler_crtc(struct intel_crtc_state *state) > { > const struct drm_display_mode *adjusted_mode = &state->base.adjusted_mode; > + bool need_scaler = false; > + > + if (state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) > + need_scaler = true; > > return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX, > &state->scaler_state.scaler_id, > state->pipe_src_w, state->pipe_src_h, > adjusted_mode->crtc_hdisplay, > - adjusted_mode->crtc_vdisplay, false, 0); > + adjusted_mode->crtc_vdisplay, NULL, need_scaler); > } > > /** > @@ -4962,13 +4957,17 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) > static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, > struct intel_plane_state *plane_state) > { > - > struct intel_plane *intel_plane = > to_intel_plane(plane_state->base.plane); > struct drm_framebuffer *fb = plane_state->base.fb; > int ret; > - > bool force_detach = !fb || !plane_state->base.visible; > + bool need_scaler = false; > + > + /* Pre-gen11 and SDR planes always need a scaler for planar formats. */ > + if (!icl_is_hdr_plane(intel_plane) && > + fb && fb->format->format == DRM_FORMAT_NV12) > + need_scaler = true; > > ret = skl_update_scaler(crtc_state, force_detach, > drm_plane_index(&intel_plane->base), > @@ -4977,7 +4976,7 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, > drm_rect_height(&plane_state->base.src) >> 16, > drm_rect_width(&plane_state->base.dst), > drm_rect_height(&plane_state->base.dst), > - fb ? true : false, fb ? fb->format->format : 0); > + fb ? fb->format : NULL, need_scaler); > > if (ret || plane_state->scaler_id < 0) > return ret; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 272de906a001..f1d196bcdb66 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2211,6 +2211,14 @@ static inline bool icl_is_nv12_y_plane(enum plane_id id) > return false; > } > > +static inline bool icl_is_hdr_plane(struct intel_plane *plane) > +{ > + if (INTEL_GEN(to_i915(plane->base.dev)) < 11) > + return false; > + > + return plane->id < PLANE_SPRITE2; > +} > + > /* intel_tv.c */ > void intel_tv_init(struct drm_i915_private *dev_priv); > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 7cd59eee5cad..a2a4328107b6 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -332,7 +332,8 @@ skl_program_scaler(struct drm_i915_private *dev_priv, > crtc_h--; > > /* TODO: handle sub-pixel coordinates */ > - if (plane_state->base.fb->format->format == DRM_FORMAT_NV12) { > + if (plane_state->base.fb->format->format == DRM_FORMAT_NV12 && > + !icl_is_hdr_plane(plane)) { > y_hphase = skl_scaler_calc_phase(1, false); > y_vphase = skl_scaler_calc_phase(1, false); > > -- > 2.19.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 81f1c601987d..8a81e7c2fe25 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6819,7 +6819,7 @@ enum { #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_SCALER_MODE_NORMAL (0 << 29) #define PS_PLANE_SEL_MASK (7 << 25) #define PS_PLANE_SEL(plane) (((plane) + 1) << 25) #define PS_FILTER_MASK (3 << 23) @@ -6836,6 +6836,8 @@ enum { #define PS_VADAPT_MODE_LEAST_ADAPT (0 << 5) #define PS_VADAPT_MODE_MOD_ADAPT (1 << 5) #define PS_VADAPT_MODE_MOST_ADAPT (3 << 5) +#define PS_PLANE_Y_SEL_MASK (7 << 5) +#define PS_PLANE_Y_SEL(plane) (((plane) + 1) << 5) #define _PS_PWR_GATE_1A 0x68160 #define _PS_PWR_GATE_2A 0x68260 diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 20bfc89c652c..7238ab615548 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -233,13 +233,23 @@ static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_sta 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)) + !IS_GEMINILAKE(dev_priv)) { mode = SKL_PS_SCALER_MODE_NV12; - else + } else if (icl_is_hdr_plane(to_intel_plane(plane_state->base.plane))) { + /* + * On gen11+'s HDR planes we only use the scaler for + * scaling. They have a dedicated chroma upsampler, so + * we don't need the scaler to upsample the UV plane. + */ + mode = PS_SCALER_MODE_NORMAL; + } else { mode = PS_SCALER_MODE_PLANAR; + if (plane_state->linked_plane) + mode |= PS_PLANE_Y_SEL(plane_state->linked_plane->id); + } } else if (INTEL_GEN(dev_priv) > 9 || IS_GEMINILAKE(dev_priv)) { - mode = PS_SCALER_MODE_PACKED; + mode = PS_SCALER_MODE_NORMAL; } else if (num_scalers_need == 1 && intel_crtc->num_scalers > 1) { /* * when only 1 scaler is in use on a pipe with 2 scalers diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cbb3fb1d5ad4..5997097177e9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4836,8 +4836,7 @@ static int skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, unsigned int scaler_user, int *scaler_id, int src_w, int src_h, int dst_w, int dst_h, - bool plane_scaler_check, - uint32_t pixel_format) + const struct drm_format_info *format, bool need_scaler) { struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state; @@ -4846,22 +4845,14 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode; - int need_scaling; /* * Src coordinates are already rotated by 270 degrees for * the 90/270 degree plane rotation cases (to match the * GTT mapping), hence no need to account for rotation here. */ - need_scaling = src_w != dst_w || src_h != dst_h; - - if (plane_scaler_check) - if (pixel_format == DRM_FORMAT_NV12) - need_scaling = true; - - if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 && - scaler_user == SKL_CRTC_INDEX) - need_scaling = true; + if (src_w != dst_w || src_h != dst_h) + need_scaler = true; /* * Scaling/fitting not supported in IF-ID mode in GEN9+ @@ -4870,7 +4861,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, * for NV12. */ if (INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable && - need_scaling && adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) { + need_scaler && adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) { DRM_DEBUG_KMS("Pipe/Plane scaling not supported with IF-ID mode\n"); return -EINVAL; } @@ -4885,7 +4876,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, * update to free the scaler is done in plane/panel-fit programming. * For this purpose crtc/plane_state->scaler_id isn't reset here. */ - if (force_detach || !need_scaling) { + if (force_detach || !need_scaler) { if (*scaler_id >= 0) { scaler_state->scaler_users &= ~(1 << scaler_user); scaler_state->scalers[*scaler_id].in_use = 0; @@ -4899,7 +4890,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, return 0; } - if (plane_scaler_check && pixel_format == DRM_FORMAT_NV12 && + if (format && format->format == DRM_FORMAT_NV12 && (src_h < SKL_MIN_YUV_420_SRC_H || src_w < SKL_MIN_YUV_420_SRC_W)) { DRM_DEBUG_KMS("NV12: src dimensions not met\n"); return -EINVAL; @@ -4942,12 +4933,16 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, int skl_update_scaler_crtc(struct intel_crtc_state *state) { const struct drm_display_mode *adjusted_mode = &state->base.adjusted_mode; + bool need_scaler = false; + + if (state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) + need_scaler = true; return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX, &state->scaler_state.scaler_id, state->pipe_src_w, state->pipe_src_h, adjusted_mode->crtc_hdisplay, - adjusted_mode->crtc_vdisplay, false, 0); + adjusted_mode->crtc_vdisplay, NULL, need_scaler); } /** @@ -4962,13 +4957,17 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state) { - struct intel_plane *intel_plane = to_intel_plane(plane_state->base.plane); struct drm_framebuffer *fb = plane_state->base.fb; int ret; - bool force_detach = !fb || !plane_state->base.visible; + bool need_scaler = false; + + /* Pre-gen11 and SDR planes always need a scaler for planar formats. */ + if (!icl_is_hdr_plane(intel_plane) && + fb && fb->format->format == DRM_FORMAT_NV12) + need_scaler = true; ret = skl_update_scaler(crtc_state, force_detach, drm_plane_index(&intel_plane->base), @@ -4977,7 +4976,7 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, drm_rect_height(&plane_state->base.src) >> 16, drm_rect_width(&plane_state->base.dst), drm_rect_height(&plane_state->base.dst), - fb ? true : false, fb ? fb->format->format : 0); + fb ? fb->format : NULL, need_scaler); if (ret || plane_state->scaler_id < 0) return ret; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 272de906a001..f1d196bcdb66 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2211,6 +2211,14 @@ static inline bool icl_is_nv12_y_plane(enum plane_id id) return false; } +static inline bool icl_is_hdr_plane(struct intel_plane *plane) +{ + if (INTEL_GEN(to_i915(plane->base.dev)) < 11) + return false; + + return plane->id < PLANE_SPRITE2; +} + /* intel_tv.c */ void intel_tv_init(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 7cd59eee5cad..a2a4328107b6 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -332,7 +332,8 @@ skl_program_scaler(struct drm_i915_private *dev_priv, crtc_h--; /* TODO: handle sub-pixel coordinates */ - if (plane_state->base.fb->format->format == DRM_FORMAT_NV12) { + if (plane_state->base.fb->format->format == DRM_FORMAT_NV12 && + !icl_is_hdr_plane(plane)) { y_hphase = skl_scaler_calc_phase(1, false); y_vphase = skl_scaler_calc_phase(1, false);
The first 3 planes (primary, sprite 0 and 1) have a dedicated chroma upsampler to upscale YUV420 to YUV444 and the scaler should only be used for upscaling. Because of this we shouldn't program the scalers in planar mode if NV12 and the chroma upsampler are used. Instead program the scalers like on normal planes. Sprite 2 and 3 have no dedicated scaler, and need to program the selected Y plane in the scaler mode. Changes since v1: - Make the comment less confusing. Changes since v2: - Fix checkpatch warning (Matt) - gen10- -> Pre-gen11 (Ville) - PS_SCALER_MODE_PACKED -> PS_SCALER_MODE_NORMAL. (Matt) - Add comment about scaler mode in intel_atomic_setup_scaler(). (Matt) - Rename need_scaling to need_scaler. (Matt) - Move the crtc need_scaling check to skl_update_scaler_crtc(). Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 4 ++- drivers/gpu/drm/i915/intel_atomic.c | 16 +++++++++--- drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++-------------- drivers/gpu/drm/i915/intel_drv.h | 8 ++++++ drivers/gpu/drm/i915/intel_sprite.c | 3 ++- 5 files changed, 44 insertions(+), 24 deletions(-)