ILK/IVB irq handler merge
Subsystems (2)
cover ILK/IVB irq handler merge
From: Paulo Zanoni <[email protected]>
This is something that have always bothered me: we have two functions that do almost the same thing, but their implementations are quite different. On this series I try to reorganize the two irq handlers so their implementations become very similar, and in the last patch I add ILK/SNB support to ivybridge_irq_handler and kill ironlake_irq_handler.
This is mostly a coding-style/OCD series, but considering how complex our irq handlers are becoming, I think it's an advantage to be able to kill one irq handler. ALso, this series removes more lines than it adds :)
Patches tested on SNB (which uses the ILK irq handler) and HSW (IVB irq handler).
Paulo Zanoni (6):
drm/i915: extract ilk_display_irq_handler drm/i915: extract ivb_display_irq_handler drm/i915: don't read or write GEN6_PMIIR on Gen 5 drm/i915: reorganize ironlake_irq_handler drm/i915: POSTING_READ(DEIER) on ivybridge_irq_handler drm/i915: add ILK/SNB support to ivybridge_irq_handler
drivers/gpu/drm/i915/i915_irq.c | 273 ++++++++++++++++++---------------------- 1 file changed, 120 insertions(+), 153 deletions(-)
Signature · 1 line
1.8.1.2
PATCH 1/6 drm/i915: extract ilk_display_irq_handler
From: Paulo Zanoni <[email protected]>
It's the code that deals with de_iir.
--- drivers/gpu/drm/i915/i915_irq.c | 104 +++++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 48 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +56 -48
@@ -1179,6 +1179,60 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)cpt_serr_int_handler (dev); } +static void ilk_display_irq_handler (struct drm_device *dev,u32 de_iir) +{ +struct drm_i915_private *dev_priv = dev->dev_private; + +if (de_iir &DE_AUX_CHANNEL_A ) +dp_aux_irq_handler (dev); + +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_PLANEA_FLIP_DONE ) { +intel_prepare_page_flip (dev,0 ); +intel_finish_page_flip_plane (dev,0 ); + } + +if (de_iir &DE_PLANEB_FLIP_DONE ) { +intel_prepare_page_flip (dev,1 ); +intel_finish_page_flip_plane (dev,1 ); + } + +/* check event from PCH */ +if (de_iir &DE_PCH_EVENT ) { +u32 pch_iir =I915_READ (SDEIIR ); + +if (HAS_PCH_CPT (dev)) +cpt_irq_handler (dev, pch_iir); +else +ibx_irq_handler (dev, pch_iir); + +/* should clear PCH hotplug event before clear CPU irq */ +I915_WRITE (SDEIIR , pch_iir); + } + +if (IS_GEN5 (dev) && de_iir &DE_PCU_EVENT ) +ironlake_handle_rps_change (dev); +} +static irqreturn_t ivybridge_irq_handler (int irq,void *arg) {struct drm_device *dev = (struct drm_device *) arg;
@@ -1337,54 +1391,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)else snb_gt_irq_handler (dev, dev_priv, gt_iir); -if (de_iir &DE_AUX_CHANNEL_A ) -dp_aux_irq_handler (dev); - -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_PLANEA_FLIP_DONE ) { -intel_prepare_page_flip (dev,0 ); -intel_finish_page_flip_plane (dev,0 ); - } - -if (de_iir &DE_PLANEB_FLIP_DONE ) { -intel_prepare_page_flip (dev,1 ); -intel_finish_page_flip_plane (dev,1 ); - } - -/* check event from PCH */ -if (de_iir &DE_PCH_EVENT ) { -u32 pch_iir =I915_READ (SDEIIR ); - -if (HAS_PCH_CPT (dev)) -cpt_irq_handler (dev, pch_iir); -else -ibx_irq_handler (dev, pch_iir); - -/* should clear PCH hotplug event before clear CPU irq */ -I915_WRITE (SDEIIR , pch_iir); - } - -if (IS_GEN5 (dev) && de_iir &DE_PCU_EVENT ) -ironlake_handle_rps_change (dev); +if (de_iir) +ilk_display_irq_handler (dev, de_iir);if (IS_GEN6 (dev) && pm_iir &GEN6_PM_RPS_EVENTS )gen6_queue_rps_work (dev_priv, pm_iir);
Signature · 1 line
1.8.1.2
PATCH 2/6 drm/i915: extract ivb_display_irq_handler
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
Just like we did with ilk_display_irq_handler.
--- drivers/gpu/drm/i915/i915_irq.c | 63 +++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 28 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +35 -28
@@ -1233,13 +1233,46 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)ironlake_handle_rps_change (dev); } +static void ivb_display_irq_handler (struct drm_device *dev,u32 de_iir) +{ +struct drm_i915_private *dev_priv = dev->dev_private; +int i; + +if (de_iir &DE_ERR_INT_IVB ) +ivb_err_int_handler (dev); + +if (de_iir &DE_AUX_CHANNEL_A_IVB ) +dp_aux_irq_handler (dev); + +if (de_iir &DE_GSE_IVB ) +intel_opregion_asle_intr (dev); + +for (i =0 ; i <3 ; i++) { +if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i))) +drm_handle_vblank (dev, i); +if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) { +intel_prepare_page_flip (dev, i); +intel_finish_page_flip_plane (dev, i); + } + } + +/* check event from PCH */ +if (!HAS_PCH_NOP (dev) && (de_iir &DE_PCH_EVENT_IVB )) { +u32 pch_iir =I915_READ (SDEIIR ); + +cpt_irq_handler (dev, pch_iir); + +/* clear PCH hotplug event before clear CPU irq */ +I915_WRITE (SDEIIR , pch_iir); + } +} +static irqreturn_t ivybridge_irq_handler (int irq,void *arg) {struct drm_device *dev = (struct drm_device *) arg;drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier =0 ;irqreturn_t ret =IRQ_NONE ; -int i;atomic_inc (&dev_priv->irq_received);
@@ -1284,33 +1317,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) de_iir =I915_READ (DEIIR );if (de_iir) { -if (de_iir &DE_ERR_INT_IVB ) -ivb_err_int_handler (dev); - -if (de_iir &DE_AUX_CHANNEL_A_IVB ) -dp_aux_irq_handler (dev); - -if (de_iir &DE_GSE_IVB ) -intel_opregion_asle_intr (dev); - -for (i =0 ; i <3 ; i++) { -if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i))) -drm_handle_vblank (dev, i); -if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) { -intel_prepare_page_flip (dev, i); -intel_finish_page_flip_plane (dev, i); - } - } - -/* check event from PCH */ -if (!HAS_PCH_NOP (dev) && (de_iir &DE_PCH_EVENT_IVB )) { -u32 pch_iir =I915_READ (SDEIIR ); - -cpt_irq_handler (dev, pch_iir); - -/* clear PCH hotplug event before clear CPU irq */ -I915_WRITE (SDEIIR , pch_iir); - } +ivb_display_irq_handler (dev, de_iir);I915_WRITE (DEIIR , de_iir); ret =IRQ_HANDLED ;
Signature · 1 line
1.8.1.2
PATCH 3/6 drm/i915: don't read or write GEN6_PMIIR on Gen 5
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
The register doesn't exist on Gen 5.
--- drivers/gpu/drm/i915/i915_irq.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +5 -3
@@ -1366,7 +1366,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)struct drm_device *dev = (struct drm_device *) arg;drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;int ret =IRQ_NONE ; -u32 de_iir, gt_iir, de_ier, pm_iir, sde_ier; +u32 de_iir, gt_iir, de_ier, pm_iir =0 , sde_ier;atomic_inc (&dev_priv->irq_received);
@@ -1386,7 +1386,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg) de_iir =I915_READ (DEIIR ); gt_iir =I915_READ (GTIIR ); - pm_iir =I915_READ (GEN6_PMIIR ); +if (IS_GEN6 (dev)) + pm_iir =I915_READ (GEN6_PMIIR );if (de_iir ==0 && gt_iir ==0 && (!IS_GEN6 (dev) || pm_iir ==0 )) goto done;
@@ -1406,7 +1407,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)I915_WRITE (GTIIR , gt_iir);I915_WRITE (DEIIR , de_iir); -I915_WRITE (GEN6_PMIIR , pm_iir); +if (IS_GEN6 (dev)) +I915_WRITE (GEN6_PMIIR , pm_iir); done:I915_WRITE (DEIER , de_ier);
Signature · 1 line
1.8.1.2
PATCH 4/6 drm/i915: reorganize ironlake_irq_handler
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
The ironlake_irq_handler and ivybridge_irq_handler functions do basically the same thing, but they have different implementation styles. With this patch we reorganize ironlake_irq_handler in a way that makes it look very similar to ivybridge_irq_handler.
One of the advantages of this new function style is that we don't write 0 to the IIR registers anymore.
--- drivers/gpu/drm/i915/i915_irq.c | 44 ++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 22 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +22 -22
@@ -1384,33 +1384,33 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)I915_WRITE (SDEIER ,0 );POSTING_READ (SDEIER ); - de_iir =I915_READ (DEIIR ); gt_iir =I915_READ (GTIIR ); -if (IS_GEN6 (dev)) - pm_iir =I915_READ (GEN6_PMIIR ); - -if (de_iir ==0 && gt_iir ==0 && (!IS_GEN6 (dev) || pm_iir ==0 )) - goto done; - - ret =IRQ_HANDLED ; - -if (IS_GEN5 (dev)) -ilk_gt_irq_handler (dev, dev_priv, gt_iir); -else -snb_gt_irq_handler (dev, dev_priv, gt_iir); +if (gt_iir) { +if (IS_GEN5 (dev)) +ilk_gt_irq_handler (dev, dev_priv, gt_iir); +else +snb_gt_irq_handler (dev, dev_priv, gt_iir); +I915_WRITE (GTIIR , gt_iir); + ret =IRQ_HANDLED ; + } -if (de_iir) + de_iir =I915_READ (DEIIR ); +if (de_iir) {ilk_display_irq_handler (dev, de_iir); +I915_WRITE (DEIIR , de_iir); + ret =IRQ_HANDLED ; + } -if (IS_GEN6 (dev) && pm_iir &GEN6_PM_RPS_EVENTS ) -gen6_queue_rps_work (dev_priv, pm_iir); - -I915_WRITE (GTIIR , gt_iir); -I915_WRITE (DEIIR , de_iir); -if (IS_GEN6 (dev)) -I915_WRITE (GEN6_PMIIR , pm_iir); +if (IS_GEN6 (dev)) { + pm_iir =I915_READ (GEN6_PMIIR ); +if (pm_iir) { +if (pm_iir &GEN6_PM_RPS_EVENTS ) +gen6_queue_rps_work (dev_priv, pm_iir); +I915_WRITE (GEN6_PMIIR , pm_iir); + ret =IRQ_HANDLED ; + } + } -done:I915_WRITE (DEIER , de_ier);POSTING_READ (DEIER );I915_WRITE (SDEIER , sde_ier);
Signature · 1 line
1.8.1.2
PATCH 5/6 drm/i915: POSTING_READ(DEIER) on ivybridge_irq_handler
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
We have this POSTING_READ inside ironlake_irq_handler. I suppose we also want it on IVB because we want to stop the IRQ handler as soon as possible at this point. Also, I'll add ILK/SNB support to ivybridge_irq_handler on the next patch.
--- drivers/gpu/drm/i915/i915_irq.c | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/i915/i915_irq.c +1 -0
@@ -1287,6 +1287,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)/* disable master interrupt before clearing iir */ de_ier =I915_READ (DEIER );I915_WRITE (DEIER , de_ier & ~DE_MASTER_IRQ_CONTROL ); +POSTING_READ (DEIER );/* Disable south interrupts. We'll only write to SDEIIR once, so further * interrupts will will be stored on its back queue, and then we'll be
Signature · 1 line
1.8.1.2
PATCH 6/6 drm/i915: add ILK/SNB support to ivybridge_irq_handler
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
And then rename it to ironlake_irq_handler. Also move ilk_gt_irq_handler up to avoid forward declarations.
In the previous patches I did small modifications to both ironlake_irq_handler an ivybridge_irq_handler so they became very similar functions. Now it should be very easy to verify that all we need to add ILK/SNB support is to call ilk_gt_irq_handler, call ilk_display_irq_handler and avoid reading pm_iir on gen 5.
--- drivers/gpu/drm/i915/i915_irq.c | 113 +++++++++++----------------------------- 1 file changed, 31 insertions(+), 82 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +31 -82
@@ -820,6 +820,17 @@ static void ivybridge_handle_parity_error(struct drm_device *dev)queue_work (dev_priv->wq, &dev_priv->l3_parity.error_work); } +static void ilk_gt_irq_handler (struct drm_device *dev, +struct drm_i915_private *dev_priv, +u32 gt_iir) +{ +if (gt_iir & + (GT_RENDER_USER_INTERRUPT |GT_RENDER_PIPECTL_NOTIFY_INTERRUPT )) +notify_ring (dev, &dev_priv->ring[RCS ]); +if (gt_iir &ILK_BSD_USER_INTERRUPT ) +notify_ring (dev, &dev_priv->ring[VCS ]); +} +static void snb_gt_irq_handler (struct drm_device *dev,struct drm_i915_private *dev_priv,u32 gt_iir)
@@ -1267,7 +1278,7 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir) } } -static irqreturn_t ivybridge_irq_handler (int irq,void *arg) +static irqreturn_t ironlake_irq_handler (int irq,void *arg) {struct drm_device *dev = (struct drm_device *) arg;drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
@@ -1311,27 +1322,34 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) gt_iir =I915_READ (GTIIR );if (gt_iir) { -snb_gt_irq_handler (dev, dev_priv, gt_iir); +if (IS_GEN5 (dev)) +ilk_gt_irq_handler (dev, dev_priv, gt_iir); +else +snb_gt_irq_handler (dev, dev_priv, gt_iir);I915_WRITE (GTIIR , gt_iir); ret =IRQ_HANDLED ; } de_iir =I915_READ (DEIIR );if (de_iir) { -ivb_display_irq_handler (dev, de_iir); - +if (INTEL_INFO (dev)->gen <=6 ) +ilk_display_irq_handler (dev, de_iir); +else +ivb_display_irq_handler (dev, de_iir);I915_WRITE (DEIIR , de_iir); ret =IRQ_HANDLED ; } - pm_iir =I915_READ (GEN6_PMIIR ); -if (pm_iir) { -if (IS_HASWELL (dev)) -hsw_pm_irq_handler (dev_priv, pm_iir); -else if (pm_iir &GEN6_PM_RPS_EVENTS ) -gen6_queue_rps_work (dev_priv, pm_iir); -I915_WRITE (GEN6_PMIIR , pm_iir); - ret =IRQ_HANDLED ; +if (!IS_GEN5 (dev)) { + pm_iir =I915_READ (GEN6_PMIIR ); +if (pm_iir) { +if (IS_HASWELL (dev)) +hsw_pm_irq_handler (dev_priv, pm_iir); +else if (pm_iir &GEN6_PM_RPS_EVENTS ) +gen6_queue_rps_work (dev_priv, pm_iir); +I915_WRITE (GEN6_PMIIR , pm_iir); + ret =IRQ_HANDLED ; + } }if (IS_HASWELL (dev)) {
@@ -1351,75 +1369,6 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)return ret; } -static void ilk_gt_irq_handler (struct drm_device *dev, -struct drm_i915_private *dev_priv, -u32 gt_iir) -{ -if (gt_iir & - (GT_RENDER_USER_INTERRUPT |GT_RENDER_PIPECTL_NOTIFY_INTERRUPT )) -notify_ring (dev, &dev_priv->ring[RCS ]); -if (gt_iir &ILK_BSD_USER_INTERRUPT ) -notify_ring (dev, &dev_priv->ring[VCS ]); -} - -static irqreturn_t ironlake_irq_handler (int irq,void *arg) -{ -struct drm_device *dev = (struct drm_device *) arg; -drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; -int ret =IRQ_NONE ; -u32 de_iir, gt_iir, de_ier, pm_iir =0 , sde_ier; - -atomic_inc (&dev_priv->irq_received); - -/* disable master interrupt before clearing iir */ - de_ier =I915_READ (DEIER ); -I915_WRITE (DEIER , de_ier & ~DE_MASTER_IRQ_CONTROL ); -POSTING_READ (DEIER ); - -/* Disable south interrupts. We'll only write to SDEIIR once, so further -* interrupts will will be stored on its back queue, and then we'll be -* able to process them after we restore SDEIER (as soon as we restore -* it, we'll get an interrupt if SDEIIR still has something to process -* due to its back queue). */ - sde_ier =I915_READ (SDEIER ); -I915_WRITE (SDEIER ,0 ); -POSTING_READ (SDEIER ); - - gt_iir =I915_READ (GTIIR ); -if (gt_iir) { -if (IS_GEN5 (dev)) -ilk_gt_irq_handler (dev, dev_priv, gt_iir); -else -snb_gt_irq_handler (dev, dev_priv, gt_iir); -I915_WRITE (GTIIR , gt_iir); - ret =IRQ_HANDLED ; - } - - de_iir =I915_READ (DEIIR ); -if (de_iir) { -ilk_display_irq_handler (dev, de_iir); -I915_WRITE (DEIIR , de_iir); - ret =IRQ_HANDLED ; - } - -if (IS_GEN6 (dev)) { - pm_iir =I915_READ (GEN6_PMIIR ); -if (pm_iir) { -if (pm_iir &GEN6_PM_RPS_EVENTS ) -gen6_queue_rps_work (dev_priv, pm_iir); -I915_WRITE (GEN6_PMIIR , pm_iir); - ret =IRQ_HANDLED ; - } - } - -I915_WRITE (DEIER , de_ier); -POSTING_READ (DEIER ); -I915_WRITE (SDEIER , sde_ier); -POSTING_READ (SDEIER ); - -return ret; -} -/** * i915_error_work_func - do process context error handling work * @work: work struct
@@ -3706,7 +3655,7 @@ void intel_irq_init(struct drm_device *dev) dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; }else if (IS_IVYBRIDGE (dev) ||IS_HASWELL (dev)) {/* Share uninstall handlers with ILK/SNB */ - dev->driver->irq_handler = ivybridge_irq_handler; + dev->driver->irq_handler = ironlake_irq_handler; dev->driver->irq_preinstall = ivybridge_irq_preinstall; dev->driver->irq_postinstall = ivybridge_irq_postinstall; dev->driver->irq_uninstall = ironlake_irq_uninstall;
Signature · 1 line
1.8.1.2