| Message ID | 20201008101608.8652-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Set all unused color plane offsets to ~0xfff again | expand |
On Thu, Oct 08, 2020 at 01:16:06PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > When the number of potential color planes grew to 4 we stopped > setting all unused color plane offsets to ~0xfff. The code > still tries to do this, but actually does nothing since the > loop limits are bogus. > > skl_check_main_surface() actually depends on this ~0xfff > behaviour as it will make sure to move the main surface > offset below the aux surface offset because the hardware > AUX_DIST must be a non-negative value [1], and for simplicity > it doesn't bother checking if the AUX plane is actually > needed or not. So currently it may end up shuffling the > main surface around based on some stale leftover AUX offset. > > The skl+ plane code also just blindly calculates the AUX_DIST > whether or not the AUX plane is actually needed by the hw or > not, and that too will now potentially use some stale AUX > surface offset in the calculation. Would seem nicer to > guarantee a consistent non-negative AUX_DIST always. > > So bring back the original ~0xfff offset behaviour for > unused color planes. Though it doesn't seem super likely > that this inconsistency would cause any real issues. > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > Fixes: 2dfbf9d2873a ("drm/i915/tgl: Gen-12 display can decompress surfaces compressed by the media engine") > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Arg. Yes skl_check_main_surface() adjusts now the address needlessly. The fix looks ok to me: Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 907e1d155443..44fd7059838f 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -4104,8 +4104,7 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state) > int skl_check_plane_surface(struct intel_plane_state *plane_state) > { > const struct drm_framebuffer *fb = plane_state->hw.fb; > - int ret; > - bool needs_aux = false; > + int ret, i; > > ret = intel_plane_compute_gtt(plane_state); > if (ret) > @@ -4119,7 +4118,6 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state) > * it. > */ > if (is_ccs_modifier(fb->modifier)) { > - needs_aux = true; > ret = skl_check_ccs_aux_surface(plane_state); > if (ret) > return ret; > @@ -4127,20 +4125,15 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state) > > if (intel_format_info_is_yuv_semiplanar(fb->format, > fb->modifier)) { > - needs_aux = true; > ret = skl_check_nv12_aux_surface(plane_state); > if (ret) > return ret; > } > > - if (!needs_aux) { > - int i; > - > - for (i = 1; i < fb->format->num_planes; i++) { > - plane_state->color_plane[i].offset = ~0xfff; > - plane_state->color_plane[i].x = 0; > - plane_state->color_plane[i].y = 0; > - } > + for (i = fb->format->num_planes; i < ARRAY_SIZE(plane_state->color_plane); i++) { > + plane_state->color_plane[i].offset = ~0xfff; > + plane_state->color_plane[i].x = 0; > + plane_state->color_plane[i].y = 0; > } > > ret = skl_check_main_surface(plane_state); > -- > 2.26.2 >
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 907e1d155443..44fd7059838f 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -4104,8 +4104,7 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state) int skl_check_plane_surface(struct intel_plane_state *plane_state) { const struct drm_framebuffer *fb = plane_state->hw.fb; - int ret; - bool needs_aux = false; + int ret, i; ret = intel_plane_compute_gtt(plane_state); if (ret) @@ -4119,7 +4118,6 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state) * it. */ if (is_ccs_modifier(fb->modifier)) { - needs_aux = true; ret = skl_check_ccs_aux_surface(plane_state); if (ret) return ret; @@ -4127,20 +4125,15 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state) if (intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier)) { - needs_aux = true; ret = skl_check_nv12_aux_surface(plane_state); if (ret) return ret; } - if (!needs_aux) { - int i; - - for (i = 1; i < fb->format->num_planes; i++) { - plane_state->color_plane[i].offset = ~0xfff; - plane_state->color_plane[i].x = 0; - plane_state->color_plane[i].y = 0; - } + for (i = fb->format->num_planes; i < ARRAY_SIZE(plane_state->color_plane); i++) { + plane_state->color_plane[i].offset = ~0xfff; + plane_state->color_plane[i].x = 0; + plane_state->color_plane[i].y = 0; } ret = skl_check_main_surface(plane_state);