On Wed, Aug 19, 2015 at 06:02:23PM -0700, Chandra Konduru wrote: > This patch swaps src width and height for dbuf/wm calculations > when rotation is 90/270 as per hw requirements. The spec is rather unclear about this. It only says: "If plane 90 or 270 rotation is enabled, use the rotated width and height in pixel rate calculations." Although it does make sense based on the fact that the display fetch will happen in the rotated direction. I tried to reconcile some if this stuff in my head but when I went to read the Yf/Ys tiling documentation I got even more confused. What it says there is that a single cacheline always contains 4 rows of pixels, but then in the next sentence it claims the aspect ratio will be 1:1 for 8,32,128 bpp, and 2:1 for 16,64 bpp. Those two things can't both be true. Can someone tell me which one is true? > > Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_pm.c | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index da3046f..c455946 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3193,6 +3193,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, > enum pipe pipe = intel_crtc->pipe; > struct drm_plane *plane; > struct drm_framebuffer *fb; > + struct intel_plane_state *plane_state; > + int src_w, src_h; These can be moved to a narrower scope. plane_state could be const I believe. > int i = 1; /* Index for sprite planes start */ > > p->active = intel_crtc->active; > @@ -3201,6 +3203,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, > p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config); > > fb = crtc->primary->state->fb; > + plane_state = to_intel_plane_state(crtc->primary->state); > /* For planar: Bpp is for uv plane, y_Bpp is for y plane */ > if (fb) { > p->plane[0].enabled = true; > @@ -3215,8 +3218,22 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, > p->plane[0].y_bytes_per_pixel = 0; > p->plane[0].tiling = DRM_FORMAT_MOD_NONE; > } > - p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w; > - p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h; > + > + if (drm_rect_width(&plane_state->src)) { > + src_w = drm_rect_width(&plane_state->src) >> 16; > + src_h = drm_rect_height(&plane_state->src) >> 16; > + } else { > + src_w = intel_crtc->config->pipe_src_w; > + src_h = intel_crtc->config->pipe_src_h; > + } Are we still not having a consistent plane state all the time, or why is this kludge needed? > + > + if (intel_rotation_90_or_270(crtc->primary->state->rotation)) { > + p->plane[0].horiz_pixels = src_h; > + p->plane[0].vert_pixels = src_w; > + } else { > + p->plane[0].horiz_pixels = src_w; > + p->plane[0].vert_pixels = src_h; > + } > p->plane[0].rotation = crtc->primary->state->rotation; > > fb = crtc->cursor->state->fb; > @@ -3750,8 +3767,15 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc, > > intel_plane->wm.enabled = enabled; > intel_plane->wm.scaled = scaled; > - intel_plane->wm.horiz_pixels = sprite_width; > - intel_plane->wm.vert_pixels = sprite_height; > + > + if (intel_rotation_90_or_270(plane->state->rotation)) { > + intel_plane->wm.horiz_pixels = sprite_height; > + intel_plane->wm.vert_pixels = sprite_width; > + } else { > + intel_plane->wm.horiz_pixels = sprite_width; > + intel_plane->wm.vert_pixels = sprite_height; > + } > + > intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE; > > /* For planar: Bpp is for UV plane, y_Bpp is for Y plane */ > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx