All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/i915: Finish drm_driver vfunc cleanup
@ 2019-06-18 14:21 Ville Syrjala
  2019-06-18 14:21 ` [PATCH 1/3] drm/i915: Fix various tracepoints for gen2 Ville Syrjala
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Ville Syrjala @ 2019-06-18 14:21 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I forgot to convert the tracepoints to the per-crtc
vfuncs in https://patchwork.freedesktop.org/series/62287/
Fortunately I had a patch ready for that in a branch (for gen2
originally) so I just refreshed that a bit. And since I had
to take another look at this stuff anyway I went ahead and
nuked the irq vfuncs too. After this I believe our drm_driver
struct should be happy in a mixed gen environment.

Ville Syrjälä (3):
  drm/i915: Fix various tracepoints for gen2
  drm/i915: Nuke drm_driver irq vfuncs
  drm/i915: Initialize drm_driver vblank funcs at compile time

 drivers/gpu/drm/i915/display/intel_display.c |   4 +-
 drivers/gpu/drm/i915/i915_drv.c              |   5 +-
 drivers/gpu/drm/i915/i915_irq.c              | 291 +++++++++----------
 drivers/gpu/drm/i915/i915_irq.h              |   5 +
 drivers/gpu/drm/i915/i915_trace.h            |  76 +++--
 5 files changed, 187 insertions(+), 194 deletions(-)

-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] drm/i915: Fix various tracepoints for gen2
  2019-06-18 14:21 [PATCH 0/3] drm/i915: Finish drm_driver vfunc cleanup Ville Syrjala
@ 2019-06-18 14:21 ` Ville Syrjala
  2019-06-18 14:46   ` Chris Wilson
  2019-06-18 14:21 ` [PATCH 2/3] drm/i915: Nuke drm_driver irq vfuncs Ville Syrjala
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjala @ 2019-06-18 14:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Shawn Guo

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Gen2 doesn't have a frame counter and apparently we no longer provide
a fake .get_vblank_counter() hook for it. That means all tracepoints
calling that hook will oops. Update the tracepoints to use
intel_crtc_get_vblank_counter() which will gracefully fall back to
using the software counter. This is actually a better approach since
we now get (hopefully accurate) frame numbers in the traces.

This also gets rid of the raw driver->get_vblank_counter() calls, which
we need to do in order to switch to the per-crtc vblank vfuncs.

v2: Deal with new tracepoints

Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Fixes: 967dd4841787 ("drm: remove drm_vblank_no_hw_counter assignment from driver code")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  4 +-
 drivers/gpu/drm/i915/i915_trace.h            | 76 +++++++++-----------
 2 files changed, 35 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index dc602a8a52fe..fcf99dc17127 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1839,7 +1839,7 @@ static void intel_enable_pipe(const struct intel_crtc_state *new_crtc_state)
 		/* FIXME: assert CPU port conditions for SNB+ */
 	}
 
-	trace_intel_pipe_enable(dev_priv, pipe);
+	trace_intel_pipe_enable(crtc);
 
 	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
@@ -1880,7 +1880,7 @@ static void intel_disable_pipe(const struct intel_crtc_state *old_crtc_state)
 	 */
 	assert_planes_disabled(crtc);
 
-	trace_intel_pipe_disable(dev_priv, pipe);
+	trace_intel_pipe_disable(crtc);
 
 	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 5c8cfaa70d72..b5aa25b89de0 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -21,24 +21,22 @@
 /* watermark/fifo updates */
 
 TRACE_EVENT(intel_pipe_enable,
-	    TP_PROTO(struct drm_i915_private *dev_priv, enum pipe pipe),
-	    TP_ARGS(dev_priv, pipe),
+	    TP_PROTO(struct intel_crtc *crtc),
+	    TP_ARGS(crtc),
 
 	    TP_STRUCT__entry(
 			     __array(u32, frame, 3)
 			     __array(u32, scanline, 3)
 			     __field(enum pipe, pipe)
 			     ),
-
 	    TP_fast_assign(
-			   enum pipe _pipe;
-			   for_each_pipe(dev_priv, _pipe) {
-				   __entry->frame[_pipe] =
-					   dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, _pipe);
-				   __entry->scanline[_pipe] =
-					   intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, _pipe));
+			   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+			   struct intel_crtc *_crtc;
+			   for_each_intel_crtc(&dev_priv->drm, _crtc) {
+				   __entry->frame[_crtc->pipe] = intel_crtc_get_vblank_counter(_crtc);
+				   __entry->scanline[_crtc->pipe] = intel_get_crtc_scanline(_crtc);
 			   }
-			   __entry->pipe = pipe;
+			   __entry->pipe = crtc->pipe;
 			   ),
 
 	    TP_printk("pipe %c enable, pipe A: frame=%u, scanline=%u, pipe B: frame=%u, scanline=%u, pipe C: frame=%u, scanline=%u",
@@ -49,8 +47,8 @@ TRACE_EVENT(intel_pipe_enable,
 );
 
 TRACE_EVENT(intel_pipe_disable,
-	    TP_PROTO(struct drm_i915_private *dev_priv, enum pipe pipe),
-	    TP_ARGS(dev_priv, pipe),
+	    TP_PROTO(struct intel_crtc *crtc),
+	    TP_ARGS(crtc),
 
 	    TP_STRUCT__entry(
 			     __array(u32, frame, 3)
@@ -59,14 +57,13 @@ TRACE_EVENT(intel_pipe_disable,
 			     ),
 
 	    TP_fast_assign(
-			   enum pipe _pipe;
-			   for_each_pipe(dev_priv, _pipe) {
-				   __entry->frame[_pipe] =
-					   dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, _pipe);
-				   __entry->scanline[_pipe] =
-					   intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, _pipe));
+			   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+			   struct intel_crtc *_crtc;
+			   for_each_intel_crtc(&dev_priv->drm, _crtc) {
+				   __entry->frame[_crtc->pipe] = intel_crtc_get_vblank_counter(_crtc);
+				   __entry->scanline[_crtc->pipe] = intel_get_crtc_scanline(_crtc);
 			   }
-			   __entry->pipe = pipe;
+			   __entry->pipe = crtc->pipe;
 			   ),
 
 	    TP_printk("pipe %c disable, pipe A: frame=%u, scanline=%u, pipe B: frame=%u, scanline=%u, pipe C: frame=%u, scanline=%u",
@@ -89,8 +86,7 @@ TRACE_EVENT(intel_pipe_crc,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
 			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   memcpy(__entry->crcs, crcs, sizeof(__entry->crcs));
 			   ),
@@ -112,9 +108,10 @@ TRACE_EVENT(intel_cpu_fifo_underrun,
 			     ),
 
 	    TP_fast_assign(
+			    struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
 			   __entry->pipe = pipe;
-			   __entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
-			   __entry->scanline = intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   ),
 
 	    TP_printk("pipe %c, frame=%u, scanline=%u",
@@ -134,9 +131,10 @@ TRACE_EVENT(intel_pch_fifo_underrun,
 
 	    TP_fast_assign(
 			   enum pipe pipe = pch_transcoder;
+			   struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
 			   __entry->pipe = pipe;
-			   __entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
-			   __entry->scanline = intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   ),
 
 	    TP_printk("pch transcoder %c, frame=%u, scanline=%u",
@@ -156,12 +154,10 @@ TRACE_EVENT(intel_memory_cxsr,
 			     ),
 
 	    TP_fast_assign(
-			   enum pipe pipe;
-			   for_each_pipe(dev_priv, pipe) {
-				   __entry->frame[pipe] =
-					   dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
-				   __entry->scanline[pipe] =
-					   intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   struct intel_crtc *crtc;
+			   for_each_intel_crtc(&dev_priv->drm, crtc) {
+				   __entry->frame[crtc->pipe] = intel_crtc_get_vblank_counter(crtc);
+				   __entry->scanline[crtc->pipe] = intel_get_crtc_scanline(crtc);
 			   }
 			   __entry->old = old;
 			   __entry->new = new;
@@ -198,8 +194,7 @@ TRACE_EVENT(g4x_wm,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
 			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   __entry->primary = wm->pipe[crtc->pipe].plane[PLANE_PRIMARY];
 			   __entry->sprite = wm->pipe[crtc->pipe].plane[PLANE_SPRITE0];
@@ -243,8 +238,7 @@ TRACE_EVENT(vlv_wm,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
 			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   __entry->level = wm->level;
 			   __entry->cxsr = wm->cxsr;
@@ -278,8 +272,7 @@ TRACE_EVENT(vlv_fifo_size,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
 			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   __entry->sprite0_start = sprite0_start;
 			   __entry->sprite1_start = sprite1_start;
@@ -310,8 +303,7 @@ TRACE_EVENT(intel_update_plane,
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
 			   __entry->name = plane->name;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
 			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   memcpy(__entry->src, &plane->state->src, sizeof(__entry->src));
 			   memcpy(__entry->dst, &plane->state->dst, sizeof(__entry->dst));
@@ -338,8 +330,7 @@ TRACE_EVENT(intel_disable_plane,
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
 			   __entry->name = plane->name;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
 			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   ),
 
@@ -364,8 +355,7 @@ TRACE_EVENT(i915_pipe_update_start,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
 			   __entry->scanline = intel_get_crtc_scanline(crtc);
 			   __entry->min = crtc->debug.min_vbl;
 			   __entry->max = crtc->debug.max_vbl;
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] drm/i915: Nuke drm_driver irq vfuncs
  2019-06-18 14:21 [PATCH 0/3] drm/i915: Finish drm_driver vfunc cleanup Ville Syrjala
  2019-06-18 14:21 ` [PATCH 1/3] drm/i915: Fix various tracepoints for gen2 Ville Syrjala
@ 2019-06-18 14:21 ` Ville Syrjala
  2019-06-18 14:54   ` Chris Wilson
  2019-06-18 14:21 ` [PATCH 3/3] drm/i915: Initialize drm_driver vblank funcs at compile time Ville Syrjala
  2019-06-18 15:12 ` ✗ Fi.CI.BAT: failure for drm/i915: Finish drm_driver vfunc cleanup Patchwork
  3 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjala @ 2019-06-18 14:21 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Stop using the irq vfuncs under drm_driver. That's not going to fly
in a mixed gen environment since the structure is shared between all
the devices.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |   2 +-
 drivers/gpu/drm/i915/i915_irq.c | 280 ++++++++++++++++----------------
 2 files changed, 140 insertions(+), 142 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f62e3397d936..ea6b06109d5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -754,7 +754,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 cleanup_modeset:
 	intel_modeset_cleanup(dev);
 cleanup_irq:
-	drm_irq_uninstall(dev);
+	intel_irq_uninstall(dev_priv);
 	intel_gmbus_teardown(dev_priv);
 cleanup_csr:
 	intel_csr_ucode_fini(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4fbe8d90950a..e9e29eed8005 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2187,8 +2187,7 @@ static void i9xx_hpd_irq_handler(struct drm_i915_private *dev_priv,
 
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -2273,8 +2272,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -2693,8 +2691,7 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
  */
 static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	u32 de_iir, gt_iir, de_ier, sde_ier = 0;
 	irqreturn_t ret = IRQ_NONE;
 
@@ -3004,7 +3001,7 @@ static inline void gen8_master_intr_enable(void __iomem * const regs)
 
 static irqreturn_t gen8_irq_handler(int irq, void *arg)
 {
-	struct drm_i915_private *dev_priv = to_i915(arg);
+	struct drm_i915_private *dev_priv = arg;
 	void __iomem * const regs = dev_priv->uncore.regs;
 	u32 master_ctl;
 	u32 gt_iir[4];
@@ -3203,7 +3200,7 @@ static inline void gen11_master_intr_enable(void __iomem * const regs)
 
 static irqreturn_t gen11_irq_handler(int irq, void *arg)
 {
-	struct drm_i915_private * const i915 = to_i915(arg);
+	struct drm_i915_private * const i915 = arg;
 	void __iomem * const regs = i915->uncore.regs;
 	u32 master_ctl;
 	u32 gu_misc_iir;
@@ -3457,10 +3454,8 @@ static void ibx_irq_reset(struct drm_i915_private *dev_priv)
  *
  * This function needs to be called before interrupts are enabled.
  */
-static void ibx_irq_pre_postinstall(struct drm_device *dev)
+static void ibx_irq_pre_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
 	if (HAS_PCH_NOP(dev_priv))
 		return;
 
@@ -3529,9 +3524,8 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
 
 /* drm_dma.h hooks
 */
-static void ironlake_irq_reset(struct drm_device *dev)
+static void ironlake_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	GEN3_IRQ_RESET(uncore, DE);
@@ -3548,10 +3542,8 @@ static void ironlake_irq_reset(struct drm_device *dev)
 	ibx_irq_reset(dev_priv);
 }
 
-static void valleyview_irq_reset(struct drm_device *dev)
+static void valleyview_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
 	I915_WRITE(VLV_MASTER_IER, 0);
 	POSTING_READ(VLV_MASTER_IER);
 
@@ -3573,9 +3565,8 @@ static void gen8_gt_irq_reset(struct drm_i915_private *dev_priv)
 	GEN8_IRQ_RESET_NDX(uncore, GT, 3);
 }
 
-static void gen8_irq_reset(struct drm_device *dev)
+static void gen8_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	int pipe;
 
@@ -3618,9 +3609,8 @@ static void gen11_gt_irq_reset(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN11_GUC_SG_INTR_MASK,  ~0);
 }
 
-static void gen11_irq_reset(struct drm_device *dev)
+static void gen11_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	int pipe;
 
@@ -3693,9 +3683,8 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
 	synchronize_irq(dev_priv->drm.irq);
 }
 
-static void cherryview_irq_reset(struct drm_device *dev)
+static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
@@ -3960,9 +3949,8 @@ static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv)
 	__bxt_hpd_detection_setup(dev_priv, enabled_irqs);
 }
 
-static void ibx_irq_postinstall(struct drm_device *dev)
+static void ibx_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 mask;
 
 	if (HAS_PCH_NOP(dev_priv))
@@ -3985,9 +3973,8 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 		spt_hpd_detection_setup(dev_priv);
 }
 
-static void gen5_gt_irq_postinstall(struct drm_device *dev)
+static void gen5_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 pm_irqs, gt_irqs;
 
@@ -4024,9 +4011,8 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 	}
 }
 
-static int ironlake_irq_postinstall(struct drm_device *dev)
+static void ironlake_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 display_mask, extra_mask;
 
@@ -4053,16 +4039,16 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~display_mask;
 
-	ibx_irq_pre_postinstall(dev);
+	ibx_irq_pre_postinstall(dev_priv);
 
 	GEN3_IRQ_INIT(uncore, DE, dev_priv->irq_mask,
 		      display_mask | extra_mask);
 
-	gen5_gt_irq_postinstall(dev);
+	gen5_gt_irq_postinstall(dev_priv);
 
 	ilk_hpd_detection_setup(dev_priv);
 
-	ibx_irq_postinstall(dev);
+	ibx_irq_postinstall(dev_priv);
 
 	if (IS_IRONLAKE_M(dev_priv)) {
 		/* Enable PCU event interrupts
@@ -4074,8 +4060,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 		ilk_enable_display_irq(dev_priv, DE_PCU_EVENT);
 		spin_unlock_irq(&dev_priv->irq_lock);
 	}
-
-	return 0;
 }
 
 void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
@@ -4107,11 +4091,9 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
 }
 
 
-static int valleyview_irq_postinstall(struct drm_device *dev)
+static void valleyview_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	gen5_gt_irq_postinstall(dev);
+	gen5_gt_irq_postinstall(dev_priv);
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	if (dev_priv->display_irqs_enabled)
@@ -4120,8 +4102,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 
 	I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
 	POSTING_READ(VLV_MASTER_IER);
-
-	return 0;
 }
 
 static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
@@ -4228,22 +4208,18 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	}
 }
 
-static int gen8_irq_postinstall(struct drm_device *dev)
+static void gen8_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
 	if (HAS_PCH_SPLIT(dev_priv))
-		ibx_irq_pre_postinstall(dev);
+		ibx_irq_pre_postinstall(dev_priv);
 
 	gen8_gt_irq_postinstall(dev_priv);
 	gen8_de_irq_postinstall(dev_priv);
 
 	if (HAS_PCH_SPLIT(dev_priv))
-		ibx_irq_postinstall(dev);
+		ibx_irq_postinstall(dev_priv);
 
 	gen8_master_intr_enable(dev_priv->uncore.regs);
-
-	return 0;
 }
 
 static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
@@ -4277,9 +4253,8 @@ static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN11_GUC_SG_INTR_MASK,  ~0);
 }
 
-static void icp_irq_postinstall(struct drm_device *dev)
+static void icp_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 mask = SDE_GMBUS_ICP;
 
 	WARN_ON(I915_READ(SDEIER) != 0);
@@ -4292,14 +4267,13 @@ static void icp_irq_postinstall(struct drm_device *dev)
 	icp_hpd_detection_setup(dev_priv);
 }
 
-static int gen11_irq_postinstall(struct drm_device *dev)
+static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 gu_misc_masked = GEN11_GU_MISC_GSE;
 
 	if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
-		icp_irq_postinstall(dev);
+		icp_irq_postinstall(dev_priv);
 
 	gen11_gt_irq_postinstall(dev_priv);
 	gen8_de_irq_postinstall(dev_priv);
@@ -4310,14 +4284,10 @@ static int gen11_irq_postinstall(struct drm_device *dev)
 
 	gen11_master_intr_enable(dev_priv->uncore.regs);
 	POSTING_READ(GEN11_GFX_MSTR_IRQ);
-
-	return 0;
 }
 
-static int cherryview_irq_postinstall(struct drm_device *dev)
+static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
 	gen8_gt_irq_postinstall(dev_priv);
 
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -4327,13 +4297,10 @@ static int cherryview_irq_postinstall(struct drm_device *dev)
 
 	I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 	POSTING_READ(GEN8_MASTER_IRQ);
-
-	return 0;
 }
 
-static void i8xx_irq_reset(struct drm_device *dev)
+static void i8xx_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	i9xx_pipestat_irq_reset(dev_priv);
@@ -4341,9 +4308,8 @@ static void i8xx_irq_reset(struct drm_device *dev)
 	GEN2_IRQ_RESET(uncore);
 }
 
-static int i8xx_irq_postinstall(struct drm_device *dev)
+static void i8xx_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u16 enable_mask;
 
@@ -4372,8 +4338,6 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_CRC_DONE_INTERRUPT_STATUS);
 	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_CRC_DONE_INTERRUPT_STATUS);
 	spin_unlock_irq(&dev_priv->irq_lock);
-
-	return 0;
 }
 
 static void i8xx_error_irq_ack(struct drm_i915_private *i915,
@@ -4454,8 +4418,7 @@ static void i9xx_error_irq_handler(struct drm_i915_private *dev_priv,
 
 static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -4498,9 +4461,8 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void i915_irq_reset(struct drm_device *dev)
+static void i915_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	if (I915_HAS_HOTPLUG(dev_priv)) {
@@ -4513,9 +4475,8 @@ static void i915_irq_reset(struct drm_device *dev)
 	GEN3_IRQ_RESET(uncore, GEN2_);
 }
 
-static int i915_irq_postinstall(struct drm_device *dev)
+static void i915_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 enable_mask;
 
@@ -4553,14 +4514,11 @@ static int i915_irq_postinstall(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	i915_enable_asle_pipestat(dev_priv);
-
-	return 0;
 }
 
 static irqreturn_t i915_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -4611,9 +4569,8 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void i965_irq_reset(struct drm_device *dev)
+static void i965_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
@@ -4624,9 +4581,8 @@ static void i965_irq_reset(struct drm_device *dev)
 	GEN3_IRQ_RESET(uncore, GEN2_);
 }
 
-static int i965_irq_postinstall(struct drm_device *dev)
+static void i965_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 enable_mask;
 	u32 error_mask;
@@ -4676,8 +4632,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	i915_enable_asle_pipestat(dev_priv);
-
-	return 0;
 }
 
 static void i915_hpd_irq_setup(struct drm_i915_private *dev_priv)
@@ -4707,8 +4661,7 @@ static void i915_hpd_irq_setup(struct drm_i915_private *dev_priv)
 
 static irqreturn_t i965_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -4839,65 +4792,18 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 	dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
 	dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
 
-	if (IS_CHERRYVIEW(dev_priv)) {
-		dev->driver->irq_handler = cherryview_irq_handler;
-		dev->driver->irq_preinstall = cherryview_irq_reset;
-		dev->driver->irq_postinstall = cherryview_irq_postinstall;
-		dev->driver->irq_uninstall = cherryview_irq_reset;
-		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
-	} else if (IS_VALLEYVIEW(dev_priv)) {
-		dev->driver->irq_handler = valleyview_irq_handler;
-		dev->driver->irq_preinstall = valleyview_irq_reset;
-		dev->driver->irq_postinstall = valleyview_irq_postinstall;
-		dev->driver->irq_uninstall = valleyview_irq_reset;
-		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
-	} else if (INTEL_GEN(dev_priv) >= 11) {
-		dev->driver->irq_handler = gen11_irq_handler;
-		dev->driver->irq_preinstall = gen11_irq_reset;
-		dev->driver->irq_postinstall = gen11_irq_postinstall;
-		dev->driver->irq_uninstall = gen11_irq_reset;
-		dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup;
-	} else if (INTEL_GEN(dev_priv) >= 8) {
-		dev->driver->irq_handler = gen8_irq_handler;
-		dev->driver->irq_preinstall = gen8_irq_reset;
-		dev->driver->irq_postinstall = gen8_irq_postinstall;
-		dev->driver->irq_uninstall = gen8_irq_reset;
-		if (IS_GEN9_LP(dev_priv))
+	if (HAS_GMCH(dev_priv)) {
+		if (I915_HAS_HOTPLUG(dev_priv))
+			dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup;
+		else if (IS_GEN9_LP(dev_priv))
 			dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
 		else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
 			dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
 		else
 			dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
-	} else if (HAS_PCH_SPLIT(dev_priv)) {
-		dev->driver->irq_handler = ironlake_irq_handler;
-		dev->driver->irq_preinstall = ironlake_irq_reset;
-		dev->driver->irq_postinstall = ironlake_irq_postinstall;
-		dev->driver->irq_uninstall = ironlake_irq_reset;
-		dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
-	} else {
-		if (IS_GEN(dev_priv, 2)) {
-			dev->driver->irq_preinstall = i8xx_irq_reset;
-			dev->driver->irq_postinstall = i8xx_irq_postinstall;
-			dev->driver->irq_handler = i8xx_irq_handler;
-			dev->driver->irq_uninstall = i8xx_irq_reset;
-		} else if (IS_I945GM(dev_priv)) {
-			dev->driver->irq_preinstall = i915_irq_reset;
-			dev->driver->irq_postinstall = i915_irq_postinstall;
-			dev->driver->irq_uninstall = i915_irq_reset;
-			dev->driver->irq_handler = i915_irq_handler;
-		} else if (IS_GEN(dev_priv, 3)) {
-			dev->driver->irq_preinstall = i915_irq_reset;
-			dev->driver->irq_postinstall = i915_irq_postinstall;
-			dev->driver->irq_uninstall = i915_irq_reset;
-			dev->driver->irq_handler = i915_irq_handler;
-		} else {
-			dev->driver->irq_preinstall = i965_irq_reset;
-			dev->driver->irq_postinstall = i965_irq_postinstall;
-			dev->driver->irq_uninstall = i965_irq_reset;
-			dev->driver->irq_handler = i965_irq_handler;
-		}
-		if (I915_HAS_HOTPLUG(dev_priv))
-			dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
 	}
 }
 
@@ -4918,6 +4824,75 @@ void intel_irq_fini(struct drm_i915_private *i915)
 		kfree(i915->l3_parity.remap_info[i]);
 }
 
+static irq_handler_t intel_irq_handler(struct drm_i915_private *dev_priv)
+{
+	if (HAS_GMCH(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			return cherryview_irq_handler;
+		else if (IS_VALLEYVIEW(dev_priv))
+			return valleyview_irq_handler;
+		else if (IS_GEN(dev_priv, 4))
+			return i965_irq_handler;
+		else if (IS_GEN(dev_priv, 3))
+			return i915_irq_handler;
+		else
+			return i8xx_irq_handler;
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			return gen11_irq_handler;
+		else if (INTEL_GEN(dev_priv) >= 8)
+			return gen8_irq_handler;
+		else
+			return ironlake_irq_handler;
+	}
+}
+
+static void intel_irq_reset(struct drm_i915_private *dev_priv)
+{
+	if (HAS_GMCH(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			cherryview_irq_reset(dev_priv);
+		else if (IS_VALLEYVIEW(dev_priv))
+			valleyview_irq_reset(dev_priv);
+		else if (IS_GEN(dev_priv, 4))
+			i965_irq_reset(dev_priv);
+		else if (IS_GEN(dev_priv, 3))
+			i915_irq_reset(dev_priv);
+		else
+			i8xx_irq_reset(dev_priv);
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			gen11_irq_reset(dev_priv);
+		else if (INTEL_GEN(dev_priv) >= 8)
+			gen8_irq_reset(dev_priv);
+		else
+			ironlake_irq_reset(dev_priv);
+	}
+}
+
+static void intel_irq_postinstall(struct drm_i915_private *dev_priv)
+{
+	if (HAS_GMCH(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			cherryview_irq_postinstall(dev_priv);
+		else if (IS_VALLEYVIEW(dev_priv))
+			valleyview_irq_postinstall(dev_priv);
+		else if (IS_GEN(dev_priv, 4))
+			i965_irq_postinstall(dev_priv);
+		else if (IS_GEN(dev_priv, 3))
+			i915_irq_postinstall(dev_priv);
+		else
+			i8xx_irq_postinstall(dev_priv);
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			gen11_irq_postinstall(dev_priv);
+		else if (INTEL_GEN(dev_priv) >= 8)
+			gen8_irq_postinstall(dev_priv);
+		else
+			ironlake_irq_postinstall(dev_priv);
+	}
+}
+
 /**
  * intel_irq_install - enables the hardware interrupt
  * @dev_priv: i915 device instance
@@ -4931,6 +4906,9 @@ void intel_irq_fini(struct drm_i915_private *i915)
  */
 int intel_irq_install(struct drm_i915_private *dev_priv)
 {
+	int irq = dev_priv->drm.pdev->irq;
+	int ret;
+
 	/*
 	 * We enable some interrupt sources in our postinstall hooks, so mark
 	 * interrupts as enabled _before_ actually enabling them to avoid
@@ -4938,7 +4916,20 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
 	 */
 	dev_priv->runtime_pm.irqs_enabled = true;
 
-	return drm_irq_install(&dev_priv->drm, dev_priv->drm.pdev->irq);
+	dev_priv->drm.irq_enabled = true;
+
+	intel_irq_reset(dev_priv);
+
+	ret = request_irq(irq, intel_irq_handler(dev_priv),
+			  IRQF_SHARED, DRIVER_NAME, dev_priv);
+	if (ret < 0) {
+		dev_priv->drm.irq_enabled = false;
+		return ret;
+	}
+
+	intel_irq_postinstall(dev_priv);
+
+	return ret;
 }
 
 /**
@@ -4950,7 +4941,14 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
  */
 void intel_irq_uninstall(struct drm_i915_private *dev_priv)
 {
-	drm_irq_uninstall(&dev_priv->drm);
+	int irq = dev_priv->drm.pdev->irq;
+
+	dev_priv->drm.irq_enabled = false;
+
+	intel_irq_reset(dev_priv);
+
+	free_irq(irq, dev_priv);
+
 	intel_hpd_cancel_work(dev_priv);
 	dev_priv->runtime_pm.irqs_enabled = false;
 }
@@ -4964,7 +4962,7 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
  */
 void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
 {
-	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
+	intel_irq_reset(dev_priv);
 	dev_priv->runtime_pm.irqs_enabled = false;
 	synchronize_irq(dev_priv->drm.irq);
 }
@@ -4979,6 +4977,6 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
 void intel_runtime_pm_enable_interrupts(struct drm_i915_private *dev_priv)
 {
 	dev_priv->runtime_pm.irqs_enabled = true;
-	dev_priv->drm.driver->irq_preinstall(&dev_priv->drm);
-	dev_priv->drm.driver->irq_postinstall(&dev_priv->drm);
+	intel_irq_reset(dev_priv);
+	intel_irq_postinstall(dev_priv);
 }
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] drm/i915: Initialize drm_driver vblank funcs at compile time
  2019-06-18 14:21 [PATCH 0/3] drm/i915: Finish drm_driver vfunc cleanup Ville Syrjala
  2019-06-18 14:21 ` [PATCH 1/3] drm/i915: Fix various tracepoints for gen2 Ville Syrjala
  2019-06-18 14:21 ` [PATCH 2/3] drm/i915: Nuke drm_driver irq vfuncs Ville Syrjala
@ 2019-06-18 14:21 ` Ville Syrjala
  2019-06-18 14:55   ` Chris Wilson
  2019-06-18 15:12 ` ✗ Fi.CI.BAT: failure for drm/i915: Finish drm_driver vfunc cleanup Patchwork
  3 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjala @ 2019-06-18 14:21 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Move the .get_vblank_timestamp() and .get_scanout_position()
initialization to happen at compile time. No point in delaying
it since we always assign the same functions.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |  3 +++
 drivers/gpu/drm/i915/i915_irq.c | 11 ++++-------
 drivers/gpu/drm/i915/i915_irq.h |  5 +++++
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ea6b06109d5a..178e9872b905 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -3216,6 +3216,9 @@ static struct drm_driver driver = {
 	.gem_prime_export = i915_gem_prime_export,
 	.gem_prime_import = i915_gem_prime_import,
 
+	.get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
+	.get_scanout_position = i915_get_crtc_scanoutpos,
+
 	.dumb_create = i915_gem_dumb_create,
 	.dumb_map_offset = i915_gem_mmap_gtt,
 	.ioctls = i915_ioctls,
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e9e29eed8005..f2aa01f35280 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1109,10 +1109,10 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 	return (position + crtc->scanline_offset) % vtotal;
 }
 
-static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
-				     bool in_vblank_irq, int *vpos, int *hpos,
-				     ktime_t *stime, ktime_t *etime,
-				     const struct drm_display_mode *mode)
+bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
+			      bool in_vblank_irq, int *vpos, int *hpos,
+			      ktime_t *stime, ktime_t *etime,
+			      const struct drm_display_mode *mode)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv,
@@ -4789,9 +4789,6 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 	 */
 	dev_priv->hotplug.hpd_short_storm_enabled = !HAS_DP_MST(dev_priv);
 
-	dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
-	dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
-
 	if (HAS_GMCH(dev_priv)) {
 		if (I915_HAS_HOTPLUG(dev_priv))
 			dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
index ef782e5ab240..5af5654f801d 100644
--- a/drivers/gpu/drm/i915/i915_irq.h
+++ b/drivers/gpu/drm/i915/i915_irq.h
@@ -114,6 +114,11 @@ void gen11_reset_guc_interrupts(struct drm_i915_private *i915);
 void gen11_enable_guc_interrupts(struct drm_i915_private *i915);
 void gen11_disable_guc_interrupts(struct drm_i915_private *i915);
 
+bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
+			      bool in_vblank_irq, int *vpos, int *hpos,
+			      ktime_t *stime, ktime_t *etime,
+			      const struct drm_display_mode *mode);
+
 u32 i915_get_vblank_counter(struct drm_crtc *crtc);
 u32 g4x_get_vblank_counter(struct drm_crtc *crtc);
 
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix various tracepoints for gen2
  2019-06-18 14:21 ` [PATCH 1/3] drm/i915: Fix various tracepoints for gen2 Ville Syrjala
@ 2019-06-18 14:46   ` Chris Wilson
  2019-06-18 15:14     ` Ville Syrjälä
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-06-18 14:46 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Daniel Vetter, Shawn Guo

Quoting Ville Syrjala (2019-06-18 15:21:06)
> @@ -59,14 +57,13 @@ TRACE_EVENT(intel_pipe_disable,
>                              ),
>  
>             TP_fast_assign(
> -                          enum pipe _pipe;
> -                          for_each_pipe(dev_priv, _pipe) {
> -                                  __entry->frame[_pipe] =
> -                                          dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, _pipe);
> -                                  __entry->scanline[_pipe] =
> -                                          intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, _pipe));
> +                          struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +                          struct intel_crtc *_crtc;
> +                          for_each_intel_crtc(&dev_priv->drm, _crtc) {
> +                                  __entry->frame[_crtc->pipe] = intel_crtc_get_vblank_counter(_crtc);
> +                                  __entry->scanline[_crtc->pipe] = intel_get_crtc_scanline(_crtc);
>                            }
> -                          __entry->pipe = pipe;
> +                          __entry->pipe = crtc->pipe;

Ok. Stared hard to make sure it was _crtc and not crtc. Would crtc__ be
more obvious, or maybe it__ so that it doesn't look anything like the
crtc argument.

>             TP_fast_assign(
> -                          enum pipe pipe;
> -                          for_each_pipe(dev_priv, pipe) {
> -                                  __entry->frame[pipe] =
> -                                          dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
> -                                  __entry->scanline[pipe] =
> -                                          intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
> +                          struct intel_crtc *crtc;
> +                          for_each_intel_crtc(&dev_priv->drm, crtc) {
> +                                  __entry->frame[crtc->pipe] = intel_crtc_get_vblank_counter(crtc);
> +                                  __entry->scanline[crtc->pipe] = intel_get_crtc_scanline(crtc);
>                            }

Or even a populate_vblanks(i915, __entry->frame, __entry->scanline)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] drm/i915: Nuke drm_driver irq vfuncs
  2019-06-18 14:21 ` [PATCH 2/3] drm/i915: Nuke drm_driver irq vfuncs Ville Syrjala
@ 2019-06-18 14:54   ` Chris Wilson
  2019-06-18 15:26     ` Ville Syrjälä
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-06-18 14:54 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Quoting Ville Syrjala (2019-06-18 15:21:07)
[snip mechanical changes]

> @@ -4839,65 +4792,18 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>         dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
>         dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
>  
> -       if (IS_CHERRYVIEW(dev_priv)) {
> -               dev->driver->irq_handler = cherryview_irq_handler;
> -               dev->driver->irq_preinstall = cherryview_irq_reset;
> -               dev->driver->irq_postinstall = cherryview_irq_postinstall;
> -               dev->driver->irq_uninstall = cherryview_irq_reset;
> -               dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> -       } else if (IS_VALLEYVIEW(dev_priv)) {
> -               dev->driver->irq_handler = valleyview_irq_handler;
> -               dev->driver->irq_preinstall = valleyview_irq_reset;
> -               dev->driver->irq_postinstall = valleyview_irq_postinstall;
> -               dev->driver->irq_uninstall = valleyview_irq_reset;
> -               dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> -       } else if (INTEL_GEN(dev_priv) >= 11) {
> -               dev->driver->irq_handler = gen11_irq_handler;
> -               dev->driver->irq_preinstall = gen11_irq_reset;
> -               dev->driver->irq_postinstall = gen11_irq_postinstall;
> -               dev->driver->irq_uninstall = gen11_irq_reset;
> -               dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup;
> -       } else if (INTEL_GEN(dev_priv) >= 8) {
> -               dev->driver->irq_handler = gen8_irq_handler;
> -               dev->driver->irq_preinstall = gen8_irq_reset;
> -               dev->driver->irq_postinstall = gen8_irq_postinstall;
> -               dev->driver->irq_uninstall = gen8_irq_reset;
> -               if (IS_GEN9_LP(dev_priv))
> +       if (HAS_GMCH(dev_priv)) {
> +               if (I915_HAS_HOTPLUG(dev_priv))
> +                       dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;

Includes chv/vlv.

> +       } else {
> +               if (INTEL_GEN(dev_priv) >= 11)
> +                       dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup;

Ok.

> +               else if (IS_GEN9_LP(dev_priv))
>                         dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
>                 else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
>                         dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;

As before.

>                 else
>                         dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;

The rest of !GMCH.

Code be one level if-chain though.

> @@ -4918,6 +4824,75 @@ void intel_irq_fini(struct drm_i915_private *i915)
>                 kfree(i915->l3_parity.remap_info[i]);
>  }
>  
> +static irq_handler_t intel_irq_handler(struct drm_i915_private *dev_priv)
> +{
> +       if (HAS_GMCH(dev_priv)) {
> +               if (IS_CHERRYVIEW(dev_priv))
> +                       return cherryview_irq_handler;
> +               else if (IS_VALLEYVIEW(dev_priv))
> +                       return valleyview_irq_handler;
> +               else if (IS_GEN(dev_priv, 4))
> +                       return i965_irq_handler;
> +               else if (IS_GEN(dev_priv, 3))
> +                       return i915_irq_handler;
> +               else
> +                       return i8xx_irq_handler;
> +       } else {
> +               if (INTEL_GEN(dev_priv) >= 11)
> +                       return gen11_irq_handler;
> +               else if (INTEL_GEN(dev_priv) >= 8)
> +                       return gen8_irq_handler;
> +               else
> +                       return ironlake_irq_handler;
> +       }
> +}
> +
> +static void intel_irq_reset(struct drm_i915_private *dev_priv)
> +{
> +       if (HAS_GMCH(dev_priv)) {
> +               if (IS_CHERRYVIEW(dev_priv))
> +                       cherryview_irq_reset(dev_priv);
> +               else if (IS_VALLEYVIEW(dev_priv))
> +                       valleyview_irq_reset(dev_priv);
> +               else if (IS_GEN(dev_priv, 4))
> +                       i965_irq_reset(dev_priv);
> +               else if (IS_GEN(dev_priv, 3))
> +                       i915_irq_reset(dev_priv);
> +               else
> +                       i8xx_irq_reset(dev_priv);
> +       } else {
> +               if (INTEL_GEN(dev_priv) >= 11)
> +                       gen11_irq_reset(dev_priv);
> +               else if (INTEL_GEN(dev_priv) >= 8)
> +                       gen8_irq_reset(dev_priv);
> +               else
> +                       ironlake_irq_reset(dev_priv);
> +       }
> +}
> +
> +static void intel_irq_postinstall(struct drm_i915_private *dev_priv)
> +{
> +       if (HAS_GMCH(dev_priv)) {
> +               if (IS_CHERRYVIEW(dev_priv))
> +                       cherryview_irq_postinstall(dev_priv);
> +               else if (IS_VALLEYVIEW(dev_priv))
> +                       valleyview_irq_postinstall(dev_priv);
> +               else if (IS_GEN(dev_priv, 4))
> +                       i965_irq_postinstall(dev_priv);
> +               else if (IS_GEN(dev_priv, 3))
> +                       i915_irq_postinstall(dev_priv);
> +               else
> +                       i8xx_irq_postinstall(dev_priv);
> +       } else {
> +               if (INTEL_GEN(dev_priv) >= 11)
> +                       gen11_irq_postinstall(dev_priv);
> +               else if (INTEL_GEN(dev_priv) >= 8)
> +                       gen8_irq_postinstall(dev_priv);
> +               else
> +                       ironlake_irq_postinstall(dev_priv);
> +       }
> +}
> +
>  /**
>   * intel_irq_install - enables the hardware interrupt
>   * @dev_priv: i915 device instance
> @@ -4931,6 +4906,9 @@ void intel_irq_fini(struct drm_i915_private *i915)
>   */
>  int intel_irq_install(struct drm_i915_private *dev_priv)
>  {
> +       int irq = dev_priv->drm.pdev->irq;
> +       int ret;
> +
>         /*
>          * We enable some interrupt sources in our postinstall hooks, so mark
>          * interrupts as enabled _before_ actually enabling them to avoid
> @@ -4938,7 +4916,20 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
>          */
>         dev_priv->runtime_pm.irqs_enabled = true;
>  
> -       return drm_irq_install(&dev_priv->drm, dev_priv->drm.pdev->irq);
> +       dev_priv->drm.irq_enabled = true;
> +
> +       intel_irq_reset(dev_priv);
> +
> +       ret = request_irq(irq, intel_irq_handler(dev_priv),

Oh fancy.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] drm/i915: Initialize drm_driver vblank funcs at compile time
  2019-06-18 14:21 ` [PATCH 3/3] drm/i915: Initialize drm_driver vblank funcs at compile time Ville Syrjala
@ 2019-06-18 14:55   ` Chris Wilson
  2019-06-19 16:18     ` Ville Syrjälä
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-06-18 14:55 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Quoting Ville Syrjala (2019-06-18 15:21:08)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the .get_vblank_timestamp() and .get_scanout_position()
> initialization to happen at compile time. No point in delaying
> it since we always assign the same functions.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  3 +++
>  drivers/gpu/drm/i915/i915_irq.c | 11 ++++-------
>  drivers/gpu/drm/i915/i915_irq.h |  5 +++++
>  3 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ea6b06109d5a..178e9872b905 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -3216,6 +3216,9 @@ static struct drm_driver driver = {
>         .gem_prime_export = i915_gem_prime_export,
>         .gem_prime_import = i915_gem_prime_import,
>  
> +       .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
> +       .get_scanout_position = i915_get_crtc_scanoutpos,

One might suggest intel_get_crtc_scanoutpos, and a push for it to be
passed drm_crtc instead of dev + pipe.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Finish drm_driver vfunc cleanup
  2019-06-18 14:21 [PATCH 0/3] drm/i915: Finish drm_driver vfunc cleanup Ville Syrjala
                   ` (2 preceding siblings ...)
  2019-06-18 14:21 ` [PATCH 3/3] drm/i915: Initialize drm_driver vblank funcs at compile time Ville Syrjala
@ 2019-06-18 15:12 ` Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-06-18 15:12 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Finish drm_driver vfunc cleanup
URL   : https://patchwork.freedesktop.org/series/62317/
State : failure

== Summary ==

Applying: drm/i915: Fix various tracepoints for gen2
Applying: drm/i915: Nuke drm_driver irq vfuncs
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_irq.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_irq.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_irq.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0002 drm/i915: Nuke drm_driver irq vfuncs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix various tracepoints for gen2
  2019-06-18 14:46   ` Chris Wilson
@ 2019-06-18 15:14     ` Ville Syrjälä
  0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2019-06-18 15:14 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, Shawn Guo

On Tue, Jun 18, 2019 at 03:46:29PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-06-18 15:21:06)
> > @@ -59,14 +57,13 @@ TRACE_EVENT(intel_pipe_disable,
> >                              ),
> >  
> >             TP_fast_assign(
> > -                          enum pipe _pipe;
> > -                          for_each_pipe(dev_priv, _pipe) {
> > -                                  __entry->frame[_pipe] =
> > -                                          dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, _pipe);
> > -                                  __entry->scanline[_pipe] =
> > -                                          intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, _pipe));
> > +                          struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +                          struct intel_crtc *_crtc;
> > +                          for_each_intel_crtc(&dev_priv->drm, _crtc) {
> > +                                  __entry->frame[_crtc->pipe] = intel_crtc_get_vblank_counter(_crtc);
> > +                                  __entry->scanline[_crtc->pipe] = intel_get_crtc_scanline(_crtc);
> >                            }
> > -                          __entry->pipe = pipe;
> > +                          __entry->pipe = crtc->pipe;
> 
> Ok. Stared hard to make sure it was _crtc and not crtc. Would crtc__ be
> more obvious, or maybe it__ so that it doesn't look anything like the
> crtc argument.

I suppose a more distinct name might be a good idea.

> 
> >             TP_fast_assign(
> > -                          enum pipe pipe;
> > -                          for_each_pipe(dev_priv, pipe) {
> > -                                  __entry->frame[pipe] =
> > -                                          dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
> > -                                  __entry->scanline[pipe] =
> > -                                          intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
> > +                          struct intel_crtc *crtc;
> > +                          for_each_intel_crtc(&dev_priv->drm, crtc) {
> > +                                  __entry->frame[crtc->pipe] = intel_crtc_get_vblank_counter(crtc);
> > +                                  __entry->scanline[crtc->pipe] = intel_get_crtc_scanline(crtc);
> >                            }
> 
> Or even a populate_vblanks(i915, __entry->frame, __entry->scanline)
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] drm/i915: Nuke drm_driver irq vfuncs
  2019-06-18 14:54   ` Chris Wilson
@ 2019-06-18 15:26     ` Ville Syrjälä
  0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2019-06-18 15:26 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Tue, Jun 18, 2019 at 03:54:01PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-06-18 15:21:07)
> [snip mechanical changes]
> 
> > @@ -4839,65 +4792,18 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> >         dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
> >         dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
> >  
> > -       if (IS_CHERRYVIEW(dev_priv)) {
> > -               dev->driver->irq_handler = cherryview_irq_handler;
> > -               dev->driver->irq_preinstall = cherryview_irq_reset;
> > -               dev->driver->irq_postinstall = cherryview_irq_postinstall;
> > -               dev->driver->irq_uninstall = cherryview_irq_reset;
> > -               dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> > -       } else if (IS_VALLEYVIEW(dev_priv)) {
> > -               dev->driver->irq_handler = valleyview_irq_handler;
> > -               dev->driver->irq_preinstall = valleyview_irq_reset;
> > -               dev->driver->irq_postinstall = valleyview_irq_postinstall;
> > -               dev->driver->irq_uninstall = valleyview_irq_reset;
> > -               dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> > -       } else if (INTEL_GEN(dev_priv) >= 11) {
> > -               dev->driver->irq_handler = gen11_irq_handler;
> > -               dev->driver->irq_preinstall = gen11_irq_reset;
> > -               dev->driver->irq_postinstall = gen11_irq_postinstall;
> > -               dev->driver->irq_uninstall = gen11_irq_reset;
> > -               dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup;
> > -       } else if (INTEL_GEN(dev_priv) >= 8) {
> > -               dev->driver->irq_handler = gen8_irq_handler;
> > -               dev->driver->irq_preinstall = gen8_irq_reset;
> > -               dev->driver->irq_postinstall = gen8_irq_postinstall;
> > -               dev->driver->irq_uninstall = gen8_irq_reset;
> > -               if (IS_GEN9_LP(dev_priv))
> > +       if (HAS_GMCH(dev_priv)) {
> > +               if (I915_HAS_HOTPLUG(dev_priv))
> > +                       dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> 
> Includes chv/vlv.
> 
> > +       } else {
> > +               if (INTEL_GEN(dev_priv) >= 11)
> > +                       dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup;
> 
> Ok.
> 
> > +               else if (IS_GEN9_LP(dev_priv))
> >                         dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
> >                 else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
> >                         dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
> 
> As before.
> 
> >                 else
> >                         dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> 
> The rest of !GMCH.
> 
> Code be one level if-chain though.

"could be ..."? Sure. Not entirely sure I'd be able to follow it though.
I guess just flattening it and checking for HAS_PCH_SPLIT() for ilk+
could be a reasonably non-confusing way to write it. Silly vlv/chv
always interfering with the mostly nice chronological order.

> 
> > @@ -4918,6 +4824,75 @@ void intel_irq_fini(struct drm_i915_private *i915)
> >                 kfree(i915->l3_parity.remap_info[i]);
> >  }
> >  
> > +static irq_handler_t intel_irq_handler(struct drm_i915_private *dev_priv)
> > +{
> > +       if (HAS_GMCH(dev_priv)) {
> > +               if (IS_CHERRYVIEW(dev_priv))
> > +                       return cherryview_irq_handler;
> > +               else if (IS_VALLEYVIEW(dev_priv))
> > +                       return valleyview_irq_handler;
> > +               else if (IS_GEN(dev_priv, 4))
> > +                       return i965_irq_handler;
> > +               else if (IS_GEN(dev_priv, 3))
> > +                       return i915_irq_handler;
> > +               else
> > +                       return i8xx_irq_handler;
> > +       } else {
> > +               if (INTEL_GEN(dev_priv) >= 11)
> > +                       return gen11_irq_handler;
> > +               else if (INTEL_GEN(dev_priv) >= 8)
> > +                       return gen8_irq_handler;
> > +               else
> > +                       return ironlake_irq_handler;
> > +       }
> > +}
> > +
> > +static void intel_irq_reset(struct drm_i915_private *dev_priv)
> > +{
> > +       if (HAS_GMCH(dev_priv)) {
> > +               if (IS_CHERRYVIEW(dev_priv))
> > +                       cherryview_irq_reset(dev_priv);
> > +               else if (IS_VALLEYVIEW(dev_priv))
> > +                       valleyview_irq_reset(dev_priv);
> > +               else if (IS_GEN(dev_priv, 4))
> > +                       i965_irq_reset(dev_priv);
> > +               else if (IS_GEN(dev_priv, 3))
> > +                       i915_irq_reset(dev_priv);
> > +               else
> > +                       i8xx_irq_reset(dev_priv);
> > +       } else {
> > +               if (INTEL_GEN(dev_priv) >= 11)
> > +                       gen11_irq_reset(dev_priv);
> > +               else if (INTEL_GEN(dev_priv) >= 8)
> > +                       gen8_irq_reset(dev_priv);
> > +               else
> > +                       ironlake_irq_reset(dev_priv);
> > +       }
> > +}
> > +
> > +static void intel_irq_postinstall(struct drm_i915_private *dev_priv)
> > +{
> > +       if (HAS_GMCH(dev_priv)) {
> > +               if (IS_CHERRYVIEW(dev_priv))
> > +                       cherryview_irq_postinstall(dev_priv);
> > +               else if (IS_VALLEYVIEW(dev_priv))
> > +                       valleyview_irq_postinstall(dev_priv);
> > +               else if (IS_GEN(dev_priv, 4))
> > +                       i965_irq_postinstall(dev_priv);
> > +               else if (IS_GEN(dev_priv, 3))
> > +                       i915_irq_postinstall(dev_priv);
> > +               else
> > +                       i8xx_irq_postinstall(dev_priv);
> > +       } else {
> > +               if (INTEL_GEN(dev_priv) >= 11)
> > +                       gen11_irq_postinstall(dev_priv);
> > +               else if (INTEL_GEN(dev_priv) >= 8)
> > +                       gen8_irq_postinstall(dev_priv);
> > +               else
> > +                       ironlake_irq_postinstall(dev_priv);
> > +       }
> > +}
> > +
> >  /**
> >   * intel_irq_install - enables the hardware interrupt
> >   * @dev_priv: i915 device instance
> > @@ -4931,6 +4906,9 @@ void intel_irq_fini(struct drm_i915_private *i915)
> >   */
> >  int intel_irq_install(struct drm_i915_private *dev_priv)
> >  {
> > +       int irq = dev_priv->drm.pdev->irq;
> > +       int ret;
> > +
> >         /*
> >          * We enable some interrupt sources in our postinstall hooks, so mark
> >          * interrupts as enabled _before_ actually enabling them to avoid
> > @@ -4938,7 +4916,20 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
> >          */
> >         dev_priv->runtime_pm.irqs_enabled = true;
> >  
> > -       return drm_irq_install(&dev_priv->drm, dev_priv->drm.pdev->irq);
> > +       dev_priv->drm.irq_enabled = true;
> > +
> > +       intel_irq_reset(dev_priv);
> > +
> > +       ret = request_irq(irq, intel_irq_handler(dev_priv),
> 
> Oh fancy.

I was debating to vfunc or not to vfunc. Seemed simple enough without.
Though the repetition of the platform checks in intel_irq_*() is a bit
annoying.

> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] drm/i915: Initialize drm_driver vblank funcs at compile time
  2019-06-18 14:55   ` Chris Wilson
@ 2019-06-19 16:18     ` Ville Syrjälä
  0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2019-06-19 16:18 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Tue, Jun 18, 2019 at 03:55:10PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-06-18 15:21:08)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Move the .get_vblank_timestamp() and .get_scanout_position()
> > initialization to happen at compile time. No point in delaying
> > it since we always assign the same functions.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c |  3 +++
> >  drivers/gpu/drm/i915/i915_irq.c | 11 ++++-------
> >  drivers/gpu/drm/i915/i915_irq.h |  5 +++++
> >  3 files changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index ea6b06109d5a..178e9872b905 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -3216,6 +3216,9 @@ static struct drm_driver driver = {
> >         .gem_prime_export = i915_gem_prime_export,
> >         .gem_prime_import = i915_gem_prime_import,
> >  
> > +       .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
> > +       .get_scanout_position = i915_get_crtc_scanoutpos,
> 
> One might suggest intel_get_crtc_scanoutpos, and a push for it to be
> passed drm_crtc instead of dev + pipe.

I suppose. There's also a FIXME about moving it to the crtc_helper funcs
instead. Or could just remove the vfunc entirely and have each driver
pass it to drm_calc_vbltimestamp_from_scanoutpos(). Either that or
eliminate the .get_vblank_timestamp() vfunc instead since I believe
everyone who has that uses drm_calc_vbltimestamp_from_scanoutpos().

Anyways, looks like there's enough material there for a good few hours
of cursing.

> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-06-19 16:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-18 14:21 [PATCH 0/3] drm/i915: Finish drm_driver vfunc cleanup Ville Syrjala
2019-06-18 14:21 ` [PATCH 1/3] drm/i915: Fix various tracepoints for gen2 Ville Syrjala
2019-06-18 14:46   ` Chris Wilson
2019-06-18 15:14     ` Ville Syrjälä
2019-06-18 14:21 ` [PATCH 2/3] drm/i915: Nuke drm_driver irq vfuncs Ville Syrjala
2019-06-18 14:54   ` Chris Wilson
2019-06-18 15:26     ` Ville Syrjälä
2019-06-18 14:21 ` [PATCH 3/3] drm/i915: Initialize drm_driver vblank funcs at compile time Ville Syrjala
2019-06-18 14:55   ` Chris Wilson
2019-06-19 16:18     ` Ville Syrjälä
2019-06-18 15:12 ` ✗ Fi.CI.BAT: failure for drm/i915: Finish drm_driver vfunc cleanup Patchwork

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.