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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox