All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv
@ 2021-10-22 16:27 Jani Nikula
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 1/4] drm/i915/audio: group audio under anonymous struct in drm_i915_private Jani Nikula
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Jani Nikula @ 2021-10-22 16:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: airlied, daniel, ville.syrjala, lucas.demarchi, jani.nikula

I'll first make the argument that drm_i915_private should not have a
single member that is not a named sub-struct defined separately. It's
just too big to have individual members like, say, framestart_delay. All
of these need context that is created using C constructs, not comments.

Ideas have been thrown around about splitting display data more, perhaps
to a separate "display private" struct. I'll also make the argument that
it is not fine-grained enough. Here, I'm taking the audio parts and
shoving them under intel_audio_private struct. It's still defined in
i915_drv.h, but it could be trivially turned into a completely opaque
type defined and allocated in intel_audio.c.

I think the driver is big enough to start requiring this kind of
abstractions, even at the cost of dynamic allocations and pointer
chasing that opaque types require. It's just too easy to poke at
drm_i915_private from wherever, breaking all abstractions.

Sure, it's no coincidence that I chose audio, it's fairly well isolated
already. Not all parts are going to be that easy. But I'm not sure I
like the thought of doing a massive display private struct refactoring
and not take one step further. Or, indeed, take this step *first*.

BR,
Jani.


Jani Nikula (4):
  drm/i915/audio: group audio under anonymous struct in drm_i915_private
  drm/i915/audio: name the audio sub-struct in drm_i915_private
  drm/i915/audio: define the audio struct separately from
    drm_i915_private
  drm/i915/audio: move intel_audio_funcs internal to intel_audio.c

 drivers/gpu/drm/i915/display/intel_audio.c    | 99 ++++++++++---------
 .../gpu/drm/i915/display/intel_lpe_audio.c    | 42 ++++----
 drivers/gpu/drm/i915/i915_drv.c               |  2 +-
 drivers/gpu/drm/i915/i915_drv.h               | 56 +++++------
 4 files changed, 101 insertions(+), 98 deletions(-)

-- 
2.30.2


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

* [Intel-gfx] [PATCH 1/4] drm/i915/audio: group audio under anonymous struct in drm_i915_private
  2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
@ 2021-10-22 16:27 ` Jani Nikula
  2021-10-22 17:22   ` Lucas De Marchi
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 2/4] drm/i915/audio: name the audio sub-struct " Jani Nikula
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2021-10-22 16:27 UTC (permalink / raw)
  To: intel-gfx
  Cc: airlied, daniel, ville.syrjala, lucas.demarchi, jani.nikula, Dave Airlie

With an anonymous struct, this can be pure hierarchical organization
without code changes.

Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 43 +++++++++++++++++----------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 19e6700a4315..ee210e12648a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -995,9 +995,6 @@ struct drm_i915_private {
 	/* Display internal color functions */
 	const struct intel_color_funcs *color_funcs;
 
-	/* Display internal audio functions */
-	const struct intel_audio_funcs *audio_funcs;
-
 	/* Display CDCLK functions */
 	const struct intel_cdclk_funcs *cdclk_funcs;
 
@@ -1084,17 +1081,6 @@ struct drm_i915_private {
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
 
-	/* hda/i915 audio component */
-	struct i915_audio_component *audio_component;
-	bool audio_component_registered;
-	/**
-	 * av_mutex - mutex for audio/video sync
-	 *
-	 */
-	struct mutex av_mutex;
-	int audio_power_refcount;
-	u32 audio_freq_cntrl;
-
 	u32 fdi_rx_config;
 
 	/* Shadow for DISPLAY_PHY_CONTROL which can't be safely read */
@@ -1227,14 +1213,29 @@ struct drm_i915_private {
 
 	bool ipc_enabled;
 
-	/* Used to save the pipe-to-encoder mapping for audio */
-	struct intel_encoder *av_enc_map[I915_MAX_PIPES];
-
-	/* necessary resource sharing with HDMI LPE audio driver. */
 	struct {
-		struct platform_device *platdev;
-		int	irq;
-	} lpe_audio;
+		/* Display internal audio functions */
+		const struct intel_audio_funcs *audio_funcs;
+
+		/* hda/i915 audio component */
+		struct i915_audio_component *audio_component;
+		bool audio_component_registered;
+		/**
+		 * av_mutex - mutex for audio/video sync
+		 */
+		struct mutex av_mutex;
+		int audio_power_refcount;
+		u32 audio_freq_cntrl;
+
+		/* Used to save the pipe-to-encoder mapping for audio */
+		struct intel_encoder *av_enc_map[I915_MAX_PIPES];
+
+		/* necessary resource sharing with HDMI LPE audio driver. */
+		struct {
+			struct platform_device *platdev;
+			int irq;
+		} lpe_audio;
+	};
 
 	struct i915_pmu pmu;
 
-- 
2.30.2


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

* [Intel-gfx] [PATCH 2/4] drm/i915/audio: name the audio sub-struct in drm_i915_private
  2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 1/4] drm/i915/audio: group audio under anonymous struct in drm_i915_private Jani Nikula
@ 2021-10-22 16:27 ` Jani Nikula
  2021-10-22 16:42   ` Ville Syrjälä
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private Jani Nikula
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2021-10-22 16:27 UTC (permalink / raw)
  To: intel-gfx
  Cc: airlied, daniel, ville.syrjala, lucas.demarchi, jani.nikula, Dave Airlie

Add name to the audio sub-struct in drm_i915_private, and remove the
tautologies and other inconsistencies in the member names.

Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c    | 90 +++++++++----------
 .../gpu/drm/i915/display/intel_lpe_audio.c    | 42 ++++-----
 drivers/gpu/drm/i915/i915_drv.c               |  2 +-
 drivers/gpu/drm/i915/i915_drv.h               | 22 +++--
 4 files changed, 77 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 03e8c05a74f6..b46f6539b9ab 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -388,7 +388,7 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder,
 			   const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct i915_audio_component *acomp = dev_priv->audio_component;
+	struct i915_audio_component *acomp = dev_priv->audio.component;
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	enum port port = encoder->port;
 	const struct dp_aud_n_m *nm;
@@ -436,7 +436,7 @@ hsw_hdmi_audio_config_update(struct intel_encoder *encoder,
 			     const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct i915_audio_component *acomp = dev_priv->audio_component;
+	struct i915_audio_component *acomp = dev_priv->audio.component;
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	enum port port = encoder->port;
 	int n, rate;
@@ -494,7 +494,7 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
 	drm_dbg_kms(&dev_priv->drm, "Disable audio codec on transcoder %s\n",
 		    transcoder_name(cpu_transcoder));
 
-	mutex_lock(&dev_priv->av_mutex);
+	mutex_lock(&dev_priv->audio.lock);
 
 	/* Disable timestamps */
 	tmp = intel_de_read(dev_priv, HSW_AUD_CFG(cpu_transcoder));
@@ -512,7 +512,7 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
 	tmp &= ~AUDIO_OUTPUT_ENABLE(cpu_transcoder);
 	intel_de_write(dev_priv, HSW_AUD_PIN_ELD_CP_VLD, tmp);
 
-	mutex_unlock(&dev_priv->av_mutex);
+	mutex_unlock(&dev_priv->audio.lock);
 }
 
 static unsigned int calc_hblank_early_prog(struct intel_encoder *encoder,
@@ -641,7 +641,7 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
 		    "Enable audio codec on transcoder %s, %u bytes ELD\n",
 		     transcoder_name(cpu_transcoder), drm_eld_size(eld));
 
-	mutex_lock(&dev_priv->av_mutex);
+	mutex_lock(&dev_priv->audio.lock);
 
 	/* Enable Audio WA for 4k DSC usecases */
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
@@ -679,7 +679,7 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
 	/* Enable timestamps */
 	hsw_audio_config_update(encoder, crtc_state);
 
-	mutex_unlock(&dev_priv->av_mutex);
+	mutex_unlock(&dev_priv->audio.lock);
 }
 
 static void ilk_audio_codec_disable(struct intel_encoder *encoder,
@@ -826,7 +826,7 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
 			      const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct i915_audio_component *acomp = dev_priv->audio_component;
+	struct i915_audio_component *acomp = dev_priv->audio.component;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_connector *connector = conn_state->connector;
 	const struct drm_display_mode *adjusted_mode =
@@ -848,17 +848,17 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
 
 	connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
 
-	if (dev_priv->audio_funcs)
-		dev_priv->audio_funcs->audio_codec_enable(encoder,
+	if (dev_priv->audio.funcs)
+		dev_priv->audio.funcs->audio_codec_enable(encoder,
 							  crtc_state,
 							  conn_state);
 
-	mutex_lock(&dev_priv->av_mutex);
+	mutex_lock(&dev_priv->audio.lock);
 	encoder->audio_connector = connector;
 
 	/* referred in audio callbacks */
-	dev_priv->av_enc_map[pipe] = encoder;
-	mutex_unlock(&dev_priv->av_mutex);
+	dev_priv->audio.encoder_map[pipe] = encoder;
+	mutex_unlock(&dev_priv->audio.lock);
 
 	if (acomp && acomp->base.audio_ops &&
 	    acomp->base.audio_ops->pin_eld_notify) {
@@ -888,20 +888,20 @@ void intel_audio_codec_disable(struct intel_encoder *encoder,
 			       const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct i915_audio_component *acomp = dev_priv->audio_component;
+	struct i915_audio_component *acomp = dev_priv->audio.component;
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
 	enum port port = encoder->port;
 	enum pipe pipe = crtc->pipe;
 
-	if (dev_priv->audio_funcs)
-		dev_priv->audio_funcs->audio_codec_disable(encoder,
+	if (dev_priv->audio.funcs)
+		dev_priv->audio.funcs->audio_codec_disable(encoder,
 							   old_crtc_state,
 							   old_conn_state);
 
-	mutex_lock(&dev_priv->av_mutex);
+	mutex_lock(&dev_priv->audio.lock);
 	encoder->audio_connector = NULL;
-	dev_priv->av_enc_map[pipe] = NULL;
-	mutex_unlock(&dev_priv->av_mutex);
+	dev_priv->audio.encoder_map[pipe] = NULL;
+	mutex_unlock(&dev_priv->audio.lock);
 
 	if (acomp && acomp->base.audio_ops &&
 	    acomp->base.audio_ops->pin_eld_notify) {
@@ -937,13 +937,13 @@ static const struct intel_audio_funcs hsw_audio_funcs = {
 void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
 {
 	if (IS_G4X(dev_priv)) {
-		dev_priv->audio_funcs = &g4x_audio_funcs;
+		dev_priv->audio.funcs = &g4x_audio_funcs;
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		dev_priv->audio_funcs = &ilk_audio_funcs;
+		dev_priv->audio.funcs = &ilk_audio_funcs;
 	} else if (IS_HASWELL(dev_priv) || DISPLAY_VER(dev_priv) >= 8) {
-		dev_priv->audio_funcs = &hsw_audio_funcs;
+		dev_priv->audio.funcs = &hsw_audio_funcs;
 	} else if (HAS_PCH_SPLIT(dev_priv)) {
-		dev_priv->audio_funcs = &ilk_audio_funcs;
+		dev_priv->audio.funcs = &ilk_audio_funcs;
 	}
 }
 
@@ -1014,13 +1014,13 @@ static unsigned long i915_audio_component_get_power(struct device *kdev)
 
 	ret = intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO_PLAYBACK);
 
-	if (dev_priv->audio_power_refcount++ == 0) {
+	if (dev_priv->audio.power_refcount++ == 0) {
 		if (DISPLAY_VER(dev_priv) >= 9) {
 			intel_de_write(dev_priv, AUD_FREQ_CNTRL,
-				       dev_priv->audio_freq_cntrl);
+				       dev_priv->audio.freq_cntrl);
 			drm_dbg_kms(&dev_priv->drm,
 				    "restored AUD_FREQ_CNTRL to 0x%x\n",
-				    dev_priv->audio_freq_cntrl);
+				    dev_priv->audio.freq_cntrl);
 		}
 
 		/* Force CDCLK to 2*BCLK as long as we need audio powered. */
@@ -1041,7 +1041,7 @@ static void i915_audio_component_put_power(struct device *kdev,
 	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
 
 	/* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
-	if (--dev_priv->audio_power_refcount == 0)
+	if (--dev_priv->audio.power_refcount == 0)
 		if (IS_GEMINILAKE(dev_priv))
 			glk_force_audio_cdclk(dev_priv, false);
 
@@ -1093,7 +1093,7 @@ static int i915_audio_component_get_cdclk_freq(struct device *kdev)
 /*
  * get the intel_encoder according to the parameter port and pipe
  * intel_encoder is saved by the index of pipe
- * MST & (pipe >= 0): return the av_enc_map[pipe],
+ * MST & (pipe >= 0): return the audio.encoder_map[pipe],
  *   when port is matched
  * MST & (pipe < 0): this is invalid
  * Non-MST & (pipe >= 0): only pipe = 0 (the first device entry)
@@ -1108,10 +1108,10 @@ static struct intel_encoder *get_saved_enc(struct drm_i915_private *dev_priv,
 	/* MST */
 	if (pipe >= 0) {
 		if (drm_WARN_ON(&dev_priv->drm,
-				pipe >= ARRAY_SIZE(dev_priv->av_enc_map)))
+				pipe >= ARRAY_SIZE(dev_priv->audio.encoder_map)))
 			return NULL;
 
-		encoder = dev_priv->av_enc_map[pipe];
+		encoder = dev_priv->audio.encoder_map[pipe];
 		/*
 		 * when bootup, audio driver may not know it is
 		 * MST or not. So it will poll all the port & pipe
@@ -1127,7 +1127,7 @@ static struct intel_encoder *get_saved_enc(struct drm_i915_private *dev_priv,
 		return NULL;
 
 	for_each_pipe(dev_priv, pipe) {
-		encoder = dev_priv->av_enc_map[pipe];
+		encoder = dev_priv->audio.encoder_map[pipe];
 		if (encoder == NULL)
 			continue;
 
@@ -1145,7 +1145,7 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 						int pipe, int rate)
 {
 	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
-	struct i915_audio_component *acomp = dev_priv->audio_component;
+	struct i915_audio_component *acomp = dev_priv->audio.component;
 	struct intel_encoder *encoder;
 	struct intel_crtc *crtc;
 	unsigned long cookie;
@@ -1155,7 +1155,7 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 		return 0;
 
 	cookie = i915_audio_component_get_power(kdev);
-	mutex_lock(&dev_priv->av_mutex);
+	mutex_lock(&dev_priv->audio.lock);
 
 	/* 1. get the pipe */
 	encoder = get_saved_enc(dev_priv, port, pipe);
@@ -1174,7 +1174,7 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 	hsw_audio_config_update(encoder, crtc->config);
 
  unlock:
-	mutex_unlock(&dev_priv->av_mutex);
+	mutex_unlock(&dev_priv->audio.lock);
 	i915_audio_component_put_power(kdev, cookie);
 	return err;
 }
@@ -1188,13 +1188,13 @@ static int i915_audio_component_get_eld(struct device *kdev, int port,
 	const u8 *eld;
 	int ret = -EINVAL;
 
-	mutex_lock(&dev_priv->av_mutex);
+	mutex_lock(&dev_priv->audio.lock);
 
 	intel_encoder = get_saved_enc(dev_priv, port, pipe);
 	if (!intel_encoder) {
 		drm_dbg_kms(&dev_priv->drm, "Not valid for port %c\n",
 			    port_name(port));
-		mutex_unlock(&dev_priv->av_mutex);
+		mutex_unlock(&dev_priv->audio.lock);
 		return ret;
 	}
 
@@ -1206,7 +1206,7 @@ static int i915_audio_component_get_eld(struct device *kdev, int port,
 		memcpy(buf, eld, min(max_bytes, ret));
 	}
 
-	mutex_unlock(&dev_priv->av_mutex);
+	mutex_unlock(&dev_priv->audio.lock);
 	return ret;
 }
 
@@ -1241,7 +1241,7 @@ static int i915_audio_component_bind(struct device *i915_kdev,
 	BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
 	for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
 		acomp->aud_sample_rate[i] = 0;
-	dev_priv->audio_component = acomp;
+	dev_priv->audio.component = acomp;
 	drm_modeset_unlock_all(&dev_priv->drm);
 
 	return 0;
@@ -1256,14 +1256,14 @@ static void i915_audio_component_unbind(struct device *i915_kdev,
 	drm_modeset_lock_all(&dev_priv->drm);
 	acomp->base.ops = NULL;
 	acomp->base.dev = NULL;
-	dev_priv->audio_component = NULL;
+	dev_priv->audio.component = NULL;
 	drm_modeset_unlock_all(&dev_priv->drm);
 
 	device_link_remove(hda_kdev, i915_kdev);
 
-	if (dev_priv->audio_power_refcount)
+	if (dev_priv->audio.power_refcount)
 		drm_err(&dev_priv->drm, "audio power refcount %d after unbind\n",
-			dev_priv->audio_power_refcount);
+			dev_priv->audio.power_refcount);
 }
 
 static const struct component_ops i915_audio_component_bind_ops = {
@@ -1327,10 +1327,10 @@ static void i915_audio_component_init(struct drm_i915_private *dev_priv)
 		drm_dbg_kms(&dev_priv->drm, "use AUD_FREQ_CNTRL of 0x%x (init value 0x%x)\n",
 			    aud_freq, aud_freq_init);
 
-		dev_priv->audio_freq_cntrl = aud_freq;
+		dev_priv->audio.freq_cntrl = aud_freq;
 	}
 
-	dev_priv->audio_component_registered = true;
+	dev_priv->audio.component_registered = true;
 }
 
 /**
@@ -1342,11 +1342,11 @@ static void i915_audio_component_init(struct drm_i915_private *dev_priv)
  */
 static void i915_audio_component_cleanup(struct drm_i915_private *dev_priv)
 {
-	if (!dev_priv->audio_component_registered)
+	if (!dev_priv->audio.component_registered)
 		return;
 
 	component_del(dev_priv->drm.dev, &i915_audio_component_bind_ops);
-	dev_priv->audio_component_registered = false;
+	dev_priv->audio.component_registered = false;
 }
 
 /**
@@ -1368,7 +1368,7 @@ void intel_audio_init(struct drm_i915_private *dev_priv)
  */
 void intel_audio_deinit(struct drm_i915_private *dev_priv)
 {
-	if ((dev_priv)->lpe_audio.platdev != NULL)
+	if ((dev_priv)->audio.lpe.platdev != NULL)
 		intel_lpe_audio_teardown(dev_priv);
 	else
 		i915_audio_component_cleanup(dev_priv);
diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.c b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
index 7f3c638c8950..4970bf146c4a 100644
--- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
@@ -74,7 +74,7 @@
 #include "intel_de.h"
 #include "intel_lpe_audio.h"
 
-#define HAS_LPE_AUDIO(dev_priv) ((dev_priv)->lpe_audio.platdev != NULL)
+#define HAS_LPE_AUDIO(dev_priv) ((dev_priv)->audio.lpe.platdev != NULL)
 
 static struct platform_device *
 lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
@@ -96,7 +96,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	rsc[0].start    = rsc[0].end = dev_priv->lpe_audio.irq;
+	rsc[0].start    = rsc[0].end = dev_priv->audio.lpe.irq;
 	rsc[0].flags    = IORESOURCE_IRQ;
 	rsc[0].name     = "hdmi-lpe-audio-irq";
 
@@ -148,7 +148,7 @@ static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
 	 * than us fiddle with its internals.
 	 */
 
-	platform_device_unregister(dev_priv->lpe_audio.platdev);
+	platform_device_unregister(dev_priv->audio.lpe.platdev);
 }
 
 static void lpe_audio_irq_unmask(struct irq_data *d)
@@ -167,7 +167,7 @@ static struct irq_chip lpe_audio_irqchip = {
 
 static int lpe_audio_irq_init(struct drm_i915_private *dev_priv)
 {
-	int irq = dev_priv->lpe_audio.irq;
+	int irq = dev_priv->audio.lpe.irq;
 
 	drm_WARN_ON(&dev_priv->drm, !intel_irqs_enabled(dev_priv));
 	irq_set_chip_and_handler_name(irq,
@@ -204,15 +204,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
 {
 	int ret;
 
-	dev_priv->lpe_audio.irq = irq_alloc_desc(0);
-	if (dev_priv->lpe_audio.irq < 0) {
+	dev_priv->audio.lpe.irq = irq_alloc_desc(0);
+	if (dev_priv->audio.lpe.irq < 0) {
 		drm_err(&dev_priv->drm, "Failed to allocate IRQ desc: %d\n",
-			dev_priv->lpe_audio.irq);
-		ret = dev_priv->lpe_audio.irq;
+			dev_priv->audio.lpe.irq);
+		ret = dev_priv->audio.lpe.irq;
 		goto err;
 	}
 
-	drm_dbg(&dev_priv->drm, "irq = %d\n", dev_priv->lpe_audio.irq);
+	drm_dbg(&dev_priv->drm, "irq = %d\n", dev_priv->audio.lpe.irq);
 
 	ret = lpe_audio_irq_init(dev_priv);
 
@@ -223,10 +223,10 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
 		goto err_free_irq;
 	}
 
-	dev_priv->lpe_audio.platdev = lpe_audio_platdev_create(dev_priv);
+	dev_priv->audio.lpe.platdev = lpe_audio_platdev_create(dev_priv);
 
-	if (IS_ERR(dev_priv->lpe_audio.platdev)) {
-		ret = PTR_ERR(dev_priv->lpe_audio.platdev);
+	if (IS_ERR(dev_priv->audio.lpe.platdev)) {
+		ret = PTR_ERR(dev_priv->audio.lpe.platdev);
 		drm_err(&dev_priv->drm,
 			"Failed to create lpe audio platform device: %d\n",
 			ret);
@@ -241,10 +241,10 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
 
 	return 0;
 err_free_irq:
-	irq_free_desc(dev_priv->lpe_audio.irq);
+	irq_free_desc(dev_priv->audio.lpe.irq);
 err:
-	dev_priv->lpe_audio.irq = -1;
-	dev_priv->lpe_audio.platdev = NULL;
+	dev_priv->audio.lpe.irq = -1;
+	dev_priv->audio.lpe.platdev = NULL;
 	return ret;
 }
 
@@ -262,7 +262,7 @@ void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv)
 	if (!HAS_LPE_AUDIO(dev_priv))
 		return;
 
-	ret = generic_handle_irq(dev_priv->lpe_audio.irq);
+	ret = generic_handle_irq(dev_priv->audio.lpe.irq);
 	if (ret)
 		drm_err_ratelimited(&dev_priv->drm,
 				    "error handling LPE audio irq: %d\n", ret);
@@ -303,10 +303,10 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
 
 	lpe_audio_platdev_destroy(dev_priv);
 
-	irq_free_desc(dev_priv->lpe_audio.irq);
+	irq_free_desc(dev_priv->audio.lpe.irq);
 
-	dev_priv->lpe_audio.irq = -1;
-	dev_priv->lpe_audio.platdev = NULL;
+	dev_priv->audio.lpe.irq = -1;
+	dev_priv->audio.lpe.platdev = NULL;
 }
 
 /**
@@ -333,7 +333,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 	if (!HAS_LPE_AUDIO(dev_priv))
 		return;
 
-	pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
+	pdata = dev_get_platdata(&dev_priv->audio.lpe.platdev->dev);
 	ppdata = &pdata->port[port - PORT_B];
 
 	spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
@@ -361,7 +361,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 	}
 
 	if (pdata->notify_audio_lpe)
-		pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port - PORT_B);
+		pdata->notify_audio_lpe(dev_priv->audio.lpe.platdev, port - PORT_B);
 
 	spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1e5b75ae9932..a56bb7479bfd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -323,7 +323,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	mutex_init(&dev_priv->sb_lock);
 	cpu_latency_qos_add_request(&dev_priv->sb_qos, PM_QOS_DEFAULT_VALUE);
 
-	mutex_init(&dev_priv->av_mutex);
+	mutex_init(&dev_priv->audio.lock);
 	mutex_init(&dev_priv->wm.wm_mutex);
 	mutex_init(&dev_priv->pps_mutex);
 	mutex_init(&dev_priv->hdcp_comp_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ee210e12648a..9c77610acf23 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1215,27 +1215,25 @@ struct drm_i915_private {
 
 	struct {
 		/* Display internal audio functions */
-		const struct intel_audio_funcs *audio_funcs;
+		const struct intel_audio_funcs *funcs;
 
 		/* hda/i915 audio component */
-		struct i915_audio_component *audio_component;
-		bool audio_component_registered;
-		/**
-		 * av_mutex - mutex for audio/video sync
-		 */
-		struct mutex av_mutex;
-		int audio_power_refcount;
-		u32 audio_freq_cntrl;
+		struct i915_audio_component *component;
+		bool component_registered;
+		/* mutex for audio/video sync */
+		struct mutex lock;
+		int power_refcount;
+		u32 freq_cntrl;
 
 		/* Used to save the pipe-to-encoder mapping for audio */
-		struct intel_encoder *av_enc_map[I915_MAX_PIPES];
+		struct intel_encoder *encoder_map[I915_MAX_PIPES];
 
 		/* necessary resource sharing with HDMI LPE audio driver. */
 		struct {
 			struct platform_device *platdev;
 			int irq;
-		} lpe_audio;
-	};
+		} lpe;
+	} audio;
 
 	struct i915_pmu pmu;
 
-- 
2.30.2


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

* [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private
  2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 1/4] drm/i915/audio: group audio under anonymous struct in drm_i915_private Jani Nikula
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 2/4] drm/i915/audio: name the audio sub-struct " Jani Nikula
@ 2021-10-22 16:27 ` Jani Nikula
  2021-10-22 16:44   ` Ville Syrjälä
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 4/4] drm/i915/audio: move intel_audio_funcs internal to intel_audio.c Jani Nikula
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2021-10-22 16:27 UTC (permalink / raw)
  To: intel-gfx
  Cc: airlied, daniel, ville.syrjala, lucas.demarchi, jani.nikula, Dave Airlie

Add a standalone definition of struct intel_audio_private, and note that
all of it is private to intel_audio.c.

Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 45 ++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9c77610acf23..ed86633a587b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -828,6 +828,29 @@ struct i915_selftest_stash {
 	struct ida mock_region_instances;
 };
 
+/* intel_audio.c private */
+struct intel_audio_private {
+	/* Display internal audio functions */
+	const struct intel_audio_funcs *funcs;
+
+	/* hda/i915 audio component */
+	struct i915_audio_component *component;
+	bool component_registered;
+	/* mutex for audio/video sync */
+	struct mutex lock;
+	int power_refcount;
+	u32 freq_cntrl;
+
+	/* Used to save the pipe-to-encoder mapping for audio */
+	struct intel_encoder *encoder_map[I915_MAX_PIPES];
+
+	/* necessary resource sharing with HDMI LPE audio driver. */
+	struct {
+		struct platform_device *platdev;
+		int irq;
+	} lpe;
+};
+
 struct drm_i915_private {
 	struct drm_device drm;
 
@@ -1213,27 +1236,7 @@ struct drm_i915_private {
 
 	bool ipc_enabled;
 
-	struct {
-		/* Display internal audio functions */
-		const struct intel_audio_funcs *funcs;
-
-		/* hda/i915 audio component */
-		struct i915_audio_component *component;
-		bool component_registered;
-		/* mutex for audio/video sync */
-		struct mutex lock;
-		int power_refcount;
-		u32 freq_cntrl;
-
-		/* Used to save the pipe-to-encoder mapping for audio */
-		struct intel_encoder *encoder_map[I915_MAX_PIPES];
-
-		/* necessary resource sharing with HDMI LPE audio driver. */
-		struct {
-			struct platform_device *platdev;
-			int irq;
-		} lpe;
-	} audio;
+	struct intel_audio_private audio;
 
 	struct i915_pmu pmu;
 
-- 
2.30.2


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

* [Intel-gfx] [PATCH 4/4] drm/i915/audio: move intel_audio_funcs internal to intel_audio.c
  2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
                   ` (2 preceding siblings ...)
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private Jani Nikula
@ 2021-10-22 16:27 ` Jani Nikula
  2021-10-22 16:48   ` Ville Syrjälä
  2021-10-22 17:29 ` [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Lucas De Marchi
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2021-10-22 16:27 UTC (permalink / raw)
  To: intel-gfx
  Cc: airlied, daniel, ville.syrjala, lucas.demarchi, jani.nikula, Dave Airlie

It's all internal to intel_audio.c.

Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c |  9 +++++++++
 drivers/gpu/drm/i915/i915_drv.h            | 10 +---------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index b46f6539b9ab..39246f64fb39 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -62,6 +62,15 @@
  * struct &i915_audio_component_audio_ops @audio_ops is called from i915 driver.
  */
 
+struct intel_audio_funcs {
+	void (*audio_codec_enable)(struct intel_encoder *encoder,
+				   const struct intel_crtc_state *crtc_state,
+				   const struct drm_connector_state *conn_state);
+	void (*audio_codec_disable)(struct intel_encoder *encoder,
+				    const struct intel_crtc_state *old_crtc_state,
+				    const struct drm_connector_state *old_conn_state);
+};
+
 /* DP N/M table */
 #define LC_810M	810000
 #define LC_540M	540000
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ed86633a587b..bb023b909e86 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -364,15 +364,6 @@ struct intel_color_funcs {
 	void (*read_luts)(struct intel_crtc_state *crtc_state);
 };
 
-struct intel_audio_funcs {
-	void (*audio_codec_enable)(struct intel_encoder *encoder,
-				   const struct intel_crtc_state *crtc_state,
-				   const struct drm_connector_state *conn_state);
-	void (*audio_codec_disable)(struct intel_encoder *encoder,
-				    const struct intel_crtc_state *old_crtc_state,
-				    const struct drm_connector_state *old_conn_state);
-};
-
 struct intel_cdclk_funcs {
 	void (*get_cdclk)(struct drm_i915_private *dev_priv,
 			  struct intel_cdclk_config *cdclk_config);
@@ -829,6 +820,7 @@ struct i915_selftest_stash {
 };
 
 /* intel_audio.c private */
+struct intel_audio_funcs;
 struct intel_audio_private {
 	/* Display internal audio functions */
 	const struct intel_audio_funcs *funcs;
-- 
2.30.2


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

* Re: [Intel-gfx] [PATCH 2/4] drm/i915/audio: name the audio sub-struct in drm_i915_private
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 2/4] drm/i915/audio: name the audio sub-struct " Jani Nikula
@ 2021-10-22 16:42   ` Ville Syrjälä
  2021-10-22 16:58     ` Jani Nikula
  0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2021-10-22 16:42 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, airlied, daniel, lucas.demarchi, Dave Airlie

On Fri, Oct 22, 2021 at 07:27:56PM +0300, Jani Nikula wrote:
> Add name to the audio sub-struct in drm_i915_private, and remove the
> tautologies and other inconsistencies in the member names.
> 
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c    | 90 +++++++++----------
>  .../gpu/drm/i915/display/intel_lpe_audio.c    | 42 ++++-----
>  drivers/gpu/drm/i915/i915_drv.c               |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h               | 22 +++--
>  4 files changed, 77 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 03e8c05a74f6..b46f6539b9ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -388,7 +388,7 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder,
>  			   const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct i915_audio_component *acomp = dev_priv->audio_component;
> +	struct i915_audio_component *acomp = dev_priv->audio.component;
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	enum port port = encoder->port;
>  	const struct dp_aud_n_m *nm;
> @@ -436,7 +436,7 @@ hsw_hdmi_audio_config_update(struct intel_encoder *encoder,
>  			     const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct i915_audio_component *acomp = dev_priv->audio_component;
> +	struct i915_audio_component *acomp = dev_priv->audio.component;
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	enum port port = encoder->port;
>  	int n, rate;
> @@ -494,7 +494,7 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
>  	drm_dbg_kms(&dev_priv->drm, "Disable audio codec on transcoder %s\n",
>  		    transcoder_name(cpu_transcoder));
>  
> -	mutex_lock(&dev_priv->av_mutex);
> +	mutex_lock(&dev_priv->audio.lock);

Maybe we should try to be consistent in our lock names and
call mutexes 'mutex' and spinlocks 'lock'?

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private Jani Nikula
@ 2021-10-22 16:44   ` Ville Syrjälä
  2021-10-22 17:07     ` Jani Nikula
  0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2021-10-22 16:44 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, airlied, daniel, lucas.demarchi, Dave Airlie

On Fri, Oct 22, 2021 at 07:27:57PM +0300, Jani Nikula wrote:
> Add a standalone definition of struct intel_audio_private, and note that
> all of it is private to intel_audio.c.
> 
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 45 ++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9c77610acf23..ed86633a587b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -828,6 +828,29 @@ struct i915_selftest_stash {
>  	struct ida mock_region_instances;
>  };
>  
> +/* intel_audio.c private */
> +struct intel_audio_private {

Not sure the "_private" is actually useful. I'd just call it
intel_audio. The fact that struct drm_i915_private
already says "private" doesn't mean anything to anyone anyway.

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH 4/4] drm/i915/audio: move intel_audio_funcs internal to intel_audio.c
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 4/4] drm/i915/audio: move intel_audio_funcs internal to intel_audio.c Jani Nikula
@ 2021-10-22 16:48   ` Ville Syrjälä
  0 siblings, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2021-10-22 16:48 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, airlied, daniel, lucas.demarchi, Dave Airlie

On Fri, Oct 22, 2021 at 07:27:58PM +0300, Jani Nikula wrote:
> It's all internal to intel_audio.c.
> 
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

I've toyed around with similar stuff myself before
(moving bunch of .av_foo/.audio_foo to .audio.foo) just
never bothered sending it out. So very much like the idea.

Series is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_audio.c |  9 +++++++++
>  drivers/gpu/drm/i915/i915_drv.h            | 10 +---------
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index b46f6539b9ab..39246f64fb39 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -62,6 +62,15 @@
>   * struct &i915_audio_component_audio_ops @audio_ops is called from i915 driver.
>   */
>  
> +struct intel_audio_funcs {
> +	void (*audio_codec_enable)(struct intel_encoder *encoder,
> +				   const struct intel_crtc_state *crtc_state,
> +				   const struct drm_connector_state *conn_state);
> +	void (*audio_codec_disable)(struct intel_encoder *encoder,
> +				    const struct intel_crtc_state *old_crtc_state,
> +				    const struct drm_connector_state *old_conn_state);
> +};
> +
>  /* DP N/M table */
>  #define LC_810M	810000
>  #define LC_540M	540000
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ed86633a587b..bb023b909e86 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -364,15 +364,6 @@ struct intel_color_funcs {
>  	void (*read_luts)(struct intel_crtc_state *crtc_state);
>  };
>  
> -struct intel_audio_funcs {
> -	void (*audio_codec_enable)(struct intel_encoder *encoder,
> -				   const struct intel_crtc_state *crtc_state,
> -				   const struct drm_connector_state *conn_state);
> -	void (*audio_codec_disable)(struct intel_encoder *encoder,
> -				    const struct intel_crtc_state *old_crtc_state,
> -				    const struct drm_connector_state *old_conn_state);
> -};
> -
>  struct intel_cdclk_funcs {
>  	void (*get_cdclk)(struct drm_i915_private *dev_priv,
>  			  struct intel_cdclk_config *cdclk_config);
> @@ -829,6 +820,7 @@ struct i915_selftest_stash {
>  };
>  
>  /* intel_audio.c private */
> +struct intel_audio_funcs;
>  struct intel_audio_private {
>  	/* Display internal audio functions */
>  	const struct intel_audio_funcs *funcs;
> -- 
> 2.30.2

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH 2/4] drm/i915/audio: name the audio sub-struct in drm_i915_private
  2021-10-22 16:42   ` Ville Syrjälä
@ 2021-10-22 16:58     ` Jani Nikula
  0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2021-10-22 16:58 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, airlied, daniel, lucas.demarchi, Dave Airlie

On Fri, 22 Oct 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Oct 22, 2021 at 07:27:56PM +0300, Jani Nikula wrote:
>> Add name to the audio sub-struct in drm_i915_private, and remove the
>> tautologies and other inconsistencies in the member names.
>> 
>> Cc: Dave Airlie <airlied@redhat.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_audio.c    | 90 +++++++++----------
>>  .../gpu/drm/i915/display/intel_lpe_audio.c    | 42 ++++-----
>>  drivers/gpu/drm/i915/i915_drv.c               |  2 +-
>>  drivers/gpu/drm/i915/i915_drv.h               | 22 +++--
>>  4 files changed, 77 insertions(+), 79 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
>> index 03e8c05a74f6..b46f6539b9ab 100644
>> --- a/drivers/gpu/drm/i915/display/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
>> @@ -388,7 +388,7 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder,
>>  			   const struct intel_crtc_state *crtc_state)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> -	struct i915_audio_component *acomp = dev_priv->audio_component;
>> +	struct i915_audio_component *acomp = dev_priv->audio.component;
>>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>  	enum port port = encoder->port;
>>  	const struct dp_aud_n_m *nm;
>> @@ -436,7 +436,7 @@ hsw_hdmi_audio_config_update(struct intel_encoder *encoder,
>>  			     const struct intel_crtc_state *crtc_state)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> -	struct i915_audio_component *acomp = dev_priv->audio_component;
>> +	struct i915_audio_component *acomp = dev_priv->audio.component;
>>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>  	enum port port = encoder->port;
>>  	int n, rate;
>> @@ -494,7 +494,7 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
>>  	drm_dbg_kms(&dev_priv->drm, "Disable audio codec on transcoder %s\n",
>>  		    transcoder_name(cpu_transcoder));
>>  
>> -	mutex_lock(&dev_priv->av_mutex);
>> +	mutex_lock(&dev_priv->audio.lock);
>
> Maybe we should try to be consistent in our lock names and
> call mutexes 'mutex' and spinlocks 'lock'?

Works for me.

Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private
  2021-10-22 16:44   ` Ville Syrjälä
@ 2021-10-22 17:07     ` Jani Nikula
  2021-10-28 17:37       ` Jani Nikula
  0 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2021-10-22 17:07 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, airlied, daniel, lucas.demarchi, Dave Airlie

On Fri, 22 Oct 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Oct 22, 2021 at 07:27:57PM +0300, Jani Nikula wrote:
>> Add a standalone definition of struct intel_audio_private, and note that
>> all of it is private to intel_audio.c.
>> 
>> Cc: Dave Airlie <airlied@redhat.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h | 45 ++++++++++++++++++---------------
>>  1 file changed, 24 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 9c77610acf23..ed86633a587b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -828,6 +828,29 @@ struct i915_selftest_stash {
>>  	struct ida mock_region_instances;
>>  };
>>  
>> +/* intel_audio.c private */
>> +struct intel_audio_private {
>
> Not sure the "_private" is actually useful. I'd just call it
> intel_audio. The fact that struct drm_i915_private
> already says "private" doesn't mean anything to anyone anyway.

I first named it just intel_audio. Then I added intel_hdcp too, and
realized it means something else, in intel_display_types.h. Then there's
intel_gmbus. Probably others. I'd kind of like to have some
consistency. I'm not hung up on "_private", but I don't like to call
this intel_audio and then have intel_hdcp_foo and intel_gmbus_bar. Ideas
welcome.

And, yeah, we could just rename drm_i915_private drm_i915_public. :p


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 1/4] drm/i915/audio: group audio under anonymous struct in drm_i915_private
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 1/4] drm/i915/audio: group audio under anonymous struct in drm_i915_private Jani Nikula
@ 2021-10-22 17:22   ` Lucas De Marchi
  2021-10-28 17:36     ` Jani Nikula
  0 siblings, 1 reply; 17+ messages in thread
From: Lucas De Marchi @ 2021-10-22 17:22 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, airlied, daniel, ville.syrjala, Dave Airlie

On Fri, Oct 22, 2021 at 07:27:55PM +0300, Jani Nikula wrote:
>With an anonymous struct, this can be pure hierarchical organization
>without code changes.

start reading from patch 1 left me confused. A sentence here that next
patches will add the sub-struct name would be good.

Lucas De Marchi

>
>Cc: Dave Airlie <airlied@redhat.com>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/i915/i915_drv.h | 43 +++++++++++++++++----------------
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 19e6700a4315..ee210e12648a 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -995,9 +995,6 @@ struct drm_i915_private {
> 	/* Display internal color functions */
> 	const struct intel_color_funcs *color_funcs;
>
>-	/* Display internal audio functions */
>-	const struct intel_audio_funcs *audio_funcs;
>-
> 	/* Display CDCLK functions */
> 	const struct intel_cdclk_funcs *cdclk_funcs;
>
>@@ -1084,17 +1081,6 @@ struct drm_i915_private {
> 	struct drm_property *broadcast_rgb_property;
> 	struct drm_property *force_audio_property;
>
>-	/* hda/i915 audio component */
>-	struct i915_audio_component *audio_component;
>-	bool audio_component_registered;
>-	/**
>-	 * av_mutex - mutex for audio/video sync
>-	 *
>-	 */
>-	struct mutex av_mutex;
>-	int audio_power_refcount;
>-	u32 audio_freq_cntrl;
>-
> 	u32 fdi_rx_config;
>
> 	/* Shadow for DISPLAY_PHY_CONTROL which can't be safely read */
>@@ -1227,14 +1213,29 @@ struct drm_i915_private {
>
> 	bool ipc_enabled;
>
>-	/* Used to save the pipe-to-encoder mapping for audio */
>-	struct intel_encoder *av_enc_map[I915_MAX_PIPES];
>-
>-	/* necessary resource sharing with HDMI LPE audio driver. */
> 	struct {
>-		struct platform_device *platdev;
>-		int	irq;
>-	} lpe_audio;
>+		/* Display internal audio functions */
>+		const struct intel_audio_funcs *audio_funcs;
>+
>+		/* hda/i915 audio component */
>+		struct i915_audio_component *audio_component;
>+		bool audio_component_registered;
>+		/**
>+		 * av_mutex - mutex for audio/video sync
>+		 */
>+		struct mutex av_mutex;
>+		int audio_power_refcount;
>+		u32 audio_freq_cntrl;
>+
>+		/* Used to save the pipe-to-encoder mapping for audio */
>+		struct intel_encoder *av_enc_map[I915_MAX_PIPES];
>+
>+		/* necessary resource sharing with HDMI LPE audio driver. */
>+		struct {
>+			struct platform_device *platdev;
>+			int irq;
>+		} lpe_audio;
>+	};
>
> 	struct i915_pmu pmu;
>
>-- 
>2.30.2
>

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

* Re: [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv
  2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
                   ` (3 preceding siblings ...)
  2021-10-22 16:27 ` [Intel-gfx] [PATCH 4/4] drm/i915/audio: move intel_audio_funcs internal to intel_audio.c Jani Nikula
@ 2021-10-22 17:29 ` Lucas De Marchi
  2021-10-22 20:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Lucas De Marchi @ 2021-10-22 17:29 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, airlied, daniel, ville.syrjala

On Fri, Oct 22, 2021 at 07:27:54PM +0300, Jani Nikula wrote:
>I'll first make the argument that drm_i915_private should not have a
>single member that is not a named sub-struct defined separately. It's
>just too big to have individual members like, say, framestart_delay. All
>of these need context that is created using C constructs, not comments.
>
>Ideas have been thrown around about splitting display data more, perhaps
>to a separate "display private" struct. I'll also make the argument that
>it is not fine-grained enough. Here, I'm taking the audio parts and
>shoving them under intel_audio_private struct. It's still defined in
>i915_drv.h, but it could be trivially turned into a completely opaque
>type defined and allocated in intel_audio.c.
>
>I think the driver is big enough to start requiring this kind of
>abstractions, even at the cost of dynamic allocations and pointer
>chasing that opaque types require. It's just too easy to poke at
>drm_i915_private from wherever, breaking all abstractions.
>
>Sure, it's no coincidence that I chose audio, it's fairly well isolated
>already. Not all parts are going to be that easy. But I'm not sure I
>like the thought of doing a massive display private struct refactoring
>and not take one step further. Or, indeed, take this step *first*.

I like it, thanks.

I only have a small comment on first patch about commit message. Also
about intel_audio_private vs intel_audio I had the same thought as
Ville, but I get your point too. And I don't have any better name
suggestion. So this is 


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks
Lucas De Marchi

>
>BR,
>Jani.
>
>
>Jani Nikula (4):
>  drm/i915/audio: group audio under anonymous struct in drm_i915_private
>  drm/i915/audio: name the audio sub-struct in drm_i915_private
>  drm/i915/audio: define the audio struct separately from
>    drm_i915_private
>  drm/i915/audio: move intel_audio_funcs internal to intel_audio.c
>
> drivers/gpu/drm/i915/display/intel_audio.c    | 99 ++++++++++---------
> .../gpu/drm/i915/display/intel_lpe_audio.c    | 42 ++++----
> drivers/gpu/drm/i915/i915_drv.c               |  2 +-
> drivers/gpu/drm/i915/i915_drv.h               | 56 +++++------
> 4 files changed, 101 insertions(+), 98 deletions(-)
>
>-- 
>2.30.2
>

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: split out audio private from dev_priv
  2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
                   ` (4 preceding siblings ...)
  2021-10-22 17:29 ` [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Lucas De Marchi
@ 2021-10-22 20:39 ` Patchwork
  2021-10-22 21:10 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2021-10-23  4:33 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
  7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2021-10-22 20:39 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: split out audio private from dev_priv
URL   : https://patchwork.freedesktop.org/series/96195/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
4d684e513941 drm/i915/audio: group audio under anonymous struct in drm_i915_private
7dde34aa454e drm/i915/audio: name the audio sub-struct in drm_i915_private
-:315: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "audio.lpe.platdev"
#315: FILE: drivers/gpu/drm/i915/display/intel_audio.c:1371:
+	if ((dev_priv)->audio.lpe.platdev != NULL)

-:328: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "audio.lpe.platdev"
#328: FILE: drivers/gpu/drm/i915/display/intel_lpe_audio.c:77:
+#define HAS_LPE_AUDIO(dev_priv) ((dev_priv)->audio.lpe.platdev != NULL)

-:337: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#337: FILE: drivers/gpu/drm/i915/display/intel_lpe_audio.c:99:
+	rsc[0].start    = rsc[0].end = dev_priv->audio.lpe.irq;

total: 0 errors, 0 warnings, 3 checks, 438 lines checked
f6bfd155afcd drm/i915/audio: define the audio struct separately from drm_i915_private
4ba26336de5d drm/i915/audio: move intel_audio_funcs internal to intel_audio.c



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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: split out audio private from dev_priv
  2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
                   ` (5 preceding siblings ...)
  2021-10-22 20:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2021-10-22 21:10 ` Patchwork
  2021-10-23  4:33 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
  7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2021-10-22 21:10 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

== Series Details ==

Series: drm/i915: split out audio private from dev_priv
URL   : https://patchwork.freedesktop.org/series/96195/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10781 -> Patchwork_21425
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (37 -> 35)
------------------------------

  Additional (2): fi-tgl-1115g4 fi-pnv-d510 
  Missing    (4): fi-ctg-p8600 fi-bsw-cyan bat-dg1-6 fi-hsw-4200u 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@query-info:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][1] ([fdo#109315])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@amdgpu/amd_basic@query-info.html

  * igt@amdgpu/amd_basic@semaphore:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][2] ([fdo#109271]) +27 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-bdw-5557u/igt@amdgpu/amd_basic@semaphore.html

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][3] ([fdo#109315] / [i915#2575]) +16 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@amdgpu/amd_cs_nop@nop-gfx0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][4] ([i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@gem_huc_copy@huc-copy.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][5] ([i915#1155])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-nick:        [PASS][6] -> [INCOMPLETE][7] ([i915#2940])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/fi-bsw-nick/igt@i915_selftest@live@execlists.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-bsw-nick/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-bsw-n3050:       [PASS][8] -> [DMESG-FAIL][9] ([i915#541])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/fi-bsw-n3050/igt@i915_selftest@live@gt_heartbeat.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-bsw-n3050/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@hangcheck:
    - fi-snb-2600:        [PASS][10] -> [INCOMPLETE][11] ([i915#3921])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][12] ([fdo#111827]) +8 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][13] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-bdw-5557u/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][14] ([i915#4103]) +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][15] ([fdo#109285])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][16] ([i915#1072]) +3 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-userptr:
    - fi-pnv-d510:        NOTRUN -> [SKIP][17] ([fdo#109271]) +53 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-pnv-d510/igt@prime_vgem@basic-userptr.html
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][18] ([i915#3301])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-tgl-1115g4/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-bsw-nick:        NOTRUN -> [FAIL][19] ([fdo#109271] / [i915#1436] / [i915#3428] / [i915#4312])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-bsw-nick/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-bdw-5557u:       [INCOMPLETE][20] ([i915#146]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/fi-bdw-5557u/igt@gem_exec_suspend@basic-s3.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-bdw-5557u/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cml-u2:          [DMESG-WARN][22] ([i915#4269]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3428]: https://gitlab.freedesktop.org/drm/intel/issues/3428
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541


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

  * Linux: CI_DRM_10781 -> Patchwork_21425

  CI-20190529: 20190529
  CI_DRM_10781: 4dbabf0625634db7aa4a674515f163dc6989376f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6259: 89629f64da9f12b144f913865b08d2c9efcd10d7 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_21425: 4ba26336de5d8e3d5566ce2a46d0e646411e4595 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4ba26336de5d drm/i915/audio: move intel_audio_funcs internal to intel_audio.c
f6bfd155afcd drm/i915/audio: define the audio struct separately from drm_i915_private
7dde34aa454e drm/i915/audio: name the audio sub-struct in drm_i915_private
4d684e513941 drm/i915/audio: group audio under anonymous struct in drm_i915_private

== Logs ==

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

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

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

* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: split out audio private from dev_priv
  2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
                   ` (6 preceding siblings ...)
  2021-10-22 21:10 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2021-10-23  4:33 ` Patchwork
  7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2021-10-23  4:33 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

== Series Details ==

Series: drm/i915: split out audio private from dev_priv
URL   : https://patchwork.freedesktop.org/series/96195/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10781_full -> Patchwork_21425_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_21425_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_21425_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_draw_crc@draw-method-xrgb8888-blt-ytiled:
    - shard-kbl:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-kbl6/igt@kms_draw_crc@draw-method-xrgb8888-blt-ytiled.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@kms_draw_crc@draw-method-xrgb8888-blt-ytiled.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-suspend:
    - shard-tglb:         [PASS][3] -> [INCOMPLETE][4] ([i915#456]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-tglb8/igt@gem_eio@in-flight-suspend.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb7/igt@gem_eio@in-flight-suspend.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-kbl:          [PASS][5] -> [FAIL][6] ([i915#2846])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-kbl2/igt@gem_exec_fair@basic-deadline.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl6/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-flow@rcs0:
    - shard-kbl:          [PASS][7] -> [SKIP][8] ([fdo#109271])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-kbl1/igt@gem_exec_fair@basic-flow@rcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl6/igt@gem_exec_fair@basic-flow@rcs0.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-kbl:          [PASS][9] -> [FAIL][10] ([i915#2842]) +2 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-kbl7/igt@gem_exec_fair@basic-none@vcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@gem_exec_fair@basic-none@vcs0.html
    - shard-tglb:         NOTRUN -> [FAIL][11] ([i915#2842]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-none@vecs0:
    - shard-apl:          [PASS][12] -> [FAIL][13] ([i915#2842] / [i915#3468])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-apl7/igt@gem_exec_fair@basic-none@vecs0.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl1/igt@gem_exec_fair@basic-none@vecs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-glk:          [PASS][14] -> [FAIL][15] ([i915#2842])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-glk1/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-glk9/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-tglb:         [PASS][16] -> [FAIL][17] ([i915#2842])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-tglb5/igt@gem_exec_fair@basic-pace@vecs0.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb8/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-iclb:         NOTRUN -> [FAIL][18] ([i915#2849])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_params@secure-non-master:
    - shard-iclb:         NOTRUN -> [SKIP][19] ([fdo#112283])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@gem_exec_params@secure-non-master.html

  * igt@gem_huc_copy@huc-copy:
    - shard-kbl:          NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#2190])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@gem_huc_copy@huc-copy.html

  * igt@gem_pread@exhaustion:
    - shard-kbl:          NOTRUN -> [WARN][21] ([i915#2658])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl7/igt@gem_pread@exhaustion.html

  * igt@gem_pxp@reject-modify-context-protection-off-2:
    - shard-iclb:         NOTRUN -> [SKIP][22] ([i915#4270])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@gem_pxp@reject-modify-context-protection-off-2.html

  * igt@gem_softpin@evict-snoop-interruptible:
    - shard-tglb:         NOTRUN -> [SKIP][23] ([fdo#109312])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@gem_softpin@evict-snoop-interruptible.html

  * igt@gen3_mixed_blits:
    - shard-iclb:         NOTRUN -> [SKIP][24] ([fdo#109289])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@gen3_mixed_blits.html

  * igt@gen7_exec_parse@basic-offset:
    - shard-skl:          NOTRUN -> [SKIP][25] ([fdo#109271]) +124 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl2/igt@gen7_exec_parse@basic-offset.html

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-glk:          NOTRUN -> [SKIP][26] ([fdo#109271] / [i915#658])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-glk5/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-kbl:          NOTRUN -> [FAIL][27] ([i915#454])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl7/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-tglb:         NOTRUN -> [WARN][28] ([i915#2681] / [i915#2684])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-tglb:         NOTRUN -> [SKIP][29] ([fdo#111644] / [i915#1397] / [i915#2411])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html

  * igt@i915_query@query-topology-known-pci-ids:
    - shard-tglb:         NOTRUN -> [SKIP][30] ([fdo#109303])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@i915_query@query-topology-known-pci-ids.html

  * igt@i915_selftest@live@gem_contexts:
    - shard-skl:          NOTRUN -> [INCOMPLETE][31] ([i915#198])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl9/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@gt_lrc:
    - shard-tglb:         NOTRUN -> [DMESG-FAIL][32] ([i915#2373])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb6/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@gt_pm:
    - shard-tglb:         NOTRUN -> [DMESG-FAIL][33] ([i915#1759])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb6/igt@i915_selftest@live@gt_pm.html
    - shard-skl:          NOTRUN -> [DMESG-FAIL][34] ([i915#1886] / [i915#2291])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl9/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@hangcheck:
    - shard-snb:          [PASS][35] -> [INCOMPLETE][36] ([i915#3921])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-snb6/igt@i915_selftest@live@hangcheck.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-snb4/igt@i915_selftest@live@hangcheck.html

  * igt@kms_big_fb@linear-32bpp-rotate-0:
    - shard-glk:          [PASS][37] -> [DMESG-WARN][38] ([i915#118]) +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-glk3/igt@kms_big_fb@linear-32bpp-rotate-0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-glk8/igt@kms_big_fb@linear-32bpp-rotate-0.html

  * igt@kms_big_fb@linear-8bpp-rotate-0:
    - shard-skl:          [PASS][39] -> [DMESG-WARN][40] ([i915#1982])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-skl1/igt@kms_big_fb@linear-8bpp-rotate-0.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl6/igt@kms_big_fb@linear-8bpp-rotate-0.html

  * igt@kms_big_fb@x-tiled-32bpp-rotate-270:
    - shard-iclb:         NOTRUN -> [SKIP][41] ([fdo#110725] / [fdo#111614]) +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_big_fb@x-tiled-32bpp-rotate-270.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-270:
    - shard-tglb:         NOTRUN -> [SKIP][42] ([fdo#111614])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_big_fb@x-tiled-64bpp-rotate-270.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-async-flip:
    - shard-skl:          NOTRUN -> [FAIL][43] ([i915#3743])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl4/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-apl:          NOTRUN -> [SKIP][44] ([fdo#109271] / [i915#3777]) +2 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl8/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-hflip:
    - shard-kbl:          NOTRUN -> [SKIP][45] ([fdo#109271] / [i915#3777]) +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-async-flip:
    - shard-skl:          NOTRUN -> [FAIL][46] ([i915#3763])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl2/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-async-flip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip:
    - shard-tglb:         NOTRUN -> [SKIP][47] ([fdo#111615])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html

  * igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc:
    - shard-glk:          NOTRUN -> [SKIP][48] ([fdo#109271] / [i915#3886])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-glk5/igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][49] ([fdo#109271] / [i915#3886]) +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl3/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_mc_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][50] ([i915#3689] / [i915#3886]) +1 similar issue
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_rc_ccs_cc:
    - shard-iclb:         NOTRUN -> [SKIP][51] ([fdo#109278] / [i915#3886]) +1 similar issue
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb7/igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][52] ([i915#3689])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_ccs.html

  * igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_mc_ccs:
    - shard-kbl:          NOTRUN -> [SKIP][53] ([fdo#109271] / [i915#3886]) +4 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc:
    - shard-skl:          NOTRUN -> [SKIP][54] ([fdo#109271] / [i915#3886]) +3 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl4/igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_chamelium@dp-hpd-for-each-pipe:
    - shard-kbl:          NOTRUN -> [SKIP][55] ([fdo#109271] / [fdo#111827]) +6 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@kms_chamelium@dp-hpd-for-each-pipe.html

  * igt@kms_chamelium@dp-hpd-storm-disable:
    - shard-apl:          NOTRUN -> [SKIP][56] ([fdo#109271] / [fdo#111827]) +4 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl8/igt@kms_chamelium@dp-hpd-storm-disable.html

  * igt@kms_color@pipe-d-ctm-negative:
    - shard-iclb:         NOTRUN -> [SKIP][57] ([fdo#109278] / [i915#1149])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_color@pipe-d-ctm-negative.html

  * igt@kms_color_chamelium@pipe-b-ctm-max:
    - shard-skl:          NOTRUN -> [SKIP][58] ([fdo#109271] / [fdo#111827]) +10 similar issues
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl4/igt@kms_color_chamelium@pipe-b-ctm-max.html

  * igt@kms_color_chamelium@pipe-c-ctm-negative:
    - shard-iclb:         NOTRUN -> [SKIP][59] ([fdo#109284] / [fdo#111827]) +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_color_chamelium@pipe-c-ctm-negative.html

  * igt@kms_color_chamelium@pipe-d-degamma:
    - shard-tglb:         NOTRUN -> [SKIP][60] ([fdo#109284] / [fdo#111827]) +4 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_color_chamelium@pipe-d-degamma.html

  * igt@kms_content_protection@atomic-dpms:
    - shard-tglb:         NOTRUN -> [SKIP][61] ([fdo#111828])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_content_protection@atomic-dpms.html

  * igt@kms_content_protection@srm:
    - shard-kbl:          NOTRUN -> [TIMEOUT][62] ([i915#1319])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl2/igt@kms_content_protection@srm.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][63] -> [DMESG-WARN][64] ([i915#180]) +3 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-32x10-rapid-movement:
    - shard-iclb:         NOTRUN -> [SKIP][65] ([fdo#109278]) +10 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_cursor_crc@pipe-b-cursor-32x10-rapid-movement.html

  * igt@kms_cursor_crc@pipe-b-cursor-32x32-sliding:
    - shard-tglb:         NOTRUN -> [SKIP][66] ([i915#3319]) +1 similar issue
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb6/igt@kms_cursor_crc@pipe-b-cursor-32x32-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][67] ([i915#180])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-d-cursor-32x10-random:
    - shard-tglb:         NOTRUN -> [SKIP][68] ([i915#3359]) +1 similar issue
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_cursor_crc@pipe-d-cursor-32x10-random.html

  * igt@kms_cursor_crc@pipe-d-cursor-512x170-sliding:
    - shard-tglb:         NOTRUN -> [SKIP][69] ([fdo#109279] / [i915#3359])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_cursor_crc@pipe-d-cursor-512x170-sliding.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-iclb:         NOTRUN -> [SKIP][70] ([fdo#109274] / [fdo#109278]) +3 similar issues
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][71] -> [FAIL][72] ([i915#2122])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-glk9/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@2x-nonexisting-fb:
    - shard-iclb:         NOTRUN -> [SKIP][73] ([fdo#109274])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_flip@2x-nonexisting-fb.html

  * igt@kms_flip@plain-flip-fb-recreate@b-edp1:
    - shard-skl:          [PASS][74] -> [FAIL][75] ([i915#2122]) +1 similar issue
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-skl8/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl2/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs:
    - shard-skl:          NOTRUN -> [INCOMPLETE][76] ([i915#3699])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl8/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs.html
    - shard-iclb:         [PASS][77] -> [SKIP][78] ([i915#3701])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-iclb5/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-mmap-cpu:
    - shard-iclb:         NOTRUN -> [SKIP][79] ([fdo#109280]) +6 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-mmap-cpu.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-shrfb-msflip-blt:
    - shard-tglb:         NOTRUN -> [SKIP][80] ([fdo#111825]) +8 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-shrfb-msflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-blt:
    - shard-apl:          NOTRUN -> [SKIP][81] ([fdo#109271]) +53 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl8/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> [SKIP][82] ([fdo#109271]) +72 similar issues
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl2/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [PASS][83] -> [FAIL][84] ([i915#1188])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-skl2/igt@kms_hdr@bpc-switch.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl6/igt@kms_hdr@bpc-switch.html

  * igt@kms_hdr@static-toggle-dpms:
    - shard-iclb:         NOTRUN -> [SKIP][85] ([i915#1187])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_hdr@static-toggle-dpms.html

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-d:
    - shard-kbl:          NOTRUN -> [SKIP][86] ([fdo#109271] / [i915#533])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@kms_pipe_crc_basic@hang-read-crc-pipe-d.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - shard-apl:          [PASS][87] -> [DMESG-WARN][88] ([i915#180])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-apl4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-skl:          NOTRUN -> [FAIL][89] ([fdo#108145] / [i915#265]) +1 similar issue
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-alpha-7efc.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-apl:          NOTRUN -> [FAIL][90] ([fdo#108145] / [i915#265])
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl8/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-kbl:          NOTRUN -> [FAIL][91] ([fdo#108145] / [i915#265])
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][92] -> [FAIL][93] ([fdo#108145] / [i915#265])
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-c-tiling-x:
    - shard-tglb:         NOTRUN -> [SKIP][94] ([i915#3536])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_plane_lowres@pipe-c-tiling-x.html

  * igt@kms_prime@basic-crc@first-to-second:
    - shard-tglb:         NOTRUN -> [SKIP][95] ([i915#1836])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb6/igt@kms_prime@basic-crc@first-to-second.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-1:
    - shard-iclb:         NOTRUN -> [SKIP][96] ([i915#2920])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-1.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-1:
    - shard-skl:          NOTRUN -> [SKIP][97] ([fdo#109271] / [i915#658]) +3 similar issues
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl2/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-1.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-0:
    - shard-apl:          NOTRUN -> [SKIP][98] ([fdo#109271] / [i915#658])
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl8/igt@kms_psr2_sf@plane-move-sf-dmg-area-0.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-2:
    - shard-tglb:         NOTRUN -> [SKIP][99] ([i915#2920])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@kms_psr2_sf@plane-move-sf-dmg-area-2.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-1:
    - shard-kbl:          NOTRUN -> [SKIP][100] ([fdo#109271] / [i915#658]) +1 similar issue
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-1.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][101] -> [SKIP][102] ([fdo#109441]) +4 similar issues
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb8/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_psr@sprite_blt:
    - shard-glk:          NOTRUN -> [SKIP][103] ([fdo#109271]) +3 similar issues
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-glk5/igt@kms_psr@sprite_blt.html

  * igt@kms_setmode@basic:
    - shard-glk:          [PASS][104] -> [FAIL][105] ([i915#31])
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-glk3/igt@kms_setmode@basic.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-glk3/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [PASS][106] -> [DMESG-WARN][107] ([i915#180] / [i915#295])
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-kbl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-skl:          [PASS][108] -> [INCOMPLETE][109] ([i915#198])
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-skl1/igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl7/igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend.html

  * igt@nouveau_crc@pipe-c-source-outp-complete:
    - shard-tglb:         NOTRUN -> [SKIP][110] ([i915#2530])
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@nouveau_crc@pipe-c-source-outp-complete.html

  * igt@perf@mi-rpc:
    - shard-tglb:         NOTRUN -> [SKIP][111] ([fdo#109289]) +1 similar issue
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@perf@mi-rpc.html

  * igt@prime_nv_api@i915_nv_import_twice:
    - shard-iclb:         NOTRUN -> [SKIP][112] ([fdo#109291])
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@prime_nv_api@i915_nv_import_twice.html

  * igt@prime_nv_pcopy@test_semaphore:
    - shard-tglb:         NOTRUN -> [SKIP][113] ([fdo#109291])
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@prime_nv_pcopy@test_semaphore.html

  * igt@sysfs_clients@create:
    - shard-tglb:         NOTRUN -> [SKIP][114] ([i915#2994])
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb2/igt@sysfs_clients@create.html

  * igt@sysfs_clients@fair-3:
    - shard-apl:          NOTRUN -> [SKIP][115] ([fdo#109271] / [i915#2994])
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl8/igt@sysfs_clients@fair-3.html

  * igt@sysfs_clients@pidname:
    - shard-iclb:         NOTRUN -> [SKIP][116] ([i915#2994])
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@sysfs_clients@pidname.html

  * igt@sysfs_clients@recycle-many:
    - shard-skl:          NOTRUN -> [SKIP][117] ([fdo#109271] / [i915#2994]) +1 similar issue
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl4/igt@sysfs_clients@recycle-many.html

  * igt@sysfs_clients@split-25:
    - shard-kbl:          NOTRUN -> [SKIP][118] ([fdo#109271] / [i915#2994])
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl4/igt@sysfs_clients@split-25.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@preservation-s3@bcs0:
    - shard-apl:          [DMESG-WARN][119] ([i915#180]) -> [PASS][120] +5 similar issues
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-apl8/igt@gem_ctx_isolation@preservation-s3@bcs0.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-apl3/igt@gem_ctx_isolation@preservation-s3@bcs0.html

  * igt@gem_ctx_param@get-priority-new-ctx:
    - shard-skl:          [DMESG-WARN][121] ([i915#1982]) -> [PASS][122]
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-skl5/igt@gem_ctx_param@get-priority-new-ctx.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl1/igt@gem_ctx_param@get-priority-new-ctx.html

  * igt@gem_exec_fair@basic-pace@vcs0:
    - shard-tglb:         [FAIL][123] ([i915#2842]) -> [PASS][124]
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-tglb5/igt@gem_exec_fair@basic-pace@vcs0.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-tglb8/igt@gem_exec_fair@basic-pace@vcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-glk:          [FAIL][125] ([i915#2842]) -> [PASS][126] +1 similar issue
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-glk6/igt@gem_exec_fair@basic-throttle@rcs0.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-glk1/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_suspend@basic-s3:
    - shard-kbl:          [INCOMPLETE][127] ([i915#4221]) -> [PASS][128]
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-kbl3/igt@gem_exec_suspend@basic-s3.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-kbl7/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_exec_whisper@basic-queues-forked-all:
    - shard-iclb:         [INCOMPLETE][129] ([i915#1895]) -> [PASS][130]
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-iclb7/igt@gem_exec_whisper@basic-queues-forked-all.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-iclb2/igt@gem_exec_whisper@basic-queues-forked-all.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [DMESG-WARN][131] ([i915#1436] / [i915#716]) -> [PASS][132]
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-skl9/igt@gen9_exec_parse@allowed-single.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21425/shard-skl7/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-iclb:         [FAIL][133] ([i915#454]) -> [PASS][134]
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10781/shard-iclb3/igt@i915_pm_dc@dc6-dpms.html
   [13

== Logs ==

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

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

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

* Re: [Intel-gfx] [PATCH 1/4] drm/i915/audio: group audio under anonymous struct in drm_i915_private
  2021-10-22 17:22   ` Lucas De Marchi
@ 2021-10-28 17:36     ` Jani Nikula
  0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2021-10-28 17:36 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, airlied, daniel, ville.syrjala, Dave Airlie

On Fri, 22 Oct 2021, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Fri, Oct 22, 2021 at 07:27:55PM +0300, Jani Nikula wrote:
>>With an anonymous struct, this can be pure hierarchical organization
>>without code changes.
>
> start reading from patch 1 left me confused. A sentence here that next
> patches will add the sub-struct name would be good.

Fixed in v2.

>
> Lucas De Marchi
>
>>
>>Cc: Dave Airlie <airlied@redhat.com>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>---
>> drivers/gpu/drm/i915/i915_drv.h | 43 +++++++++++++++++----------------
>> 1 file changed, 22 insertions(+), 21 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>index 19e6700a4315..ee210e12648a 100644
>>--- a/drivers/gpu/drm/i915/i915_drv.h
>>+++ b/drivers/gpu/drm/i915/i915_drv.h
>>@@ -995,9 +995,6 @@ struct drm_i915_private {
>> 	/* Display internal color functions */
>> 	const struct intel_color_funcs *color_funcs;
>>
>>-	/* Display internal audio functions */
>>-	const struct intel_audio_funcs *audio_funcs;
>>-
>> 	/* Display CDCLK functions */
>> 	const struct intel_cdclk_funcs *cdclk_funcs;
>>
>>@@ -1084,17 +1081,6 @@ struct drm_i915_private {
>> 	struct drm_property *broadcast_rgb_property;
>> 	struct drm_property *force_audio_property;
>>
>>-	/* hda/i915 audio component */
>>-	struct i915_audio_component *audio_component;
>>-	bool audio_component_registered;
>>-	/**
>>-	 * av_mutex - mutex for audio/video sync
>>-	 *
>>-	 */
>>-	struct mutex av_mutex;
>>-	int audio_power_refcount;
>>-	u32 audio_freq_cntrl;
>>-
>> 	u32 fdi_rx_config;
>>
>> 	/* Shadow for DISPLAY_PHY_CONTROL which can't be safely read */
>>@@ -1227,14 +1213,29 @@ struct drm_i915_private {
>>
>> 	bool ipc_enabled;
>>
>>-	/* Used to save the pipe-to-encoder mapping for audio */
>>-	struct intel_encoder *av_enc_map[I915_MAX_PIPES];
>>-
>>-	/* necessary resource sharing with HDMI LPE audio driver. */
>> 	struct {
>>-		struct platform_device *platdev;
>>-		int	irq;
>>-	} lpe_audio;
>>+		/* Display internal audio functions */
>>+		const struct intel_audio_funcs *audio_funcs;
>>+
>>+		/* hda/i915 audio component */
>>+		struct i915_audio_component *audio_component;
>>+		bool audio_component_registered;
>>+		/**
>>+		 * av_mutex - mutex for audio/video sync
>>+		 */
>>+		struct mutex av_mutex;
>>+		int audio_power_refcount;
>>+		u32 audio_freq_cntrl;
>>+
>>+		/* Used to save the pipe-to-encoder mapping for audio */
>>+		struct intel_encoder *av_enc_map[I915_MAX_PIPES];
>>+
>>+		/* necessary resource sharing with HDMI LPE audio driver. */
>>+		struct {
>>+			struct platform_device *platdev;
>>+			int irq;
>>+		} lpe_audio;
>>+	};
>>
>> 	struct i915_pmu pmu;
>>
>>-- 
>>2.30.2
>>

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private
  2021-10-22 17:07     ` Jani Nikula
@ 2021-10-28 17:37       ` Jani Nikula
  0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2021-10-28 17:37 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, airlied, daniel, lucas.demarchi, Dave Airlie

On Fri, 22 Oct 2021, Jani Nikula <jani.nikula@intel.com> wrote:
> On Fri, 22 Oct 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Fri, Oct 22, 2021 at 07:27:57PM +0300, Jani Nikula wrote:
>>> Add a standalone definition of struct intel_audio_private, and note that
>>> all of it is private to intel_audio.c.
>>> 
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_drv.h | 45 ++++++++++++++++++---------------
>>>  1 file changed, 24 insertions(+), 21 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 9c77610acf23..ed86633a587b 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -828,6 +828,29 @@ struct i915_selftest_stash {
>>>  	struct ida mock_region_instances;
>>>  };
>>>  
>>> +/* intel_audio.c private */
>>> +struct intel_audio_private {
>>
>> Not sure the "_private" is actually useful. I'd just call it
>> intel_audio. The fact that struct drm_i915_private
>> already says "private" doesn't mean anything to anyone anyway.
>
> I first named it just intel_audio. Then I added intel_hdcp too, and
> realized it means something else, in intel_display_types.h. Then there's
> intel_gmbus. Probably others. I'd kind of like to have some
> consistency. I'm not hung up on "_private", but I don't like to call
> this intel_audio and then have intel_hdcp_foo and intel_gmbus_bar. Ideas
> welcome.

I ended up keeping the intel_audio_private name for now in v2. It's a
two-line patch to rename later. Though I admit it sets a precedent.

BR,
Jani.

>
> And, yeah, we could just rename drm_i915_private drm_i915_public. :p
>
>
> BR,
> Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

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

end of thread, other threads:[~2021-10-28 17:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-22 16:27 [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Jani Nikula
2021-10-22 16:27 ` [Intel-gfx] [PATCH 1/4] drm/i915/audio: group audio under anonymous struct in drm_i915_private Jani Nikula
2021-10-22 17:22   ` Lucas De Marchi
2021-10-28 17:36     ` Jani Nikula
2021-10-22 16:27 ` [Intel-gfx] [PATCH 2/4] drm/i915/audio: name the audio sub-struct " Jani Nikula
2021-10-22 16:42   ` Ville Syrjälä
2021-10-22 16:58     ` Jani Nikula
2021-10-22 16:27 ` [Intel-gfx] [PATCH 3/4] drm/i915/audio: define the audio struct separately from drm_i915_private Jani Nikula
2021-10-22 16:44   ` Ville Syrjälä
2021-10-22 17:07     ` Jani Nikula
2021-10-28 17:37       ` Jani Nikula
2021-10-22 16:27 ` [Intel-gfx] [PATCH 4/4] drm/i915/audio: move intel_audio_funcs internal to intel_audio.c Jani Nikula
2021-10-22 16:48   ` Ville Syrjälä
2021-10-22 17:29 ` [Intel-gfx] [PATCH 0/4] drm/i915: split out audio private from dev_priv Lucas De Marchi
2021-10-22 20:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2021-10-22 21:10 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-10-23  4:33 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.