[3/3] drm/i915: Pass dev_priv to intel_is_dual_link_lvds()

Message ID	20190318202653.15217-3-ville.syrjala@linux.intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces@lists.freedesktop.org> From: Ville Syrjala <ville.syrjala@linux.intel.com> To: intel-gfx@lists.freedesktop.org Date: Mon, 18 Mar 2019 22:26:53 +0200 Message-Id: <20190318202653.15217-3-ville.syrjala@linux.intel.com> In-Reply-To: <20190318202653.15217-1-ville.syrjala@linux.intel.com> References: <20190318202653.15217-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 3/3] drm/i915: Pass dev_priv to intel_is_dual_link_lvds() Precedence: list Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
Series	[1/3] drm/i915: Add some missing curly braces \| expand [1/3] drm/i915: Add some missing curly braces [2/3] drm/i915: Polish intel_get_lvds_encoder() [3/3] drm/i915: Pass dev_priv to intel_is_dual_link_lvds()

Ville Syrjala March 18, 2019, 8:26 p.m. UTC

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make things look a bit nicer by passing dev_priv to
intel_is_dual_link_lvds().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c    |  4 ++--
 3 files changed, 15 insertions(+), 19 deletions(-)

Rodrigo Vivi March 18, 2019, 8:35 p.m. UTC | #1

On Mon, Mar 18, 2019 at 10:26:53PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make things look a bit nicer by passing dev_priv to
> intel_is_dual_link_lvds().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


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


> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>  drivers/gpu/drm/i915/intel_lvds.c    |  4 ++--
>  3 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bfe792789a52..ff807277d2fe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -595,7 +595,7 @@ i9xx_select_p2_div(const struct intel_limit *limit,
>  		   const struct intel_crtc_state *crtc_state,
>  		   int target)
>  {
> -	struct drm_device *dev = crtc_state->base.crtc->dev;
> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>  
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		/*
> @@ -603,7 +603,7 @@ i9xx_select_p2_div(const struct intel_limit *limit,
>  		 * We haven't figured out how to reliably set up different
>  		 * single/dual channel state, if we even can.
>  		 */
> -		if (intel_is_dual_link_lvds(dev))
> +		if (intel_is_dual_link_lvds(dev_priv))
>  			return limit->p2.p2_fast;
>  		else
>  			return limit->p2.p2_slow;
> @@ -6872,8 +6872,7 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>  static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  				     struct intel_crtc_state *pipe_config)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	int clock_limit = dev_priv->max_dotclk_freq;
>  
> @@ -6923,7 +6922,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		}
>  
>  		if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_LVDS) &&
> -		    intel_is_dual_link_lvds(dev)) {
> +		    intel_is_dual_link_lvds(dev_priv)) {
>  			DRM_DEBUG_KMS("Odd pipe source width not supported with dual link LVDS\n");
>  			return -EINVAL;
>  		}
> @@ -7813,8 +7812,7 @@ static int i8xx_crtc_compute_clock(struct intel_crtc *crtc,
>  static int g4x_crtc_compute_clock(struct intel_crtc *crtc,
>  				  struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	const struct intel_limit *limit;
>  	int refclk = 96000;
>  
> @@ -7827,7 +7825,7 @@ static int g4x_crtc_compute_clock(struct intel_crtc *crtc,
>  			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
>  		}
>  
> -		if (intel_is_dual_link_lvds(dev))
> +		if (intel_is_dual_link_lvds(dev_priv))
>  			limit = &intel_limits_g4x_dual_channel_lvds;
>  		else
>  			limit = &intel_limits_g4x_single_channel_lvds;
> @@ -8861,13 +8859,11 @@ static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
>  	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
>  }
>  
> -static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> +static void ironlake_compute_dpll(struct intel_crtc *crtc,
>  				  struct intel_crtc_state *crtc_state,
>  				  struct dpll *reduced_clock)
>  {
> -	struct drm_crtc *crtc = &intel_crtc->base;
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	u32 dpll, fp, fp2;
>  	int factor;
>  
> @@ -8876,7 +8872,8 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		if ((intel_panel_use_ssc(dev_priv) &&
>  		     dev_priv->vbt.lvds_ssc_freq == 100000) ||
> -		    (HAS_PCH_IBX(dev_priv) && intel_is_dual_link_lvds(dev)))
> +		    (HAS_PCH_IBX(dev_priv) &&
> +		     intel_is_dual_link_lvds(dev_priv)))
>  			factor = 25;
>  	} else if (crtc_state->sdvo_tv_clock) {
>  		factor = 20;
> @@ -8967,8 +8964,7 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  				       struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	const struct intel_limit *limit;
>  	int refclk = 120000;
>  
> @@ -8986,7 +8982,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  			refclk = dev_priv->vbt.lvds_ssc_freq;
>  		}
>  
> -		if (intel_is_dual_link_lvds(dev)) {
> +		if (intel_is_dual_link_lvds(dev_priv)) {
>  			if (refclk == 100000)
>  				limit = &intel_limits_ironlake_dual_lvds_100m;
>  			else
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ac8a67ba704d..1711a717ee23 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2118,7 +2118,7 @@ bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv,
>  			     i915_reg_t lvds_reg, enum pipe *pipe);
>  void intel_lvds_init(struct drm_i915_private *dev_priv);
>  struct intel_encoder *intel_get_lvds_encoder(struct drm_i915_private *dev_priv);
> -bool intel_is_dual_link_lvds(struct drm_device *dev);
> +bool intel_is_dual_link_lvds(struct drm_i915_private *dev_priv);
>  
>  /* intel_overlay.c */
>  void intel_overlay_setup(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 306bc321fdaa..845792aa0abe 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -754,9 +754,9 @@ struct intel_encoder *intel_get_lvds_encoder(struct drm_i915_private *dev_priv)
>  	return NULL;
>  }
>  
> -bool intel_is_dual_link_lvds(struct drm_device *dev)
> +bool intel_is_dual_link_lvds(struct drm_i915_private *dev_priv)
>  {
> -	struct intel_encoder *encoder = intel_get_lvds_encoder(to_i915(dev));
> +	struct intel_encoder *encoder = intel_get_lvds_encoder(dev_priv);
>  
>  	return encoder && to_lvds_encoder(&encoder->base)->is_dual_link;
>  }
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Michal Wajdeczko March 18, 2019, 8:45 p.m. UTC | #2

> Make things look a bit nicer by passing dev_priv to

In other places we are changing naming from dev_priv to i915.
Can we do the same here ?

~Michal

Ville Syrjala March 18, 2019, 8:59 p.m. UTC | #3

On Mon, Mar 18, 2019 at 09:45:16PM +0100, Michal Wajdeczko wrote:
> > Make things look a bit nicer by passing dev_priv to
> 
> In other places we are changing naming from dev_priv to i915.
> Can we do the same here ?

The display code uses dev_priv quite consistently,. We could
go for i915 I suppose but I'd prefer a mass conversion rather
than the death by a thousand cuts approach. The display
code is already messy enough when it comes to naming local
variables.

Jani Nikula March 19, 2019, 3:50 p.m. UTC | #4

On Mon, 18 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Mar 18, 2019 at 09:45:16PM +0100, Michal Wajdeczko wrote:
>> > Make things look a bit nicer by passing dev_priv to
>> 
>> In other places we are changing naming from dev_priv to i915.
>> Can we do the same here ?
>
> The display code uses dev_priv quite consistently,. We could
> go for i915 I suppose but I'd prefer a mass conversion rather
> than the death by a thousand cuts approach. The display
> code is already messy enough when it comes to naming local
> variables.

We could use i915 in display code as a convention for code that does not
do register access. ;)

BR,
Jani.

Rodrigo Vivi March 19, 2019, 5:31 p.m. UTC | #5

On Tue, Mar 19, 2019 at 05:50:22PM +0200, Jani Nikula wrote:
> On Mon, 18 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Mon, Mar 18, 2019 at 09:45:16PM +0100, Michal Wajdeczko wrote:
> >> > Make things look a bit nicer by passing dev_priv to
> >> 
> >> In other places we are changing naming from dev_priv to i915.
> >> Can we do the same here ?
> >
> > The display code uses dev_priv quite consistently,. We could
> > go for i915 I suppose but I'd prefer a mass conversion rather
> > than the death by a thousand cuts approach. The display
> > code is already messy enough when it comes to naming local
> > variables.

As Ville I'm more in favor of the mass conversion using sed and/or
coccinelle. It's less painful and less confusing.

> 
> We could use i915 in display code as a convention for code that does not
> do register access. ;)

For me this is the worst part of gem code... the mix
of i915 and dev_priv for these cases. Something that a massive
and global change could fix?

> 
> BR,
> Jani.
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Chris Wilson March 19, 2019, 5:36 p.m. UTC | #6

Quoting Rodrigo Vivi (2019-03-19 17:31:40)
> On Tue, Mar 19, 2019 at 05:50:22PM +0200, Jani Nikula wrote:
> > On Mon, 18 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > On Mon, Mar 18, 2019 at 09:45:16PM +0100, Michal Wajdeczko wrote:
> > >> > Make things look a bit nicer by passing dev_priv to
> > >> 
> > >> In other places we are changing naming from dev_priv to i915.
> > >> Can we do the same here ?
> > >
> > > The display code uses dev_priv quite consistently,. We could
> > > go for i915 I suppose but I'd prefer a mass conversion rather
> > > than the death by a thousand cuts approach. The display
> > > code is already messy enough when it comes to naming local
> > > variables.
> 
> As Ville I'm more in favor of the mass conversion using sed and/or
> coccinelle. It's less painful and less confusing.
> 
> > 
> > We could use i915 in display code as a convention for code that does not
> > do register access. ;)
> 
> For me this is the worst part of gem code... the mix
> of i915 and dev_priv for these cases. Something that a massive
> and global change could fix?

Yes, killing I915_READ(). If only plans were afoot...
-Chris

[3/3] drm/i915: Pass dev_priv to intel_is_dual_link_lvds()

Commit Message

Comments

Patch