| Message ID | 20191017132105.15528-14-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/14] drm/i915: Rework watermark readout to use plane api | expand |
Op 17-10-2019 om 15:21 schreef Maarten Lankhorst: > Now that we split plane_state which I didn't want to do yet, we can > program the slave plane without requiring the master plane. > > This is useful for programming bigjoiner slave planes as well. We > will no longer need the master's plane_state. > > Changes since v1: > - set src/dst rectangles after copy_uapi_to_hw_state. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > .../gpu/drm/i915/display/intel_atomic_plane.c | 30 +--------- > .../gpu/drm/i915/display/intel_atomic_plane.h | 3 - > drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++ > .../drm/i915/display/intel_display_types.h | 6 +- > drivers/gpu/drm/i915/display/intel_sprite.c | 55 ++++++------------- > 5 files changed, 39 insertions(+), 73 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > index d9b65e9c45fc..54d112408716 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > @@ -309,16 +309,6 @@ void intel_update_plane(struct intel_plane *plane, > plane->update_plane(plane, crtc_state, plane_state); > } > > -void intel_update_slave(struct intel_plane *plane, > - const struct intel_crtc_state *crtc_state, > - const struct intel_plane_state *plane_state) > -{ > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > - > - trace_intel_update_plane(&plane->base, crtc); > - plane->update_slave(plane, crtc_state, plane_state); > -} > - > void intel_disable_plane(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state) > { > @@ -351,25 +341,9 @@ void skl_update_planes_on_crtc(struct intel_atomic_state *state, > struct intel_plane_state *new_plane_state = > intel_atomic_get_new_plane_state(state, plane); > > - if (new_plane_state->uapi.visible) { > + if (new_plane_state->uapi.visible || > + new_plane_state->planar_slave) { > intel_update_plane(plane, new_crtc_state, new_plane_state); > - } else if (new_plane_state->planar_slave) { > - struct intel_plane *master = > - new_plane_state->planar_linked_plane; > - > - /* > - * We update the slave plane from this function because > - * programming it from the master plane's update_plane > - * callback runs into issues when the Y plane is > - * reassigned, disabled or used by a different plane. > - * > - * The slave plane is updated with the master plane's > - * plane_state. > - */ > - new_plane_state = > - intel_atomic_get_new_plane_state(state, master); > - > - intel_update_slave(plane, new_crtc_state, new_plane_state); > } else { > intel_disable_plane(plane, new_crtc_state); > } > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h > index 123404a9cf23..726ececd6abd 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h > @@ -25,9 +25,6 @@ void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state, > void intel_update_plane(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state); > -void intel_update_slave(struct intel_plane *plane, > - const struct intel_crtc_state *crtc_state, > - const struct intel_plane_state *plane_state); > void intel_disable_plane(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state); > struct intel_plane *intel_plane_alloc(void); > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index de520d5f1374..88f149cac198 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -11768,6 +11768,24 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state) > crtc_state->active_planes |= BIT(linked->id); > crtc_state->update_planes |= BIT(linked->id); > DRM_DEBUG_KMS("Using %s as Y plane for %s\n", linked->base.name, plane->base.name); > + > + /* Copy parameters to slave plane */ > + linked_state->ctl = plane_state->ctl | PLANE_CTL_YUV420_Y_PLANE; > + linked_state->color_ctl = plane_state->color_ctl; > + linked_state->color_plane[0] = plane_state->color_plane[0]; > + > + intel_plane_copy_uapi_to_hw_state(linked_state, plane_state); > + linked_state->uapi.src = plane_state->uapi.src; > + linked_state->uapi.dst = plane_state->uapi.dst; > + > + if (icl_is_hdr_plane(dev_priv, plane->id)) { > + if (linked->id == PLANE_SPRITE5) > + plane_state->cus_ctl |= PLANE_CUS_PLANE_7; > + else if (linked->id == PLANE_SPRITE4) > + plane_state->cus_ctl |= PLANE_CUS_PLANE_6; > + else > + MISSING_CASE(linked->id); > + } > } > > return 0; > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index 5379b93c2fab..dbca1b3c5c67 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -555,6 +555,9 @@ struct intel_plane_state { > /* plane color control register */ > u32 color_ctl; > > + /* chroma upsampler control register */ > + u32 cus_ctl; > + > /* > * scaler_id > * = -1 : not using a scaler > @@ -1106,9 +1109,6 @@ struct intel_plane { > void (*update_plane)(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state); > - void (*update_slave)(struct intel_plane *plane, > - const struct intel_crtc_state *crtc_state, > - const struct intel_plane_state *plane_state); > void (*disable_plane)(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state); > bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c > index 00f83d37abcf..20123fccd081 100644 > --- a/drivers/gpu/drm/i915/display/intel_sprite.c > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c > @@ -526,7 +526,7 @@ static void > skl_program_plane(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state, > - int color_plane, bool slave, u32 plane_ctl) > + int color_plane) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum plane_id plane_id = plane->id; > @@ -541,12 +541,12 @@ skl_program_plane(struct intel_plane *plane, > u32 y = plane_state->color_plane[color_plane].y; > u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16; > u32 src_h = drm_rect_height(&plane_state->uapi.src) >> 16; > - struct intel_plane *linked = plane_state->planar_linked_plane; > const struct drm_framebuffer *fb = plane_state->hw.fb; > u8 alpha = plane_state->hw.alpha >> 8; > u32 plane_color_ctl = 0; > unsigned long irqflags; > u32 keymsk, keymax; > + u32 plane_ctl = plane_state->ctl; > > plane_ctl |= skl_plane_ctl_crtc(crtc_state); > > @@ -578,26 +578,8 @@ skl_program_plane(struct intel_plane *plane, > I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id), > (plane_state->color_plane[1].offset - surf_addr) | aux_stride); > > - if (icl_is_hdr_plane(dev_priv, plane_id)) { > - u32 cus_ctl = 0; > - > - if (linked) { > - /* Enable and use MPEG-2 chroma siting */ > - cus_ctl = PLANE_CUS_ENABLE | > - PLANE_CUS_HPHASE_0 | > - PLANE_CUS_VPHASE_SIGN_NEGATIVE | > - PLANE_CUS_VPHASE_0_25; > - > - if (linked->id == PLANE_SPRITE5) > - cus_ctl |= PLANE_CUS_PLANE_7; > - else if (linked->id == PLANE_SPRITE4) > - cus_ctl |= PLANE_CUS_PLANE_6; > - else > - MISSING_CASE(linked->id); > - } > - > - I915_WRITE_FW(PLANE_CUS_CTL(pipe, plane_id), cus_ctl); > - } > + if (icl_is_hdr_plane(dev_priv, plane_id)) > + I915_WRITE_FW(PLANE_CUS_CTL(pipe, plane_id), plane_state->cus_ctl); > > if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), plane_color_ctl); > @@ -627,7 +609,7 @@ skl_program_plane(struct intel_plane *plane, > I915_WRITE_FW(PLANE_SURF(pipe, plane_id), > intel_plane_ggtt_offset(plane_state) + surf_addr); > > - if (!slave && plane_state->scaler_id >= 0) > + if (plane_state->scaler_id >= 0) > skl_program_scaler(plane, crtc_state, plane_state); > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > @@ -640,24 +622,13 @@ skl_update_plane(struct intel_plane *plane, > { > int color_plane = 0; > > - if (plane_state->planar_linked_plane) { > + if (drm_format_info_is_yuv_semiplanar(plane_state->hw.fb->format) && > + !plane_state->planar_slave) > /* Program the UV plane */ > color_plane = 1; > - } And for pre-icl, this explains the test failures, needs a gen11+ check. > > - skl_program_plane(plane, crtc_state, plane_state, > - color_plane, false, plane_state->ctl); > + skl_program_plane(plane, crtc_state, plane_state, color_plane); > } > - > -static void > -icl_update_slave(struct intel_plane *plane, > - const struct intel_crtc_state *crtc_state, > - const struct intel_plane_state *plane_state) > -{ > - skl_program_plane(plane, crtc_state, plane_state, 0, true, > - plane_state->ctl | PLANE_CTL_YUV420_Y_PLANE); > -} > - > static void > skl_disable_plane(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state) > @@ -1844,6 +1815,14 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, > plane_state->color_ctl = glk_plane_color_ctl(crtc_state, > plane_state); > > + if (icl_is_hdr_plane(dev_priv, plane->id) && fb->format->is_yuv) > + /* Enable and use MPEG-2 chroma siting */ > + plane_state->cus_ctl = PLANE_CUS_ENABLE | > + PLANE_CUS_HPHASE_0 | > + PLANE_CUS_VPHASE_SIGN_NEGATIVE | PLANE_CUS_VPHASE_0_25; > + else > + plane_state->cus_ctl = 0; needs drm_format_info_is_yuv_semiplanar(fb->format) instead of is_yuv check. This explains the test failures on icl+. > return 0; > } > > @@ -2512,8 +2491,6 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv, > plane->disable_plane = skl_disable_plane; > plane->get_hw_state = skl_plane_get_hw_state; > plane->check_plane = skl_plane_check; > - if (icl_is_nv12_y_plane(plane_id)) > - plane->update_slave = icl_update_slave; > > if (INTEL_GEN(dev_priv) >= 11) > formats = icl_get_plane_formats(dev_priv, pipe,
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index d9b65e9c45fc..54d112408716 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -309,16 +309,6 @@ void intel_update_plane(struct intel_plane *plane, plane->update_plane(plane, crtc_state, plane_state); } -void intel_update_slave(struct intel_plane *plane, - const struct intel_crtc_state *crtc_state, - const struct intel_plane_state *plane_state) -{ - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); - - trace_intel_update_plane(&plane->base, crtc); - plane->update_slave(plane, crtc_state, plane_state); -} - void intel_disable_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state) { @@ -351,25 +341,9 @@ void skl_update_planes_on_crtc(struct intel_atomic_state *state, struct intel_plane_state *new_plane_state = intel_atomic_get_new_plane_state(state, plane); - if (new_plane_state->uapi.visible) { + if (new_plane_state->uapi.visible || + new_plane_state->planar_slave) { intel_update_plane(plane, new_crtc_state, new_plane_state); - } else if (new_plane_state->planar_slave) { - struct intel_plane *master = - new_plane_state->planar_linked_plane; - - /* - * We update the slave plane from this function because - * programming it from the master plane's update_plane - * callback runs into issues when the Y plane is - * reassigned, disabled or used by a different plane. - * - * The slave plane is updated with the master plane's - * plane_state. - */ - new_plane_state = - intel_atomic_get_new_plane_state(state, master); - - intel_update_slave(plane, new_crtc_state, new_plane_state); } else { intel_disable_plane(plane, new_crtc_state); } diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 123404a9cf23..726ececd6abd 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -25,9 +25,6 @@ void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state, void intel_update_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state); -void intel_update_slave(struct intel_plane *plane, - const struct intel_crtc_state *crtc_state, - const struct intel_plane_state *plane_state); void intel_disable_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state); struct intel_plane *intel_plane_alloc(void); diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index de520d5f1374..88f149cac198 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -11768,6 +11768,24 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state) crtc_state->active_planes |= BIT(linked->id); crtc_state->update_planes |= BIT(linked->id); DRM_DEBUG_KMS("Using %s as Y plane for %s\n", linked->base.name, plane->base.name); + + /* Copy parameters to slave plane */ + linked_state->ctl = plane_state->ctl | PLANE_CTL_YUV420_Y_PLANE; + linked_state->color_ctl = plane_state->color_ctl; + linked_state->color_plane[0] = plane_state->color_plane[0]; + + intel_plane_copy_uapi_to_hw_state(linked_state, plane_state); + linked_state->uapi.src = plane_state->uapi.src; + linked_state->uapi.dst = plane_state->uapi.dst; + + if (icl_is_hdr_plane(dev_priv, plane->id)) { + if (linked->id == PLANE_SPRITE5) + plane_state->cus_ctl |= PLANE_CUS_PLANE_7; + else if (linked->id == PLANE_SPRITE4) + plane_state->cus_ctl |= PLANE_CUS_PLANE_6; + else + MISSING_CASE(linked->id); + } } return 0; diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 5379b93c2fab..dbca1b3c5c67 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -555,6 +555,9 @@ struct intel_plane_state { /* plane color control register */ u32 color_ctl; + /* chroma upsampler control register */ + u32 cus_ctl; + /* * scaler_id * = -1 : not using a scaler @@ -1106,9 +1109,6 @@ struct intel_plane { void (*update_plane)(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state); - void (*update_slave)(struct intel_plane *plane, - const struct intel_crtc_state *crtc_state, - const struct intel_plane_state *plane_state); void (*disable_plane)(struct intel_plane *plane, const struct intel_crtc_state *crtc_state); bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c index 00f83d37abcf..20123fccd081 100644 --- a/drivers/gpu/drm/i915/display/intel_sprite.c +++ b/drivers/gpu/drm/i915/display/intel_sprite.c @@ -526,7 +526,7 @@ static void skl_program_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state, - int color_plane, bool slave, u32 plane_ctl) + int color_plane) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum plane_id plane_id = plane->id; @@ -541,12 +541,12 @@ skl_program_plane(struct intel_plane *plane, u32 y = plane_state->color_plane[color_plane].y; u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16; u32 src_h = drm_rect_height(&plane_state->uapi.src) >> 16; - struct intel_plane *linked = plane_state->planar_linked_plane; const struct drm_framebuffer *fb = plane_state->hw.fb; u8 alpha = plane_state->hw.alpha >> 8; u32 plane_color_ctl = 0; unsigned long irqflags; u32 keymsk, keymax; + u32 plane_ctl = plane_state->ctl; plane_ctl |= skl_plane_ctl_crtc(crtc_state); @@ -578,26 +578,8 @@ skl_program_plane(struct intel_plane *plane, I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id), (plane_state->color_plane[1].offset - surf_addr) | aux_stride); - if (icl_is_hdr_plane(dev_priv, plane_id)) { - u32 cus_ctl = 0; - - if (linked) { - /* Enable and use MPEG-2 chroma siting */ - cus_ctl = PLANE_CUS_ENABLE | - PLANE_CUS_HPHASE_0 | - PLANE_CUS_VPHASE_SIGN_NEGATIVE | - PLANE_CUS_VPHASE_0_25; - - if (linked->id == PLANE_SPRITE5) - cus_ctl |= PLANE_CUS_PLANE_7; - else if (linked->id == PLANE_SPRITE4) - cus_ctl |= PLANE_CUS_PLANE_6; - else - MISSING_CASE(linked->id); - } - - I915_WRITE_FW(PLANE_CUS_CTL(pipe, plane_id), cus_ctl); - } + if (icl_is_hdr_plane(dev_priv, plane_id)) + I915_WRITE_FW(PLANE_CUS_CTL(pipe, plane_id), plane_state->cus_ctl); if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), plane_color_ctl); @@ -627,7 +609,7 @@ skl_program_plane(struct intel_plane *plane, I915_WRITE_FW(PLANE_SURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + surf_addr); - if (!slave && plane_state->scaler_id >= 0) + if (plane_state->scaler_id >= 0) skl_program_scaler(plane, crtc_state, plane_state); spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); @@ -640,24 +622,13 @@ skl_update_plane(struct intel_plane *plane, { int color_plane = 0; - if (plane_state->planar_linked_plane) { + if (drm_format_info_is_yuv_semiplanar(plane_state->hw.fb->format) && + !plane_state->planar_slave) /* Program the UV plane */ color_plane = 1; - } - skl_program_plane(plane, crtc_state, plane_state, - color_plane, false, plane_state->ctl); + skl_program_plane(plane, crtc_state, plane_state, color_plane); } - -static void -icl_update_slave(struct intel_plane *plane, - const struct intel_crtc_state *crtc_state, - const struct intel_plane_state *plane_state) -{ - skl_program_plane(plane, crtc_state, plane_state, 0, true, - plane_state->ctl | PLANE_CTL_YUV420_Y_PLANE); -} - static void skl_disable_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state) @@ -1844,6 +1815,14 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, plane_state->color_ctl = glk_plane_color_ctl(crtc_state, plane_state); + if (icl_is_hdr_plane(dev_priv, plane->id) && fb->format->is_yuv) + /* Enable and use MPEG-2 chroma siting */ + plane_state->cus_ctl = PLANE_CUS_ENABLE | + PLANE_CUS_HPHASE_0 | + PLANE_CUS_VPHASE_SIGN_NEGATIVE | PLANE_CUS_VPHASE_0_25; + else + plane_state->cus_ctl = 0; + return 0; } @@ -2512,8 +2491,6 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv, plane->disable_plane = skl_disable_plane; plane->get_hw_state = skl_plane_get_hw_state; plane->check_plane = skl_plane_check; - if (icl_is_nv12_y_plane(plane_id)) - plane->update_slave = icl_update_slave; if (INTEL_GEN(dev_priv) >= 11) formats = icl_get_plane_formats(dev_priv, pipe,
Now that we split plane_state which I didn't want to do yet, we can program the slave plane without requiring the master plane. This is useful for programming bigjoiner slave planes as well. We will no longer need the master's plane_state. Changes since v1: - set src/dst rectangles after copy_uapi_to_hw_state. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- .../gpu/drm/i915/display/intel_atomic_plane.c | 30 +--------- .../gpu/drm/i915/display/intel_atomic_plane.h | 3 - drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++ .../drm/i915/display/intel_display_types.h | 6 +- drivers/gpu/drm/i915/display/intel_sprite.c | 55 ++++++------------- 5 files changed, 39 insertions(+), 73 deletions(-)