From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/7] drm/i915: Use passed plane state for sprite planes, v4.
Date: Thu, 7 Jan 2016 14:15:56 +0200 [thread overview]
Message-ID: <20160107121556.GT4437@intel.com> (raw)
In-Reply-To: <1452164052-21752-2-git-send-email-maarten.lankhorst@linux.intel.com>
On Thu, Jan 07, 2016 at 11:54:06AM +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.
> Changes since v3:
> - Small whitespace changes and only assign 1 variable per line.
> - Constify plane_state and crtc_state. (vsyrjala)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 10 +--
> drivers/gpu/drm/i915/intel_sprite.c | 124 ++++++++++++++++++++----------------
> 2 files changed, 71 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a3b2025da563..068050884353 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -658,16 +658,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);
> + const struct intel_crtc_state *crtc_state,
> + const 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 4ff7a1f4183e..9f64289333e8 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -178,28 +178,33 @@ 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,
> + const struct intel_crtc_state *crtc_state,
> + const 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;
> u32 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;
> + int 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;
> + uint32_t 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 +213,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 +259,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 +337,29 @@ 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,
> + const struct intel_crtc_state *crtc_state,
> + const 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;
> + int 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;
> + uint32_t 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 +429,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 +482,28 @@ 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,
> + const struct intel_crtc_state *crtc_state,
> + const 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;
> + int 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;
> + uint32_t 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 +563,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 +625,28 @@ 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,
> + const struct intel_crtc_state *crtc_state,
> + const 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;
> + int 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;
> + uint32_t 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 +702,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 +935,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);
> }
> }
>
> --
> 2.1.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-01-07 12:16 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-07 10:54 [PATCH 0/7] Explicitly pass crtc_state and plane_state to plane update functions Maarten Lankhorst
2016-01-07 10:54 ` [PATCH 1/7] drm/i915: Use passed plane state for sprite planes, v4 Maarten Lankhorst
2016-01-07 12:15 ` Ville Syrjälä [this message]
2016-01-07 10:54 ` [PATCH 2/7] drm/i915: Do not use commit_plane for sprite planes Maarten Lankhorst
2016-01-07 12:20 ` Ville Syrjälä
2016-01-07 10:54 ` [PATCH 3/7] drm/i915: Remove some visibility checks from intel_crtc_update_cursor Maarten Lankhorst
2016-01-07 12:22 ` Ville Syrjälä
2016-01-07 10:54 ` [PATCH 4/7] drm/i915: Make disable_cursor_plane similar to commit_cursor_plane Maarten Lankhorst
2016-01-07 12:25 ` Ville Syrjälä
2016-01-07 10:54 ` [PATCH 5/7] drm/i915: Use the plane state for cursor updates Maarten Lankhorst
2016-01-07 12:29 ` Ville Syrjälä
2016-01-07 10:54 ` [PATCH 6/7] drm/i915: Use plane state for primary plane updates Maarten Lankhorst
2016-01-07 12:38 ` Ville Syrjälä
2016-01-07 10:54 ` [PATCH 7/7] drm/i915: Remove commit_plane function pointer Maarten Lankhorst
2016-01-07 12:39 ` Ville Syrjälä
2016-01-11 8:53 ` ✗ failure: Fi.CI.BAT Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160107121556.GT4437@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.