From: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/11] drm/i915: Use passed plane state for sprite planes.
Date: Thu, 22 Oct 2015 14:58:19 +0200 [thread overview]
Message-ID: <20151022125819.GS16848@phenom.ffwll.local> (raw)
In-Reply-To: <1445514996-18733-2-git-send-email-maarten.lankhorst@linux.intel.com>
On Thu, Oct 22, 2015 at 01:56:26PM +0200, Maarten Lankhorst wrote:
> Don't use plane->state directly, use the pointer from commit_plane.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 8 ++---
> drivers/gpu/drm/i915/intel_sprite.c | 67 +++++++++++++++++++++++--------------
> 2 files changed, 45 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4e35557bbd41..51722e657b91 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -631,16 +631,16 @@ 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,
> + struct intel_plane_state *plane_state,
> 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);
> + uint32_t src_w, uint32_t src_h,
> + struct intel_scaler *scaler);
This is an ugly hack imo. Better to add the scaler pointer to the plane
state, set it in compute_config once we're done and not pass it around
like this. Or we also pass around a pointer to the right crtc_state.
Also, if we pass around plane_state I think all the crtc_x/y/w/h and x/y
and src_x/y/w/h should disappear, for a really clean vfunc interface.
-Daniel
> 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 2551335ada04..599d001b1415 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -178,16 +178,19 @@ 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,
> +skl_update_plane(struct drm_plane *drm_plane,
> + struct intel_plane_state *plane_state,
> 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)
> + uint32_t src_w, uint32_t src_h,
> + struct intel_scaler *scaler)
> {
> 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_crtc *crtc = plane_state->base.crtc;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> const int pipe = intel_plane->pipe;
> const int plane = intel_plane->plane + 1;
> @@ -199,8 +202,6 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> 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;
>
> plane_ctl = PLANE_CTL_ENABLE |
> PLANE_CTL_PIPE_GAMMA_ENABLE |
> @@ -219,8 +220,6 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> 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--;
> @@ -261,13 +260,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 (scaler) {
> 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);
> @@ -341,16 +340,18 @@ 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,
> +vlv_update_plane(struct drm_plane *dplane,
> + struct intel_plane_state *plane_state,
> 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)
> + uint32_t src_w, uint32_t src_h,
> + struct intel_scaler *scaler)
> {
> 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;
> @@ -481,16 +482,19 @@ 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,
> +ivb_update_plane(struct drm_plane *plane,
> + struct intel_plane_state *plane_state,
> 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)
> + uint32_t src_w, uint32_t src_h,
> + struct intel_scaler *scaler)
> {
> 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_crtc *crtc = plane_state->base.crtc;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> enum pipe pipe = intel_plane->pipe;
> u32 sprctl, sprscale = 0;
> @@ -623,16 +627,19 @@ 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,
> +ilk_update_plane(struct drm_plane *plane,
> + struct intel_plane_state *plane_state,
> 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)
> + uint32_t src_w, uint32_t src_h,
> + struct intel_scaler *scaler)
> {
> 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_crtc *crtc = plane_state->base.crtc;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> int pipe = intel_plane->pipe;
> unsigned long dvssurf_offset, linear_offset;
> @@ -932,23 +939,31 @@ 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,
> + struct intel_scaler *scaler = NULL;
> +
> + if (state->scaler_id >= 0) {
> + struct intel_crtc_state *crtc_state =
> + to_intel_crtc(state->base.crtc)->config;
> +
> + scaler = &crtc_state->scaler_state.scalers[state->scaler_id];
> + }
> +
> + intel_plane->update_plane(plane, state,
> 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);
> + drm_rect_height(&state->src) >> 16,
> + scaler);
> } 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-10-22 12:58 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-22 11:56 [PATCH 00/11] Kill off intel_crtc->atomic! Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 01/11] drm/i915: Use passed plane state for sprite planes Maarten Lankhorst
2015-10-22 12:58 ` Daniel Vetter [this message]
2015-10-22 13:09 ` Maarten Lankhorst
2015-10-26 9:30 ` [PATCH v2 01/11] drm/i915: Use passed plane state for sprite planes, v2 Maarten Lankhorst
2015-11-17 15:23 ` Daniel Vetter
2015-10-22 11:56 ` [PATCH 02/11] drm/i915: Do not acquire crtc state to check clock during modeset Maarten Lankhorst
2015-10-22 13:08 ` Daniel Vetter
2015-10-22 13:42 ` Maarten Lankhorst
2015-10-22 13:55 ` Daniel Vetter
2015-10-22 13:31 ` Ville Syrjälä
2015-10-27 13:26 ` [PATCH 1/3] drm/i915: Do not acquire crtc state to check clock during modeset, v2 Maarten Lankhorst
2015-10-27 13:26 ` [PATCH 2/3] drm/i915: Handle cdclk limits on broadwell Maarten Lankhorst
2015-10-27 13:26 ` [PATCH 3/3] drm/i915/bxt: Use the bypass frequency if there are no active pipes Maarten Lankhorst
2015-10-27 13:31 ` Ville Syrjälä
2015-10-27 13:29 ` [PATCH 1/3] drm/i915: Do not acquire crtc state to check clock during modeset, v2 Ville Syrjälä
2015-10-27 13:41 ` Maarten Lankhorst
2015-10-27 13:49 ` Ville Syrjälä
2015-10-27 13:51 ` Maarten Lankhorst
2015-10-27 14:27 ` Ville Syrjälä
2015-10-22 11:56 ` [PATCH 03/11] drm/i915: Kill off intel_crtc->atomic.wait_vblank Maarten Lankhorst
2015-10-22 13:09 ` Daniel Vetter
2015-10-22 13:30 ` Ville Syrjälä
2015-10-22 14:50 ` Maarten Lankhorst
2015-10-22 15:15 ` Ville Syrjälä
2015-10-26 8:31 ` Maarten Lankhorst
2015-11-17 15:25 ` Daniel Vetter
2015-10-22 11:56 ` [PATCH 04/11] drm/i915: Update watermark related members in the crtc_state Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 05/11] drm/i915: Remove intel_crtc->atomic.disable_ips Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 06/11] drm/i915: Remove atomic.pre_disable_primary Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 07/11] drm/i915: Remove some post-commit members from intel_crtc->atomic Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 08/11] drm/i915: Nuke fbc " Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 09/11] drm/i915/skl: Prevent unclaimed register writes on skylake Maarten Lankhorst
2015-10-22 13:11 ` [Intel-gfx] " Daniel Vetter
2015-10-23 7:54 ` Jani Nikula
2015-11-02 8:01 ` Jani Nikula
2015-10-22 11:56 ` [PATCH 10/11] drm/i915/skl: Update watermarks before the crtc is disabled Maarten Lankhorst
2015-10-22 13:15 ` [Intel-gfx] " Daniel Vetter
2015-10-22 11:56 ` [PATCH 11/11] drm/i915/skl: Do not allow scaling when " Maarten Lankhorst
2015-10-22 13:17 ` Daniel Vetter
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=20151022125819.GS16848@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox