[PATCH 0/5] drm/i915: intel_display conversions, cleanups

[PATCH 0/5] drm/i915: intel_display conversions, cleanups

[Jani Nikula]

Some struct intel_display conversions, with the goal to reduce direct
struct intel_display member access from i915 core code. Also some
related cleanups.

BR,
Jani.


Jani Nikula (5):
  drm/i915/overlay: convert to struct intel_display
  drm/i915/overlay: add intel_overlay_available() and use it
  drm/i915/plane: convert initial plane setup to struct intel_display
  drm/i915/irq: hide display_irqs_enabled access
  drm/i915/irq: emphasize display_irqs_enabled is only about VLV/CHV

 .../gpu/drm/i915/display/intel_display_core.h |   9 +-
 .../drm/i915/display/intel_display_driver.c   |   6 +-
 .../gpu/drm/i915/display/intel_display_irq.c  |  37 ++--
 .../gpu/drm/i915/display/intel_hotplug_irq.c  |   6 +-
 drivers/gpu/drm/i915/display/intel_overlay.c  | 184 +++++++++---------
 drivers/gpu/drm/i915/display/intel_overlay.h  |  19 +-
 .../drm/i915/display/intel_plane_initial.c    |  56 +++---
 .../drm/i915/display/intel_plane_initial.h    |   4 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
 drivers/gpu/drm/i915/i915_getparam.c          |   5 +-
 drivers/gpu/drm/i915/i915_irq.c               |  12 +-
 drivers/gpu/drm/xe/display/xe_plane_initial.c |   8 +-
 12 files changed, 187 insertions(+), 162 deletions(-)

-- 
2.39.5

[PATCH 1/5] drm/i915/overlay: convert to struct intel_display

[Jani Nikula]

struct intel_display replaces struct drm_i915_private as the main
display device pointer. Convert overlay to it, as much as possible.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_display_driver.c   |   4 +-
 drivers/gpu/drm/i915/display/intel_overlay.c  | 179 +++++++++---------
 drivers/gpu/drm/i915/display/intel_overlay.h  |  14 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
 4 files changed, 102 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 56b78cf6b854..5983570b510f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -518,7 +518,7 @@ int intel_display_driver_probe(struct drm_i915_private *i915)
 	if (ret)
 		drm_dbg_kms(&i915->drm, "Initial modeset failed, %d\n", ret);
 
-	intel_overlay_setup(i915);
+	intel_overlay_setup(display);
 
 	/* Only enable hotplug handling once the fbdev is fully set up. */
 	intel_hpd_init(i915);
@@ -607,7 +607,7 @@ void intel_display_driver_remove_noirq(struct drm_i915_private *i915)
 
 	intel_dp_tunnel_mgr_cleanup(display);
 
-	intel_overlay_cleanup(i915);
+	intel_overlay_cleanup(display);
 
 	intel_gmbus_teardown(display);
 
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index 2ec14096ba9c..57eaf81651c4 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -183,7 +183,7 @@ struct overlay_registers {
 };
 
 struct intel_overlay {
-	struct drm_i915_private *i915;
+	struct intel_display *display;
 	struct intel_context *context;
 	struct intel_crtc *crtc;
 	struct i915_vma *vma;
@@ -205,17 +205,17 @@ struct intel_overlay {
 	void (*flip_complete)(struct intel_overlay *ovl);
 };
 
-static void i830_overlay_clock_gating(struct drm_i915_private *dev_priv,
+static void i830_overlay_clock_gating(struct intel_display *display,
 				      bool enable)
 {
-	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
+	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
 	u8 val;
 
 	/* WA_OVERLAY_CLKGATE:alm */
 	if (enable)
-		intel_de_write(dev_priv, DSPCLK_GATE_D(dev_priv), 0);
+		intel_de_write(display, DSPCLK_GATE_D(display), 0);
 	else
-		intel_de_write(dev_priv, DSPCLK_GATE_D(dev_priv),
+		intel_de_write(display, DSPCLK_GATE_D(display),
 			       OVRUNIT_CLOCK_GATE_DISABLE);
 
 	/* WA_DISABLE_L2CACHE_CLOCK_GATING:alm */
@@ -253,11 +253,11 @@ alloc_request(struct intel_overlay *overlay, void (*fn)(struct intel_overlay *))
 /* overlay needs to be disable in OCMD reg */
 static int intel_overlay_on(struct intel_overlay *overlay)
 {
-	struct drm_i915_private *dev_priv = overlay->i915;
+	struct intel_display *display = overlay->display;
 	struct i915_request *rq;
 	u32 *cs;
 
-	drm_WARN_ON(&dev_priv->drm, overlay->active);
+	drm_WARN_ON(display->drm, overlay->active);
 
 	rq = alloc_request(overlay, NULL);
 	if (IS_ERR(rq))
@@ -271,8 +271,8 @@ static int intel_overlay_on(struct intel_overlay *overlay)
 
 	overlay->active = true;
 
-	if (IS_I830(dev_priv))
-		i830_overlay_clock_gating(dev_priv, false);
+	if (display->platform.i830)
+		i830_overlay_clock_gating(display, false);
 
 	*cs++ = MI_OVERLAY_FLIP | MI_OVERLAY_ON;
 	*cs++ = overlay->flip_addr | OFC_UPDATE;
@@ -288,10 +288,12 @@ static int intel_overlay_on(struct intel_overlay *overlay)
 static void intel_overlay_flip_prepare(struct intel_overlay *overlay,
 				       struct i915_vma *vma)
 {
+	struct intel_display *display = overlay->display;
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	enum pipe pipe = overlay->crtc->pipe;
 	struct intel_frontbuffer *frontbuffer = NULL;
 
-	drm_WARN_ON(&overlay->i915->drm, overlay->old_vma);
+	drm_WARN_ON(display->drm, overlay->old_vma);
 
 	if (vma)
 		frontbuffer = intel_frontbuffer_get(intel_bo_to_drm_bo(vma->obj));
@@ -303,8 +305,7 @@ static void intel_overlay_flip_prepare(struct intel_overlay *overlay,
 		intel_frontbuffer_put(overlay->frontbuffer);
 	overlay->frontbuffer = frontbuffer;
 
-	intel_frontbuffer_flip_prepare(overlay->i915,
-				       INTEL_FRONTBUFFER_OVERLAY(pipe));
+	intel_frontbuffer_flip_prepare(i915, INTEL_FRONTBUFFER_OVERLAY(pipe));
 
 	overlay->old_vma = overlay->vma;
 	if (vma)
@@ -318,20 +319,20 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
 				  struct i915_vma *vma,
 				  bool load_polyphase_filter)
 {
-	struct drm_i915_private *dev_priv = overlay->i915;
+	struct intel_display *display = overlay->display;
 	struct i915_request *rq;
 	u32 flip_addr = overlay->flip_addr;
 	u32 tmp, *cs;
 
-	drm_WARN_ON(&dev_priv->drm, !overlay->active);
+	drm_WARN_ON(display->drm, !overlay->active);
 
 	if (load_polyphase_filter)
 		flip_addr |= OFC_UPDATE;
 
 	/* check for underruns */
-	tmp = intel_de_read(dev_priv, DOVSTA);
+	tmp = intel_de_read(display, DOVSTA);
 	if (tmp & (1 << 17))
-		drm_dbg(&dev_priv->drm, "overlay underrun, DOVSTA: %x\n", tmp);
+		drm_dbg(display->drm, "overlay underrun, DOVSTA: %x\n", tmp);
 
 	rq = alloc_request(overlay, NULL);
 	if (IS_ERR(rq))
@@ -355,14 +356,15 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
 
 static void intel_overlay_release_old_vma(struct intel_overlay *overlay)
 {
+	struct intel_display *display = overlay->display;
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct i915_vma *vma;
 
 	vma = fetch_and_zero(&overlay->old_vma);
-	if (drm_WARN_ON(&overlay->i915->drm, !vma))
+	if (drm_WARN_ON(display->drm, !vma))
 		return;
 
-	intel_frontbuffer_flip_complete(overlay->i915,
-					INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
+	intel_frontbuffer_flip_complete(i915, INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
 
 	i915_vma_unpin(vma);
 	i915_vma_put(vma);
@@ -376,7 +378,7 @@ intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
 
 static void intel_overlay_off_tail(struct intel_overlay *overlay)
 {
-	struct drm_i915_private *dev_priv = overlay->i915;
+	struct intel_display *display = overlay->display;
 
 	intel_overlay_release_old_vma(overlay);
 
@@ -384,8 +386,8 @@ static void intel_overlay_off_tail(struct intel_overlay *overlay)
 	overlay->crtc = NULL;
 	overlay->active = false;
 
-	if (IS_I830(dev_priv))
-		i830_overlay_clock_gating(dev_priv, true);
+	if (display->platform.i830)
+		i830_overlay_clock_gating(display, true);
 }
 
 static void intel_overlay_last_flip_retire(struct i915_active *active)
@@ -400,10 +402,11 @@ static void intel_overlay_last_flip_retire(struct i915_active *active)
 /* overlay needs to be disabled in OCMD reg */
 static int intel_overlay_off(struct intel_overlay *overlay)
 {
+	struct intel_display *display = overlay->display;
 	struct i915_request *rq;
 	u32 *cs, flip_addr = overlay->flip_addr;
 
-	drm_WARN_ON(&overlay->i915->drm, !overlay->active);
+	drm_WARN_ON(display->drm, !overlay->active);
 
 	/* According to intel docs the overlay hw may hang (when switching
 	 * off) without loading the filter coeffs. It is however unclear whether
@@ -452,7 +455,7 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
  */
 static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
 {
-	struct drm_i915_private *dev_priv = overlay->i915;
+	struct intel_display *display = overlay->display;
 	struct i915_request *rq;
 	u32 *cs;
 
@@ -463,7 +466,7 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
 	if (!overlay->old_vma)
 		return 0;
 
-	if (!(intel_de_read(dev_priv, GEN2_ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT)) {
+	if (!(intel_de_read(display, GEN2_ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT)) {
 		intel_overlay_release_old_vid_tail(overlay);
 		return 0;
 	}
@@ -487,9 +490,9 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
 	return i915_active_wait(&overlay->last_flip);
 }
 
-void intel_overlay_reset(struct drm_i915_private *dev_priv)
+void intel_overlay_reset(struct intel_display *display)
 {
-	struct intel_overlay *overlay = dev_priv->display.overlay;
+	struct intel_overlay *overlay = display->overlay;
 
 	if (!overlay)
 		return;
@@ -550,11 +553,11 @@ static int uv_vsubsampling(u32 format)
 	}
 }
 
-static u32 calc_swidthsw(struct drm_i915_private *dev_priv, u32 offset, u32 width)
+static u32 calc_swidthsw(struct intel_display *display, u32 offset, u32 width)
 {
 	u32 sw;
 
-	if (DISPLAY_VER(dev_priv) == 2)
+	if (DISPLAY_VER(display) == 2)
 		sw = ALIGN((offset & 31) + width, 32);
 	else
 		sw = ALIGN((offset & 63) + width, 64);
@@ -789,16 +792,17 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 				      struct drm_i915_gem_object *new_bo,
 				      struct drm_intel_overlay_put_image *params)
 {
+	struct intel_display *display = overlay->display;
+	struct drm_i915_private *dev_priv = to_i915(display->drm);
 	struct overlay_registers __iomem *regs = overlay->regs;
-	struct drm_i915_private *dev_priv = overlay->i915;
 	u32 swidth, swidthsw, sheight, ostride;
 	enum pipe pipe = overlay->crtc->pipe;
 	bool scale_changed = false;
 	struct i915_vma *vma;
 	int ret, tmp_width;
 
-	drm_WARN_ON(&dev_priv->drm,
-		    !drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
+	drm_WARN_ON(display->drm,
+		    !drm_modeset_is_locked(&display->drm->mode_config.connection_mutex));
 
 	ret = intel_overlay_release_old_vid(overlay);
 	if (ret != 0)
@@ -824,7 +828,7 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 			oconfig |= OCONF_CC_OUT_8BIT;
 		if (crtc_state->gamma_enable)
 			oconfig |= OCONF_GAMMA2_ENABLE;
-		if (DISPLAY_VER(dev_priv) == 4)
+		if (DISPLAY_VER(display) == 4)
 			oconfig |= OCONF_CSC_MODE_BT709;
 		oconfig |= pipe == 0 ?
 			OCONF_PIPE_A : OCONF_PIPE_B;
@@ -845,7 +849,7 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 		tmp_width = params->src_width;
 
 	swidth = params->src_width;
-	swidthsw = calc_swidthsw(dev_priv, params->offset_Y, tmp_width);
+	swidthsw = calc_swidthsw(display, params->offset_Y, tmp_width);
 	sheight = params->src_height;
 	iowrite32(i915_ggtt_offset(vma) + params->offset_Y, &regs->OBUF_0Y);
 	ostride = params->stride_Y;
@@ -858,9 +862,9 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 		swidth |= (params->src_width / uv_hscale) << 16;
 		sheight |= (params->src_height / uv_vscale) << 16;
 
-		tmp_U = calc_swidthsw(dev_priv, params->offset_U,
+		tmp_U = calc_swidthsw(display, params->offset_U,
 				      params->src_width / uv_hscale);
-		tmp_V = calc_swidthsw(dev_priv, params->offset_V,
+		tmp_V = calc_swidthsw(display, params->offset_V,
 				      params->src_width / uv_hscale);
 		swidthsw |= max(tmp_U, tmp_V) << 16;
 
@@ -899,11 +903,11 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 
 int intel_overlay_switch_off(struct intel_overlay *overlay)
 {
-	struct drm_i915_private *dev_priv = overlay->i915;
+	struct intel_display *display = overlay->display;
 	int ret;
 
-	drm_WARN_ON(&dev_priv->drm,
-		    !drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
+	drm_WARN_ON(display->drm,
+		    !drm_modeset_is_locked(&display->drm->mode_config.connection_mutex));
 
 	ret = intel_overlay_recover_from_interrupt(overlay);
 	if (ret != 0)
@@ -936,26 +940,24 @@ static int check_overlay_possible_on_crtc(struct intel_overlay *overlay,
 
 static void update_pfit_vscale_ratio(struct intel_overlay *overlay)
 {
-	struct drm_i915_private *dev_priv = overlay->i915;
+	struct intel_display *display = overlay->display;
 	u32 ratio;
 
 	/* XXX: This is not the same logic as in the xorg driver, but more in
 	 * line with the intel documentation for the i965
 	 */
-	if (DISPLAY_VER(dev_priv) >= 4) {
-		u32 tmp = intel_de_read(dev_priv, PFIT_PGM_RATIOS(dev_priv));
+	if (DISPLAY_VER(display) >= 4) {
+		u32 tmp = intel_de_read(display, PFIT_PGM_RATIOS(display));
 
 		/* on i965 use the PGM reg to read out the autoscaler values */
 		ratio = REG_FIELD_GET(PFIT_VERT_SCALE_MASK_965, tmp);
 	} else {
 		u32 tmp;
 
-		if (intel_de_read(dev_priv, PFIT_CONTROL(dev_priv)) & PFIT_VERT_AUTO_SCALE)
-			tmp = intel_de_read(dev_priv,
-					    PFIT_AUTO_RATIOS(dev_priv));
+		if (intel_de_read(display, PFIT_CONTROL(display)) & PFIT_VERT_AUTO_SCALE)
+			tmp = intel_de_read(display, PFIT_AUTO_RATIOS(display));
 		else
-			tmp = intel_de_read(dev_priv,
-					    PFIT_PGM_RATIOS(dev_priv));
+			tmp = intel_de_read(display, PFIT_PGM_RATIOS(display));
 
 		ratio = REG_FIELD_GET(PFIT_VERT_SCALE_MASK, tmp);
 	}
@@ -1000,7 +1002,7 @@ static int check_overlay_scaling(struct drm_intel_overlay_put_image *rec)
 	return 0;
 }
 
-static int check_overlay_src(struct drm_i915_private *dev_priv,
+static int check_overlay_src(struct intel_display *display,
 			     struct drm_intel_overlay_put_image *rec,
 			     struct drm_i915_gem_object *new_bo)
 {
@@ -1011,7 +1013,7 @@ static int check_overlay_src(struct drm_i915_private *dev_priv,
 	u32 tmp;
 
 	/* check src dimensions */
-	if (IS_I845G(dev_priv) || IS_I830(dev_priv)) {
+	if (display->platform.i845g || display->platform.i830) {
 		if (rec->src_height > IMAGE_MAX_HEIGHT_LEGACY ||
 		    rec->src_width  > IMAGE_MAX_WIDTH_LEGACY)
 			return -EINVAL;
@@ -1063,14 +1065,14 @@ static int check_overlay_src(struct drm_i915_private *dev_priv,
 		return -EINVAL;
 
 	/* stride checking */
-	if (IS_I830(dev_priv) || IS_I845G(dev_priv))
+	if (display->platform.i830 || display->platform.i845g)
 		stride_mask = 255;
 	else
 		stride_mask = 63;
 
 	if (rec->stride_Y & stride_mask || rec->stride_UV & stride_mask)
 		return -EINVAL;
-	if (DISPLAY_VER(dev_priv) == 4 && rec->stride_Y < 512)
+	if (DISPLAY_VER(display) == 4 && rec->stride_Y < 512)
 		return -EINVAL;
 
 	tmp = (rec->flags & I915_OVERLAY_TYPE_MASK) == I915_OVERLAY_YUV_PLANAR ?
@@ -1114,17 +1116,17 @@ static int check_overlay_src(struct drm_i915_private *dev_priv,
 int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file_priv)
 {
+	struct intel_display *display = to_intel_display(dev);
 	struct drm_intel_overlay_put_image *params = data;
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_overlay *overlay;
 	struct drm_crtc *drmmode_crtc;
 	struct intel_crtc *crtc;
 	struct drm_i915_gem_object *new_bo;
 	int ret;
 
-	overlay = dev_priv->display.overlay;
+	overlay = display->overlay;
 	if (!overlay) {
-		drm_dbg(&dev_priv->drm, "userspace bug: no overlay\n");
+		drm_dbg(display->drm, "userspace bug: no overlay\n");
 		return -ENODEV;
 	}
 
@@ -1148,7 +1150,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 	drm_modeset_lock_all(dev);
 
 	if (i915_gem_object_is_tiled(new_bo)) {
-		drm_dbg_kms(&dev_priv->drm,
+		drm_dbg_kms(display->drm,
 			    "buffer used for overlay image can not be tiled\n");
 		ret = -EINVAL;
 		goto out_unlock;
@@ -1197,7 +1199,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 		goto out_unlock;
 	}
 
-	ret = check_overlay_src(dev_priv, params, new_bo);
+	ret = check_overlay_src(display, params, new_bo);
 	if (ret != 0)
 		goto out_unlock;
 
@@ -1277,14 +1279,14 @@ static int check_gamma(struct drm_intel_overlay_attrs *attrs)
 int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv)
 {
+	struct intel_display *display = to_intel_display(dev);
 	struct drm_intel_overlay_attrs *attrs = data;
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_overlay *overlay;
 	int ret;
 
-	overlay = dev_priv->display.overlay;
+	overlay = display->overlay;
 	if (!overlay) {
-		drm_dbg(&dev_priv->drm, "userspace bug: no overlay\n");
+		drm_dbg(display->drm, "userspace bug: no overlay\n");
 		return -ENODEV;
 	}
 
@@ -1297,13 +1299,13 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 		attrs->contrast   = overlay->contrast;
 		attrs->saturation = overlay->saturation;
 
-		if (DISPLAY_VER(dev_priv) != 2) {
-			attrs->gamma0 = intel_de_read(dev_priv, OGAMC0);
-			attrs->gamma1 = intel_de_read(dev_priv, OGAMC1);
-			attrs->gamma2 = intel_de_read(dev_priv, OGAMC2);
-			attrs->gamma3 = intel_de_read(dev_priv, OGAMC3);
-			attrs->gamma4 = intel_de_read(dev_priv, OGAMC4);
-			attrs->gamma5 = intel_de_read(dev_priv, OGAMC5);
+		if (DISPLAY_VER(display) != 2) {
+			attrs->gamma0 = intel_de_read(display, OGAMC0);
+			attrs->gamma1 = intel_de_read(display, OGAMC1);
+			attrs->gamma2 = intel_de_read(display, OGAMC2);
+			attrs->gamma3 = intel_de_read(display, OGAMC3);
+			attrs->gamma4 = intel_de_read(display, OGAMC4);
+			attrs->gamma5 = intel_de_read(display, OGAMC5);
 		}
 	} else {
 		if (attrs->brightness < -128 || attrs->brightness > 127)
@@ -1321,7 +1323,7 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 		update_reg_attrs(overlay, overlay->regs);
 
 		if (attrs->flags & I915_OVERLAY_UPDATE_GAMMA) {
-			if (DISPLAY_VER(dev_priv) == 2)
+			if (DISPLAY_VER(display) == 2)
 				goto out_unlock;
 
 			if (overlay->active) {
@@ -1333,12 +1335,12 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 			if (ret)
 				goto out_unlock;
 
-			intel_de_write(dev_priv, OGAMC0, attrs->gamma0);
-			intel_de_write(dev_priv, OGAMC1, attrs->gamma1);
-			intel_de_write(dev_priv, OGAMC2, attrs->gamma2);
-			intel_de_write(dev_priv, OGAMC3, attrs->gamma3);
-			intel_de_write(dev_priv, OGAMC4, attrs->gamma4);
-			intel_de_write(dev_priv, OGAMC5, attrs->gamma5);
+			intel_de_write(display, OGAMC0, attrs->gamma0);
+			intel_de_write(display, OGAMC1, attrs->gamma1);
+			intel_de_write(display, OGAMC2, attrs->gamma2);
+			intel_de_write(display, OGAMC3, attrs->gamma3);
+			intel_de_write(display, OGAMC4, attrs->gamma4);
+			intel_de_write(display, OGAMC5, attrs->gamma5);
 		}
 	}
 	overlay->color_key_enabled = (attrs->flags & I915_OVERLAY_DISABLE_DEST_COLORKEY) == 0;
@@ -1352,12 +1354,13 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 
 static int get_registers(struct intel_overlay *overlay, bool use_phys)
 {
-	struct drm_i915_private *i915 = overlay->i915;
+	struct intel_display *display = overlay->display;
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct drm_i915_gem_object *obj = ERR_PTR(-ENODEV);
 	struct i915_vma *vma;
 	int err;
 
-	if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */
+	if (!display->platform.meteorlake) /* Wa_22018444074 */
 		obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
 	if (IS_ERR(obj))
 		obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
@@ -1390,13 +1393,14 @@ static int get_registers(struct intel_overlay *overlay, bool use_phys)
 	return err;
 }
 
-void intel_overlay_setup(struct drm_i915_private *dev_priv)
+void intel_overlay_setup(struct intel_display *display)
 {
+	struct drm_i915_private *dev_priv = to_i915(display->drm);
 	struct intel_overlay *overlay;
 	struct intel_engine_cs *engine;
 	int ret;
 
-	if (!HAS_OVERLAY(dev_priv))
+	if (!HAS_OVERLAY(display))
 		return;
 
 	engine = to_gt(dev_priv)->engine[RCS0];
@@ -1407,7 +1411,7 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
 	if (!overlay)
 		return;
 
-	overlay->i915 = dev_priv;
+	overlay->display = display;
 	overlay->context = engine->kernel_context;
 	overlay->color_key = 0x0101fe;
 	overlay->color_key_enabled = true;
@@ -1418,7 +1422,7 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
 	i915_active_init(&overlay->last_flip,
 			 NULL, intel_overlay_last_flip_retire, 0);
 
-	ret = get_registers(overlay, OVERLAY_NEEDS_PHYSICAL(dev_priv));
+	ret = get_registers(overlay, OVERLAY_NEEDS_PHYSICAL(display));
 	if (ret)
 		goto out_free;
 
@@ -1426,19 +1430,19 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
 	update_polyphase_filter(overlay->regs);
 	update_reg_attrs(overlay, overlay->regs);
 
-	dev_priv->display.overlay = overlay;
-	drm_info(&dev_priv->drm, "Initialized overlay support.\n");
+	display->overlay = overlay;
+	drm_info(display->drm, "Initialized overlay support.\n");
 	return;
 
 out_free:
 	kfree(overlay);
 }
 
-void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
+void intel_overlay_cleanup(struct intel_display *display)
 {
 	struct intel_overlay *overlay;
 
-	overlay = fetch_and_zero(&dev_priv->display.overlay);
+	overlay = fetch_and_zero(&display->overlay);
 	if (!overlay)
 		return;
 
@@ -1447,7 +1451,7 @@ void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
 	 * Furthermore modesetting teardown happens beforehand so the
 	 * hardware should be off already.
 	 */
-	drm_WARN_ON(&dev_priv->drm, overlay->active);
+	drm_WARN_ON(display->drm, overlay->active);
 
 	i915_gem_object_put(overlay->reg_bo);
 	i915_active_fini(&overlay->last_flip);
@@ -1467,8 +1471,7 @@ struct intel_overlay_snapshot {
 struct intel_overlay_snapshot *
 intel_overlay_snapshot_capture(struct intel_display *display)
 {
-	struct drm_i915_private *dev_priv = to_i915(display->drm);
-	struct intel_overlay *overlay = dev_priv->display.overlay;
+	struct intel_overlay *overlay = display->overlay;
 	struct intel_overlay_snapshot *error;
 
 	if (!overlay || !overlay->active)
@@ -1478,8 +1481,8 @@ intel_overlay_snapshot_capture(struct intel_display *display)
 	if (error == NULL)
 		return NULL;
 
-	error->dovsta = intel_de_read(dev_priv, DOVSTA);
-	error->isr = intel_de_read(dev_priv, GEN2_ISR);
+	error->dovsta = intel_de_read(display, DOVSTA);
+	error->isr = intel_de_read(display, GEN2_ISR);
 	error->base = overlay->flip_addr;
 
 	memcpy_fromio(&error->regs, overlay->regs, sizeof(error->regs));
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.h b/drivers/gpu/drm/i915/display/intel_overlay.h
index eafac24d1de8..dc885edf39e6 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.h
+++ b/drivers/gpu/drm/i915/display/intel_overlay.h
@@ -17,19 +17,19 @@ struct intel_overlay;
 struct intel_overlay_snapshot;
 
 #ifdef I915
-void intel_overlay_setup(struct drm_i915_private *dev_priv);
-void intel_overlay_cleanup(struct drm_i915_private *dev_priv);
+void intel_overlay_setup(struct intel_display *display);
+void intel_overlay_cleanup(struct intel_display *display);
 int intel_overlay_switch_off(struct intel_overlay *overlay);
 int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file_priv);
 int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv);
-void intel_overlay_reset(struct drm_i915_private *dev_priv);
+void intel_overlay_reset(struct intel_display *display);
 #else
-static inline void intel_overlay_setup(struct drm_i915_private *dev_priv)
+static inline void intel_overlay_setup(struct intel_display *display)
 {
 }
-static inline void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
+static inline void intel_overlay_cleanup(struct intel_display *display)
 {
 }
 static inline int intel_overlay_switch_off(struct intel_overlay *overlay)
@@ -37,7 +37,7 @@ static inline int intel_overlay_switch_off(struct intel_overlay *overlay)
 	return 0;
 }
 static inline int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
-				  struct drm_file *file_priv)
+						struct drm_file *file_priv)
 {
 	return 0;
 }
@@ -46,7 +46,7 @@ static inline int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 {
 	return 0;
 }
-static inline void intel_overlay_reset(struct drm_i915_private *dev_priv)
+static inline void intel_overlay_reset(struct intel_display *display)
 {
 }
 #endif
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index f42f21632306..c2fe3fc78e76 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1198,6 +1198,7 @@ void intel_gt_reset(struct intel_gt *gt,
 		    intel_engine_mask_t stalled_mask,
 		    const char *reason)
 {
+	struct intel_display *display = &gt->i915->display;
 	intel_engine_mask_t awake;
 	int ret;
 
@@ -1243,7 +1244,7 @@ void intel_gt_reset(struct intel_gt *gt,
 	if (INTEL_INFO(gt->i915)->gpu_reset_clobbers_display)
 		intel_irq_resume(gt->i915);
 
-	intel_overlay_reset(gt->i915);
+	intel_overlay_reset(display);
 
 	/* sanitize uC after engine reset */
 	if (!intel_uc_uses_guc_submission(&gt->uc))
-- 
2.39.5

Re: [PATCH 1/5] drm/i915/overlay: convert to struct intel_display

[Rodrigo Vivi]

On Mon, Nov 11, 2024 at 07:53:30PM +0200, Jani Nikula wrote:
> struct intel_display replaces struct drm_i915_private as the main
> display device pointer. Convert overlay to it, as much as possible.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../drm/i915/display/intel_display_driver.c   |   4 +-
>  drivers/gpu/drm/i915/display/intel_overlay.c  | 179 +++++++++---------
>  drivers/gpu/drm/i915/display/intel_overlay.h  |  14 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
>  4 files changed, 102 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 56b78cf6b854..5983570b510f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -518,7 +518,7 @@ int intel_display_driver_probe(struct drm_i915_private *i915)
>  	if (ret)
>  		drm_dbg_kms(&i915->drm, "Initial modeset failed, %d\n", ret);
>  
> -	intel_overlay_setup(i915);
> +	intel_overlay_setup(display);
>  
>  	/* Only enable hotplug handling once the fbdev is fully set up. */
>  	intel_hpd_init(i915);
> @@ -607,7 +607,7 @@ void intel_display_driver_remove_noirq(struct drm_i915_private *i915)
>  
>  	intel_dp_tunnel_mgr_cleanup(display);
>  
> -	intel_overlay_cleanup(i915);
> +	intel_overlay_cleanup(display);
>  
>  	intel_gmbus_teardown(display);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
> index 2ec14096ba9c..57eaf81651c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
> @@ -183,7 +183,7 @@ struct overlay_registers {
>  };
>  
>  struct intel_overlay {
> -	struct drm_i915_private *i915;
> +	struct intel_display *display;
>  	struct intel_context *context;
>  	struct intel_crtc *crtc;
>  	struct i915_vma *vma;
> @@ -205,17 +205,17 @@ struct intel_overlay {
>  	void (*flip_complete)(struct intel_overlay *ovl);
>  };
>  
> -static void i830_overlay_clock_gating(struct drm_i915_private *dev_priv,
> +static void i830_overlay_clock_gating(struct intel_display *display,
>  				      bool enable)
>  {
> -	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> +	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
>  	u8 val;
>  
>  	/* WA_OVERLAY_CLKGATE:alm */
>  	if (enable)
> -		intel_de_write(dev_priv, DSPCLK_GATE_D(dev_priv), 0);
> +		intel_de_write(display, DSPCLK_GATE_D(display), 0);
>  	else
> -		intel_de_write(dev_priv, DSPCLK_GATE_D(dev_priv),
> +		intel_de_write(display, DSPCLK_GATE_D(display),
>  			       OVRUNIT_CLOCK_GATE_DISABLE);
>  
>  	/* WA_DISABLE_L2CACHE_CLOCK_GATING:alm */
> @@ -253,11 +253,11 @@ alloc_request(struct intel_overlay *overlay, void (*fn)(struct intel_overlay *))
>  /* overlay needs to be disable in OCMD reg */
>  static int intel_overlay_on(struct intel_overlay *overlay)
>  {
> -	struct drm_i915_private *dev_priv = overlay->i915;
> +	struct intel_display *display = overlay->display;
>  	struct i915_request *rq;
>  	u32 *cs;
>  
> -	drm_WARN_ON(&dev_priv->drm, overlay->active);
> +	drm_WARN_ON(display->drm, overlay->active);
>  
>  	rq = alloc_request(overlay, NULL);
>  	if (IS_ERR(rq))
> @@ -271,8 +271,8 @@ static int intel_overlay_on(struct intel_overlay *overlay)
>  
>  	overlay->active = true;
>  
> -	if (IS_I830(dev_priv))
> -		i830_overlay_clock_gating(dev_priv, false);
> +	if (display->platform.i830)
> +		i830_overlay_clock_gating(display, false);
>  
>  	*cs++ = MI_OVERLAY_FLIP | MI_OVERLAY_ON;
>  	*cs++ = overlay->flip_addr | OFC_UPDATE;
> @@ -288,10 +288,12 @@ static int intel_overlay_on(struct intel_overlay *overlay)
>  static void intel_overlay_flip_prepare(struct intel_overlay *overlay,
>  				       struct i915_vma *vma)
>  {
> +	struct intel_display *display = overlay->display;
> +	struct drm_i915_private *i915 = to_i915(display->drm);
>  	enum pipe pipe = overlay->crtc->pipe;
>  	struct intel_frontbuffer *frontbuffer = NULL;
>  
> -	drm_WARN_ON(&overlay->i915->drm, overlay->old_vma);
> +	drm_WARN_ON(display->drm, overlay->old_vma);
>  
>  	if (vma)
>  		frontbuffer = intel_frontbuffer_get(intel_bo_to_drm_bo(vma->obj));
> @@ -303,8 +305,7 @@ static void intel_overlay_flip_prepare(struct intel_overlay *overlay,
>  		intel_frontbuffer_put(overlay->frontbuffer);
>  	overlay->frontbuffer = frontbuffer;
>  
> -	intel_frontbuffer_flip_prepare(overlay->i915,
> -				       INTEL_FRONTBUFFER_OVERLAY(pipe));
> +	intel_frontbuffer_flip_prepare(i915, INTEL_FRONTBUFFER_OVERLAY(pipe));
>  
>  	overlay->old_vma = overlay->vma;
>  	if (vma)
> @@ -318,20 +319,20 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
>  				  struct i915_vma *vma,
>  				  bool load_polyphase_filter)
>  {
> -	struct drm_i915_private *dev_priv = overlay->i915;
> +	struct intel_display *display = overlay->display;
>  	struct i915_request *rq;
>  	u32 flip_addr = overlay->flip_addr;
>  	u32 tmp, *cs;
>  
> -	drm_WARN_ON(&dev_priv->drm, !overlay->active);
> +	drm_WARN_ON(display->drm, !overlay->active);
>  
>  	if (load_polyphase_filter)
>  		flip_addr |= OFC_UPDATE;
>  
>  	/* check for underruns */
> -	tmp = intel_de_read(dev_priv, DOVSTA);
> +	tmp = intel_de_read(display, DOVSTA);
>  	if (tmp & (1 << 17))
> -		drm_dbg(&dev_priv->drm, "overlay underrun, DOVSTA: %x\n", tmp);
> +		drm_dbg(display->drm, "overlay underrun, DOVSTA: %x\n", tmp);
>  
>  	rq = alloc_request(overlay, NULL);
>  	if (IS_ERR(rq))
> @@ -355,14 +356,15 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
>  
>  static void intel_overlay_release_old_vma(struct intel_overlay *overlay)
>  {
> +	struct intel_display *display = overlay->display;
> +	struct drm_i915_private *i915 = to_i915(display->drm);
>  	struct i915_vma *vma;
>  
>  	vma = fetch_and_zero(&overlay->old_vma);
> -	if (drm_WARN_ON(&overlay->i915->drm, !vma))
> +	if (drm_WARN_ON(display->drm, !vma))
>  		return;
>  
> -	intel_frontbuffer_flip_complete(overlay->i915,
> -					INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
> +	intel_frontbuffer_flip_complete(i915, INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
>  
>  	i915_vma_unpin(vma);
>  	i915_vma_put(vma);
> @@ -376,7 +378,7 @@ intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
>  
>  static void intel_overlay_off_tail(struct intel_overlay *overlay)
>  {
> -	struct drm_i915_private *dev_priv = overlay->i915;
> +	struct intel_display *display = overlay->display;
>  
>  	intel_overlay_release_old_vma(overlay);
>  
> @@ -384,8 +386,8 @@ static void intel_overlay_off_tail(struct intel_overlay *overlay)
>  	overlay->crtc = NULL;
>  	overlay->active = false;
>  
> -	if (IS_I830(dev_priv))
> -		i830_overlay_clock_gating(dev_priv, true);
> +	if (display->platform.i830)
> +		i830_overlay_clock_gating(display, true);
>  }
>  
>  static void intel_overlay_last_flip_retire(struct i915_active *active)
> @@ -400,10 +402,11 @@ static void intel_overlay_last_flip_retire(struct i915_active *active)
>  /* overlay needs to be disabled in OCMD reg */
>  static int intel_overlay_off(struct intel_overlay *overlay)
>  {
> +	struct intel_display *display = overlay->display;
>  	struct i915_request *rq;
>  	u32 *cs, flip_addr = overlay->flip_addr;
>  
> -	drm_WARN_ON(&overlay->i915->drm, !overlay->active);
> +	drm_WARN_ON(display->drm, !overlay->active);
>  
>  	/* According to intel docs the overlay hw may hang (when switching
>  	 * off) without loading the filter coeffs. It is however unclear whether
> @@ -452,7 +455,7 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
>   */
>  static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
>  {
> -	struct drm_i915_private *dev_priv = overlay->i915;
> +	struct intel_display *display = overlay->display;
>  	struct i915_request *rq;
>  	u32 *cs;
>  
> @@ -463,7 +466,7 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
>  	if (!overlay->old_vma)
>  		return 0;
>  
> -	if (!(intel_de_read(dev_priv, GEN2_ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT)) {
> +	if (!(intel_de_read(display, GEN2_ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT)) {
>  		intel_overlay_release_old_vid_tail(overlay);
>  		return 0;
>  	}
> @@ -487,9 +490,9 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
>  	return i915_active_wait(&overlay->last_flip);
>  }
>  
> -void intel_overlay_reset(struct drm_i915_private *dev_priv)
> +void intel_overlay_reset(struct intel_display *display)
>  {
> -	struct intel_overlay *overlay = dev_priv->display.overlay;
> +	struct intel_overlay *overlay = display->overlay;
>  
>  	if (!overlay)
>  		return;
> @@ -550,11 +553,11 @@ static int uv_vsubsampling(u32 format)
>  	}
>  }
>  
> -static u32 calc_swidthsw(struct drm_i915_private *dev_priv, u32 offset, u32 width)
> +static u32 calc_swidthsw(struct intel_display *display, u32 offset, u32 width)
>  {
>  	u32 sw;
>  
> -	if (DISPLAY_VER(dev_priv) == 2)
> +	if (DISPLAY_VER(display) == 2)
>  		sw = ALIGN((offset & 31) + width, 32);
>  	else
>  		sw = ALIGN((offset & 63) + width, 64);
> @@ -789,16 +792,17 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
>  				      struct drm_i915_gem_object *new_bo,
>  				      struct drm_intel_overlay_put_image *params)
>  {
> +	struct intel_display *display = overlay->display;
> +	struct drm_i915_private *dev_priv = to_i915(display->drm);
>  	struct overlay_registers __iomem *regs = overlay->regs;
> -	struct drm_i915_private *dev_priv = overlay->i915;
>  	u32 swidth, swidthsw, sheight, ostride;
>  	enum pipe pipe = overlay->crtc->pipe;
>  	bool scale_changed = false;
>  	struct i915_vma *vma;
>  	int ret, tmp_width;
>  
> -	drm_WARN_ON(&dev_priv->drm,
> -		    !drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
> +	drm_WARN_ON(display->drm,
> +		    !drm_modeset_is_locked(&display->drm->mode_config.connection_mutex));
>  
>  	ret = intel_overlay_release_old_vid(overlay);
>  	if (ret != 0)
> @@ -824,7 +828,7 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
>  			oconfig |= OCONF_CC_OUT_8BIT;
>  		if (crtc_state->gamma_enable)
>  			oconfig |= OCONF_GAMMA2_ENABLE;
> -		if (DISPLAY_VER(dev_priv) == 4)
> +		if (DISPLAY_VER(display) == 4)
>  			oconfig |= OCONF_CSC_MODE_BT709;
>  		oconfig |= pipe == 0 ?
>  			OCONF_PIPE_A : OCONF_PIPE_B;
> @@ -845,7 +849,7 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
>  		tmp_width = params->src_width;
>  
>  	swidth = params->src_width;
> -	swidthsw = calc_swidthsw(dev_priv, params->offset_Y, tmp_width);
> +	swidthsw = calc_swidthsw(display, params->offset_Y, tmp_width);
>  	sheight = params->src_height;
>  	iowrite32(i915_ggtt_offset(vma) + params->offset_Y, &regs->OBUF_0Y);
>  	ostride = params->stride_Y;
> @@ -858,9 +862,9 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
>  		swidth |= (params->src_width / uv_hscale) << 16;
>  		sheight |= (params->src_height / uv_vscale) << 16;
>  
> -		tmp_U = calc_swidthsw(dev_priv, params->offset_U,
> +		tmp_U = calc_swidthsw(display, params->offset_U,
>  				      params->src_width / uv_hscale);
> -		tmp_V = calc_swidthsw(dev_priv, params->offset_V,
> +		tmp_V = calc_swidthsw(display, params->offset_V,
>  				      params->src_width / uv_hscale);
>  		swidthsw |= max(tmp_U, tmp_V) << 16;
>  
> @@ -899,11 +903,11 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
>  
>  int intel_overlay_switch_off(struct intel_overlay *overlay)
>  {
> -	struct drm_i915_private *dev_priv = overlay->i915;
> +	struct intel_display *display = overlay->display;
>  	int ret;
>  
> -	drm_WARN_ON(&dev_priv->drm,
> -		    !drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
> +	drm_WARN_ON(display->drm,
> +		    !drm_modeset_is_locked(&display->drm->mode_config.connection_mutex));
>  
>  	ret = intel_overlay_recover_from_interrupt(overlay);
>  	if (ret != 0)
> @@ -936,26 +940,24 @@ static int check_overlay_possible_on_crtc(struct intel_overlay *overlay,
>  
>  static void update_pfit_vscale_ratio(struct intel_overlay *overlay)
>  {
> -	struct drm_i915_private *dev_priv = overlay->i915;
> +	struct intel_display *display = overlay->display;
>  	u32 ratio;
>  
>  	/* XXX: This is not the same logic as in the xorg driver, but more in
>  	 * line with the intel documentation for the i965
>  	 */
> -	if (DISPLAY_VER(dev_priv) >= 4) {
> -		u32 tmp = intel_de_read(dev_priv, PFIT_PGM_RATIOS(dev_priv));
> +	if (DISPLAY_VER(display) >= 4) {
> +		u32 tmp = intel_de_read(display, PFIT_PGM_RATIOS(display));
>  
>  		/* on i965 use the PGM reg to read out the autoscaler values */
>  		ratio = REG_FIELD_GET(PFIT_VERT_SCALE_MASK_965, tmp);
>  	} else {
>  		u32 tmp;
>  
> -		if (intel_de_read(dev_priv, PFIT_CONTROL(dev_priv)) & PFIT_VERT_AUTO_SCALE)
> -			tmp = intel_de_read(dev_priv,
> -					    PFIT_AUTO_RATIOS(dev_priv));
> +		if (intel_de_read(display, PFIT_CONTROL(display)) & PFIT_VERT_AUTO_SCALE)
> +			tmp = intel_de_read(display, PFIT_AUTO_RATIOS(display));
>  		else
> -			tmp = intel_de_read(dev_priv,
> -					    PFIT_PGM_RATIOS(dev_priv));
> +			tmp = intel_de_read(display, PFIT_PGM_RATIOS(display));
>  
>  		ratio = REG_FIELD_GET(PFIT_VERT_SCALE_MASK, tmp);
>  	}
> @@ -1000,7 +1002,7 @@ static int check_overlay_scaling(struct drm_intel_overlay_put_image *rec)
>  	return 0;
>  }
>  
> -static int check_overlay_src(struct drm_i915_private *dev_priv,
> +static int check_overlay_src(struct intel_display *display,
>  			     struct drm_intel_overlay_put_image *rec,
>  			     struct drm_i915_gem_object *new_bo)
>  {
> @@ -1011,7 +1013,7 @@ static int check_overlay_src(struct drm_i915_private *dev_priv,
>  	u32 tmp;
>  
>  	/* check src dimensions */
> -	if (IS_I845G(dev_priv) || IS_I830(dev_priv)) {
> +	if (display->platform.i845g || display->platform.i830) {
>  		if (rec->src_height > IMAGE_MAX_HEIGHT_LEGACY ||
>  		    rec->src_width  > IMAGE_MAX_WIDTH_LEGACY)
>  			return -EINVAL;
> @@ -1063,14 +1065,14 @@ static int check_overlay_src(struct drm_i915_private *dev_priv,
>  		return -EINVAL;
>  
>  	/* stride checking */
> -	if (IS_I830(dev_priv) || IS_I845G(dev_priv))
> +	if (display->platform.i830 || display->platform.i845g)
>  		stride_mask = 255;
>  	else
>  		stride_mask = 63;
>  
>  	if (rec->stride_Y & stride_mask || rec->stride_UV & stride_mask)
>  		return -EINVAL;
> -	if (DISPLAY_VER(dev_priv) == 4 && rec->stride_Y < 512)
> +	if (DISPLAY_VER(display) == 4 && rec->stride_Y < 512)
>  		return -EINVAL;
>  
>  	tmp = (rec->flags & I915_OVERLAY_TYPE_MASK) == I915_OVERLAY_YUV_PLANAR ?
> @@ -1114,17 +1116,17 @@ static int check_overlay_src(struct drm_i915_private *dev_priv,
>  int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  				  struct drm_file *file_priv)
>  {
> +	struct intel_display *display = to_intel_display(dev);
>  	struct drm_intel_overlay_put_image *params = data;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_overlay *overlay;
>  	struct drm_crtc *drmmode_crtc;
>  	struct intel_crtc *crtc;
>  	struct drm_i915_gem_object *new_bo;
>  	int ret;
>  
> -	overlay = dev_priv->display.overlay;
> +	overlay = display->overlay;
>  	if (!overlay) {
> -		drm_dbg(&dev_priv->drm, "userspace bug: no overlay\n");
> +		drm_dbg(display->drm, "userspace bug: no overlay\n");
>  		return -ENODEV;
>  	}
>  
> @@ -1148,7 +1150,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  	drm_modeset_lock_all(dev);
>  
>  	if (i915_gem_object_is_tiled(new_bo)) {
> -		drm_dbg_kms(&dev_priv->drm,
> +		drm_dbg_kms(display->drm,
>  			    "buffer used for overlay image can not be tiled\n");
>  		ret = -EINVAL;
>  		goto out_unlock;
> @@ -1197,7 +1199,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  		goto out_unlock;
>  	}
>  
> -	ret = check_overlay_src(dev_priv, params, new_bo);
> +	ret = check_overlay_src(display, params, new_bo);
>  	if (ret != 0)
>  		goto out_unlock;
>  
> @@ -1277,14 +1279,14 @@ static int check_gamma(struct drm_intel_overlay_attrs *attrs)
>  int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  			      struct drm_file *file_priv)
>  {
> +	struct intel_display *display = to_intel_display(dev);
>  	struct drm_intel_overlay_attrs *attrs = data;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_overlay *overlay;
>  	int ret;
>  
> -	overlay = dev_priv->display.overlay;
> +	overlay = display->overlay;
>  	if (!overlay) {
> -		drm_dbg(&dev_priv->drm, "userspace bug: no overlay\n");
> +		drm_dbg(display->drm, "userspace bug: no overlay\n");
>  		return -ENODEV;
>  	}
>  
> @@ -1297,13 +1299,13 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  		attrs->contrast   = overlay->contrast;
>  		attrs->saturation = overlay->saturation;
>  
> -		if (DISPLAY_VER(dev_priv) != 2) {
> -			attrs->gamma0 = intel_de_read(dev_priv, OGAMC0);
> -			attrs->gamma1 = intel_de_read(dev_priv, OGAMC1);
> -			attrs->gamma2 = intel_de_read(dev_priv, OGAMC2);
> -			attrs->gamma3 = intel_de_read(dev_priv, OGAMC3);
> -			attrs->gamma4 = intel_de_read(dev_priv, OGAMC4);
> -			attrs->gamma5 = intel_de_read(dev_priv, OGAMC5);
> +		if (DISPLAY_VER(display) != 2) {
> +			attrs->gamma0 = intel_de_read(display, OGAMC0);
> +			attrs->gamma1 = intel_de_read(display, OGAMC1);
> +			attrs->gamma2 = intel_de_read(display, OGAMC2);
> +			attrs->gamma3 = intel_de_read(display, OGAMC3);
> +			attrs->gamma4 = intel_de_read(display, OGAMC4);
> +			attrs->gamma5 = intel_de_read(display, OGAMC5);
>  		}
>  	} else {
>  		if (attrs->brightness < -128 || attrs->brightness > 127)
> @@ -1321,7 +1323,7 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  		update_reg_attrs(overlay, overlay->regs);
>  
>  		if (attrs->flags & I915_OVERLAY_UPDATE_GAMMA) {
> -			if (DISPLAY_VER(dev_priv) == 2)
> +			if (DISPLAY_VER(display) == 2)
>  				goto out_unlock;
>  
>  			if (overlay->active) {
> @@ -1333,12 +1335,12 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  			if (ret)
>  				goto out_unlock;
>  
> -			intel_de_write(dev_priv, OGAMC0, attrs->gamma0);
> -			intel_de_write(dev_priv, OGAMC1, attrs->gamma1);
> -			intel_de_write(dev_priv, OGAMC2, attrs->gamma2);
> -			intel_de_write(dev_priv, OGAMC3, attrs->gamma3);
> -			intel_de_write(dev_priv, OGAMC4, attrs->gamma4);
> -			intel_de_write(dev_priv, OGAMC5, attrs->gamma5);
> +			intel_de_write(display, OGAMC0, attrs->gamma0);
> +			intel_de_write(display, OGAMC1, attrs->gamma1);
> +			intel_de_write(display, OGAMC2, attrs->gamma2);
> +			intel_de_write(display, OGAMC3, attrs->gamma3);
> +			intel_de_write(display, OGAMC4, attrs->gamma4);
> +			intel_de_write(display, OGAMC5, attrs->gamma5);
>  		}
>  	}
>  	overlay->color_key_enabled = (attrs->flags & I915_OVERLAY_DISABLE_DEST_COLORKEY) == 0;
> @@ -1352,12 +1354,13 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  
>  static int get_registers(struct intel_overlay *overlay, bool use_phys)
>  {
> -	struct drm_i915_private *i915 = overlay->i915;
> +	struct intel_display *display = overlay->display;
> +	struct drm_i915_private *i915 = to_i915(display->drm);
>  	struct drm_i915_gem_object *obj = ERR_PTR(-ENODEV);
>  	struct i915_vma *vma;
>  	int err;
>  
> -	if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */
> +	if (!display->platform.meteorlake) /* Wa_22018444074 */
>  		obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
>  	if (IS_ERR(obj))
>  		obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
> @@ -1390,13 +1393,14 @@ static int get_registers(struct intel_overlay *overlay, bool use_phys)
>  	return err;
>  }
>  
> -void intel_overlay_setup(struct drm_i915_private *dev_priv)
> +void intel_overlay_setup(struct intel_display *display)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(display->drm);
>  	struct intel_overlay *overlay;
>  	struct intel_engine_cs *engine;
>  	int ret;
>  
> -	if (!HAS_OVERLAY(dev_priv))
> +	if (!HAS_OVERLAY(display))
>  		return;
>  
>  	engine = to_gt(dev_priv)->engine[RCS0];
> @@ -1407,7 +1411,7 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
>  	if (!overlay)
>  		return;
>  
> -	overlay->i915 = dev_priv;
> +	overlay->display = display;
>  	overlay->context = engine->kernel_context;
>  	overlay->color_key = 0x0101fe;
>  	overlay->color_key_enabled = true;
> @@ -1418,7 +1422,7 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
>  	i915_active_init(&overlay->last_flip,
>  			 NULL, intel_overlay_last_flip_retire, 0);
>  
> -	ret = get_registers(overlay, OVERLAY_NEEDS_PHYSICAL(dev_priv));
> +	ret = get_registers(overlay, OVERLAY_NEEDS_PHYSICAL(display));

I thought that you were converting the macros separately...
but if this is working, no hard feelings

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

>  	if (ret)
>  		goto out_free;
>  
> @@ -1426,19 +1430,19 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
>  	update_polyphase_filter(overlay->regs);
>  	update_reg_attrs(overlay, overlay->regs);
>  
> -	dev_priv->display.overlay = overlay;
> -	drm_info(&dev_priv->drm, "Initialized overlay support.\n");
> +	display->overlay = overlay;
> +	drm_info(display->drm, "Initialized overlay support.\n");
>  	return;
>  
>  out_free:
>  	kfree(overlay);
>  }
>  
> -void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
> +void intel_overlay_cleanup(struct intel_display *display)
>  {
>  	struct intel_overlay *overlay;
>  
> -	overlay = fetch_and_zero(&dev_priv->display.overlay);
> +	overlay = fetch_and_zero(&display->overlay);
>  	if (!overlay)
>  		return;
>  
> @@ -1447,7 +1451,7 @@ void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
>  	 * Furthermore modesetting teardown happens beforehand so the
>  	 * hardware should be off already.
>  	 */
> -	drm_WARN_ON(&dev_priv->drm, overlay->active);
> +	drm_WARN_ON(display->drm, overlay->active);
>  
>  	i915_gem_object_put(overlay->reg_bo);
>  	i915_active_fini(&overlay->last_flip);
> @@ -1467,8 +1471,7 @@ struct intel_overlay_snapshot {
>  struct intel_overlay_snapshot *
>  intel_overlay_snapshot_capture(struct intel_display *display)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(display->drm);
> -	struct intel_overlay *overlay = dev_priv->display.overlay;
> +	struct intel_overlay *overlay = display->overlay;
>  	struct intel_overlay_snapshot *error;
>  
>  	if (!overlay || !overlay->active)
> @@ -1478,8 +1481,8 @@ intel_overlay_snapshot_capture(struct intel_display *display)
>  	if (error == NULL)
>  		return NULL;
>  
> -	error->dovsta = intel_de_read(dev_priv, DOVSTA);
> -	error->isr = intel_de_read(dev_priv, GEN2_ISR);
> +	error->dovsta = intel_de_read(display, DOVSTA);
> +	error->isr = intel_de_read(display, GEN2_ISR);
>  	error->base = overlay->flip_addr;
>  
>  	memcpy_fromio(&error->regs, overlay->regs, sizeof(error->regs));
> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.h b/drivers/gpu/drm/i915/display/intel_overlay.h
> index eafac24d1de8..dc885edf39e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_overlay.h
> +++ b/drivers/gpu/drm/i915/display/intel_overlay.h
> @@ -17,19 +17,19 @@ struct intel_overlay;
>  struct intel_overlay_snapshot;
>  
>  #ifdef I915
> -void intel_overlay_setup(struct drm_i915_private *dev_priv);
> -void intel_overlay_cleanup(struct drm_i915_private *dev_priv);
> +void intel_overlay_setup(struct intel_display *display);
> +void intel_overlay_cleanup(struct intel_display *display);
>  int intel_overlay_switch_off(struct intel_overlay *overlay);
>  int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  				  struct drm_file *file_priv);
>  int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  			      struct drm_file *file_priv);
> -void intel_overlay_reset(struct drm_i915_private *dev_priv);
> +void intel_overlay_reset(struct intel_display *display);
>  #else
> -static inline void intel_overlay_setup(struct drm_i915_private *dev_priv)
> +static inline void intel_overlay_setup(struct intel_display *display)
>  {
>  }
> -static inline void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
> +static inline void intel_overlay_cleanup(struct intel_display *display)
>  {
>  }
>  static inline int intel_overlay_switch_off(struct intel_overlay *overlay)
> @@ -37,7 +37,7 @@ static inline int intel_overlay_switch_off(struct intel_overlay *overlay)
>  	return 0;
>  }
>  static inline int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
> -				  struct drm_file *file_priv)
> +						struct drm_file *file_priv)
>  {
>  	return 0;
>  }
> @@ -46,7 +46,7 @@ static inline int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  {
>  	return 0;
>  }
> -static inline void intel_overlay_reset(struct drm_i915_private *dev_priv)
> +static inline void intel_overlay_reset(struct intel_display *display)
>  {
>  }
>  #endif
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index f42f21632306..c2fe3fc78e76 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -1198,6 +1198,7 @@ void intel_gt_reset(struct intel_gt *gt,
>  		    intel_engine_mask_t stalled_mask,
>  		    const char *reason)
>  {
> +	struct intel_display *display = &gt->i915->display;
>  	intel_engine_mask_t awake;
>  	int ret;
>  
> @@ -1243,7 +1244,7 @@ void intel_gt_reset(struct intel_gt *gt,
>  	if (INTEL_INFO(gt->i915)->gpu_reset_clobbers_display)
>  		intel_irq_resume(gt->i915);
>  
> -	intel_overlay_reset(gt->i915);
> +	intel_overlay_reset(display);
>  
>  	/* sanitize uC after engine reset */
>  	if (!intel_uc_uses_guc_submission(&gt->uc))
> -- 
> 2.39.5
> 

Re: [PATCH 1/5] drm/i915/overlay: convert to struct intel_display

[Jani Nikula]

On Tue, 12 Nov 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Mon, Nov 11, 2024 at 07:53:30PM +0200, Jani Nikula wrote:
>> @@ -1418,7 +1422,7 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
>>  	i915_active_init(&overlay->last_flip,
>>  			 NULL, intel_overlay_last_flip_retire, 0);
>>  
>> -	ret = get_registers(overlay, OVERLAY_NEEDS_PHYSICAL(dev_priv));
>> +	ret = get_registers(overlay, OVERLAY_NEEDS_PHYSICAL(display));
>
> I thought that you were converting the macros separately...
> but if this is working, no hard feelings

I only converted the ones that actually needed dev_priv, and couldn't
use generics. They now accept display only. This one uses generics and
can take either.

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thanks,
Jani.


-- 
Jani Nikula, Intel

[PATCH 2/5] drm/i915/overlay: add intel_overlay_available() and use it

[Jani Nikula]

Avoid accessing struct intel_display members directly from
i915_getparam_ioctl(). Add intel_overlay_available() function to provide
the information for I915_PARAM_HAS_OVERLAY.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_overlay.c | 5 +++++
 drivers/gpu/drm/i915/display/intel_overlay.h | 5 +++++
 drivers/gpu/drm/i915/i915_getparam.c         | 5 +++--
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index 57eaf81651c4..ca30fff61876 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -1438,6 +1438,11 @@ void intel_overlay_setup(struct intel_display *display)
 	kfree(overlay);
 }
 
+bool intel_overlay_available(struct intel_display *display)
+{
+	return display->overlay;
+}
+
 void intel_overlay_cleanup(struct intel_display *display)
 {
 	struct intel_overlay *overlay;
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.h b/drivers/gpu/drm/i915/display/intel_overlay.h
index dc885edf39e6..45a42fce754e 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.h
+++ b/drivers/gpu/drm/i915/display/intel_overlay.h
@@ -18,6 +18,7 @@ struct intel_overlay_snapshot;
 
 #ifdef I915
 void intel_overlay_setup(struct intel_display *display);
+bool intel_overlay_available(struct intel_display *display);
 void intel_overlay_cleanup(struct intel_display *display);
 int intel_overlay_switch_off(struct intel_overlay *overlay);
 int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
@@ -29,6 +30,10 @@ void intel_overlay_reset(struct intel_display *display);
 static inline void intel_overlay_setup(struct intel_display *display)
 {
 }
+static inline bool intel_overlay_available(struct intel_display *display)
+{
+	return false;
+}
 static inline void intel_overlay_cleanup(struct intel_display *display)
 {
 }
diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
index a62405787e77..be8149e46281 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -2,9 +2,9 @@
  * SPDX-License-Identifier: MIT
  */
 
+#include "display/intel_overlay.h"
 #include "gem/i915_gem_mman.h"
 #include "gt/intel_engine_user.h"
-
 #include "pxp/intel_pxp.h"
 
 #include "i915_cmd_parser.h"
@@ -16,6 +16,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file_priv)
 {
 	struct drm_i915_private *i915 = to_i915(dev);
+	struct intel_display *display = &i915->display;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	const struct sseu_dev_info *sseu = &to_gt(i915)->info.sseu;
 	drm_i915_getparam_t *param = data;
@@ -38,7 +39,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
 		value = to_gt(i915)->ggtt->num_fences;
 		break;
 	case I915_PARAM_HAS_OVERLAY:
-		value = !!i915->display.overlay;
+		value = intel_overlay_available(display);
 		break;
 	case I915_PARAM_HAS_BSD:
 		value = !!intel_engine_lookup_user(i915,
-- 
2.39.5

Re: [PATCH 2/5] drm/i915/overlay: add intel_overlay_available() and use it

[Rodrigo Vivi]

On Mon, Nov 11, 2024 at 07:53:31PM +0200, Jani Nikula wrote:
> Avoid accessing struct intel_display members directly from
> i915_getparam_ioctl(). Add intel_overlay_available() function to provide
> the information for I915_PARAM_HAS_OVERLAY.
> 

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_overlay.c | 5 +++++
>  drivers/gpu/drm/i915/display/intel_overlay.h | 5 +++++
>  drivers/gpu/drm/i915/i915_getparam.c         | 5 +++--
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
> index 57eaf81651c4..ca30fff61876 100644
> --- a/drivers/gpu/drm/i915/display/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
> @@ -1438,6 +1438,11 @@ void intel_overlay_setup(struct intel_display *display)
>  	kfree(overlay);
>  }
>  
> +bool intel_overlay_available(struct intel_display *display)
> +{
> +	return display->overlay;
> +}
> +
>  void intel_overlay_cleanup(struct intel_display *display)
>  {
>  	struct intel_overlay *overlay;
> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.h b/drivers/gpu/drm/i915/display/intel_overlay.h
> index dc885edf39e6..45a42fce754e 100644
> --- a/drivers/gpu/drm/i915/display/intel_overlay.h
> +++ b/drivers/gpu/drm/i915/display/intel_overlay.h
> @@ -18,6 +18,7 @@ struct intel_overlay_snapshot;
>  
>  #ifdef I915
>  void intel_overlay_setup(struct intel_display *display);
> +bool intel_overlay_available(struct intel_display *display);
>  void intel_overlay_cleanup(struct intel_display *display);
>  int intel_overlay_switch_off(struct intel_overlay *overlay);
>  int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
> @@ -29,6 +30,10 @@ void intel_overlay_reset(struct intel_display *display);
>  static inline void intel_overlay_setup(struct intel_display *display)
>  {
>  }
> +static inline bool intel_overlay_available(struct intel_display *display)
> +{
> +	return false;
> +}
>  static inline void intel_overlay_cleanup(struct intel_display *display)
>  {
>  }
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> index a62405787e77..be8149e46281 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -2,9 +2,9 @@
>   * SPDX-License-Identifier: MIT
>   */
>  
> +#include "display/intel_overlay.h"
>  #include "gem/i915_gem_mman.h"
>  #include "gt/intel_engine_user.h"
> -
>  #include "pxp/intel_pxp.h"
>  
>  #include "i915_cmd_parser.h"
> @@ -16,6 +16,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
>  			struct drm_file *file_priv)
>  {
>  	struct drm_i915_private *i915 = to_i915(dev);
> +	struct intel_display *display = &i915->display;
>  	struct pci_dev *pdev = to_pci_dev(dev->dev);
>  	const struct sseu_dev_info *sseu = &to_gt(i915)->info.sseu;
>  	drm_i915_getparam_t *param = data;
> @@ -38,7 +39,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
>  		value = to_gt(i915)->ggtt->num_fences;
>  		break;
>  	case I915_PARAM_HAS_OVERLAY:
> -		value = !!i915->display.overlay;
> +		value = intel_overlay_available(display);
>  		break;
>  	case I915_PARAM_HAS_BSD:
>  		value = !!intel_engine_lookup_user(i915,
> -- 
> 2.39.5
> 

[PATCH 3/5] drm/i915/plane: convert initial plane setup to struct intel_display

[Jani Nikula]

struct intel_display replaces struct drm_i915_private as the main
display device pointer. Convert initial plane setup to it, as much as
possible.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_display_driver.c   |  2 +-
 .../drm/i915/display/intel_plane_initial.c    | 56 ++++++++++---------
 .../drm/i915/display/intel_plane_initial.h    |  4 +-
 drivers/gpu/drm/xe/display/xe_plane_initial.c |  8 +--
 4 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 5983570b510f..8daf48d2ba7d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -472,7 +472,7 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915)
 	intel_acpi_assign_connector_fwnodes(display);
 	drm_modeset_unlock_all(dev);
 
-	intel_initial_plane_config(i915);
+	intel_initial_plane_config(display);
 
 	/*
 	 * Make sure hardware watermarks really match the state we read out.
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index 62401f6a04e4..6789b7f14095 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -20,10 +20,10 @@ intel_reuse_initial_plane_obj(struct intel_crtc *this,
 			      struct drm_framebuffer **fb,
 			      struct i915_vma **vma)
 {
-	struct drm_i915_private *i915 = to_i915(this->base.dev);
+	struct intel_display *display = to_intel_display(this);
 	struct intel_crtc *crtc;
 
-	for_each_intel_crtc(&i915->drm, crtc) {
+	for_each_intel_crtc(display->drm, crtc) {
 		struct intel_plane *plane =
 			to_intel_plane(crtc->base.primary);
 		const struct intel_plane_state *plane_state =
@@ -48,9 +48,10 @@ intel_reuse_initial_plane_obj(struct intel_crtc *this,
 }
 
 static bool
-initial_plane_phys_lmem(struct drm_i915_private *i915,
+initial_plane_phys_lmem(struct intel_display *display,
 			struct intel_initial_plane_config *plane_config)
 {
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
 	struct intel_memory_region *mem;
 	dma_addr_t dma_addr;
@@ -63,7 +64,7 @@ initial_plane_phys_lmem(struct drm_i915_private *i915,
 
 	pte = ioread64(gte);
 	if (!(pte & GEN12_GGTT_PTE_LM)) {
-		drm_err(&i915->drm,
+		drm_err(display->drm,
 			"Initial plane programming missing PTE_LM bit\n");
 		return false;
 	}
@@ -75,7 +76,7 @@ initial_plane_phys_lmem(struct drm_i915_private *i915,
 	else
 		mem = i915->mm.stolen_region;
 	if (!mem) {
-		drm_dbg_kms(&i915->drm,
+		drm_dbg_kms(display->drm,
 			    "Initial plane memory region not initialized\n");
 		return false;
 	}
@@ -85,13 +86,13 @@ initial_plane_phys_lmem(struct drm_i915_private *i915,
 	 * ever be placed in the stolen portion.
 	 */
 	if (dma_addr < mem->region.start || dma_addr > mem->region.end) {
-		drm_err(&i915->drm,
+		drm_err(display->drm,
 			"Initial plane programming using invalid range, dma_addr=%pa (%s [%pa-%pa])\n",
 			&dma_addr, mem->region.name, &mem->region.start, &mem->region.end);
 		return false;
 	}
 
-	drm_dbg(&i915->drm,
+	drm_dbg(display->drm,
 		"Using dma_addr=%pa, based on initial plane programming\n",
 		&dma_addr);
 
@@ -102,9 +103,10 @@ initial_plane_phys_lmem(struct drm_i915_private *i915,
 }
 
 static bool
-initial_plane_phys_smem(struct drm_i915_private *i915,
+initial_plane_phys_smem(struct intel_display *display,
 			struct intel_initial_plane_config *plane_config)
 {
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct intel_memory_region *mem;
 	u32 base;
 
@@ -112,7 +114,7 @@ initial_plane_phys_smem(struct drm_i915_private *i915,
 
 	mem = i915->mm.stolen_region;
 	if (!mem) {
-		drm_dbg_kms(&i915->drm,
+		drm_dbg_kms(display->drm,
 			    "Initial plane memory region not initialized\n");
 		return false;
 	}
@@ -125,19 +127,22 @@ initial_plane_phys_smem(struct drm_i915_private *i915,
 }
 
 static bool
-initial_plane_phys(struct drm_i915_private *i915,
+initial_plane_phys(struct intel_display *display,
 		   struct intel_initial_plane_config *plane_config)
 {
+	struct drm_i915_private *i915 = to_i915(display->drm);
+
 	if (IS_DGFX(i915) || HAS_LMEMBAR_SMEM_STOLEN(i915))
-		return initial_plane_phys_lmem(i915, plane_config);
+		return initial_plane_phys_lmem(display, plane_config);
 	else
-		return initial_plane_phys_smem(i915, plane_config);
+		return initial_plane_phys_smem(display, plane_config);
 }
 
 static struct i915_vma *
-initial_plane_vma(struct drm_i915_private *i915,
+initial_plane_vma(struct intel_display *display,
 		  struct intel_initial_plane_config *plane_config)
 {
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct intel_memory_region *mem;
 	struct drm_i915_gem_object *obj;
 	struct drm_mm_node orig_mm = {};
@@ -149,7 +154,7 @@ initial_plane_vma(struct drm_i915_private *i915,
 	if (plane_config->size == 0)
 		return NULL;
 
-	if (!initial_plane_phys(i915, plane_config))
+	if (!initial_plane_phys(display, plane_config))
 		return NULL;
 
 	phys_base = plane_config->phys_base;
@@ -168,7 +173,7 @@ initial_plane_vma(struct drm_i915_private *i915,
 	if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
 	    mem == i915->mm.stolen_region &&
 	    size * 2 > i915->dsm.usable_size) {
-		drm_dbg_kms(&i915->drm, "Initial FB size exceeds half of stolen, discarding\n");
+		drm_dbg_kms(display->drm, "Initial FB size exceeds half of stolen, discarding\n");
 		return NULL;
 	}
 
@@ -176,7 +181,7 @@ initial_plane_vma(struct drm_i915_private *i915,
 					       I915_BO_ALLOC_USER |
 					       I915_BO_PREALLOC);
 	if (IS_ERR(obj)) {
-		drm_dbg_kms(&i915->drm, "Failed to preallocate initial FB in %s\n",
+		drm_dbg_kms(display->drm, "Failed to preallocate initial FB in %s\n",
 			    mem->region.name);
 		return NULL;
 	}
@@ -254,7 +259,7 @@ initial_plane_vma(struct drm_i915_private *i915,
 	if (drm_mm_node_allocated(&orig_mm))
 		drm_mm_remove_node(&orig_mm);
 
-	drm_dbg_kms(&i915->drm,
+	drm_dbg_kms(display->drm,
 		    "Initial plane fb bound to 0x%x in the ggtt (original 0x%x)\n",
 		    i915_ggtt_offset(vma), plane_config->base);
 
@@ -271,8 +276,7 @@ static bool
 intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
 			      struct intel_initial_plane_config *plane_config)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_display *display = to_intel_display(crtc);
 	struct drm_mode_fb_cmd2 mode_cmd = {};
 	struct drm_framebuffer *fb = &plane_config->fb->base;
 	struct i915_vma *vma;
@@ -284,13 +288,13 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
 	case I915_FORMAT_MOD_4_TILED:
 		break;
 	default:
-		drm_dbg(&dev_priv->drm,
+		drm_dbg(display->drm,
 			"Unsupported modifier for initial FB: 0x%llx\n",
 			fb->modifier);
 		return false;
 	}
 
-	vma = initial_plane_vma(dev_priv, plane_config);
+	vma = initial_plane_vma(display, plane_config);
 	if (!vma)
 		return false;
 
@@ -303,7 +307,7 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
 
 	if (intel_framebuffer_init(to_intel_framebuffer(fb),
 				   intel_bo_to_drm_bo(vma->obj), &mode_cmd)) {
-		drm_dbg_kms(&dev_priv->drm, "intel fb init failed\n");
+		drm_dbg_kms(display->drm, "intel fb init failed\n");
 		goto err_vma;
 	}
 
@@ -410,12 +414,12 @@ static void plane_config_fini(struct intel_initial_plane_config *plane_config)
 		i915_vma_put(plane_config->vma);
 }
 
-void intel_initial_plane_config(struct drm_i915_private *i915)
+void intel_initial_plane_config(struct intel_display *display)
 {
 	struct intel_initial_plane_config plane_configs[I915_MAX_PIPES] = {};
 	struct intel_crtc *crtc;
 
-	for_each_intel_crtc(&i915->drm, crtc) {
+	for_each_intel_crtc(display->drm, crtc) {
 		struct intel_initial_plane_config *plane_config =
 			&plane_configs[crtc->pipe];
 
@@ -429,7 +433,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
 		 * can even allow for smooth boot transitions if the BIOS
 		 * fb is large enough for the active pipe configuration.
 		 */
-		i915->display.funcs.display->get_initial_plane_config(crtc, plane_config);
+		display->funcs.display->get_initial_plane_config(crtc, plane_config);
 
 		/*
 		 * If the fb is shared between multiple heads, we'll
@@ -437,7 +441,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
 		 */
 		intel_find_initial_plane_obj(crtc, plane_configs);
 
-		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
+		if (display->funcs.display->fixup_initial_plane_config(crtc, plane_config))
 			intel_crtc_wait_for_next_vblank(crtc);
 
 		plane_config_fini(plane_config);
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.h b/drivers/gpu/drm/i915/display/intel_plane_initial.h
index 64ab95239cd4..6c6aa717ed21 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.h
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.h
@@ -6,8 +6,8 @@
 #ifndef __INTEL_PLANE_INITIAL_H__
 #define __INTEL_PLANE_INITIAL_H__
 
-struct drm_i915_private;
+struct intel_display;
 
-void intel_initial_plane_config(struct drm_i915_private *i915);
+void intel_initial_plane_config(struct intel_display *display);
 
 #endif
diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c b/drivers/gpu/drm/xe/display/xe_plane_initial.c
index 8c113463a3d5..2eb9633f163a 100644
--- a/drivers/gpu/drm/xe/display/xe_plane_initial.c
+++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c
@@ -275,12 +275,12 @@ static void plane_config_fini(struct intel_initial_plane_config *plane_config)
 	}
 }
 
-void intel_initial_plane_config(struct drm_i915_private *i915)
+void intel_initial_plane_config(struct intel_display *display)
 {
 	struct intel_initial_plane_config plane_configs[I915_MAX_PIPES] = {};
 	struct intel_crtc *crtc;
 
-	for_each_intel_crtc(&i915->drm, crtc) {
+	for_each_intel_crtc(display->drm, crtc) {
 		struct intel_initial_plane_config *plane_config =
 			&plane_configs[crtc->pipe];
 
@@ -294,7 +294,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
 		 * can even allow for smooth boot transitions if the BIOS
 		 * fb is large enough for the active pipe configuration.
 		 */
-		i915->display.funcs.display->get_initial_plane_config(crtc, plane_config);
+		display->funcs.display->get_initial_plane_config(crtc, plane_config);
 
 		/*
 		 * If the fb is shared between multiple heads, we'll
@@ -302,7 +302,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
 		 */
 		intel_find_initial_plane_obj(crtc, plane_configs);
 
-		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
+		if (display->funcs.display->fixup_initial_plane_config(crtc, plane_config))
 			intel_crtc_wait_for_next_vblank(crtc);
 
 		plane_config_fini(plane_config);
-- 
2.39.5

Re: [PATCH 3/5] drm/i915/plane: convert initial plane setup to struct intel_display

[Rodrigo Vivi]

On Mon, Nov 11, 2024 at 07:53:32PM +0200, Jani Nikula wrote:
> struct intel_display replaces struct drm_i915_private as the main
> display device pointer. Convert initial plane setup to it, as much as
> possible.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../drm/i915/display/intel_display_driver.c   |  2 +-
>  .../drm/i915/display/intel_plane_initial.c    | 56 ++++++++++---------
>  .../drm/i915/display/intel_plane_initial.h    |  4 +-
>  drivers/gpu/drm/xe/display/xe_plane_initial.c |  8 +--
>  4 files changed, 37 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 5983570b510f..8daf48d2ba7d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -472,7 +472,7 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915)
>  	intel_acpi_assign_connector_fwnodes(display);
>  	drm_modeset_unlock_all(dev);
>  
> -	intel_initial_plane_config(i915);
> +	intel_initial_plane_config(display);
>  
>  	/*
>  	 * Make sure hardware watermarks really match the state we read out.
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> index 62401f6a04e4..6789b7f14095 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> @@ -20,10 +20,10 @@ intel_reuse_initial_plane_obj(struct intel_crtc *this,
>  			      struct drm_framebuffer **fb,
>  			      struct i915_vma **vma)
>  {
> -	struct drm_i915_private *i915 = to_i915(this->base.dev);
> +	struct intel_display *display = to_intel_display(this);
>  	struct intel_crtc *crtc;
>  
> -	for_each_intel_crtc(&i915->drm, crtc) {
> +	for_each_intel_crtc(display->drm, crtc) {
>  		struct intel_plane *plane =
>  			to_intel_plane(crtc->base.primary);
>  		const struct intel_plane_state *plane_state =
> @@ -48,9 +48,10 @@ intel_reuse_initial_plane_obj(struct intel_crtc *this,
>  }
>  
>  static bool
> -initial_plane_phys_lmem(struct drm_i915_private *i915,
> +initial_plane_phys_lmem(struct intel_display *display,
>  			struct intel_initial_plane_config *plane_config)
>  {
> +	struct drm_i915_private *i915 = to_i915(display->drm);
>  	gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
>  	struct intel_memory_region *mem;
>  	dma_addr_t dma_addr;
> @@ -63,7 +64,7 @@ initial_plane_phys_lmem(struct drm_i915_private *i915,
>  
>  	pte = ioread64(gte);
>  	if (!(pte & GEN12_GGTT_PTE_LM)) {
> -		drm_err(&i915->drm,
> +		drm_err(display->drm,
>  			"Initial plane programming missing PTE_LM bit\n");
>  		return false;
>  	}
> @@ -75,7 +76,7 @@ initial_plane_phys_lmem(struct drm_i915_private *i915,
>  	else
>  		mem = i915->mm.stolen_region;
>  	if (!mem) {
> -		drm_dbg_kms(&i915->drm,
> +		drm_dbg_kms(display->drm,
>  			    "Initial plane memory region not initialized\n");
>  		return false;
>  	}
> @@ -85,13 +86,13 @@ initial_plane_phys_lmem(struct drm_i915_private *i915,
>  	 * ever be placed in the stolen portion.
>  	 */
>  	if (dma_addr < mem->region.start || dma_addr > mem->region.end) {
> -		drm_err(&i915->drm,
> +		drm_err(display->drm,
>  			"Initial plane programming using invalid range, dma_addr=%pa (%s [%pa-%pa])\n",
>  			&dma_addr, mem->region.name, &mem->region.start, &mem->region.end);
>  		return false;
>  	}
>  
> -	drm_dbg(&i915->drm,
> +	drm_dbg(display->drm,
>  		"Using dma_addr=%pa, based on initial plane programming\n",
>  		&dma_addr);
>  
> @@ -102,9 +103,10 @@ initial_plane_phys_lmem(struct drm_i915_private *i915,
>  }
>  
>  static bool
> -initial_plane_phys_smem(struct drm_i915_private *i915,
> +initial_plane_phys_smem(struct intel_display *display,
>  			struct intel_initial_plane_config *plane_config)
>  {
> +	struct drm_i915_private *i915 = to_i915(display->drm);
>  	struct intel_memory_region *mem;
>  	u32 base;
>  
> @@ -112,7 +114,7 @@ initial_plane_phys_smem(struct drm_i915_private *i915,
>  
>  	mem = i915->mm.stolen_region;
>  	if (!mem) {
> -		drm_dbg_kms(&i915->drm,
> +		drm_dbg_kms(display->drm,
>  			    "Initial plane memory region not initialized\n");
>  		return false;
>  	}
> @@ -125,19 +127,22 @@ initial_plane_phys_smem(struct drm_i915_private *i915,
>  }
>  
>  static bool
> -initial_plane_phys(struct drm_i915_private *i915,
> +initial_plane_phys(struct intel_display *display,
>  		   struct intel_initial_plane_config *plane_config)
>  {
> +	struct drm_i915_private *i915 = to_i915(display->drm);
> +
>  	if (IS_DGFX(i915) || HAS_LMEMBAR_SMEM_STOLEN(i915))
> -		return initial_plane_phys_lmem(i915, plane_config);
> +		return initial_plane_phys_lmem(display, plane_config);
>  	else
> -		return initial_plane_phys_smem(i915, plane_config);
> +		return initial_plane_phys_smem(display, plane_config);
>  }
>  
>  static struct i915_vma *
> -initial_plane_vma(struct drm_i915_private *i915,
> +initial_plane_vma(struct intel_display *display,
>  		  struct intel_initial_plane_config *plane_config)
>  {
> +	struct drm_i915_private *i915 = to_i915(display->drm);
>  	struct intel_memory_region *mem;
>  	struct drm_i915_gem_object *obj;
>  	struct drm_mm_node orig_mm = {};
> @@ -149,7 +154,7 @@ initial_plane_vma(struct drm_i915_private *i915,
>  	if (plane_config->size == 0)
>  		return NULL;
>  
> -	if (!initial_plane_phys(i915, plane_config))
> +	if (!initial_plane_phys(display, plane_config))
>  		return NULL;
>  
>  	phys_base = plane_config->phys_base;
> @@ -168,7 +173,7 @@ initial_plane_vma(struct drm_i915_private *i915,
>  	if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
>  	    mem == i915->mm.stolen_region &&
>  	    size * 2 > i915->dsm.usable_size) {
> -		drm_dbg_kms(&i915->drm, "Initial FB size exceeds half of stolen, discarding\n");
> +		drm_dbg_kms(display->drm, "Initial FB size exceeds half of stolen, discarding\n");
>  		return NULL;
>  	}
>  
> @@ -176,7 +181,7 @@ initial_plane_vma(struct drm_i915_private *i915,
>  					       I915_BO_ALLOC_USER |
>  					       I915_BO_PREALLOC);
>  	if (IS_ERR(obj)) {
> -		drm_dbg_kms(&i915->drm, "Failed to preallocate initial FB in %s\n",
> +		drm_dbg_kms(display->drm, "Failed to preallocate initial FB in %s\n",
>  			    mem->region.name);
>  		return NULL;
>  	}
> @@ -254,7 +259,7 @@ initial_plane_vma(struct drm_i915_private *i915,
>  	if (drm_mm_node_allocated(&orig_mm))
>  		drm_mm_remove_node(&orig_mm);
>  
> -	drm_dbg_kms(&i915->drm,
> +	drm_dbg_kms(display->drm,
>  		    "Initial plane fb bound to 0x%x in the ggtt (original 0x%x)\n",
>  		    i915_ggtt_offset(vma), plane_config->base);
>  
> @@ -271,8 +276,7 @@ static bool
>  intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
>  			      struct intel_initial_plane_config *plane_config)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_display *display = to_intel_display(crtc);
>  	struct drm_mode_fb_cmd2 mode_cmd = {};
>  	struct drm_framebuffer *fb = &plane_config->fb->base;
>  	struct i915_vma *vma;
> @@ -284,13 +288,13 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
>  	case I915_FORMAT_MOD_4_TILED:
>  		break;
>  	default:
> -		drm_dbg(&dev_priv->drm,
> +		drm_dbg(display->drm,
>  			"Unsupported modifier for initial FB: 0x%llx\n",
>  			fb->modifier);
>  		return false;
>  	}
>  
> -	vma = initial_plane_vma(dev_priv, plane_config);
> +	vma = initial_plane_vma(display, plane_config);
>  	if (!vma)
>  		return false;
>  
> @@ -303,7 +307,7 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
>  
>  	if (intel_framebuffer_init(to_intel_framebuffer(fb),
>  				   intel_bo_to_drm_bo(vma->obj), &mode_cmd)) {
> -		drm_dbg_kms(&dev_priv->drm, "intel fb init failed\n");
> +		drm_dbg_kms(display->drm, "intel fb init failed\n");
>  		goto err_vma;
>  	}
>  
> @@ -410,12 +414,12 @@ static void plane_config_fini(struct intel_initial_plane_config *plane_config)
>  		i915_vma_put(plane_config->vma);
>  }
>  
> -void intel_initial_plane_config(struct drm_i915_private *i915)
> +void intel_initial_plane_config(struct intel_display *display)
>  {
>  	struct intel_initial_plane_config plane_configs[I915_MAX_PIPES] = {};
>  	struct intel_crtc *crtc;
>  
> -	for_each_intel_crtc(&i915->drm, crtc) {
> +	for_each_intel_crtc(display->drm, crtc) {
>  		struct intel_initial_plane_config *plane_config =
>  			&plane_configs[crtc->pipe];
>  
> @@ -429,7 +433,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
>  		 * can even allow for smooth boot transitions if the BIOS
>  		 * fb is large enough for the active pipe configuration.
>  		 */
> -		i915->display.funcs.display->get_initial_plane_config(crtc, plane_config);
> +		display->funcs.display->get_initial_plane_config(crtc, plane_config);
>  
>  		/*
>  		 * If the fb is shared between multiple heads, we'll
> @@ -437,7 +441,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
>  		 */
>  		intel_find_initial_plane_obj(crtc, plane_configs);
>  
> -		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
> +		if (display->funcs.display->fixup_initial_plane_config(crtc, plane_config))
>  			intel_crtc_wait_for_next_vblank(crtc);
>  
>  		plane_config_fini(plane_config);
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.h b/drivers/gpu/drm/i915/display/intel_plane_initial.h
> index 64ab95239cd4..6c6aa717ed21 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.h
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.h
> @@ -6,8 +6,8 @@
>  #ifndef __INTEL_PLANE_INITIAL_H__
>  #define __INTEL_PLANE_INITIAL_H__
>  
> -struct drm_i915_private;
> +struct intel_display;
>  
> -void intel_initial_plane_config(struct drm_i915_private *i915);
> +void intel_initial_plane_config(struct intel_display *display);
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c b/drivers/gpu/drm/xe/display/xe_plane_initial.c
> index 8c113463a3d5..2eb9633f163a 100644
> --- a/drivers/gpu/drm/xe/display/xe_plane_initial.c
> +++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c
> @@ -275,12 +275,12 @@ static void plane_config_fini(struct intel_initial_plane_config *plane_config)
>  	}
>  }
>  
> -void intel_initial_plane_config(struct drm_i915_private *i915)
> +void intel_initial_plane_config(struct intel_display *display)
>  {
>  	struct intel_initial_plane_config plane_configs[I915_MAX_PIPES] = {};
>  	struct intel_crtc *crtc;
>  
> -	for_each_intel_crtc(&i915->drm, crtc) {
> +	for_each_intel_crtc(display->drm, crtc) {
>  		struct intel_initial_plane_config *plane_config =
>  			&plane_configs[crtc->pipe];
>  
> @@ -294,7 +294,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
>  		 * can even allow for smooth boot transitions if the BIOS
>  		 * fb is large enough for the active pipe configuration.
>  		 */
> -		i915->display.funcs.display->get_initial_plane_config(crtc, plane_config);
> +		display->funcs.display->get_initial_plane_config(crtc, plane_config);
>  
>  		/*
>  		 * If the fb is shared between multiple heads, we'll
> @@ -302,7 +302,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
>  		 */
>  		intel_find_initial_plane_obj(crtc, plane_configs);
>  
> -		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
> +		if (display->funcs.display->fixup_initial_plane_config(crtc, plane_config))
>  			intel_crtc_wait_for_next_vblank(crtc);
>  
>  		plane_config_fini(plane_config);
> -- 
> 2.39.5
> 

[PATCH 4/5] drm/i915/irq: hide display_irqs_enabled access

[Jani Nikula]

Move the check for display_irqs_enabled within vlv_display_irq_reset()
and vlv_display_irq_postinstall() to avoid looking at struct
intel_display members within i915 core irq code.

Within display irq code, vlv_display_irq_reset() may need to be called
with !display_irqs_enabled, so add a small wrapper.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_irq.c | 15 ++++++++++++---
 drivers/gpu/drm/i915/i915_irq.c                  | 12 ++++--------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
index e1547ebce60e..d5458b0d976b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_irq.c
+++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
@@ -1479,7 +1479,7 @@ void bdw_disable_vblank(struct drm_crtc *_crtc)
 		schedule_work(&display->irq.vblank_dc_work);
 }
 
-void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
+static void _vlv_display_irq_reset(struct drm_i915_private *dev_priv)
 {
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
@@ -1497,6 +1497,12 @@ void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
 	dev_priv->irq_mask = ~0u;
 }
 
+void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
+{
+	if (dev_priv->display.irq.display_irqs_enabled)
+		_vlv_display_irq_reset(dev_priv);
+}
+
 void i9xx_display_irq_reset(struct drm_i915_private *i915)
 {
 	if (I915_HAS_HOTPLUG(i915)) {
@@ -1516,6 +1522,9 @@ void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
 	u32 enable_mask;
 	enum pipe pipe;
 
+	if (!dev_priv->display.irq.display_irqs_enabled)
+		return;
+
 	pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
 
 	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS);
@@ -1694,7 +1703,7 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
 	dev_priv->display.irq.display_irqs_enabled = true;
 
 	if (intel_irqs_enabled(dev_priv)) {
-		vlv_display_irq_reset(dev_priv);
+		_vlv_display_irq_reset(dev_priv);
 		vlv_display_irq_postinstall(dev_priv);
 	}
 }
@@ -1709,7 +1718,7 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
 	dev_priv->display.irq.display_irqs_enabled = false;
 
 	if (intel_irqs_enabled(dev_priv))
-		vlv_display_irq_reset(dev_priv);
+		_vlv_display_irq_reset(dev_priv);
 }
 
 void ilk_de_irq_postinstall(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f75cbf5b8a1c..7920ad9585ae 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -658,8 +658,7 @@ static void valleyview_irq_reset(struct drm_i915_private *dev_priv)
 	gen5_gt_irq_reset(to_gt(dev_priv));
 
 	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display.irq.display_irqs_enabled)
-		vlv_display_irq_reset(dev_priv);
+	vlv_display_irq_reset(dev_priv);
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
 
@@ -723,8 +722,7 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
 	gen2_irq_reset(uncore, GEN8_PCU_IRQ_REGS);
 
 	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display.irq.display_irqs_enabled)
-		vlv_display_irq_reset(dev_priv);
+	vlv_display_irq_reset(dev_priv);
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
 
@@ -740,8 +738,7 @@ static void valleyview_irq_postinstall(struct drm_i915_private *dev_priv)
 	gen5_gt_irq_postinstall(to_gt(dev_priv));
 
 	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display.irq.display_irqs_enabled)
-		vlv_display_irq_postinstall(dev_priv);
+	vlv_display_irq_postinstall(dev_priv);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	intel_uncore_write(&dev_priv->uncore, VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
@@ -794,8 +791,7 @@ static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
 	gen8_gt_irq_postinstall(to_gt(dev_priv));
 
 	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display.irq.display_irqs_enabled)
-		vlv_display_irq_postinstall(dev_priv);
+	vlv_display_irq_postinstall(dev_priv);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	intel_uncore_write(&dev_priv->uncore, GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
-- 
2.39.5

Re: [PATCH 4/5] drm/i915/irq: hide display_irqs_enabled access

[Rodrigo Vivi]

On Mon, Nov 11, 2024 at 07:53:33PM +0200, Jani Nikula wrote:
> Move the check for display_irqs_enabled within vlv_display_irq_reset()
> and vlv_display_irq_postinstall() to avoid looking at struct
> intel_display members within i915 core irq code.
> 
> Within display irq code, vlv_display_irq_reset() may need to be called
> with !display_irqs_enabled, so add a small wrapper.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_irq.c | 15 ++++++++++++---
>  drivers/gpu/drm/i915/i915_irq.c                  | 12 ++++--------
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index e1547ebce60e..d5458b0d976b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -1479,7 +1479,7 @@ void bdw_disable_vblank(struct drm_crtc *_crtc)
>  		schedule_work(&display->irq.vblank_dc_work);
>  }
>  
> -void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> +static void _vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_uncore *uncore = &dev_priv->uncore;
>  
> @@ -1497,6 +1497,12 @@ void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>  	dev_priv->irq_mask = ~0u;
>  }
>  
> +void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> +{
> +	if (dev_priv->display.irq.display_irqs_enabled)
> +		_vlv_display_irq_reset(dev_priv);
> +}
> +
>  void i9xx_display_irq_reset(struct drm_i915_private *i915)
>  {
>  	if (I915_HAS_HOTPLUG(i915)) {
> @@ -1516,6 +1522,9 @@ void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
>  	u32 enable_mask;
>  	enum pipe pipe;
>  
> +	if (!dev_priv->display.irq.display_irqs_enabled)
> +		return;

I got confused here. this likely deserves a separate patch?

> +
>  	pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
>  
>  	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS);
> @@ -1694,7 +1703,7 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
>  	dev_priv->display.irq.display_irqs_enabled = true;
>  
>  	if (intel_irqs_enabled(dev_priv)) {
> -		vlv_display_irq_reset(dev_priv);
> +		_vlv_display_irq_reset(dev_priv);
>  		vlv_display_irq_postinstall(dev_priv);
>  	}
>  }
> @@ -1709,7 +1718,7 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
>  	dev_priv->display.irq.display_irqs_enabled = false;
>  
>  	if (intel_irqs_enabled(dev_priv))
> -		vlv_display_irq_reset(dev_priv);
> +		_vlv_display_irq_reset(dev_priv);
>  }
>  
>  void ilk_de_irq_postinstall(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f75cbf5b8a1c..7920ad9585ae 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -658,8 +658,7 @@ static void valleyview_irq_reset(struct drm_i915_private *dev_priv)
>  	gen5_gt_irq_reset(to_gt(dev_priv));
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.irq.display_irqs_enabled)
> -		vlv_display_irq_reset(dev_priv);
> +	vlv_display_irq_reset(dev_priv);
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  }
>  
> @@ -723,8 +722,7 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>  	gen2_irq_reset(uncore, GEN8_PCU_IRQ_REGS);
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.irq.display_irqs_enabled)
> -		vlv_display_irq_reset(dev_priv);
> +	vlv_display_irq_reset(dev_priv);
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  }
>  
> @@ -740,8 +738,7 @@ static void valleyview_irq_postinstall(struct drm_i915_private *dev_priv)
>  	gen5_gt_irq_postinstall(to_gt(dev_priv));
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.irq.display_irqs_enabled)
> -		vlv_display_irq_postinstall(dev_priv);
> +	vlv_display_irq_postinstall(dev_priv);
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
>  	intel_uncore_write(&dev_priv->uncore, VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
> @@ -794,8 +791,7 @@ static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
>  	gen8_gt_irq_postinstall(to_gt(dev_priv));
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.irq.display_irqs_enabled)
> -		vlv_display_irq_postinstall(dev_priv);
> +	vlv_display_irq_postinstall(dev_priv);
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
>  	intel_uncore_write(&dev_priv->uncore, GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> -- 
> 2.39.5
> 

Re: [PATCH 4/5] drm/i915/irq: hide display_irqs_enabled access

[Jani Nikula]

On Thu, 14 Nov 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Mon, Nov 11, 2024 at 07:53:33PM +0200, Jani Nikula wrote:
>> Move the check for display_irqs_enabled within vlv_display_irq_reset()
>> and vlv_display_irq_postinstall() to avoid looking at struct
>> intel_display members within i915 core irq code.
>> 
>> Within display irq code, vlv_display_irq_reset() may need to be called
>> with !display_irqs_enabled, so add a small wrapper.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_irq.c | 15 ++++++++++++---
>>  drivers/gpu/drm/i915/i915_irq.c                  | 12 ++++--------
>>  2 files changed, 16 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
>> index e1547ebce60e..d5458b0d976b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
>> @@ -1479,7 +1479,7 @@ void bdw_disable_vblank(struct drm_crtc *_crtc)
>>  		schedule_work(&display->irq.vblank_dc_work);
>>  }
>>  
>> -void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>> +static void _vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>>  {
>>  	struct intel_uncore *uncore = &dev_priv->uncore;
>>  
>> @@ -1497,6 +1497,12 @@ void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>>  	dev_priv->irq_mask = ~0u;
>>  }
>>  
>> +void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>> +{
>> +	if (dev_priv->display.irq.display_irqs_enabled)
>> +		_vlv_display_irq_reset(dev_priv);
>> +}
>> +
>>  void i9xx_display_irq_reset(struct drm_i915_private *i915)
>>  {
>>  	if (I915_HAS_HOTPLUG(i915)) {
>> @@ -1516,6 +1522,9 @@ void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
>>  	u32 enable_mask;
>>  	enum pipe pipe;
>>  
>> +	if (!dev_priv->display.irq.display_irqs_enabled)
>> +		return;
>
> I got confused here. this likely deserves a separate patch?

I thought I explained it in the commit message. The check is being moved
from the callers to the callees. But for vlv_display_irq_reset() we also
need to be able to call without the check, so that gets an additional
wrapper.

BR,
Jani.

>
>> +
>>  	pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
>>  
>>  	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS);
>> @@ -1694,7 +1703,7 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
>>  	dev_priv->display.irq.display_irqs_enabled = true;
>>  
>>  	if (intel_irqs_enabled(dev_priv)) {
>> -		vlv_display_irq_reset(dev_priv);
>> +		_vlv_display_irq_reset(dev_priv);
>>  		vlv_display_irq_postinstall(dev_priv);
>>  	}
>>  }
>> @@ -1709,7 +1718,7 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
>>  	dev_priv->display.irq.display_irqs_enabled = false;
>>  
>>  	if (intel_irqs_enabled(dev_priv))
>> -		vlv_display_irq_reset(dev_priv);
>> +		_vlv_display_irq_reset(dev_priv);
>>  }
>>  
>>  void ilk_de_irq_postinstall(struct drm_i915_private *i915)
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index f75cbf5b8a1c..7920ad9585ae 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -658,8 +658,7 @@ static void valleyview_irq_reset(struct drm_i915_private *dev_priv)
>>  	gen5_gt_irq_reset(to_gt(dev_priv));
>>  
>>  	spin_lock_irq(&dev_priv->irq_lock);
>> -	if (dev_priv->display.irq.display_irqs_enabled)
>> -		vlv_display_irq_reset(dev_priv);
>> +	vlv_display_irq_reset(dev_priv);
>>  	spin_unlock_irq(&dev_priv->irq_lock);
>>  }
>>  
>> @@ -723,8 +722,7 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>>  	gen2_irq_reset(uncore, GEN8_PCU_IRQ_REGS);
>>  
>>  	spin_lock_irq(&dev_priv->irq_lock);
>> -	if (dev_priv->display.irq.display_irqs_enabled)
>> -		vlv_display_irq_reset(dev_priv);
>> +	vlv_display_irq_reset(dev_priv);
>>  	spin_unlock_irq(&dev_priv->irq_lock);
>>  }
>>  
>> @@ -740,8 +738,7 @@ static void valleyview_irq_postinstall(struct drm_i915_private *dev_priv)
>>  	gen5_gt_irq_postinstall(to_gt(dev_priv));
>>  
>>  	spin_lock_irq(&dev_priv->irq_lock);
>> -	if (dev_priv->display.irq.display_irqs_enabled)
>> -		vlv_display_irq_postinstall(dev_priv);
>> +	vlv_display_irq_postinstall(dev_priv);
>>  	spin_unlock_irq(&dev_priv->irq_lock);
>>  
>>  	intel_uncore_write(&dev_priv->uncore, VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
>> @@ -794,8 +791,7 @@ static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
>>  	gen8_gt_irq_postinstall(to_gt(dev_priv));
>>  
>>  	spin_lock_irq(&dev_priv->irq_lock);
>> -	if (dev_priv->display.irq.display_irqs_enabled)
>> -		vlv_display_irq_postinstall(dev_priv);
>> +	vlv_display_irq_postinstall(dev_priv);
>>  	spin_unlock_irq(&dev_priv->irq_lock);
>>  
>>  	intel_uncore_write(&dev_priv->uncore, GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
>> -- 
>> 2.39.5
>> 

-- 
Jani Nikula, Intel

Re: [PATCH 4/5] drm/i915/irq: hide display_irqs_enabled access

[Rodrigo Vivi]

On Fri, Nov 15, 2024 at 03:13:31PM +0200, Jani Nikula wrote:
> On Thu, 14 Nov 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Mon, Nov 11, 2024 at 07:53:33PM +0200, Jani Nikula wrote:
> >> Move the check for display_irqs_enabled within vlv_display_irq_reset()
> >> and vlv_display_irq_postinstall() to avoid looking at struct
> >> intel_display members within i915 core irq code.
> >> 
> >> Within display irq code, vlv_display_irq_reset() may need to be called
> >> with !display_irqs_enabled, so add a small wrapper.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_display_irq.c | 15 ++++++++++++---
> >>  drivers/gpu/drm/i915/i915_irq.c                  | 12 ++++--------
> >>  2 files changed, 16 insertions(+), 11 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> >> index e1547ebce60e..d5458b0d976b 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> >> @@ -1479,7 +1479,7 @@ void bdw_disable_vblank(struct drm_crtc *_crtc)
> >>  		schedule_work(&display->irq.vblank_dc_work);
> >>  }
> >>  
> >> -void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> >> +static void _vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> >>  {
> >>  	struct intel_uncore *uncore = &dev_priv->uncore;
> >>  
> >> @@ -1497,6 +1497,12 @@ void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> >>  	dev_priv->irq_mask = ~0u;
> >>  }
> >>  
> >> +void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> >> +{
> >> +	if (dev_priv->display.irq.display_irqs_enabled)
> >> +		_vlv_display_irq_reset(dev_priv);
> >> +}
> >> +
> >>  void i9xx_display_irq_reset(struct drm_i915_private *i915)
> >>  {
> >>  	if (I915_HAS_HOTPLUG(i915)) {
> >> @@ -1516,6 +1522,9 @@ void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
> >>  	u32 enable_mask;
> >>  	enum pipe pipe;
> >>  
> >> +	if (!dev_priv->display.irq.display_irqs_enabled)
> >> +		return;
> >
> > I got confused here. this likely deserves a separate patch?
> 
> I thought I explained it in the commit message. The check is being moved
> from the callers to the callees. But for vlv_display_irq_reset() we also
> need to be able to call without the check, so that gets an additional
> wrapper.

doh!

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> BR,
> Jani.
> 
> >
> >> +
> >>  	pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
> >>  
> >>  	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS);
> >> @@ -1694,7 +1703,7 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
> >>  	dev_priv->display.irq.display_irqs_enabled = true;
> >>  
> >>  	if (intel_irqs_enabled(dev_priv)) {
> >> -		vlv_display_irq_reset(dev_priv);
> >> +		_vlv_display_irq_reset(dev_priv);
> >>  		vlv_display_irq_postinstall(dev_priv);
> >>  	}
> >>  }
> >> @@ -1709,7 +1718,7 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
> >>  	dev_priv->display.irq.display_irqs_enabled = false;
> >>  
> >>  	if (intel_irqs_enabled(dev_priv))
> >> -		vlv_display_irq_reset(dev_priv);
> >> +		_vlv_display_irq_reset(dev_priv);
> >>  }
> >>  
> >>  void ilk_de_irq_postinstall(struct drm_i915_private *i915)
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index f75cbf5b8a1c..7920ad9585ae 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -658,8 +658,7 @@ static void valleyview_irq_reset(struct drm_i915_private *dev_priv)
> >>  	gen5_gt_irq_reset(to_gt(dev_priv));
> >>  
> >>  	spin_lock_irq(&dev_priv->irq_lock);
> >> -	if (dev_priv->display.irq.display_irqs_enabled)
> >> -		vlv_display_irq_reset(dev_priv);
> >> +	vlv_display_irq_reset(dev_priv);
> >>  	spin_unlock_irq(&dev_priv->irq_lock);
> >>  }
> >>  
> >> @@ -723,8 +722,7 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
> >>  	gen2_irq_reset(uncore, GEN8_PCU_IRQ_REGS);
> >>  
> >>  	spin_lock_irq(&dev_priv->irq_lock);
> >> -	if (dev_priv->display.irq.display_irqs_enabled)
> >> -		vlv_display_irq_reset(dev_priv);
> >> +	vlv_display_irq_reset(dev_priv);
> >>  	spin_unlock_irq(&dev_priv->irq_lock);
> >>  }
> >>  
> >> @@ -740,8 +738,7 @@ static void valleyview_irq_postinstall(struct drm_i915_private *dev_priv)
> >>  	gen5_gt_irq_postinstall(to_gt(dev_priv));
> >>  
> >>  	spin_lock_irq(&dev_priv->irq_lock);
> >> -	if (dev_priv->display.irq.display_irqs_enabled)
> >> -		vlv_display_irq_postinstall(dev_priv);
> >> +	vlv_display_irq_postinstall(dev_priv);
> >>  	spin_unlock_irq(&dev_priv->irq_lock);
> >>  
> >>  	intel_uncore_write(&dev_priv->uncore, VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
> >> @@ -794,8 +791,7 @@ static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
> >>  	gen8_gt_irq_postinstall(to_gt(dev_priv));
> >>  
> >>  	spin_lock_irq(&dev_priv->irq_lock);
> >> -	if (dev_priv->display.irq.display_irqs_enabled)
> >> -		vlv_display_irq_postinstall(dev_priv);
> >> +	vlv_display_irq_postinstall(dev_priv);
> >>  	spin_unlock_irq(&dev_priv->irq_lock);
> >>  
> >>  	intel_uncore_write(&dev_priv->uncore, GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> >> -- 
> >> 2.39.5
> >> 
> 
> -- 
> Jani Nikula, Intel

[PATCH 5/5] drm/i915/irq: emphasize display_irqs_enabled is only about VLV/CHV

[Jani Nikula]

Rename display_irqs_enabled to vlv_display_irqs_enabled, to emphasize
it's really only about VLV/CHV. Only access it when running on VLV/CHV.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../gpu/drm/i915/display/intel_display_core.h |  9 ++++++-
 .../gpu/drm/i915/display/intel_display_irq.c  | 26 ++++++-------------
 .../gpu/drm/i915/display/intel_hotplug_irq.c  |  6 ++++-
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index 45b7c6900adc..5ad66df1a85e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -453,7 +453,14 @@ struct intel_display {
 	} ips;
 
 	struct {
-		bool display_irqs_enabled;
+		/*
+		 * Most platforms treat the display irq block as an always-on
+		 * power domain. vlv/chv can disable it at runtime and need
+		 * special care to avoid writing any of the display block
+		 * registers outside of the power domain. We defer setting up
+		 * the display irqs in this case to the runtime pm.
+		 */
+		bool vlv_display_irqs_enabled;
 
 		/* For i915gm/i945gm vblank irq workaround */
 		u8 vblank_enabled;
diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
index d5458b0d976b..50c1ca062b80 100644
--- a/drivers/gpu/drm/i915/display/intel_display_irq.c
+++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
@@ -434,7 +434,8 @@ void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv,
 
 	spin_lock(&dev_priv->irq_lock);
 
-	if (!dev_priv->display.irq.display_irqs_enabled) {
+	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
+	    !dev_priv->display.irq.vlv_display_irqs_enabled) {
 		spin_unlock(&dev_priv->irq_lock);
 		return;
 	}
@@ -1499,7 +1500,7 @@ static void _vlv_display_irq_reset(struct drm_i915_private *dev_priv)
 
 void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
 {
-	if (dev_priv->display.irq.display_irqs_enabled)
+	if (dev_priv->display.irq.vlv_display_irqs_enabled)
 		_vlv_display_irq_reset(dev_priv);
 }
 
@@ -1522,7 +1523,7 @@ void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
 	u32 enable_mask;
 	enum pipe pipe;
 
-	if (!dev_priv->display.irq.display_irqs_enabled)
+	if (!dev_priv->display.irq.vlv_display_irqs_enabled)
 		return;
 
 	pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
@@ -1697,10 +1698,10 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
 {
 	lockdep_assert_held(&dev_priv->irq_lock);
 
-	if (dev_priv->display.irq.display_irqs_enabled)
+	if (dev_priv->display.irq.vlv_display_irqs_enabled)
 		return;
 
-	dev_priv->display.irq.display_irqs_enabled = true;
+	dev_priv->display.irq.vlv_display_irqs_enabled = true;
 
 	if (intel_irqs_enabled(dev_priv)) {
 		_vlv_display_irq_reset(dev_priv);
@@ -1712,10 +1713,10 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
 {
 	lockdep_assert_held(&dev_priv->irq_lock);
 
-	if (!dev_priv->display.irq.display_irqs_enabled)
+	if (!dev_priv->display.irq.vlv_display_irqs_enabled)
 		return;
 
-	dev_priv->display.irq.display_irqs_enabled = false;
+	dev_priv->display.irq.vlv_display_irqs_enabled = false;
 
 	if (intel_irqs_enabled(dev_priv))
 		_vlv_display_irq_reset(dev_priv);
@@ -1911,17 +1912,6 @@ void intel_display_irq_init(struct drm_i915_private *i915)
 {
 	i915->drm.vblank_disable_immediate = true;
 
-	/*
-	 * Most platforms treat the display irq block as an always-on power
-	 * domain. vlv/chv can disable it at runtime and need special care to
-	 * avoid writing any of the display block registers outside of the power
-	 * domain. We defer setting up the display irqs in this case to the
-	 * runtime pm.
-	 */
-	i915->display.irq.display_irqs_enabled = true;
-	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
-		i915->display.irq.display_irqs_enabled = false;
-
 	intel_hotplug_irq_init(i915);
 
 	INIT_WORK(&i915->display.irq.vblank_dc_work,
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
index cb64c6f0ad1b..476ac88087e0 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
@@ -1457,7 +1457,11 @@ void intel_hpd_enable_detection(struct intel_encoder *encoder)
 
 void intel_hpd_irq_setup(struct drm_i915_private *i915)
 {
-	if (i915->display.irq.display_irqs_enabled && i915->display.funcs.hotplug)
+	if ((IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) &&
+	    !i915->display.irq.vlv_display_irqs_enabled)
+		return;
+
+	if (i915->display.funcs.hotplug)
 		i915->display.funcs.hotplug->hpd_irq_setup(i915);
 }
 
-- 
2.39.5

Re: [PATCH 5/5] drm/i915/irq: emphasize display_irqs_enabled is only about VLV/CHV

[Rodrigo Vivi]

On Mon, Nov 11, 2024 at 07:53:34PM +0200, Jani Nikula wrote:
> Rename display_irqs_enabled to vlv_display_irqs_enabled, to emphasize
> it's really only about VLV/CHV. Only access it when running on VLV/CHV.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_display_core.h |  9 ++++++-
>  .../gpu/drm/i915/display/intel_display_irq.c  | 26 ++++++-------------
>  .../gpu/drm/i915/display/intel_hotplug_irq.c  |  6 ++++-
>  3 files changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 45b7c6900adc..5ad66df1a85e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -453,7 +453,14 @@ struct intel_display {
>  	} ips;
>  
>  	struct {
> -		bool display_irqs_enabled;
> +		/*
> +		 * Most platforms treat the display irq block as an always-on
> +		 * power domain. vlv/chv can disable it at runtime and need
> +		 * special care to avoid writing any of the display block
> +		 * registers outside of the power domain. We defer setting up
> +		 * the display irqs in this case to the runtime pm.
> +		 */
> +		bool vlv_display_irqs_enabled;
>  
>  		/* For i915gm/i945gm vblank irq workaround */
>  		u8 vblank_enabled;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index d5458b0d976b..50c1ca062b80 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -434,7 +434,8 @@ void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv,
>  
>  	spin_lock(&dev_priv->irq_lock);
>  
> -	if (!dev_priv->display.irq.display_irqs_enabled) {
> +	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
> +	    !dev_priv->display.irq.vlv_display_irqs_enabled) {
>  		spin_unlock(&dev_priv->irq_lock);
>  		return;
>  	}
> @@ -1499,7 +1500,7 @@ static void _vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>  
>  void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>  {
> -	if (dev_priv->display.irq.display_irqs_enabled)
> +	if (dev_priv->display.irq.vlv_display_irqs_enabled)
>  		_vlv_display_irq_reset(dev_priv);
>  }
>  
> @@ -1522,7 +1523,7 @@ void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
>  	u32 enable_mask;
>  	enum pipe pipe;
>  
> -	if (!dev_priv->display.irq.display_irqs_enabled)
> +	if (!dev_priv->display.irq.vlv_display_irqs_enabled)
>  		return;
>  
>  	pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
> @@ -1697,10 +1698,10 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
>  {
>  	lockdep_assert_held(&dev_priv->irq_lock);
>  
> -	if (dev_priv->display.irq.display_irqs_enabled)
> +	if (dev_priv->display.irq.vlv_display_irqs_enabled)
>  		return;
>  
> -	dev_priv->display.irq.display_irqs_enabled = true;
> +	dev_priv->display.irq.vlv_display_irqs_enabled = true;
>  
>  	if (intel_irqs_enabled(dev_priv)) {
>  		_vlv_display_irq_reset(dev_priv);
> @@ -1712,10 +1713,10 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
>  {
>  	lockdep_assert_held(&dev_priv->irq_lock);
>  
> -	if (!dev_priv->display.irq.display_irqs_enabled)
> +	if (!dev_priv->display.irq.vlv_display_irqs_enabled)
>  		return;
>  
> -	dev_priv->display.irq.display_irqs_enabled = false;
> +	dev_priv->display.irq.vlv_display_irqs_enabled = false;
>  
>  	if (intel_irqs_enabled(dev_priv))
>  		_vlv_display_irq_reset(dev_priv);
> @@ -1911,17 +1912,6 @@ void intel_display_irq_init(struct drm_i915_private *i915)
>  {
>  	i915->drm.vblank_disable_immediate = true;
>  
> -	/*
> -	 * Most platforms treat the display irq block as an always-on power
> -	 * domain. vlv/chv can disable it at runtime and need special care to
> -	 * avoid writing any of the display block registers outside of the power
> -	 * domain. We defer setting up the display irqs in this case to the
> -	 * runtime pm.
> -	 */
> -	i915->display.irq.display_irqs_enabled = true;
> -	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> -		i915->display.irq.display_irqs_enabled = false;
> -
>  	intel_hotplug_irq_init(i915);
>  
>  	INIT_WORK(&i915->display.irq.vblank_dc_work,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
> index cb64c6f0ad1b..476ac88087e0 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
> @@ -1457,7 +1457,11 @@ void intel_hpd_enable_detection(struct intel_encoder *encoder)
>  
>  void intel_hpd_irq_setup(struct drm_i915_private *i915)
>  {
> -	if (i915->display.irq.display_irqs_enabled && i915->display.funcs.hotplug)
> +	if ((IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) &&

I like the overal change, but it is not just a 'Rename' as the commit message
tells if we are changing conditions.

> +	    !i915->display.irq.vlv_display_irqs_enabled)
> +		return;
> +
> +	if (i915->display.funcs.hotplug)
>  		i915->display.funcs.hotplug->hpd_irq_setup(i915);
>  }
>  
> -- 
> 2.39.5
> 

Re: [PATCH 5/5] drm/i915/irq: emphasize display_irqs_enabled is only about VLV/CHV

[Jani Nikula]

On Thu, 14 Nov 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Mon, Nov 11, 2024 at 07:53:34PM +0200, Jani Nikula wrote:
>> Rename display_irqs_enabled to vlv_display_irqs_enabled, to emphasize
>> it's really only about VLV/CHV. Only access it when running on VLV/CHV.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  .../gpu/drm/i915/display/intel_display_core.h |  9 ++++++-
>>  .../gpu/drm/i915/display/intel_display_irq.c  | 26 ++++++-------------
>>  .../gpu/drm/i915/display/intel_hotplug_irq.c  |  6 ++++-
>>  3 files changed, 21 insertions(+), 20 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
>> index 45b7c6900adc..5ad66df1a85e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
>> @@ -453,7 +453,14 @@ struct intel_display {
>>  	} ips;
>>  
>>  	struct {
>> -		bool display_irqs_enabled;
>> +		/*
>> +		 * Most platforms treat the display irq block as an always-on
>> +		 * power domain. vlv/chv can disable it at runtime and need
>> +		 * special care to avoid writing any of the display block
>> +		 * registers outside of the power domain. We defer setting up
>> +		 * the display irqs in this case to the runtime pm.
>> +		 */
>> +		bool vlv_display_irqs_enabled;
>>  
>>  		/* For i915gm/i945gm vblank irq workaround */
>>  		u8 vblank_enabled;
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
>> index d5458b0d976b..50c1ca062b80 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
>> @@ -434,7 +434,8 @@ void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv,
>>  
>>  	spin_lock(&dev_priv->irq_lock);
>>  
>> -	if (!dev_priv->display.irq.display_irqs_enabled) {
>> +	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
>> +	    !dev_priv->display.irq.vlv_display_irqs_enabled) {
>>  		spin_unlock(&dev_priv->irq_lock);
>>  		return;
>>  	}
>> @@ -1499,7 +1500,7 @@ static void _vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>>  
>>  void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>>  {
>> -	if (dev_priv->display.irq.display_irqs_enabled)
>> +	if (dev_priv->display.irq.vlv_display_irqs_enabled)
>>  		_vlv_display_irq_reset(dev_priv);
>>  }
>>  
>> @@ -1522,7 +1523,7 @@ void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
>>  	u32 enable_mask;
>>  	enum pipe pipe;
>>  
>> -	if (!dev_priv->display.irq.display_irqs_enabled)
>> +	if (!dev_priv->display.irq.vlv_display_irqs_enabled)
>>  		return;
>>  
>>  	pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
>> @@ -1697,10 +1698,10 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
>>  {
>>  	lockdep_assert_held(&dev_priv->irq_lock);
>>  
>> -	if (dev_priv->display.irq.display_irqs_enabled)
>> +	if (dev_priv->display.irq.vlv_display_irqs_enabled)
>>  		return;
>>  
>> -	dev_priv->display.irq.display_irqs_enabled = true;
>> +	dev_priv->display.irq.vlv_display_irqs_enabled = true;
>>  
>>  	if (intel_irqs_enabled(dev_priv)) {
>>  		_vlv_display_irq_reset(dev_priv);
>> @@ -1712,10 +1713,10 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
>>  {
>>  	lockdep_assert_held(&dev_priv->irq_lock);
>>  
>> -	if (!dev_priv->display.irq.display_irqs_enabled)
>> +	if (!dev_priv->display.irq.vlv_display_irqs_enabled)
>>  		return;
>>  
>> -	dev_priv->display.irq.display_irqs_enabled = false;
>> +	dev_priv->display.irq.vlv_display_irqs_enabled = false;
>>  
>>  	if (intel_irqs_enabled(dev_priv))
>>  		_vlv_display_irq_reset(dev_priv);
>> @@ -1911,17 +1912,6 @@ void intel_display_irq_init(struct drm_i915_private *i915)
>>  {
>>  	i915->drm.vblank_disable_immediate = true;
>>  
>> -	/*
>> -	 * Most platforms treat the display irq block as an always-on power
>> -	 * domain. vlv/chv can disable it at runtime and need special care to
>> -	 * avoid writing any of the display block registers outside of the power
>> -	 * domain. We defer setting up the display irqs in this case to the
>> -	 * runtime pm.
>> -	 */
>> -	i915->display.irq.display_irqs_enabled = true;
>> -	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>> -		i915->display.irq.display_irqs_enabled = false;
>> -
>>  	intel_hotplug_irq_init(i915);
>>  
>>  	INIT_WORK(&i915->display.irq.vblank_dc_work,
>> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
>> index cb64c6f0ad1b..476ac88087e0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
>> @@ -1457,7 +1457,11 @@ void intel_hpd_enable_detection(struct intel_encoder *encoder)
>>  
>>  void intel_hpd_irq_setup(struct drm_i915_private *i915)
>>  {
>> -	if (i915->display.irq.display_irqs_enabled && i915->display.funcs.hotplug)
>> +	if ((IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) &&
>
> I like the overal change, but it is not just a 'Rename' as the commit message
> tells if we are changing conditions.

The commit message does say, "Only access it when running on VLV/CHV.",
but yeah, I could rephrase it.

BR,
Jani.



>
>> +	    !i915->display.irq.vlv_display_irqs_enabled)
>> +		return;
>> +
>> +	if (i915->display.funcs.hotplug)
>>  		i915->display.funcs.hotplug->hpd_irq_setup(i915);
>>  }
>>  
>> -- 
>> 2.39.5
>> 

-- 
Jani Nikula, Intel

Re: [PATCH 5/5] drm/i915/irq: emphasize display_irqs_enabled is only about VLV/CHV

[Rodrigo Vivi]

On Fri, Nov 15, 2024 at 03:15:15PM +0200, Jani Nikula wrote:
> On Thu, 14 Nov 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Mon, Nov 11, 2024 at 07:53:34PM +0200, Jani Nikula wrote:
> >> Rename display_irqs_enabled to vlv_display_irqs_enabled, to emphasize
> >> it's really only about VLV/CHV. Only access it when running on VLV/CHV.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  .../gpu/drm/i915/display/intel_display_core.h |  9 ++++++-
> >>  .../gpu/drm/i915/display/intel_display_irq.c  | 26 ++++++-------------
> >>  .../gpu/drm/i915/display/intel_hotplug_irq.c  |  6 ++++-
> >>  3 files changed, 21 insertions(+), 20 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> >> index 45b7c6900adc..5ad66df1a85e 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> >> @@ -453,7 +453,14 @@ struct intel_display {
> >>  	} ips;
> >>  
> >>  	struct {
> >> -		bool display_irqs_enabled;
> >> +		/*
> >> +		 * Most platforms treat the display irq block as an always-on
> >> +		 * power domain. vlv/chv can disable it at runtime and need
> >> +		 * special care to avoid writing any of the display block
> >> +		 * registers outside of the power domain. We defer setting up
> >> +		 * the display irqs in this case to the runtime pm.
> >> +		 */
> >> +		bool vlv_display_irqs_enabled;
> >>  
> >>  		/* For i915gm/i945gm vblank irq workaround */
> >>  		u8 vblank_enabled;
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> >> index d5458b0d976b..50c1ca062b80 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> >> @@ -434,7 +434,8 @@ void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv,
> >>  
> >>  	spin_lock(&dev_priv->irq_lock);
> >>  
> >> -	if (!dev_priv->display.irq.display_irqs_enabled) {
> >> +	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
> >> +	    !dev_priv->display.irq.vlv_display_irqs_enabled) {
> >>  		spin_unlock(&dev_priv->irq_lock);
> >>  		return;
> >>  	}
> >> @@ -1499,7 +1500,7 @@ static void _vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> >>  
> >>  void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> >>  {
> >> -	if (dev_priv->display.irq.display_irqs_enabled)
> >> +	if (dev_priv->display.irq.vlv_display_irqs_enabled)
> >>  		_vlv_display_irq_reset(dev_priv);
> >>  }
> >>  
> >> @@ -1522,7 +1523,7 @@ void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
> >>  	u32 enable_mask;
> >>  	enum pipe pipe;
> >>  
> >> -	if (!dev_priv->display.irq.display_irqs_enabled)
> >> +	if (!dev_priv->display.irq.vlv_display_irqs_enabled)
> >>  		return;
> >>  
> >>  	pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
> >> @@ -1697,10 +1698,10 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
> >>  {
> >>  	lockdep_assert_held(&dev_priv->irq_lock);
> >>  
> >> -	if (dev_priv->display.irq.display_irqs_enabled)
> >> +	if (dev_priv->display.irq.vlv_display_irqs_enabled)
> >>  		return;
> >>  
> >> -	dev_priv->display.irq.display_irqs_enabled = true;
> >> +	dev_priv->display.irq.vlv_display_irqs_enabled = true;
> >>  
> >>  	if (intel_irqs_enabled(dev_priv)) {
> >>  		_vlv_display_irq_reset(dev_priv);
> >> @@ -1712,10 +1713,10 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
> >>  {
> >>  	lockdep_assert_held(&dev_priv->irq_lock);
> >>  
> >> -	if (!dev_priv->display.irq.display_irqs_enabled)
> >> +	if (!dev_priv->display.irq.vlv_display_irqs_enabled)
> >>  		return;
> >>  
> >> -	dev_priv->display.irq.display_irqs_enabled = false;
> >> +	dev_priv->display.irq.vlv_display_irqs_enabled = false;
> >>  
> >>  	if (intel_irqs_enabled(dev_priv))
> >>  		_vlv_display_irq_reset(dev_priv);
> >> @@ -1911,17 +1912,6 @@ void intel_display_irq_init(struct drm_i915_private *i915)
> >>  {
> >>  	i915->drm.vblank_disable_immediate = true;
> >>  
> >> -	/*
> >> -	 * Most platforms treat the display irq block as an always-on power
> >> -	 * domain. vlv/chv can disable it at runtime and need special care to
> >> -	 * avoid writing any of the display block registers outside of the power
> >> -	 * domain. We defer setting up the display irqs in this case to the
> >> -	 * runtime pm.
> >> -	 */
> >> -	i915->display.irq.display_irqs_enabled = true;
> >> -	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> >> -		i915->display.irq.display_irqs_enabled = false;
> >> -
> >>  	intel_hotplug_irq_init(i915);
> >>  
> >>  	INIT_WORK(&i915->display.irq.vblank_dc_work,
> >> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
> >> index cb64c6f0ad1b..476ac88087e0 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c
> >> @@ -1457,7 +1457,11 @@ void intel_hpd_enable_detection(struct intel_encoder *encoder)
> >>  
> >>  void intel_hpd_irq_setup(struct drm_i915_private *i915)
> >>  {
> >> -	if (i915->display.irq.display_irqs_enabled && i915->display.funcs.hotplug)
> >> +	if ((IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) &&
> >
> > I like the overal change, but it is not just a 'Rename' as the commit message
> > tells if we are changing conditions.
> 
> The commit message does say, "Only access it when running on VLV/CHV.",
> but yeah, I could rephrase it.

looking again and everything really looks good. My only problem is
with the word 'Rename' in the commit message...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> BR,
> Jani.
> 
> 
> 
> >
> >> +	    !i915->display.irq.vlv_display_irqs_enabled)
> >> +		return;
> >> +
> >> +	if (i915->display.funcs.hotplug)
> >>  		i915->display.funcs.hotplug->hpd_irq_setup(i915);
> >>  }
> >>  
> >> -- 
> >> 2.39.5
> >> 
> 
> -- 
> Jani Nikula, Intel

✗ Fi.CI.CHECKPATCH: warning for drm/i915: intel_display conversions, cleanups

[Patchwork]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups
URL   : https://patchwork.freedesktop.org/series/141176/
State : warning

== Summary ==

Error: dim checkpatch failed
8c43f2b99cec drm/i915/overlay: convert to struct intel_display
-:608: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#608: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:32:
 }
+static inline void intel_overlay_cleanup(struct intel_display *display)

-:626: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#626: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:49:
 }
+static inline void intel_overlay_reset(struct intel_display *display)

total: 0 errors, 0 warnings, 2 checks, 578 lines checked
3b4e013e3418 drm/i915/overlay: add intel_overlay_available() and use it
-:44: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#44: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:33:
 }
+static inline bool intel_overlay_available(struct intel_display *display)

-:56: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#56: FILE: drivers/gpu/drm/i915/i915_getparam.c:2:
  * SPDX-License-Identifier: MIT

total: 0 errors, 1 warnings, 1 checks, 53 lines checked
010264e35e1b drm/i915/plane: convert initial plane setup to struct intel_display
87290e739ca1 drm/i915/irq: hide display_irqs_enabled access
2bb2fb50ffd7 drm/i915/irq: emphasize display_irqs_enabled is only about VLV/CHV

✗ Fi.CI.SPARSE: warning for drm/i915: intel_display conversions, cleanups

[Patchwork]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups
URL   : https://patchwork.freedesktop.org/series/141176/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for drm/i915: intel_display conversions, cleanups

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6763 bytes --]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups
URL   : https://patchwork.freedesktop.org/series/141176/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15674 -> Patchwork_141176v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_141176v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_141176v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/index.html

Participating hosts (46 -> 45)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_141176v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live:
    - bat-dg2-14:         [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15674/bat-dg2-14/igt@i915_selftest@live.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-dg2-14/igt@i915_selftest@live.html

  
Known issues
------------

  Here are the changes found in Patchwork_141176v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-rpls-4:         NOTRUN -> [SKIP][3] ([i915#9318])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@debugfs_test@basic-hwmon.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - bat-rpls-4:         NOTRUN -> [SKIP][4] ([i915#4613]) +3 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_tiled_pread_basic:
    - bat-rpls-4:         NOTRUN -> [SKIP][5] ([i915#3282])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@gem_tiled_pread_basic.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-14:         [PASS][6] -> [DMESG-FAIL][7] ([i915#9500])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15674/bat-dg2-14/igt@i915_selftest@live@workarounds.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-dg2-14/igt@i915_selftest@live@workarounds.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-rpls-4:         NOTRUN -> [SKIP][8] ([i915#4103]) +1 other test skip
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-rpls-4:         NOTRUN -> [SKIP][9] ([i915#3555] / [i915#3840] / [i915#9886])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-rpls-4:         NOTRUN -> [SKIP][10]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-rpls-4:         NOTRUN -> [SKIP][11] ([i915#5354])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_psr@psr-primary-page-flip:
    - bat-rpls-4:         NOTRUN -> [SKIP][12] ([i915#1072] / [i915#9732]) +3 other tests skip
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@kms_psr@psr-primary-page-flip.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-rpls-4:         NOTRUN -> [SKIP][13] ([i915#3555])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-read:
    - bat-rpls-4:         NOTRUN -> [SKIP][14] ([i915#3708]) +2 other tests skip
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@prime_vgem@basic-read.html

  
#### Possible fixes ####

  * igt@i915_module_load@load:
    - bat-rpls-4:         [ABORT][15] -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15674/bat-rpls-4/igt@i915_module_load@load.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-rpls-4/igt@i915_module_load@load.html
    - bat-adlp-6:         [DMESG-WARN][17] ([i915#12253]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15674/bat-adlp-6/igt@i915_module_load@load.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-adlp-6/igt@i915_module_load@load.html

  * igt@i915_selftest@live@workarounds:
    - bat-adlp-6:         [INCOMPLETE][19] ([i915#9413]) -> [PASS][20] +1 other test pass
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15674/bat-adlp-6/igt@i915_selftest@live@workarounds.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/bat-adlp-6/igt@i915_selftest@live@workarounds.html

  
  [i915#1072]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1072
  [i915#12253]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12253
  [i915#3282]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3708
  [i915#3840]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3840
  [i915#4103]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4103
  [i915#4613]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4613
  [i915#5354]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5354
  [i915#9318]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9318
  [i915#9413]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9413
  [i915#9500]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9500
  [i915#9732]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9732
  [i915#9886]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9886


Build changes
-------------

  * Linux: CI_DRM_15674 -> Patchwork_141176v1

  CI-20190529: 20190529
  CI_DRM_15674: 6d0fd3fb958e77717cedf35f49325c7c0dfa11c9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8102: a05b40911bfb79c9bdf6ff7e8ab1a68948afbbf6 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_141176v1: 6d0fd3fb958e77717cedf35f49325c7c0dfa11c9 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v1/index.html

[-- Attachment #2: Type: text/html, Size: 7798 bytes --]

✗ Fi.CI.CHECKPATCH: warning for drm/i915: intel_display conversions, cleanups (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups (rev2)
URL   : https://patchwork.freedesktop.org/series/141176/
State : warning

== Summary ==

Error: dim checkpatch failed
b645721d453f drm/i915/overlay: convert to struct intel_display
-:609: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#609: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:32:
 }
+static inline void intel_overlay_cleanup(struct intel_display *display)

-:627: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#627: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:49:
 }
+static inline void intel_overlay_reset(struct intel_display *display)

total: 0 errors, 0 warnings, 2 checks, 578 lines checked
04eb6c7b2e09 drm/i915/overlay: add intel_overlay_available() and use it
-:45: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#45: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:33:
 }
+static inline bool intel_overlay_available(struct intel_display *display)

-:57: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#57: FILE: drivers/gpu/drm/i915/i915_getparam.c:2:
  * SPDX-License-Identifier: MIT

total: 0 errors, 1 warnings, 1 checks, 53 lines checked
e77ac58ee0a9 drm/i915/plane: convert initial plane setup to struct intel_display
4c3100f86e6e drm/i915/irq: hide display_irqs_enabled access
22cfe562a273 drm/i915/irq: emphasize display_irqs_enabled is only about VLV/CHV

✗ Fi.CI.SPARSE: warning for drm/i915: intel_display conversions, cleanups (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups (rev2)
URL   : https://patchwork.freedesktop.org/series/141176/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for drm/i915: intel_display conversions, cleanups (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 8729 bytes --]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups (rev2)
URL   : https://patchwork.freedesktop.org/series/141176/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15715 -> Patchwork_141176v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_141176v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_141176v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/index.html

Participating hosts (46 -> 45)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_141176v2:

### IGT changes ###

#### Possible regressions ####

  * igt@dmabuf@all-tests:
    - bat-apl-1:          [PASS][1] -> [INCOMPLETE][2] +1 other test incomplete
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15715/bat-apl-1/igt@dmabuf@all-tests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-apl-1/igt@dmabuf@all-tests.html

  * igt@i915_pm_rpm@module-reload:
    - bat-adls-6:         [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15715/bat-adls-6/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-adls-6/igt@i915_pm_rpm@module-reload.html

  
Known issues
------------

  Here are the changes found in Patchwork_141176v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap@basic:
    - bat-dg2-14:         NOTRUN -> [SKIP][5] ([i915#4083])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@gem_mmap@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-dg2-14:         NOTRUN -> [SKIP][6] ([i915#4079]) +1 other test skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-dg2-14:         NOTRUN -> [SKIP][7] ([i915#4077]) +2 other tests skip
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@gem_tiled_fence_blits@basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg2-14:         NOTRUN -> [SKIP][8] ([i915#11681] / [i915#6621])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@i915_pm_rps@basic-api.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-dg2-14:         NOTRUN -> [SKIP][9] ([i915#4212] / [i915#5190])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-dg2-14:         NOTRUN -> [SKIP][10] ([i915#4212] / [i915#4215] / [i915#5190])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@tile-pitch-mismatch:
    - bat-dg2-14:         NOTRUN -> [SKIP][11] ([i915#4212]) +7 other tests skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_addfb_basic@tile-pitch-mismatch.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-dg2-14:         NOTRUN -> [SKIP][12] ([i915#4103] / [i915#4213]) +1 other test skip
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-dg2-14:         NOTRUN -> [SKIP][13] ([i915#3555] / [i915#3840])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-dg2-14:         NOTRUN -> [SKIP][14]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-dg2-14:         NOTRUN -> [SKIP][15] ([i915#5274])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-dg2-14:         NOTRUN -> [SKIP][16] ([i915#5354])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_psr@psr-sprite-plane-onoff:
    - bat-dg2-14:         NOTRUN -> [SKIP][17] ([i915#1072] / [i915#9732]) +3 other tests skip
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_psr@psr-sprite-plane-onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg2-14:         NOTRUN -> [SKIP][18] ([i915#3555])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-dg2-14:         NOTRUN -> [SKIP][19] ([i915#3708])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-gtt:
    - bat-dg2-14:         NOTRUN -> [SKIP][20] ([i915#3708] / [i915#4077]) +1 other test skip
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@prime_vgem@basic-gtt.html

  * igt@prime_vgem@basic-write:
    - bat-dg2-14:         NOTRUN -> [SKIP][21] ([i915#3291] / [i915#3708]) +2 other tests skip
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-14/igt@prime_vgem@basic-write.html

  * igt@runner@aborted:
    - bat-dg2-13:         NOTRUN -> [FAIL][22] ([i915#12658])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/bat-dg2-13/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@dmabuf@all-tests@dma_fence_chain:
    - fi-bsw-nick:        [INCOMPLETE][23] -> [PASS][24] +1 other test pass
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15715/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-kbl-7567u:       [DMESG-WARN][25] -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15715/fi-kbl-7567u/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/fi-kbl-7567u/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  
  [i915#1072]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1072
  [i915#11681]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11681
  [i915#12658]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12658
  [i915#3291]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3291
  [i915#3555]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3708
  [i915#3840]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3840
  [i915#4077]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4215
  [i915#5190]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/6621
  [i915#9732]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9732


Build changes
-------------

  * Linux: CI_DRM_15715 -> Patchwork_141176v2

  CI-20190529: 20190529
  CI_DRM_15715: 4f08eb8dc2721d6ab0d1df079c2b2ef34f322fed @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8114: 8114
  Patchwork_141176v2: 4f08eb8dc2721d6ab0d1df079c2b2ef34f322fed @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v2/index.html

[-- Attachment #2: Type: text/html, Size: 10423 bytes --]

✗ Fi.CI.CHECKPATCH: warning for drm/i915: intel_display conversions, cleanups (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups (rev3)
URL   : https://patchwork.freedesktop.org/series/141176/
State : warning

== Summary ==

Error: dim checkpatch failed
d05fd1c38fb0 drm/i915/overlay: convert to struct intel_display
-:609: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#609: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:32:
 }
+static inline void intel_overlay_cleanup(struct intel_display *display)

-:627: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#627: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:49:
 }
+static inline void intel_overlay_reset(struct intel_display *display)

total: 0 errors, 0 warnings, 2 checks, 578 lines checked
6377f2781c28 drm/i915/overlay: add intel_overlay_available() and use it
-:45: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#45: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:33:
 }
+static inline bool intel_overlay_available(struct intel_display *display)

-:57: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#57: FILE: drivers/gpu/drm/i915/i915_getparam.c:2:
  * SPDX-License-Identifier: MIT

total: 0 errors, 1 warnings, 1 checks, 53 lines checked
f430efa311a6 drm/i915/plane: convert initial plane setup to struct intel_display
b1a2aab65cd3 drm/i915/irq: hide display_irqs_enabled access
c3642b13af95 drm/i915/irq: emphasize display_irqs_enabled is only about VLV/CHV

✗ Fi.CI.SPARSE: warning for drm/i915: intel_display conversions, cleanups (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups (rev3)
URL   : https://patchwork.freedesktop.org/series/141176/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for drm/i915: intel_display conversions, cleanups (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4483 bytes --]

== Series Details ==

Series: drm/i915: intel_display conversions, cleanups (rev3)
URL   : https://patchwork.freedesktop.org/series/141176/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15719 -> Patchwork_141176v3
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_141176v3 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_141176v3, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/index.html

Participating hosts (46 -> 45)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_141176v3:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_busy@basic@modeset:
    - fi-kbl-7567u:       [PASS][1] -> [DMESG-WARN][2] +1 other test dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15719/fi-kbl-7567u/igt@kms_busy@basic@modeset.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/fi-kbl-7567u/igt@kms_busy@basic@modeset.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-dp1:
    - bat-apl-1:          [PASS][3] -> [DMESG-WARN][4] +1 other test dmesg-warn
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15719/bat-apl-1/igt@kms_flip@basic-flip-vs-wf_vblank@b-dp1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/bat-apl-1/igt@kms_flip@basic-flip-vs-wf_vblank@b-dp1.html

  
Known issues
------------

  Here are the changes found in Patchwork_141176v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - bat-dg1-7:          [PASS][5] -> [FAIL][6] ([i915#12903])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15719/bat-dg1-7/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/bat-dg1-7/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live:
    - bat-arlh-3:         [PASS][7] -> [ABORT][8] ([i915#12829])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15719/bat-arlh-3/igt@i915_selftest@live.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/bat-arlh-3/igt@i915_selftest@live.html
    - bat-twl-1:          NOTRUN -> [ABORT][9] ([i915#12919]) +1 other test abort
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/bat-twl-1/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-arlh-3:         [PASS][10] -> [ABORT][11] ([i915#12061])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15719/bat-arlh-3/igt@i915_selftest@live@workarounds.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/bat-arlh-3/igt@i915_selftest@live@workarounds.html

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - bat-apl-1:          [PASS][12] -> [DMESG-WARN][13] ([i915#1982])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15719/bat-apl-1/igt@kms_flip@basic-flip-vs-wf_vblank.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/bat-apl-1/igt@kms_flip@basic-flip-vs-wf_vblank.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12829]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12829
  [i915#12903]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12903
  [i915#12919]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12919
  [i915#1982]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1982


Build changes
-------------

  * Linux: CI_DRM_15719 -> Patchwork_141176v3

  CI-20190529: 20190529
  CI_DRM_15719: 6af9eb4707b2f65024ac9412f3a9d76cc2516a9a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8116: 10a7dcaa598461073d6c7268704f32043af8722f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_141176v3: 6af9eb4707b2f65024ac9412f3a9d76cc2516a9a @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141176v3/index.html

[-- Attachment #2: Type: text/html, Size: 5249 bytes --]