From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/i915: refactor ilk display interrupt handling
Date: Mon, 21 Oct 2013 19:48:58 +0300 [thread overview]
Message-ID: <20131021164858.GC13047@intel.com> (raw)
In-Reply-To: <1382371476-25639-2-git-send-email-daniel.vetter@ffwll.ch>
On Mon, Oct 21, 2013 at 06:04:36PM +0200, Daniel Vetter wrote:
> - Use a for_each_loop and add the corresponding #defines.
> - Drop the _ILK postfix on the existing DE_PIPE_VBLANK macro for
> consistency with everything else.
> - Also use macros (and add the missing one for plane flips) for the
> ivb display interrupt handler.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 50 +++++++++++++++++------------------------
> drivers/gpu/drm/i915/i915_reg.h | 7 ++++--
> 2 files changed, 26 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ce94afc..062a6d6 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1535,6 +1535,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + enum pipe pipe;
>
> if (de_iir & DE_AUX_CHANNEL_A)
> dp_aux_irq_handler(dev);
> @@ -1542,37 +1543,26 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> if (de_iir & DE_GSE)
> intel_opregion_asle_intr(dev);
>
> - if (de_iir & DE_PIPEA_VBLANK)
> - drm_handle_vblank(dev, 0);
> -
> - if (de_iir & DE_PIPEB_VBLANK)
> - drm_handle_vblank(dev, 1);
> -
> if (de_iir & DE_POISON)
> DRM_ERROR("Poison interrupt\n");
>
> - if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
> - if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
> - DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
> -
> - if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
> - if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
> - DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
> -
> - if (de_iir & DE_PIPEA_CRC_DONE)
> - i9xx_pipe_crc_irq_handler(dev, PIPE_A);
> + for_each_pipe(pipe) {
> + if (de_iir & DE_PIPE_VBLANK(pipe))
> + drm_handle_vblank(dev, pipe);
>
> - if (de_iir & DE_PIPEB_CRC_DONE)
> - i9xx_pipe_crc_irq_handler(dev, PIPE_B);
> + if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
> + if (intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> + DRM_DEBUG_DRIVER("Pipe %c FIFO underrun\n",
> + pipe_name(pipe));
>
> - if (de_iir & DE_PLANEA_FLIP_DONE) {
> - intel_prepare_page_flip(dev, 0);
> - intel_finish_page_flip_plane(dev, 0);
> - }
> + if (de_iir & DE_PIPE_CRC_DONE(pipe))
> + i9xx_pipe_crc_irq_handler(dev, pipe);
>
> - if (de_iir & DE_PLANEB_FLIP_DONE) {
> - intel_prepare_page_flip(dev, 1);
> - intel_finish_page_flip_plane(dev, 1);
> + /* plane/pipes map 1:1 on ilk+ */
> + if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
> + intel_prepare_page_flip(dev, pipe);
> + intel_finish_page_flip_plane(dev, pipe);
> + }
> }
>
> /* check event from PCH */
> @@ -1607,9 +1597,11 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
> intel_opregion_asle_intr(dev);
>
> for_each_pipe(i) {
> - if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
> + if (de_iir & (DE_PIPE_VBLANK_IVB(i)))
^ ^
> drm_handle_vblank(dev, i);
> - if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
> +
> + /* plane/pipes map 1:1 on ilk+ */
> + if (de_iir & (DE_PLANE_FLIP_DONE_IVB(i))) {
^ ^
Just a minor complaint about the useless parens.
But otherwise both patches look good, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> intel_prepare_page_flip(dev, i);
> intel_finish_page_flip_plane(dev, i);
> }
> @@ -2012,7 +2004,7 @@ static int ironlake_enable_vblank(struct drm_device *dev, int pipe)
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> unsigned long irqflags;
> uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> - DE_PIPE_VBLANK_ILK(pipe);
> + DE_PIPE_VBLANK(pipe);
>
> if (!i915_pipe_enabled(dev, pipe))
> return -EINVAL;
> @@ -2070,7 +2062,7 @@ static void ironlake_disable_vblank(struct drm_device *dev, int pipe)
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> unsigned long irqflags;
> uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> - DE_PIPE_VBLANK_ILK(pipe);
> + DE_PIPE_VBLANK(pipe);
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> ironlake_disable_display_irq(dev_priv, bit);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c97fc94..1af080a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3928,6 +3928,7 @@
> #define DE_SPRITEA_FLIP_DONE (1 << 28)
> #define DE_PLANEB_FLIP_DONE (1 << 27)
> #define DE_PLANEA_FLIP_DONE (1 << 26)
> +#define DE_PLANE_FLIP_DONE(plane) (1 << (26 + (plane)))
> #define DE_PCU_EVENT (1 << 25)
> #define DE_GTT_FAULT (1 << 24)
> #define DE_POISON (1 << 23)
> @@ -3944,12 +3945,15 @@
> #define DE_PIPEB_CRC_DONE (1 << 10)
> #define DE_PIPEB_FIFO_UNDERRUN (1 << 8)
> #define DE_PIPEA_VBLANK (1 << 7)
> +#define DE_PIPE_VBLANK(pipe) (1 << (7 + 8*(pipe)))
> #define DE_PIPEA_EVEN_FIELD (1 << 6)
> #define DE_PIPEA_ODD_FIELD (1 << 5)
> #define DE_PIPEA_LINE_COMPARE (1 << 4)
> #define DE_PIPEA_VSYNC (1 << 3)
> #define DE_PIPEA_CRC_DONE (1 << 2)
> +#define DE_PIPE_CRC_DONE(pipe) (1 << (2 + 8*(pipe)))
> #define DE_PIPEA_FIFO_UNDERRUN (1 << 0)
> +#define DE_PIPE_FIFO_UNDERRUN(pipe) (1 << (8*(pipe)))
>
> /* More Ivybridge lolz */
> #define DE_ERR_INT_IVB (1<<30)
> @@ -3965,9 +3969,8 @@
> #define DE_PIPEB_VBLANK_IVB (1<<5)
> #define DE_SPRITEA_FLIP_DONE_IVB (1<<4)
> #define DE_PLANEA_FLIP_DONE_IVB (1<<3)
> +#define DE_PLANE_FLIP_DONE_IVB(plane) (1<< (3 + 5*(plane)))
> #define DE_PIPEA_VBLANK_IVB (1<<0)
> -
> -#define DE_PIPE_VBLANK_ILK(pipe) (1 << ((pipe * 8) + 7))
> #define DE_PIPE_VBLANK_IVB(pipe) (1 << (pipe * 5))
>
> #define VLV_MASTER_IER 0x4400c /* Gunit master IER */
> --
> 1.8.4.rc3
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2013-10-21 16:50 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-21 16:04 [PATCH 1/2] drm/i915: use enum pipe consistently in i915_irq.c Daniel Vetter
2013-10-21 16:04 ` [PATCH 2/2] drm/i915: refactor ilk display interrupt handling Daniel Vetter
2013-10-21 16:48 ` Ville Syrjälä [this message]
2013-10-30 10: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=20131021164858.GC13047@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
/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.