| Message ID | 1446535913-31970-2-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 03, 2015 at 08:31:40AM +0100, Maarten Lankhorst wrote: > Don't use plane->state directly, use the pointer from commit_plane. > > Changes since v1: > - Fix uses of plane->state->rotation and color key to use the passed state too. > - Only pass crtc_state and plane_state to update_plane. > Changes since v2: > - Rebased. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_drv.h | 10 +-- > drivers/gpu/drm/i915/intel_sprite.c | 120 +++++++++++++++++++----------------- > 2 files changed, 67 insertions(+), 63 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index d1a6071afabe..16d4627364c5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -647,16 +647,12 @@ struct intel_plane { > /* > * NOTE: Do not place new plane state fields here (e.g., when adding > * new plane properties). New runtime state should now be placed in > - * the intel_plane_state structure and accessed via drm_plane->state. > + * the intel_plane_state structure and accessed via plane_state. > */ > > void (*update_plane)(struct drm_plane *plane, > - struct drm_crtc *crtc, > - struct drm_framebuffer *fb, > - int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t x, uint32_t y, > - uint32_t src_w, uint32_t src_h); > + struct intel_crtc_state *crtc_state, > + struct intel_plane_state *plane_state); These should be const. > void (*disable_plane)(struct drm_plane *plane, > struct drm_crtc *crtc); > int (*check_plane)(struct drm_plane *plane, > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 4276c135b9f2..6d3047f9c80f 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -178,28 +178,32 @@ void intel_pipe_update_end(struct intel_crtc *crtc) > } > > static void > -skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, > - int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t x, uint32_t y, > - uint32_t src_w, uint32_t src_h) > +skl_update_plane(struct drm_plane *drm_plane, > + struct intel_crtc_state *crtc_state, > + struct intel_plane_state *plane_state) > { > struct drm_device *dev = drm_plane->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_plane *intel_plane = to_intel_plane(drm_plane); > + struct drm_framebuffer *fb = plane_state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > const int pipe = intel_plane->pipe; > const int plane = intel_plane->plane + 1; > u32 plane_ctl, stride_div, stride; > - const struct drm_intel_sprite_colorkey *key = > - &to_intel_plane_state(drm_plane->state)->ckey; > + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; > unsigned long surf_addr; > u32 tile_height, plane_offset, plane_size; > unsigned int rotation; > int x_offset, y_offset; > - struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config; > - int scaler_id; > + What's with the extra newline? > + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; > + uint32_t crtc_w = drm_rect_width(&plane_state->dst); > + uint32_t crtc_h = drm_rect_height(&plane_state->dst); > + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; > + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; > + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; > + const struct intel_scaler *scaler = > + &crtc_state->scaler_state.scalers[plane_state->scaler_id]; This looks messy. I would at least get rid of the "assign two things on one line thing". > > plane_ctl = PLANE_CTL_ENABLE | > PLANE_CTL_PIPE_GAMMA_ENABLE | > @@ -208,14 +212,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, > plane_ctl |= skl_plane_ctl_format(fb->pixel_format); > plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]); > > - rotation = drm_plane->state->rotation; > + rotation = plane_state->base.rotation; > plane_ctl |= skl_plane_ctl_rotation(rotation); > > stride_div = intel_fb_stride_alignment(dev, fb->modifier[0], > fb->pixel_format); > > - scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id; > - > /* Sizes are 0 based */ > src_w--; > src_h--; > @@ -256,13 +258,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, > I915_WRITE(PLANE_SIZE(pipe, plane), plane_size); > > /* program plane scaler */ > - if (scaler_id >= 0) { > + if (plane_state->scaler_id >= 0) { > uint32_t ps_ctrl = 0; > + int scaler_id = plane_state->scaler_id; > > DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane, > PS_PLANE_SEL(plane)); > - ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | > - crtc_state->scaler_state.scalers[scaler_id].mode; > + ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | scaler->mode; > I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl); > I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0); > I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y); > @@ -334,24 +336,28 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format) > } > > static void > -vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, > - int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t x, uint32_t y, > - uint32_t src_w, uint32_t src_h) > +vlv_update_plane(struct drm_plane *dplane, > + struct intel_crtc_state *crtc_state, > + struct intel_plane_state *plane_state) > { > struct drm_device *dev = dplane->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_plane *intel_plane = to_intel_plane(dplane); > + struct drm_framebuffer *fb = plane_state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > int pipe = intel_plane->pipe; > int plane = intel_plane->plane; > u32 sprctl; > unsigned long sprsurf_offset, linear_offset; > int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > - const struct drm_intel_sprite_colorkey *key = > - &to_intel_plane_state(dplane->state)->ckey; > + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; > + > + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; > + uint32_t crtc_w = drm_rect_width(&plane_state->dst); > + uint32_t crtc_h = drm_rect_height(&plane_state->dst); > + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; > + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; > + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; > > sprctl = SP_ENABLE; > > @@ -421,7 +427,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, > fb->pitches[0]); > linear_offset -= sprsurf_offset; > > - if (dplane->state->rotation == BIT(DRM_ROTATE_180)) { > + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { > sprctl |= SP_ROTATE_180; > > x += src_w; > @@ -474,23 +480,27 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc) > } > > static void > -ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, > - int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t x, uint32_t y, > - uint32_t src_w, uint32_t src_h) > +ivb_update_plane(struct drm_plane *plane, > + struct intel_crtc_state *crtc_state, > + struct intel_plane_state *plane_state) > { > struct drm_device *dev = plane->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_plane *intel_plane = to_intel_plane(plane); > + struct drm_framebuffer *fb = plane_state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > enum pipe pipe = intel_plane->pipe; > u32 sprctl, sprscale = 0; > unsigned long sprsurf_offset, linear_offset; > int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > - const struct drm_intel_sprite_colorkey *key = > - &to_intel_plane_state(plane->state)->ckey; > + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; > + > + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; > + uint32_t crtc_w = drm_rect_width(&plane_state->dst); > + uint32_t crtc_h = drm_rect_height(&plane_state->dst); > + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; > + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; > + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; > > sprctl = SPRITE_ENABLE; > > @@ -550,7 +560,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > pixel_size, fb->pitches[0]); > linear_offset -= sprsurf_offset; > > - if (plane->state->rotation == BIT(DRM_ROTATE_180)) { > + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { > sprctl |= SPRITE_ROTATE_180; > > /* HSW and BDW does this automagically in hardware */ > @@ -612,23 +622,27 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) > } > > static void > -ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, > - int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t x, uint32_t y, > - uint32_t src_w, uint32_t src_h) > +ilk_update_plane(struct drm_plane *plane, > + struct intel_crtc_state *crtc_state, > + struct intel_plane_state *plane_state) > { > struct drm_device *dev = plane->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_plane *intel_plane = to_intel_plane(plane); > + struct drm_framebuffer *fb = plane_state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > int pipe = intel_plane->pipe; > unsigned long dvssurf_offset, linear_offset; > u32 dvscntr, dvsscale; > int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > - const struct drm_intel_sprite_colorkey *key = > - &to_intel_plane_state(plane->state)->ckey; > + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; > + > + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; > + uint32_t crtc_w = drm_rect_width(&plane_state->dst); > + uint32_t crtc_h = drm_rect_height(&plane_state->dst); > + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; > + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; > + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; > > dvscntr = DVS_ENABLE; > > @@ -684,7 +698,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > pixel_size, fb->pitches[0]); > linear_offset -= dvssurf_offset; > > - if (plane->state->rotation == BIT(DRM_ROTATE_180)) { > + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { > dvscntr |= DVS_ROTATE_180; > > x += src_w; > @@ -917,23 +931,17 @@ static void > intel_commit_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) I wonder why that isn't const... > { > - struct drm_crtc *crtc = state->base.crtc; > struct intel_plane *intel_plane = to_intel_plane(plane); > - struct drm_framebuffer *fb = state->base.fb; > - > - crtc = crtc ? crtc : plane->crtc; > > if (state->visible) { > - intel_plane->update_plane(plane, crtc, fb, > - state->dst.x1, state->dst.y1, > - drm_rect_width(&state->dst), > - drm_rect_height(&state->dst), > - state->src.x1 >> 16, > - state->src.y1 >> 16, > - drm_rect_width(&state->src) >> 16, > - drm_rect_height(&state->src) >> 16); > + struct intel_crtc_state *crtc_state = > + to_intel_crtc(state->base.crtc)->config; > + > + intel_plane->update_plane(plane, crtc_state, state); > } else { > - intel_plane->disable_plane(plane, crtc); > + struct drm_crtc *crtc = state->base.crtc; > + > + intel_plane->disable_plane(plane, crtc ?: plane->crtc); > } > } > > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 03-11-15 om 10:22 schreef Ville Syrjälä: > On Tue, Nov 03, 2015 at 08:31:40AM +0100, Maarten Lankhorst wrote: >> Don't use plane->state directly, use the pointer from commit_plane. >> >> Changes since v1: >> - Fix uses of plane->state->rotation and color key to use the passed state too. >> - Only pass crtc_state and plane_state to update_plane. >> Changes since v2: >> - Rebased. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_drv.h | 10 +-- >> drivers/gpu/drm/i915/intel_sprite.c | 120 +++++++++++++++++++----------------- >> 2 files changed, 67 insertions(+), 63 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index d1a6071afabe..16d4627364c5 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -647,16 +647,12 @@ struct intel_plane { >> /* >> * NOTE: Do not place new plane state fields here (e.g., when adding >> * new plane properties). New runtime state should now be placed in >> - * the intel_plane_state structure and accessed via drm_plane->state. >> + * the intel_plane_state structure and accessed via plane_state. >> */ >> >> void (*update_plane)(struct drm_plane *plane, >> - struct drm_crtc *crtc, >> - struct drm_framebuffer *fb, >> - int crtc_x, int crtc_y, >> - unsigned int crtc_w, unsigned int crtc_h, >> - uint32_t x, uint32_t y, >> - uint32_t src_w, uint32_t src_h); >> + struct intel_crtc_state *crtc_state, >> + struct intel_plane_state *plane_state); > These should be const. > >> void (*disable_plane)(struct drm_plane *plane, >> struct drm_crtc *crtc); >> int (*check_plane)(struct drm_plane *plane, >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c >> index 4276c135b9f2..6d3047f9c80f 100644 >> --- a/drivers/gpu/drm/i915/intel_sprite.c >> +++ b/drivers/gpu/drm/i915/intel_sprite.c >> @@ -178,28 +178,32 @@ void intel_pipe_update_end(struct intel_crtc *crtc) >> } >> >> static void >> -skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, >> - struct drm_framebuffer *fb, >> - int crtc_x, int crtc_y, >> - unsigned int crtc_w, unsigned int crtc_h, >> - uint32_t x, uint32_t y, >> - uint32_t src_w, uint32_t src_h) >> +skl_update_plane(struct drm_plane *drm_plane, >> + struct intel_crtc_state *crtc_state, >> + struct intel_plane_state *plane_state) >> { >> struct drm_device *dev = drm_plane->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_plane *intel_plane = to_intel_plane(drm_plane); >> + struct drm_framebuffer *fb = plane_state->base.fb; >> struct drm_i915_gem_object *obj = intel_fb_obj(fb); >> const int pipe = intel_plane->pipe; >> const int plane = intel_plane->plane + 1; >> u32 plane_ctl, stride_div, stride; >> - const struct drm_intel_sprite_colorkey *key = >> - &to_intel_plane_state(drm_plane->state)->ckey; >> + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; >> unsigned long surf_addr; >> u32 tile_height, plane_offset, plane_size; >> unsigned int rotation; >> int x_offset, y_offset; >> - struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config; >> - int scaler_id; >> + > What's with the extra newline? > >> + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; >> + uint32_t crtc_w = drm_rect_width(&plane_state->dst); >> + uint32_t crtc_h = drm_rect_height(&plane_state->dst); >> + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; >> + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; >> + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; >> + const struct intel_scaler *scaler = >> + &crtc_state->scaler_state.scalers[plane_state->scaler_id]; > This looks messy. I would at least get rid of the "assign two things on > one line thing". > >> >> plane_ctl = PLANE_CTL_ENABLE | >> PLANE_CTL_PIPE_GAMMA_ENABLE | >> @@ -208,14 +212,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, >> plane_ctl |= skl_plane_ctl_format(fb->pixel_format); >> plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]); >> >> - rotation = drm_plane->state->rotation; >> + rotation = plane_state->base.rotation; >> plane_ctl |= skl_plane_ctl_rotation(rotation); >> >> stride_div = intel_fb_stride_alignment(dev, fb->modifier[0], >> fb->pixel_format); >> >> - scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id; >> - >> /* Sizes are 0 based */ >> src_w--; >> src_h--; >> @@ -256,13 +258,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, >> I915_WRITE(PLANE_SIZE(pipe, plane), plane_size); >> >> /* program plane scaler */ >> - if (scaler_id >= 0) { >> + if (plane_state->scaler_id >= 0) { >> uint32_t ps_ctrl = 0; >> + int scaler_id = plane_state->scaler_id; >> >> DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane, >> PS_PLANE_SEL(plane)); >> - ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | >> - crtc_state->scaler_state.scalers[scaler_id].mode; >> + ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | scaler->mode; >> I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl); >> I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0); >> I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y); >> @@ -334,24 +336,28 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format) >> } >> >> static void >> -vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, >> - struct drm_framebuffer *fb, >> - int crtc_x, int crtc_y, >> - unsigned int crtc_w, unsigned int crtc_h, >> - uint32_t x, uint32_t y, >> - uint32_t src_w, uint32_t src_h) >> +vlv_update_plane(struct drm_plane *dplane, >> + struct intel_crtc_state *crtc_state, >> + struct intel_plane_state *plane_state) >> { >> struct drm_device *dev = dplane->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_plane *intel_plane = to_intel_plane(dplane); >> + struct drm_framebuffer *fb = plane_state->base.fb; >> struct drm_i915_gem_object *obj = intel_fb_obj(fb); >> int pipe = intel_plane->pipe; >> int plane = intel_plane->plane; >> u32 sprctl; >> unsigned long sprsurf_offset, linear_offset; >> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); >> - const struct drm_intel_sprite_colorkey *key = >> - &to_intel_plane_state(dplane->state)->ckey; >> + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; >> + >> + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; >> + uint32_t crtc_w = drm_rect_width(&plane_state->dst); >> + uint32_t crtc_h = drm_rect_height(&plane_state->dst); >> + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; >> + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; >> + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; >> >> sprctl = SP_ENABLE; >> >> @@ -421,7 +427,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, >> fb->pitches[0]); >> linear_offset -= sprsurf_offset; >> >> - if (dplane->state->rotation == BIT(DRM_ROTATE_180)) { >> + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { >> sprctl |= SP_ROTATE_180; >> >> x += src_w; >> @@ -474,23 +480,27 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc) >> } >> >> static void >> -ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, >> - struct drm_framebuffer *fb, >> - int crtc_x, int crtc_y, >> - unsigned int crtc_w, unsigned int crtc_h, >> - uint32_t x, uint32_t y, >> - uint32_t src_w, uint32_t src_h) >> +ivb_update_plane(struct drm_plane *plane, >> + struct intel_crtc_state *crtc_state, >> + struct intel_plane_state *plane_state) >> { >> struct drm_device *dev = plane->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_plane *intel_plane = to_intel_plane(plane); >> + struct drm_framebuffer *fb = plane_state->base.fb; >> struct drm_i915_gem_object *obj = intel_fb_obj(fb); >> enum pipe pipe = intel_plane->pipe; >> u32 sprctl, sprscale = 0; >> unsigned long sprsurf_offset, linear_offset; >> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); >> - const struct drm_intel_sprite_colorkey *key = >> - &to_intel_plane_state(plane->state)->ckey; >> + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; >> + >> + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; >> + uint32_t crtc_w = drm_rect_width(&plane_state->dst); >> + uint32_t crtc_h = drm_rect_height(&plane_state->dst); >> + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; >> + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; >> + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; >> >> sprctl = SPRITE_ENABLE; >> >> @@ -550,7 +560,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, >> pixel_size, fb->pitches[0]); >> linear_offset -= sprsurf_offset; >> >> - if (plane->state->rotation == BIT(DRM_ROTATE_180)) { >> + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { >> sprctl |= SPRITE_ROTATE_180; >> >> /* HSW and BDW does this automagically in hardware */ >> @@ -612,23 +622,27 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) >> } >> >> static void >> -ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, >> - struct drm_framebuffer *fb, >> - int crtc_x, int crtc_y, >> - unsigned int crtc_w, unsigned int crtc_h, >> - uint32_t x, uint32_t y, >> - uint32_t src_w, uint32_t src_h) >> +ilk_update_plane(struct drm_plane *plane, >> + struct intel_crtc_state *crtc_state, >> + struct intel_plane_state *plane_state) >> { >> struct drm_device *dev = plane->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_plane *intel_plane = to_intel_plane(plane); >> + struct drm_framebuffer *fb = plane_state->base.fb; >> struct drm_i915_gem_object *obj = intel_fb_obj(fb); >> int pipe = intel_plane->pipe; >> unsigned long dvssurf_offset, linear_offset; >> u32 dvscntr, dvsscale; >> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); >> - const struct drm_intel_sprite_colorkey *key = >> - &to_intel_plane_state(plane->state)->ckey; >> + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; >> + >> + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; >> + uint32_t crtc_w = drm_rect_width(&plane_state->dst); >> + uint32_t crtc_h = drm_rect_height(&plane_state->dst); >> + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; >> + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; >> + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; >> >> dvscntr = DVS_ENABLE; >> >> @@ -684,7 +698,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, >> pixel_size, fb->pitches[0]); >> linear_offset -= dvssurf_offset; >> >> - if (plane->state->rotation == BIT(DRM_ROTATE_180)) { >> + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { >> dvscntr |= DVS_ROTATE_180; >> >> x += src_w; >> @@ -917,23 +931,17 @@ static void >> intel_commit_sprite_plane(struct drm_plane *plane, >> struct intel_plane_state *state) > I wonder why that isn't const... > I'm guessing because until recently crtc_state wasn't really that const. :) I agree we should constify it, but shouldn't that be in a followup patch for readability? ~Maarten
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d1a6071afabe..16d4627364c5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -647,16 +647,12 @@ struct intel_plane { /* * NOTE: Do not place new plane state fields here (e.g., when adding * new plane properties). New runtime state should now be placed in - * the intel_plane_state structure and accessed via drm_plane->state. + * the intel_plane_state structure and accessed via plane_state. */ void (*update_plane)(struct drm_plane *plane, - struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t x, uint32_t y, - uint32_t src_w, uint32_t src_h); + struct intel_crtc_state *crtc_state, + struct intel_plane_state *plane_state); void (*disable_plane)(struct drm_plane *plane, struct drm_crtc *crtc); int (*check_plane)(struct drm_plane *plane, diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 4276c135b9f2..6d3047f9c80f 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -178,28 +178,32 @@ void intel_pipe_update_end(struct intel_crtc *crtc) } static void -skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t x, uint32_t y, - uint32_t src_w, uint32_t src_h) +skl_update_plane(struct drm_plane *drm_plane, + struct intel_crtc_state *crtc_state, + struct intel_plane_state *plane_state) { struct drm_device *dev = drm_plane->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(drm_plane); + struct drm_framebuffer *fb = plane_state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); const int pipe = intel_plane->pipe; const int plane = intel_plane->plane + 1; u32 plane_ctl, stride_div, stride; - const struct drm_intel_sprite_colorkey *key = - &to_intel_plane_state(drm_plane->state)->ckey; + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; unsigned long surf_addr; u32 tile_height, plane_offset, plane_size; unsigned int rotation; int x_offset, y_offset; - struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config; - int scaler_id; + + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; + uint32_t crtc_w = drm_rect_width(&plane_state->dst); + uint32_t crtc_h = drm_rect_height(&plane_state->dst); + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; + const struct intel_scaler *scaler = + &crtc_state->scaler_state.scalers[plane_state->scaler_id]; plane_ctl = PLANE_CTL_ENABLE | PLANE_CTL_PIPE_GAMMA_ENABLE | @@ -208,14 +212,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, plane_ctl |= skl_plane_ctl_format(fb->pixel_format); plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]); - rotation = drm_plane->state->rotation; + rotation = plane_state->base.rotation; plane_ctl |= skl_plane_ctl_rotation(rotation); stride_div = intel_fb_stride_alignment(dev, fb->modifier[0], fb->pixel_format); - scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id; - /* Sizes are 0 based */ src_w--; src_h--; @@ -256,13 +258,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, I915_WRITE(PLANE_SIZE(pipe, plane), plane_size); /* program plane scaler */ - if (scaler_id >= 0) { + if (plane_state->scaler_id >= 0) { uint32_t ps_ctrl = 0; + int scaler_id = plane_state->scaler_id; DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane, PS_PLANE_SEL(plane)); - ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | - crtc_state->scaler_state.scalers[scaler_id].mode; + ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | scaler->mode; I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl); I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0); I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y); @@ -334,24 +336,28 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format) } static void -vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t x, uint32_t y, - uint32_t src_w, uint32_t src_h) +vlv_update_plane(struct drm_plane *dplane, + struct intel_crtc_state *crtc_state, + struct intel_plane_state *plane_state) { struct drm_device *dev = dplane->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(dplane); + struct drm_framebuffer *fb = plane_state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); int pipe = intel_plane->pipe; int plane = intel_plane->plane; u32 sprctl; unsigned long sprsurf_offset, linear_offset; int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); - const struct drm_intel_sprite_colorkey *key = - &to_intel_plane_state(dplane->state)->ckey; + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; + + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; + uint32_t crtc_w = drm_rect_width(&plane_state->dst); + uint32_t crtc_h = drm_rect_height(&plane_state->dst); + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; sprctl = SP_ENABLE; @@ -421,7 +427,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, fb->pitches[0]); linear_offset -= sprsurf_offset; - if (dplane->state->rotation == BIT(DRM_ROTATE_180)) { + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { sprctl |= SP_ROTATE_180; x += src_w; @@ -474,23 +480,27 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc) } static void -ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t x, uint32_t y, - uint32_t src_w, uint32_t src_h) +ivb_update_plane(struct drm_plane *plane, + struct intel_crtc_state *crtc_state, + struct intel_plane_state *plane_state) { struct drm_device *dev = plane->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(plane); + struct drm_framebuffer *fb = plane_state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); enum pipe pipe = intel_plane->pipe; u32 sprctl, sprscale = 0; unsigned long sprsurf_offset, linear_offset; int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); - const struct drm_intel_sprite_colorkey *key = - &to_intel_plane_state(plane->state)->ckey; + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; + + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; + uint32_t crtc_w = drm_rect_width(&plane_state->dst); + uint32_t crtc_h = drm_rect_height(&plane_state->dst); + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; sprctl = SPRITE_ENABLE; @@ -550,7 +560,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, pixel_size, fb->pitches[0]); linear_offset -= sprsurf_offset; - if (plane->state->rotation == BIT(DRM_ROTATE_180)) { + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { sprctl |= SPRITE_ROTATE_180; /* HSW and BDW does this automagically in hardware */ @@ -612,23 +622,27 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) } static void -ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t x, uint32_t y, - uint32_t src_w, uint32_t src_h) +ilk_update_plane(struct drm_plane *plane, + struct intel_crtc_state *crtc_state, + struct intel_plane_state *plane_state) { struct drm_device *dev = plane->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(plane); + struct drm_framebuffer *fb = plane_state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); int pipe = intel_plane->pipe; unsigned long dvssurf_offset, linear_offset; u32 dvscntr, dvsscale; int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); - const struct drm_intel_sprite_colorkey *key = - &to_intel_plane_state(plane->state)->ckey; + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey; + + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; + uint32_t crtc_w = drm_rect_width(&plane_state->dst); + uint32_t crtc_h = drm_rect_height(&plane_state->dst); + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16; + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16; dvscntr = DVS_ENABLE; @@ -684,7 +698,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, pixel_size, fb->pitches[0]); linear_offset -= dvssurf_offset; - if (plane->state->rotation == BIT(DRM_ROTATE_180)) { + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) { dvscntr |= DVS_ROTATE_180; x += src_w; @@ -917,23 +931,17 @@ static void intel_commit_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) { - struct drm_crtc *crtc = state->base.crtc; struct intel_plane *intel_plane = to_intel_plane(plane); - struct drm_framebuffer *fb = state->base.fb; - - crtc = crtc ? crtc : plane->crtc; if (state->visible) { - intel_plane->update_plane(plane, crtc, fb, - state->dst.x1, state->dst.y1, - drm_rect_width(&state->dst), - drm_rect_height(&state->dst), - state->src.x1 >> 16, - state->src.y1 >> 16, - drm_rect_width(&state->src) >> 16, - drm_rect_height(&state->src) >> 16); + struct intel_crtc_state *crtc_state = + to_intel_crtc(state->base.crtc)->config; + + intel_plane->update_plane(plane, crtc_state, state); } else { - intel_plane->disable_plane(plane, crtc); + struct drm_crtc *crtc = state->base.crtc; + + intel_plane->disable_plane(plane, crtc ?: plane->crtc); } }
Don't use plane->state directly, use the pointer from commit_plane. Changes since v1: - Fix uses of plane->state->rotation and color key to use the passed state too. - Only pass crtc_state and plane_state to update_plane. Changes since v2: - Rebased. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_drv.h | 10 +-- drivers/gpu/drm/i915/intel_sprite.c | 120 +++++++++++++++++++----------------- 2 files changed, 67 insertions(+), 63 deletions(-)