| Message ID | 20191022103149.17236-1-dev@mblankhorst.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Remove special case slave handling during hw programming, v3. | expand |
On Tue, Oct 22, 2019 at 12:31:49PM +0200, Maarten Lankhorst wrote: > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > 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. > Changes since v2: > - Use the correct color_plane for pre-gen11 by using planar_linked_plane != NULL. > - Use drm_format_info_is_yuv_semiplanar in skl_plane_check() to fix gen11+. > > 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 | 57 ++++++------------- > 5 files changed, 40 insertions(+), 74 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 490f09264c7f..e51cbf6b4159 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -11911,6 +11911,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); > + } That stuff looks like it deserves a function of its own. Also a bit annoying to copy if piecemeal like that. I guess don't have a convenient way to just copy the whole thing in one go. The use of intel_plane_copy_uapi_to_hw_state() also seems wrong for the bigjoiner slave case. Shouldn't we just copy the hw state and ignore uapi state here entirely? > } > > 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 a38d0f20ef2b..ff481af2b24b 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 > @@ -1112,9 +1115,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..0036ca6c1aba 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,12 @@ skl_update_plane(struct intel_plane *plane, > { > int color_plane = 0; > > - if (plane_state->planar_linked_plane) { > - /* Program the UV plane */ > + if (plane_state->planar_linked_plane && !plane_state->planar_slave) > + /* Program the UV plane on planar master */ > color_plane = 1; > - } > - > - skl_program_plane(plane, crtc_state, plane_state, > - color_plane, false, plane_state->ctl); > -} > > -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); > + skl_program_plane(plane, crtc_state, plane_state, color_plane); > } > - > static void > skl_disable_plane(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state) > @@ -1844,6 +1814,15 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, > plane_state->color_ctl = glk_plane_color_ctl(crtc_state, > plane_state); > > + if (drm_format_info_is_yuv_semiplanar(fb->format) && > + icl_is_hdr_plane(dev_priv, plane->id)) > + /* 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, > -- > 2.23.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 22-10-2019 om 20:14 schreef Ville Syrjälä: > On Tue, Oct 22, 2019 at 12:31:49PM +0200, Maarten Lankhorst wrote: >> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> >> 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. >> Changes since v2: >> - Use the correct color_plane for pre-gen11 by using planar_linked_plane != NULL. >> - Use drm_format_info_is_yuv_semiplanar in skl_plane_check() to fix gen11+. >> >> 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 | 57 ++++++------------- >> 5 files changed, 40 insertions(+), 74 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 490f09264c7f..e51cbf6b4159 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -11911,6 +11911,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); >> + } > That stuff looks like it deserves a function of its own. Also a bit > annoying to copy if piecemeal like that. I guess don't have a > convenient way to just copy the whole thing in one go. > > The use of intel_plane_copy_uapi_to_hw_state() also seems wrong > for the bigjoiner slave case. Shouldn't we just copy the hw state > and ignore uapi state here entirely? When using bigjoiner, we copy the props from master_plane_state, then copy the rects from the slave. We're using uapi.visible/src/dst, it's a tad annoying because there is no separate props struct in drm_plane/crtc_state. Else the distinction would be more clear. It's very clear in src_x vs src.x1 that there is room for confusion. :(
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 490f09264c7f..e51cbf6b4159 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -11911,6 +11911,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 a38d0f20ef2b..ff481af2b24b 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 @@ -1112,9 +1115,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..0036ca6c1aba 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,12 @@ skl_update_plane(struct intel_plane *plane, { int color_plane = 0; - if (plane_state->planar_linked_plane) { - /* Program the UV plane */ + if (plane_state->planar_linked_plane && !plane_state->planar_slave) + /* Program the UV plane on planar master */ color_plane = 1; - } - - skl_program_plane(plane, crtc_state, plane_state, - color_plane, false, plane_state->ctl); -} -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); + skl_program_plane(plane, crtc_state, plane_state, color_plane); } - static void skl_disable_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state) @@ -1844,6 +1814,15 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, plane_state->color_ctl = glk_plane_color_ctl(crtc_state, plane_state); + if (drm_format_info_is_yuv_semiplanar(fb->format) && + icl_is_hdr_plane(dev_priv, plane->id)) + /* 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,