| Message ID | 1443643545-3603-1-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 30, 2015 at 05:05:43PM -0300, Paulo Zanoni wrote: > The comment suggests the check was there for some non-fully-atomic > case, and I couldn't find a case where we wouldn't correctly > initialize plane_state, so remove the check. > > Let's leave a WARN there just in case. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++--------------------- > 1 file changed, 13 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index e547fe7..88657cb 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3131,27 +3131,19 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > fb->pixel_format); > surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0); > > - /* > - * FIXME: intel_plane_state->src, dst aren't set when transitional > - * update_plane helpers are called from legacy paths. > - * Once full atomic crtc is available, below check can be avoided. > - */ > - if (drm_rect_width(&plane_state->src)) { > - scaler_id = plane_state->scaler_id; > - src_x = plane_state->src.x1 >> 16; > - src_y = plane_state->src.y1 >> 16; > - src_w = drm_rect_width(&plane_state->src) >> 16; > - src_h = drm_rect_height(&plane_state->src) >> 16; > - dst_x = plane_state->dst.x1; > - dst_y = plane_state->dst.y1; > - dst_w = drm_rect_width(&plane_state->dst); > - dst_h = drm_rect_height(&plane_state->dst); > - > - WARN_ON(x != src_x || y != src_y); > - } else { > - src_w = intel_crtc->config->pipe_src_w; > - src_h = intel_crtc->config->pipe_src_h; > - } > + WARN_ON(drm_rect_width(&plane_state->src) == 0); > + > + scaler_id = plane_state->scaler_id; > + src_x = plane_state->src.x1 >> 16; > + src_y = plane_state->src.y1 >> 16; > + src_w = drm_rect_width(&plane_state->src) >> 16; > + src_h = drm_rect_height(&plane_state->src) >> 16; > + dst_x = plane_state->dst.x1; > + dst_y = plane_state->dst.y1; > + dst_w = drm_rect_width(&plane_state->dst); > + dst_h = drm_rect_height(&plane_state->dst); > + > + WARN_ON(x != src_x || y != src_y); I guess I should resurrect some of my primary plane cleanup patches since no one else has taken the bait :( Anyway the patch makes sense, well, assuming we've gotten better at maintaining the plane state. Maarten might be able to answer that. Me, I'm too lazy to look right now, so I'll give this an ack. Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > if (intel_rotation_90_or_270(rotation)) { > /* stride = Surface height in tiles */ > -- > 2.5.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e547fe7..88657cb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3131,27 +3131,19 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, fb->pixel_format); surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0); - /* - * FIXME: intel_plane_state->src, dst aren't set when transitional - * update_plane helpers are called from legacy paths. - * Once full atomic crtc is available, below check can be avoided. - */ - if (drm_rect_width(&plane_state->src)) { - scaler_id = plane_state->scaler_id; - src_x = plane_state->src.x1 >> 16; - src_y = plane_state->src.y1 >> 16; - src_w = drm_rect_width(&plane_state->src) >> 16; - src_h = drm_rect_height(&plane_state->src) >> 16; - dst_x = plane_state->dst.x1; - dst_y = plane_state->dst.y1; - dst_w = drm_rect_width(&plane_state->dst); - dst_h = drm_rect_height(&plane_state->dst); - - WARN_ON(x != src_x || y != src_y); - } else { - src_w = intel_crtc->config->pipe_src_w; - src_h = intel_crtc->config->pipe_src_h; - } + WARN_ON(drm_rect_width(&plane_state->src) == 0); + + scaler_id = plane_state->scaler_id; + src_x = plane_state->src.x1 >> 16; + src_y = plane_state->src.y1 >> 16; + src_w = drm_rect_width(&plane_state->src) >> 16; + src_h = drm_rect_height(&plane_state->src) >> 16; + dst_x = plane_state->dst.x1; + dst_y = plane_state->dst.y1; + dst_w = drm_rect_width(&plane_state->dst); + dst_h = drm_rect_height(&plane_state->dst); + + WARN_ON(x != src_x || y != src_y); if (intel_rotation_90_or_270(rotation)) { /* stride = Surface height in tiles */
The comment suggests the check was there for some non-fully-atomic case, and I couldn't find a case where we wouldn't correctly initialize plane_state, so remove the check. Let's leave a WARN there just in case. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)