Re: [PATCH 4/8] drm/i915: Use a helper to get the aux power domain

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

 



On Tue, Oct 30, 2018 at 05:40:47PM +0200, Imre Deak wrote:
> From ICL onwards the AUX power domain may change dynamically based on
> whether a DDI/TypeC port is in thunderbolt or non-thunderbolt mode, so
> use a helper function instead of a static field to get the current
> domain.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++
>  drivers/gpu/drm/i915/intel_dp.c      | 73 +++++++++++++++---------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
>  4 files changed, 56 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 3739ef003819..5bb459011a49 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2099,7 +2099,7 @@ intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
>  	 * returns the correct domain for other ports too.
>  	 */
>  	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> -					      intel_dp->aux_power_domain;
> +					      intel_aux_power_domain(dig_port);
>  }
>  
>  static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c3cadc09f859..36710a30fb37 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5949,6 +5949,28 @@ enum intel_display_power_domain intel_port_to_power_domain(enum port port)
>  	}
>  }
>  
> +enum intel_display_power_domain
> +intel_aux_power_domain(struct intel_digital_port *dig_port)
> +{

couldn't we just move aux_power_domain up like yod did for aux_ch?  This way we initialize the
power domain bassed on the aux channel, and just use it on the other functions. I think this is
more future-proof for platforms changing the mapping aux <-> power domain. And avoids passing
dev_priv around later just for checking that.

Lucas De Marchi

> +	switch (dig_port->aux_ch) {
> +	case AUX_CH_A:
> +		return POWER_DOMAIN_AUX_A;
> +	case AUX_CH_B:
> +		return POWER_DOMAIN_AUX_B;
> +	case AUX_CH_C:
> +		return POWER_DOMAIN_AUX_C;
> +	case AUX_CH_D:
> +		return POWER_DOMAIN_AUX_D;
> +	case AUX_CH_E:
> +		return POWER_DOMAIN_AUX_E;
> +	case AUX_CH_F:
> +		return POWER_DOMAIN_AUX_F;
> +	default:
> +		MISSING_CASE(dig_port->aux_ch);
> +		return POWER_DOMAIN_AUX_A;
> +	}
> +}
> +
>  static u64 get_crtc_power_domains(struct drm_crtc *crtc,
>  				  struct intel_crtc_state *crtc_state)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6645c9faca9a..e6f59ef59be6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -690,7 +690,8 @@ static void pps_lock(struct intel_dp *intel_dp)
>  	 * See intel_power_sequencer_reset() why we need
>  	 * a power domain reference here.
>  	 */
> -	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_get(dev_priv,
> +				intel_aux_power_domain(dp_to_dig_port(intel_dp)));
>  
>  	mutex_lock(&dev_priv->pps_mutex);
>  }
> @@ -701,7 +702,8 @@ static void pps_unlock(struct intel_dp *intel_dp)
>  
>  	mutex_unlock(&dev_priv->pps_mutex);
>  
> -	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_put(dev_priv,
> +				intel_aux_power_domain(dp_to_dig_port(intel_dp)));
>  }
>  
>  static void
> @@ -1505,29 +1507,6 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  	return ret;
>  }
>  
> -static enum intel_display_power_domain
> -intel_aux_power_domain(struct intel_dp *intel_dp)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -
> -	switch (dig_port->aux_ch) {
> -	case AUX_CH_A:
> -		return POWER_DOMAIN_AUX_A;
> -	case AUX_CH_B:
> -		return POWER_DOMAIN_AUX_B;
> -	case AUX_CH_C:
> -		return POWER_DOMAIN_AUX_C;
> -	case AUX_CH_D:
> -		return POWER_DOMAIN_AUX_D;
> -	case AUX_CH_E:
> -		return POWER_DOMAIN_AUX_E;
> -	case AUX_CH_F:
> -		return POWER_DOMAIN_AUX_F;
> -	default:
> -		MISSING_CASE(dig_port->aux_ch);
> -		return POWER_DOMAIN_AUX_A;
> -	}
> -}
>  
>  static i915_reg_t g4x_aux_ctl_reg(struct intel_dp *intel_dp)
>  {
> @@ -1654,8 +1633,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp)
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct intel_encoder *encoder = &dig_port->base;
>  
> -	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
> -
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		intel_dp->aux_ch_ctl_reg = skl_aux_ctl_reg;
>  		intel_dp->aux_ch_data_reg = skl_aux_data_reg;
> @@ -2356,7 +2333,8 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
>  	if (edp_have_panel_vdd(intel_dp))
>  		return need_to_disable;
>  
> -	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_get(dev_priv,
> +				intel_aux_power_domain(intel_dig_port));
>  
>  	DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
>  		      port_name(intel_dig_port->base.port));
> @@ -2442,7 +2420,8 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>  	if ((pp & PANEL_POWER_ON) == 0)
>  		intel_dp->panel_power_off_time = ktime_get_boottime();
>  
> -	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_put(dev_priv,
> +				intel_aux_power_domain(intel_dig_port));
>  }
>  
>  static void edp_panel_vdd_work(struct work_struct *__work)
> @@ -2555,6 +2534,7 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
>  static void edp_panel_off(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	u32 pp;
>  	i915_reg_t pp_ctrl_reg;
>  
> @@ -2564,10 +2544,10 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>  		return;
>  
>  	DRM_DEBUG_KMS("Turn eDP port %c panel power off\n",
> -		      port_name(dp_to_dig_port(intel_dp)->base.port));
> +		      port_name(dig_port->base.port));
>  
>  	WARN(!intel_dp->want_panel_vdd, "Need eDP port %c VDD to turn off panel\n",
> -	     port_name(dp_to_dig_port(intel_dp)->base.port));
> +	     port_name(dig_port->base.port));
>  
>  	pp = ironlake_get_pp_control(intel_dp);
>  	/* We need to switch off panel power _and_ force vdd, for otherwise some
> @@ -2586,7 +2566,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>  	intel_dp->panel_power_off_time = ktime_get_boottime();
>  
>  	/* We got a reference when we enabled the VDD. */
> -	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_put(dev_priv, intel_aux_power_domain(dig_port));
>  }
>  
>  void intel_edp_panel_off(struct intel_dp *intel_dp)
> @@ -5069,14 +5049,17 @@ intel_dp_detect(struct drm_connector *connector,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *encoder = &dig_port->base;
>  	enum drm_connector_status status;
> +	enum intel_display_power_domain aux_domain =
> +		intel_aux_power_domain(dig_port);
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, connector->name);
>  	WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
>  
> -	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_get(dev_priv, aux_domain);
>  
>  	/* Can't disconnect eDP */
>  	if (intel_dp_is_edp(intel_dp))
> @@ -5138,7 +5121,7 @@ intel_dp_detect(struct drm_connector *connector,
>  		ret = intel_dp_retrain_link(encoder, ctx);
>  		if (ret) {
>  			intel_display_power_put(dev_priv,
> -						intel_dp->aux_power_domain);
> +						intel_aux_power_domain(dig_port));
>  			return ret;
>  		}
>  	}
> @@ -5162,7 +5145,7 @@ intel_dp_detect(struct drm_connector *connector,
>  	if (status != connector_status_connected && !intel_dp->is_mst)
>  		intel_dp_unset_edid(intel_dp);
>  
> -	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_put(dev_priv, aux_domain);
>  	return status;
>  }
>  
> @@ -5170,8 +5153,11 @@ static void
>  intel_dp_force(struct drm_connector *connector)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *intel_encoder = &dig_port->base;
>  	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
> +	enum intel_display_power_domain aux_domain =
> +		intel_aux_power_domain(dig_port);
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, connector->name);
> @@ -5180,11 +5166,11 @@ intel_dp_force(struct drm_connector *connector)
>  	if (connector->status != connector_status_connected)
>  		return;
>  
> -	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_get(dev_priv, aux_domain);
>  
>  	intel_dp_set_edid(intel_dp);
>  
> -	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_put(dev_priv, aux_domain);
>  }
>  
>  static int intel_dp_get_modes(struct drm_connector *connector)
> @@ -5530,6 +5516,7 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
>  static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> @@ -5543,7 +5530,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
>  	 * indefinitely.
>  	 */
>  	DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
> -	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_get(dev_priv, intel_aux_power_domain(dig_port));
>  
>  	edp_panel_vdd_schedule_off(intel_dp);
>  }
> @@ -5641,7 +5628,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  		return IRQ_NONE;
>  	}
>  
> -	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_get(dev_priv,
> +				intel_aux_power_domain(intel_dig_port));
>  
>  	if (intel_dp->is_mst) {
>  		if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
> @@ -5670,7 +5658,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  	ret = IRQ_HANDLED;
>  
>  put_power:
> -	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> +	intel_display_power_put(dev_priv,
> +				intel_aux_power_domain(intel_dig_port));
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a242a118389d..a3d7b93ecddd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1130,7 +1130,6 @@ struct intel_dp {
>  	/* sink or branch descriptor */
>  	struct drm_dp_desc desc;
>  	struct drm_dp_aux aux;
> -	enum intel_display_power_domain aux_power_domain;
>  	uint8_t train_set[4];
>  	int panel_power_up_delay;
>  	int panel_power_down_delay;
> @@ -1709,6 +1708,8 @@ bool hsw_crtc_state_ips_capable(const struct intel_crtc_state *crtc_state);
>  void hsw_enable_ips(const struct intel_crtc_state *crtc_state);
>  void hsw_disable_ips(const struct intel_crtc_state *crtc_state);
>  enum intel_display_power_domain intel_port_to_power_domain(enum port port);
> +enum intel_display_power_domain
> +intel_aux_power_domain(struct intel_digital_port *dig_port);
>  void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  				 struct intel_crtc_state *pipe_config);
>  void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
> -- 
> 2.13.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




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

  Powered by Linux