From: Matt Roper <matthew.d.roper@intel.com>
To: ville.syrjala@linux.intel.com
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/9] drm/i915: Pass primary plane size to .update_primary_plane()
Date: Tue, 10 Mar 2015 10:10:40 -0700 [thread overview]
Message-ID: <20150310171040.GI27526@intel.com> (raw)
In-Reply-To: <1425986129-15547-6-git-send-email-ville.syrjala@linux.intel.com>
On Tue, Mar 10, 2015 at 01:15:25PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> In preparation to movable/resizable primary planes pass the clipped
> plane size to .update_primary_plane().
Personally I feel like it would make more sense to just completely kill
off .update_primary_plane() now rather than trying to evolve it. We
already have an intel_plane->update_plane() function pointer which is
never set or called for non-sprites at the moment. We could unify the
handling of low-level plane programming by just using that function
pointer for primary planes as well.
I wouldn't mind also seeing the name of that low-level entrypoint
renamed to something like 'update_hw_plane' to avoid confusion with the
drm_plane entrypoint...
Matt
>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_display.c | 63 ++++++++++++++++++++----------------
> 2 files changed, 38 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b16c0a7..e99eef0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -578,7 +578,8 @@ struct drm_i915_display_funcs {
> uint32_t flags);
> void (*update_primary_plane)(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> - int x, int y);
> + int x, int y,
> + int crtc_w, int crtc_h);
> void (*hpd_irq_setup)(struct drm_device *dev);
> /* clock updates for mode set */
> /* cursor updates */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fdc83f1..1a789f0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2482,7 +2482,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
>
> static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> - int x, int y)
> + int x, int y,
> + int crtc_w, int crtc_h)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2517,20 +2518,16 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> if (INTEL_INFO(dev)->gen < 4) {
> if (intel_crtc->pipe == PIPE_B)
> dspcntr |= DISPPLANE_SEL_PIPE_B;
> -
> - /* pipesrc and dspsize control the size that is scaled from,
> - * which should always be the user's requested size.
> - */
> - I915_WRITE(DSPSIZE(plane),
> - ((intel_crtc->config->pipe_src_h - 1) << 16) |
> - (intel_crtc->config->pipe_src_w - 1));
> + I915_WRITE(DSPSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1));
> I915_WRITE(DSPPOS(plane), 0);
> } else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
> - I915_WRITE(PRIMSIZE(plane),
> - ((intel_crtc->config->pipe_src_h - 1) << 16) |
> - (intel_crtc->config->pipe_src_w - 1));
> + I915_WRITE(PRIMSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1));
> I915_WRITE(PRIMPOS(plane), 0);
> I915_WRITE(PRIMCNSTALPHA(plane), 0);
> + } else {
> + WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
> + crtc_h != intel_crtc->config->pipe_src_h,
> + "primary plane size doesn't match pipe size\n");
> }
>
> switch (fb->pixel_format) {
> @@ -2586,14 +2583,14 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
> dspcntr |= DISPPLANE_ROTATE_180;
>
> - x += (intel_crtc->config->pipe_src_w - 1);
> - y += (intel_crtc->config->pipe_src_h - 1);
> + x += (crtc_w - 1);
> + y += (crtc_h - 1);
>
> /* Finding the last pixel of the last line of the display
> data and adding to linear_offset*/
> linear_offset +=
> - (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
> - (intel_crtc->config->pipe_src_w - 1) * pixel_size;
> + (crtc_h - 1) * fb->pitches[0] +
> + (crtc_w - 1) * pixel_size;
> }
>
> I915_WRITE(reg, dspcntr);
> @@ -2611,7 +2608,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>
> static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> - int x, int y)
> + int x, int y,
> + int crtc_w, int crtc_h)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2643,6 +2641,10 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
>
> + WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
> + crtc_h != intel_crtc->config->pipe_src_h,
> + "primary plane size doesn't match pipe size\n");
> +
> switch (fb->pixel_format) {
> case DRM_FORMAT_C8:
> dspcntr |= DISPPLANE_8BPP;
> @@ -2686,14 +2688,14 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> dspcntr |= DISPPLANE_ROTATE_180;
>
> if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> - x += (intel_crtc->config->pipe_src_w - 1);
> - y += (intel_crtc->config->pipe_src_h - 1);
> + x += (crtc_w - 1);
> + y += (crtc_h - 1);
>
> /* Finding the last pixel of the last line of the display
> data and adding to linear_offset*/
> linear_offset +=
> - (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
> - (intel_crtc->config->pipe_src_w - 1) * pixel_size;
> + (crtc_h - 1) * fb->pitches[0] +
> + (crtc_w - 1) * pixel_size;
> }
> }
>
> @@ -2747,7 +2749,8 @@ u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
>
> static void skylake_update_primary_plane(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> - int x, int y)
> + int x, int y,
> + int crtc_w, int crtc_h)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2826,9 +2829,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>
> I915_WRITE(PLANE_POS(pipe, 0), 0);
> I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
> - I915_WRITE(PLANE_SIZE(pipe, 0),
> - (intel_crtc->config->pipe_src_h - 1) << 16 |
> - (intel_crtc->config->pipe_src_w - 1));
> + I915_WRITE(PLANE_SIZE(pipe, 0), ((crtc_h - 1) << 16) | (crtc_w - 1));
> I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
> I915_WRITE(PLANE_SURF(pipe, 0), i915_gem_obj_ggtt_offset(obj));
>
> @@ -2845,12 +2846,16 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> if (dev_priv->display.disable_fbc)
> dev_priv->display.disable_fbc(dev);
>
> + /* FIXME: this will go badly if the fb isn't big enough */
> to_intel_crtc(crtc)->primary_enabled = true;
> - dev_priv->display.update_primary_plane(crtc, fb, x, y);
> + dev_priv->display.update_primary_plane(crtc, fb, x, y,
> + intel_crtc->config->pipe_src_w,
> + intel_crtc->config->pipe_src_h);
>
> return 0;
> }
> @@ -2885,7 +2890,9 @@ static void intel_update_primary_planes(struct drm_device *dev)
> dev_priv->display.update_primary_plane(crtc,
> state->base.fb,
> state->src.x1 >> 16,
> - state->src.y1 >> 16);
> + state->src.y1 >> 16,
> + drm_rect_width(&state->dst),
> + drm_rect_height(&state->dst));
> }
>
> drm_modeset_unlock(&crtc->mutex);
> @@ -12019,7 +12026,9 @@ intel_commit_primary_plane(struct drm_plane *plane,
> dev_priv->display.update_primary_plane(crtc,
> state->base.fb,
> state->src.x1 >> 16,
> - state->src.y1 >> 16);
> + state->src.y1 >> 16,
> + drm_rect_width(&state->dst),
> + drm_rect_height(&state->dst));
> }
> }
>
> --
> 2.0.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-03-10 17:11 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-10 11:15 [PATCH 0/9] drm/i915: Update derived plane state at crtc enable/disable ville.syrjala
2015-03-10 11:15 ` [PATCH 1/9] drm/i915: Remove debug prints from primary plane update funcs ville.syrjala
2015-03-10 15:55 ` Jesse Barnes
2015-03-10 11:15 ` [PATCH 2/9] drm/i915: Reduce clutter by using the local plane pointer ville.syrjala
2015-03-10 16:00 ` Jesse Barnes
2015-03-10 11:15 ` [PATCH 3/9] drm/i915: Use plane->state->fb instead of plane->fb in intel_plane_restore() ville.syrjala
2015-03-10 17:01 ` Matt Roper
2015-03-10 17:48 ` Ville Syrjälä
2015-03-11 9:41 ` Daniel Vetter
2015-03-11 10:04 ` Daniel Vetter
2015-03-10 11:15 ` [PATCH 4/9] drm/i915: Make derived plane state correct after crtc_enable ville.syrjala
2015-03-10 17:01 ` Matt Roper
2015-03-10 17:57 ` Ville Syrjälä
2015-03-11 9:52 ` Daniel Vetter
2015-03-11 10:03 ` Daniel Vetter
2015-03-11 10:05 ` Ville Syrjälä
2015-03-11 10:24 ` Daniel Vetter
2015-03-11 12:19 ` Ville Syrjälä
2015-03-11 16:23 ` Daniel Vetter
2015-03-11 16:40 ` Ville Syrjälä
2015-03-10 11:15 ` [PATCH 5/9] drm/i915: Pass primary plane size to .update_primary_plane() ville.syrjala
2015-03-10 17:10 ` Matt Roper [this message]
2015-03-10 17:59 ` Ville Syrjälä
2015-03-10 20:57 ` Matt Roper
2015-03-11 9:42 ` Ville Syrjälä
2015-03-11 9:57 ` Daniel Vetter
2015-03-11 5:09 ` sonika
2015-03-11 9:27 ` Ville Syrjälä
2015-03-11 9:26 ` sonika
2015-03-19 14:28 ` [PATCH v2 " ville.syrjala
2015-03-20 9:49 ` Jindal, Sonika
2015-03-20 10:04 ` Ville Syrjälä
2015-03-20 10:53 ` Jindal, Sonika
2015-03-20 14:26 ` Ville Syrjälä
2015-03-23 4:11 ` sonika
2015-03-10 11:15 ` [PATCH 6/9] drm/i915: Pass the primary plane position " ville.syrjala
2015-03-19 14:29 ` [PATCH v2 " ville.syrjala
2015-03-20 11:22 ` Jindal, Sonika
2015-03-10 11:15 ` [PATCH 7/9] drm/i915: Update watermarks after the derived plane state is uptodate ville.syrjala
2015-03-10 17:13 ` Matt Roper
2015-03-11 9:59 ` Daniel Vetter
2015-03-10 11:15 ` [PATCH 8/9] drm/i915: Use state->visible in wm calculation ville.syrjala
2015-03-10 17:19 ` Matt Roper
2015-03-10 18:01 ` Ville Syrjälä
2015-03-19 14:31 ` [PATCH v2 " ville.syrjala
2015-03-10 11:15 ` [PATCH 9/9] drm/i915: Don't re-enable an explicitly disabled primary plane due to sprite coverage changes ville.syrjala
2015-03-10 17:58 ` shuang.he
2015-03-11 10:00 ` Daniel Vetter
2015-03-11 10:09 ` Ville Syrjälä
2015-03-11 10:28 ` 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=20150310171040.GI27526@intel.com \
--to=matthew.d.roper@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@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