Re: [PATCH 06/14] drm/i915: Split current joiner hw state readout

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 9/6/2024 8:59 PM, Ville Syrjälä wrote:
On Fri, Sep 06, 2024 at 06:27:59PM +0530, Ankit Nautiyal wrote:
From: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>

We need to add a new sanity checks and also do
some preparations for adding ultrajoiner hw state readout.
Lets first split reading of the uncompressed joiner and bigjoiner
bit masks into separate functions.

v2: Fixed checkpatch warnings (Ankit)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
Reviewed-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
---
  drivers/gpu/drm/i915/display/intel_display.c | 65 +++++++++++++++-----
  1 file changed, 48 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3278debf47cc..cdc7531311fc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3580,51 +3580,82 @@ static bool transcoder_ddi_func_is_enabled(struct drm_i915_private *dev_priv,
  	return tmp & TRANS_DDI_FUNC_ENABLE;
  }
-static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
-				 u8 *primary_pipes, u8 *secondary_pipes)
+static void enabled_uncompressed_joiner_pipes(struct drm_i915_private *dev_priv,
+					      u8 *primary_pipes, u8 *secondary_pipes)
  {
  	struct intel_crtc *crtc;
*primary_pipes = 0;
  	*secondary_pipes = 0;
+ if (DISPLAY_VER(dev_priv) < 13)
+		return;
+
  	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc,
  					 joiner_pipes(dev_priv)) {
  		enum intel_display_power_domain power_domain;
  		enum pipe pipe = crtc->pipe;
  		intel_wakeref_t wakeref;
- power_domain = intel_dsc_power_domain(crtc, (enum transcoder) pipe);
+		power_domain = POWER_DOMAIN_PIPE(pipe);
  		with_intel_display_power_if_enabled(dev_priv, power_domain, wakeref) {
  			u32 tmp = intel_de_read(dev_priv, ICL_PIPE_DSS_CTL1(pipe));
- if (!(tmp & BIG_JOINER_ENABLE))
-				continue;
-
-			if (tmp & PRIMARY_BIG_JOINER_ENABLE)
+			if (tmp & UNCOMPRESSED_JOINER_PRIMARY)
  				*primary_pipes |= BIT(pipe);
-			else
+			if (tmp & UNCOMPRESSED_JOINER_SECONDARY)
  				*secondary_pipes |= BIT(pipe);
  		}
+	}
+}
- if (DISPLAY_VER(dev_priv) < 13)
-			continue;
+static void enabled_bigjoiner_pipes(struct drm_i915_private *dev_priv,
+				    u8 *primary_pipes, u8 *secondary_pipes)
+{
+	struct intel_crtc *crtc;
- power_domain = POWER_DOMAIN_PIPE(pipe);
+	*primary_pipes = 0;
+	*secondary_pipes = 0;
We seem to be missing any kind of check to make sure bigjoiner
is actually present in the hardware. Or am I just blind?
If that is the case we need to fix it up before the
refactoring goes in.

We have check in DP, and in one place while checking for maxdotclock.

But yes its missing here. I am thinking something like: https://patchwork.freedesktop.org/patch/612875/?series=133800&rev=7

Will try to introduce a the check here prior to any other patch, that can be used in other places.


+
+	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc,
+					 joiner_pipes(dev_priv)) {
+		enum intel_display_power_domain power_domain;
+		enum pipe pipe = crtc->pipe;
+		intel_wakeref_t wakeref;
+
+		power_domain = intel_dsc_power_domain(crtc, (enum transcoder)pipe);
  		with_intel_display_power_if_enabled(dev_priv, power_domain, wakeref) {
  			u32 tmp = intel_de_read(dev_priv, ICL_PIPE_DSS_CTL1(pipe));
- if (tmp & UNCOMPRESSED_JOINER_PRIMARY)
+			if (!(tmp & BIG_JOINER_ENABLE))
+				continue;
+
+			if (tmp & PRIMARY_BIG_JOINER_ENABLE)
  				*primary_pipes |= BIT(pipe);
-			if (tmp & UNCOMPRESSED_JOINER_SECONDARY)
+			else
  				*secondary_pipes |= BIT(pipe);
  		}
  	}
+}
+
+static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
+				 u8 *primary_pipes, u8 *secondary_pipes)
+{
+	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
+	u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes;
+
+	enabled_uncompressed_joiner_pipes(dev_priv, &primary_uncompressed_joiner_pipes,
+					  &secondary_uncompressed_joiner_pipes);
+
+	enabled_bigjoiner_pipes(dev_priv, &primary_bigjoiner_pipes,
+				&secondary_bigjoiner_pipes);
+
+	*primary_pipes = 0;
+	*secondary_pipes = 0;
These seem redundant.

Will do away with these.


+
+	*primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes;
- /* Joiner pipes should always be consecutive primary and secondary */
-	drm_WARN(&dev_priv->drm, *secondary_pipes != *primary_pipes << 1,
-		 "Joiner misconfigured (primary pipes 0x%x, secondary pipes 0x%x)\n",
-		 *primary_pipes, *secondary_pipes);
Where did this go?

I think this was unintended. Will fix this.

Regards,

Ankit


+	*secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes;
  }
static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
--
2.45.2



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux