diff mbox series

[07/11] drm/i915/pps: pass intel_dp to pps_name()

Message ID f2a7fec4a2ff1f09cb73e6734604fae99ab6b11a.1725012870.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: hdmi and dp related struct intel_display conversions | expand

Commit Message

Jani Nikula Aug. 30, 2024, 10:15 a.m. UTC 
Currently all of intel_pps.c passes struct intel_dp around. Do the same
with pps_name() instead of passing both struct drm_i915_private and
struct intel_pps.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_pps.c | 61 +++++++++++++-----------
 1 file changed, 32 insertions(+), 29 deletions(-)

Comments

Ville Syrjälä Sept. 3, 2024, 12:33 p.m. UTC | #1 
On Fri, Aug 30, 2024 at 01:15:44PM +0300, Jani Nikula wrote:
> Currently all of intel_pps.c passes struct intel_dp around. Do the same
> with pps_name() instead of passing both struct drm_i915_private and
> struct intel_pps.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_pps.c | 61 +++++++++++++-----------
>  1 file changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index 68141af4da54..1e87ce95c85d 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -24,9 +24,12 @@ static void vlv_steal_power_sequencer(struct drm_i915_private *dev_priv,
>  static void pps_init_delays(struct intel_dp *intel_dp);
>  static void pps_init_registers(struct intel_dp *intel_dp, bool force_disable_vdd);
>  
> -static const char *pps_name(struct drm_i915_private *i915,
> -			    struct intel_pps *pps)
> +static const char *pps_name(struct intel_dp *intel_dp)
>  {
> +	struct intel_display *display = to_intel_display(intel_dp);
> +	struct drm_i915_private *i915 = to_i915(display->drm);
> +	struct intel_pps *pps = &intel_dp->pps;
> +

I've been thinking that we'd eventually turn intel_pps into some kind of
proper object with a 1:1 relationship to the corresponding hw block.
This is sort of going in the opposite direction, but looks trivial
enough to deal with if/when we get to reworking intel_pps.

Series is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Jani Nikula Sept. 3, 2024, 2:32 p.m. UTC | #2 
On Tue, 03 Sep 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Aug 30, 2024 at 01:15:44PM +0300, Jani Nikula wrote:
>> Currently all of intel_pps.c passes struct intel_dp around. Do the same
>> with pps_name() instead of passing both struct drm_i915_private and
>> struct intel_pps.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_pps.c | 61 +++++++++++++-----------
>>  1 file changed, 32 insertions(+), 29 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
>> index 68141af4da54..1e87ce95c85d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> @@ -24,9 +24,12 @@ static void vlv_steal_power_sequencer(struct drm_i915_private *dev_priv,
>>  static void pps_init_delays(struct intel_dp *intel_dp);
>>  static void pps_init_registers(struct intel_dp *intel_dp, bool force_disable_vdd);
>>  
>> -static const char *pps_name(struct drm_i915_private *i915,
>> -			    struct intel_pps *pps)
>> +static const char *pps_name(struct intel_dp *intel_dp)
>>  {
>> +	struct intel_display *display = to_intel_display(intel_dp);
>> +	struct drm_i915_private *i915 = to_i915(display->drm);
>> +	struct intel_pps *pps = &intel_dp->pps;
>> +
>
> I've been thinking that we'd eventually turn intel_pps into some kind of
> proper object with a 1:1 relationship to the corresponding hw block.
> This is sort of going in the opposite direction, but looks trivial
> enough to deal with if/when we get to reworking intel_pps.

Right. I think there are more problematic cases than this one.

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

Thanks, appreciated. Pushed the lot to drm-intel-next.

BR,
Jani.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
index 68141af4da54..1e87ce95c85d 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -24,9 +24,12 @@  static void vlv_steal_power_sequencer(struct drm_i915_private *dev_priv,
 static void pps_init_delays(struct intel_dp *intel_dp);
 static void pps_init_registers(struct intel_dp *intel_dp, bool force_disable_vdd);
 
-static const char *pps_name(struct drm_i915_private *i915,
-			    struct intel_pps *pps)
+static const char *pps_name(struct intel_dp *intel_dp)
 {
+	struct intel_display *display = to_intel_display(intel_dp);
+	struct drm_i915_private *i915 = to_i915(display->drm);
+	struct intel_pps *pps = &intel_dp->pps;
+
 	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) {
 		switch (pps->pps_pipe) {
 		case INVALID_PIPE:
@@ -97,13 +100,13 @@  vlv_power_sequencer_kick(struct intel_dp *intel_dp)
 	if (drm_WARN(&dev_priv->drm,
 		     intel_de_read(dev_priv, intel_dp->output_reg) & DP_PORT_EN,
 		     "skipping %s kick due to [ENCODER:%d:%s] being active\n",
-		     pps_name(dev_priv, &intel_dp->pps),
+		     pps_name(intel_dp),
 		     dig_port->base.base.base.id, dig_port->base.base.name))
 		return;
 
 	drm_dbg_kms(&dev_priv->drm,
 		    "kicking %s for [ENCODER:%d:%s]\n",
-		    pps_name(dev_priv, &intel_dp->pps),
+		    pps_name(intel_dp),
 		    dig_port->base.base.base.id, dig_port->base.base.name);
 
 	/* Preserve the BIOS-computed detected bit. This is
@@ -227,7 +230,7 @@  vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
 
 	drm_dbg_kms(&dev_priv->drm,
 		    "picked %s for [ENCODER:%d:%s]\n",
-		    pps_name(dev_priv, &intel_dp->pps),
+		    pps_name(intel_dp),
 		    dig_port->base.base.base.id, dig_port->base.base.name);
 
 	/* init power sequencer on this pipe and port */
@@ -340,7 +343,7 @@  vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
 	drm_dbg_kms(&dev_priv->drm,
 		    "[ENCODER:%d:%s] initial power sequencer: %s\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps));
+		    pps_name(intel_dp));
 }
 
 static int intel_num_pps(struct drm_i915_private *i915)
@@ -424,12 +427,12 @@  pps_initial_setup(struct intel_dp *intel_dp)
 		drm_dbg_kms(&i915->drm,
 			    "[ENCODER:%d:%s] no initial power sequencer, assuming %s\n",
 			    encoder->base.base.id, encoder->base.name,
-			    pps_name(i915, &intel_dp->pps));
+			    pps_name(intel_dp));
 	} else {
 		drm_dbg_kms(&i915->drm,
 			    "[ENCODER:%d:%s] initial power sequencer: %s\n",
 			    encoder->base.base.id, encoder->base.name,
-			    pps_name(i915, &intel_dp->pps));
+			    pps_name(intel_dp));
 	}
 
 	return intel_pps_is_valid(intel_dp);
@@ -565,11 +568,11 @@  void intel_pps_check_power_unlocked(struct intel_dp *intel_dp)
 		drm_WARN(&dev_priv->drm, 1,
 			 "[ENCODER:%d:%s] %s powered off while attempting AUX CH communication.\n",
 			 dig_port->base.base.base.id, dig_port->base.base.name,
-			 pps_name(dev_priv, &intel_dp->pps));
+			 pps_name(intel_dp));
 		drm_dbg_kms(&dev_priv->drm,
 			    "[ENCODER:%d:%s] %s PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
 			    dig_port->base.base.base.id, dig_port->base.base.name,
-			    pps_name(dev_priv, &intel_dp->pps),
+			    pps_name(intel_dp),
 			    intel_de_read(dev_priv, _pp_stat_reg(intel_dp)),
 			    intel_de_read(dev_priv, _pp_ctrl_reg(intel_dp)));
 	}
@@ -603,7 +606,7 @@  static void wait_panel_status(struct intel_dp *intel_dp,
 	drm_dbg_kms(&dev_priv->drm,
 		    "[ENCODER:%d:%s] %s mask: 0x%08x value: 0x%08x PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps),
+		    pps_name(intel_dp),
 		    mask, value,
 		    intel_de_read(dev_priv, pp_stat_reg),
 		    intel_de_read(dev_priv, pp_ctrl_reg));
@@ -612,7 +615,7 @@  static void wait_panel_status(struct intel_dp *intel_dp,
 		drm_err(&dev_priv->drm,
 			"[ENCODER:%d:%s] %s panel status timeout: PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
 			dig_port->base.base.base.id, dig_port->base.base.name,
-			pps_name(dev_priv, &intel_dp->pps),
+			pps_name(intel_dp),
 			intel_de_read(dev_priv, pp_stat_reg),
 			intel_de_read(dev_priv, pp_ctrl_reg));
 
@@ -626,7 +629,7 @@  static void wait_panel_on(struct intel_dp *intel_dp)
 
 	drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s] %s wait for panel power on\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(i915, &intel_dp->pps));
+		    pps_name(intel_dp));
 	wait_panel_status(intel_dp, IDLE_ON_MASK, IDLE_ON_VALUE);
 }
 
@@ -637,7 +640,7 @@  static void wait_panel_off(struct intel_dp *intel_dp)
 
 	drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s] %s wait for panel power off time\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(i915, &intel_dp->pps));
+		    pps_name(intel_dp));
 	wait_panel_status(intel_dp, IDLE_OFF_MASK, IDLE_OFF_VALUE);
 }
 
@@ -650,7 +653,7 @@  static void wait_panel_power_cycle(struct intel_dp *intel_dp)
 
 	drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s] %s wait for panel power cycle\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(i915, &intel_dp->pps));
+		    pps_name(intel_dp));
 
 	/* take the difference of current time and panel power off time
 	 * and then make panel wait for t11_t12 if needed. */
@@ -742,7 +745,7 @@  bool intel_pps_vdd_on_unlocked(struct intel_dp *intel_dp)
 
 	drm_dbg_kms(&dev_priv->drm, "[ENCODER:%d:%s] %s turning VDD on\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps));
+		    pps_name(intel_dp));
 
 	if (!edp_have_panel_power(intel_dp))
 		wait_panel_power_cycle(intel_dp);
@@ -754,7 +757,7 @@  bool intel_pps_vdd_on_unlocked(struct intel_dp *intel_dp)
 	intel_de_posting_read(dev_priv, pp_ctrl_reg);
 	drm_dbg_kms(&dev_priv->drm, "[ENCODER:%d:%s] %s PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps),
+		    pps_name(intel_dp),
 		    intel_de_read(dev_priv, pp_stat_reg),
 		    intel_de_read(dev_priv, pp_ctrl_reg));
 	/*
@@ -764,7 +767,7 @@  bool intel_pps_vdd_on_unlocked(struct intel_dp *intel_dp)
 		drm_dbg_kms(&dev_priv->drm,
 			    "[ENCODER:%d:%s] %s panel power wasn't enabled\n",
 			    dig_port->base.base.base.id, dig_port->base.base.name,
-			    pps_name(dev_priv, &intel_dp->pps));
+			    pps_name(intel_dp));
 		msleep(intel_dp->pps.panel_power_up_delay);
 	}
 
@@ -792,7 +795,7 @@  void intel_pps_vdd_on(struct intel_dp *intel_dp)
 	I915_STATE_WARN(i915, !vdd, "[ENCODER:%d:%s] %s VDD already requested on\n",
 			dp_to_dig_port(intel_dp)->base.base.base.id,
 			dp_to_dig_port(intel_dp)->base.base.name,
-			pps_name(i915, &intel_dp->pps));
+			pps_name(intel_dp));
 }
 
 static void intel_pps_vdd_off_sync_unlocked(struct intel_dp *intel_dp)
@@ -812,7 +815,7 @@  static void intel_pps_vdd_off_sync_unlocked(struct intel_dp *intel_dp)
 
 	drm_dbg_kms(&dev_priv->drm, "[ENCODER:%d:%s] %s turning VDD off\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps));
+		    pps_name(intel_dp));
 
 	pp = ilk_get_pp_control(intel_dp);
 	pp &= ~EDP_FORCE_VDD;
@@ -826,7 +829,7 @@  static void intel_pps_vdd_off_sync_unlocked(struct intel_dp *intel_dp)
 	/* Make sure sequencer is idle before allowing subsequent activity */
 	drm_dbg_kms(&dev_priv->drm, "[ENCODER:%d:%s] %s PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps),
+		    pps_name(intel_dp),
 		    intel_de_read(dev_priv, pp_stat_reg),
 		    intel_de_read(dev_priv, pp_ctrl_reg));
 
@@ -907,7 +910,7 @@  void intel_pps_vdd_off_unlocked(struct intel_dp *intel_dp, bool sync)
 			"[ENCODER:%d:%s] %s VDD not forced on",
 			dp_to_dig_port(intel_dp)->base.base.base.id,
 			dp_to_dig_port(intel_dp)->base.base.name,
-			pps_name(dev_priv, &intel_dp->pps));
+			pps_name(intel_dp));
 
 	intel_dp->pps.want_panel_vdd = false;
 
@@ -931,13 +934,13 @@  void intel_pps_on_unlocked(struct intel_dp *intel_dp)
 	drm_dbg_kms(&dev_priv->drm, "[ENCODER:%d:%s] %s turn panel power on\n",
 		    dp_to_dig_port(intel_dp)->base.base.base.id,
 		    dp_to_dig_port(intel_dp)->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps));
+		    pps_name(intel_dp));
 
 	if (drm_WARN(&dev_priv->drm, edp_have_panel_power(intel_dp),
 		     "[ENCODER:%d:%s] %s panel power already on\n",
 		     dp_to_dig_port(intel_dp)->base.base.base.id,
 		     dp_to_dig_port(intel_dp)->base.base.name,
-		     pps_name(dev_priv, &intel_dp->pps)))
+		     pps_name(intel_dp)))
 		return;
 
 	wait_panel_power_cycle(intel_dp);
@@ -1005,12 +1008,12 @@  void intel_pps_off_unlocked(struct intel_dp *intel_dp)
 
 	drm_dbg_kms(&dev_priv->drm, "[ENCODER:%d:%s] %s turn panel power off\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps));
+		    pps_name(intel_dp));
 
 	drm_WARN(&dev_priv->drm, !intel_dp->pps.want_panel_vdd,
 		 "[ENCODER:%d:%s] %s need VDD to turn off panel\n",
 		 dig_port->base.base.base.id, dig_port->base.base.name,
-		 pps_name(dev_priv, &intel_dp->pps));
+		 pps_name(intel_dp));
 
 	pp = ilk_get_pp_control(intel_dp);
 	/* We need to switch off panel power _and_ force vdd, for otherwise some
@@ -1146,7 +1149,7 @@  static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
 	 */
 	drm_dbg_kms(&dev_priv->drm,
 		    "detaching %s from [ENCODER:%d:%s]\n",
-		    pps_name(dev_priv, &intel_dp->pps),
+		    pps_name(intel_dp),
 		    dig_port->base.base.base.id, dig_port->base.base.name);
 	intel_de_write(dev_priv, pp_on_reg, 0);
 	intel_de_posting_read(dev_priv, pp_on_reg);
@@ -1219,7 +1222,7 @@  void vlv_pps_init(struct intel_encoder *encoder,
 
 	drm_dbg_kms(&dev_priv->drm,
 		    "initializing %s for [ENCODER:%d:%s]\n",
-		    pps_name(dev_priv, &intel_dp->pps),
+		    pps_name(intel_dp),
 		    encoder->base.base.id, encoder->base.name);
 
 	/* init power sequencer on this pipe and port */
@@ -1246,7 +1249,7 @@  static void pps_vdd_init(struct intel_dp *intel_dp)
 	drm_dbg_kms(&dev_priv->drm,
 		    "[ENCODER:%d:%s] %s VDD left on by BIOS, adjusting state tracking\n",
 		    dig_port->base.base.base.id, dig_port->base.base.name,
-		    pps_name(dev_priv, &intel_dp->pps));
+		    pps_name(intel_dp));
 	drm_WARN_ON(&dev_priv->drm, intel_dp->pps.vdd_wakeref);
 	intel_dp->pps.vdd_wakeref = intel_display_power_get(dev_priv,
 							    intel_aux_power_domain(dig_port));