[06/11] drm/i915/cdclk: Extract vlv_dsi_min_cdclk()

Message ID	20241029215217.3697-7-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 Subject: [PATCH 06/11] drm/i915/cdclk: Extract vlv_dsi_min_cdclk() Date: Tue, 29 Oct 2024 23:52:12 +0200 Message-ID: <20241029215217.3697-7-ville.syrjala@linux.intel.com> In-Reply-To: <20241029215217.3697-1-ville.syrjala@linux.intel.com> References: <20241029215217.3697-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: list Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
Series	drm/i915/cdclk: Declutter CDCLK code \| expand [00/11] drm/i915/cdclk: Declutter CDCLK code [01/11] drm/i915: Introduce HAS_DOUBLE_WIDE() [02/11] drm/i915/cdclk: Extract intel_cdclk_guardband() and intel_cdclk_ppc() [03/11] drm/i915/cdclk: Extract hsw_ips_min_cdclk() [04/11] drm/i915/cdclk: Extract intel_audio_min_cdclk() [05/11] drm/i915/cdclk: Factor out has_audio check in intel_audio_min_cdclk() [06/11] drm/i915/cdclk: Extract vlv_dsi_min_cdclk() [07/11] drm/i915/cdclk: Factor out INTEL_OUTPUT_DSI check in vlv_dsi_min_cdclk() [08/11] drm/i915/cdclk: Suck the compression_enable check into intel_vdsc_min_cdclk() [09/11] drm/i915/cdclk: Drop pointles max_t() usage in intel_vdsc_min_cdclk() [10/11] drm/i915/cdclk: Relocate intel_vdsc_min_cdclk() [11/11] drm/i915/cdclk: Unify cdclk max() parameter order

Message ID

20241029215217.3697-7-ville.syrjala@linux.intel.com (mailing list archive)

State

New, archived

Headers

From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 06/11] drm/i915/cdclk: Extract vlv_dsi_min_cdclk()
Date: Tue, 29 Oct 2024 23:52:12 +0200
Message-ID: <20241029215217.3697-7-ville.syrjala@linux.intel.com>
In-Reply-To: <20241029215217.3697-1-ville.syrjala@linux.intel.com>
References: <20241029215217.3697-1-ville.syrjala@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: list
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Series

drm/i915/cdclk: Declutter CDCLK code | expand

Commit Message

Ville Syrjälä Oct. 29, 2024, 9:52 p.m. UTC

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

Pull the DSI min cdclk calculation into a helper and hide
it inside vlv_dsi.c in order to keep most DSI related
details in one place.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++------------------
 drivers/gpu/drm/i915/display/vlv_dsi.c     | 25 ++++++++++++++++++++++
 drivers/gpu/drm/i915/display/vlv_dsi.h     |  8 +++++++
 3 files changed, 35 insertions(+), 21 deletions(-)

Comments

Jani Nikula Oct. 30, 2024, 11:34 a.m. UTC | #1

On Tue, 29 Oct 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Pull the DSI min cdclk calculation into a helper and hide
> it inside vlv_dsi.c in order to keep most DSI related
> details in one place.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++------------------
>  drivers/gpu/drm/i915/display/vlv_dsi.c     | 25 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/vlv_dsi.h     |  8 +++++++
>  3 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 89d12c521411..e10378744607 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -46,6 +46,7 @@
>  #include "intel_vdsc.h"
>  #include "skl_watermark.h"
>  #include "skl_watermark_regs.h"
> +#include "vlv_dsi.h"
>  #include "vlv_sideband.h"
>  
>  /**
> @@ -2849,8 +2850,6 @@ static int intel_vdsc_min_cdclk(const struct intel_crtc_state *crtc_state)
>  
>  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_display *display = to_intel_display(crtc_state);
> -	struct drm_i915_private *dev_priv = to_i915(display->drm);
>  	int min_cdclk;
>  
>  	if (!crtc_state->hw.enable)
> @@ -2859,25 +2858,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
>  	min_cdclk = max(hsw_ips_min_cdclk(crtc_state), min_cdclk);
>  	min_cdclk = max(intel_audio_min_cdclk(crtc_state), min_cdclk);
> -
> -	/*
> -	 * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
> -	 * than 320000KHz.
> -	 */
> -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> -	    IS_VALLEYVIEW(dev_priv))
> -		min_cdclk = max(320000, min_cdclk);
> -
> -	/*
> -	 * On Geminilake once the CDCLK gets as low as 79200
> -	 * picture gets unstable, despite that values are
> -	 * correct for DSI PLL and DE PLL.
> -	 */
> -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> -	    IS_GEMINILAKE(dev_priv))
> -		min_cdclk = max(158400, min_cdclk);
> -
> -	/* Account for additional needs from the planes */
> +	min_cdclk = max(vlv_dsi_min_cdclk(crtc_state), min_cdclk);
>  	min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
>  
>  	if (crtc_state->dsc.compression_enable)
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 9383eedee2d4..49a895589150 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1760,6 +1760,31 @@ static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
>  	intel_dsi_log_params(intel_dsi);
>  }
>  
> +int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> +	int min_cdclk = 0;
> +
> +	/*
> +	 * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
> +	 * than 320000KHz.
> +	 */
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> +	    IS_VALLEYVIEW(dev_priv))
> +		min_cdclk = max(320000, min_cdclk);
> +
> +	/*
> +	 * On Geminilake once the CDCLK gets as low as 79200
> +	 * picture gets unstable, despite that values are
> +	 * correct for DSI PLL and DE PLL.
> +	 */
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> +	    IS_GEMINILAKE(dev_priv))
> +		min_cdclk = max(158400, min_cdclk);
> +
> +	return min_cdclk;
> +}
> +
>  typedef void (*vlv_dsi_dmi_quirk_func)(struct intel_dsi *intel_dsi);
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.h b/drivers/gpu/drm/i915/display/vlv_dsi.h
> index cf9d7b82f288..5f99059b4c48 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.h
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.h
> @@ -8,13 +8,17 @@
>  
>  #include <linux/types.h>
>  
> +#include <drm/drm_mipi_dsi.h>
> +

Huh, why is this required? At least it's unrelated to the patch.

Other than that,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>  enum port;
>  struct drm_i915_private;
> +struct intel_crtc_state;
>  struct intel_dsi;
>  
>  #ifdef I915
>  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
>  enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
> +int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state);
>  void vlv_dsi_init(struct drm_i915_private *dev_priv);
>  #else
>  static inline void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
> @@ -24,6 +28,10 @@ static inline enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt
>  {
>  	return 0;
>  }
> +static inline int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
> +{
> +	return 0;
> +}
>  static inline void vlv_dsi_init(struct drm_i915_private *dev_priv)
>  {
>  }

Ville Syrjälä Oct. 30, 2024, 1:22 p.m. UTC | #2

On Wed, Oct 30, 2024 at 01:34:38PM +0200, Jani Nikula wrote:
> On Tue, 29 Oct 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Pull the DSI min cdclk calculation into a helper and hide
> > it inside vlv_dsi.c in order to keep most DSI related
> > details in one place.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++------------------
> >  drivers/gpu/drm/i915/display/vlv_dsi.c     | 25 ++++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/vlv_dsi.h     |  8 +++++++
> >  3 files changed, 35 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 89d12c521411..e10378744607 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -46,6 +46,7 @@
> >  #include "intel_vdsc.h"
> >  #include "skl_watermark.h"
> >  #include "skl_watermark_regs.h"
> > +#include "vlv_dsi.h"
> >  #include "vlv_sideband.h"
> >  
> >  /**
> > @@ -2849,8 +2850,6 @@ static int intel_vdsc_min_cdclk(const struct intel_crtc_state *crtc_state)
> >  
> >  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> >  {
> > -	struct intel_display *display = to_intel_display(crtc_state);
> > -	struct drm_i915_private *dev_priv = to_i915(display->drm);
> >  	int min_cdclk;
> >  
> >  	if (!crtc_state->hw.enable)
> > @@ -2859,25 +2858,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> >  	min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
> >  	min_cdclk = max(hsw_ips_min_cdclk(crtc_state), min_cdclk);
> >  	min_cdclk = max(intel_audio_min_cdclk(crtc_state), min_cdclk);
> > -
> > -	/*
> > -	 * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
> > -	 * than 320000KHz.
> > -	 */
> > -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> > -	    IS_VALLEYVIEW(dev_priv))
> > -		min_cdclk = max(320000, min_cdclk);
> > -
> > -	/*
> > -	 * On Geminilake once the CDCLK gets as low as 79200
> > -	 * picture gets unstable, despite that values are
> > -	 * correct for DSI PLL and DE PLL.
> > -	 */
> > -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> > -	    IS_GEMINILAKE(dev_priv))
> > -		min_cdclk = max(158400, min_cdclk);
> > -
> > -	/* Account for additional needs from the planes */
> > +	min_cdclk = max(vlv_dsi_min_cdclk(crtc_state), min_cdclk);
> >  	min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
> >  
> >  	if (crtc_state->dsc.compression_enable)
> > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> > index 9383eedee2d4..49a895589150 100644
> > --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> > @@ -1760,6 +1760,31 @@ static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
> >  	intel_dsi_log_params(intel_dsi);
> >  }
> >  
> > +int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> > +	int min_cdclk = 0;
> > +
> > +	/*
> > +	 * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
> > +	 * than 320000KHz.
> > +	 */
> > +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> > +	    IS_VALLEYVIEW(dev_priv))
> > +		min_cdclk = max(320000, min_cdclk);
> > +
> > +	/*
> > +	 * On Geminilake once the CDCLK gets as low as 79200
> > +	 * picture gets unstable, despite that values are
> > +	 * correct for DSI PLL and DE PLL.
> > +	 */
> > +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> > +	    IS_GEMINILAKE(dev_priv))
> > +		min_cdclk = max(158400, min_cdclk);
> > +
> > +	return min_cdclk;
> > +}
> > +
> >  typedef void (*vlv_dsi_dmi_quirk_func)(struct intel_dsi *intel_dsi);
> >  
> >  /*
> > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.h b/drivers/gpu/drm/i915/display/vlv_dsi.h
> > index cf9d7b82f288..5f99059b4c48 100644
> > --- a/drivers/gpu/drm/i915/display/vlv_dsi.h
> > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.h
> > @@ -8,13 +8,17 @@
> >  
> >  #include <linux/types.h>
> >  
> > +#include <drm/drm_mipi_dsi.h>
> > +
> 
> Huh, why is this required? At least it's unrelated to the patch.

Sorry meant to note that in the commit msg, but forgot.

xe fails to build without this:
../drivers/gpu/drm/i915/display/vlv_dsi.h:28:42: error: return type is an incomplete type

It looks like a forward declaration is not enough for
return types of static inline functions. i915 on the other
hand builds fine.

I suppose one alternative would be to declare is as just
'int' for xe.

> 
> Other than that,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> >  enum port;
> >  struct drm_i915_private;
> > +struct intel_crtc_state;
> >  struct intel_dsi;
> >  
> >  #ifdef I915
> >  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
> >  enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
> > +int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state);
> >  void vlv_dsi_init(struct drm_i915_private *dev_priv);
> >  #else
> >  static inline void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
> > @@ -24,6 +28,10 @@ static inline enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt
> >  {
> >  	return 0;
> >  }
> > +static inline int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
> > +{
> > +	return 0;
> > +}
> >  static inline void vlv_dsi_init(struct drm_i915_private *dev_priv)
> >  {
> >  }
> 
> -- 
> Jani Nikula, Intel

Jani Nikula Oct. 30, 2024, 5:55 p.m. UTC | #3

On Wed, 30 Oct 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Oct 30, 2024 at 01:34:38PM +0200, Jani Nikula wrote:
>> On Tue, 29 Oct 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Pull the DSI min cdclk calculation into a helper and hide
>> > it inside vlv_dsi.c in order to keep most DSI related
>> > details in one place.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++------------------
>> >  drivers/gpu/drm/i915/display/vlv_dsi.c     | 25 ++++++++++++++++++++++
>> >  drivers/gpu/drm/i915/display/vlv_dsi.h     |  8 +++++++
>> >  3 files changed, 35 insertions(+), 21 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > index 89d12c521411..e10378744607 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > @@ -46,6 +46,7 @@
>> >  #include "intel_vdsc.h"
>> >  #include "skl_watermark.h"
>> >  #include "skl_watermark_regs.h"
>> > +#include "vlv_dsi.h"
>> >  #include "vlv_sideband.h"
>> >  
>> >  /**
>> > @@ -2849,8 +2850,6 @@ static int intel_vdsc_min_cdclk(const struct intel_crtc_state *crtc_state)
>> >  
>> >  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>> >  {
>> > -	struct intel_display *display = to_intel_display(crtc_state);
>> > -	struct drm_i915_private *dev_priv = to_i915(display->drm);
>> >  	int min_cdclk;
>> >  
>> >  	if (!crtc_state->hw.enable)
>> > @@ -2859,25 +2858,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>> >  	min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
>> >  	min_cdclk = max(hsw_ips_min_cdclk(crtc_state), min_cdclk);
>> >  	min_cdclk = max(intel_audio_min_cdclk(crtc_state), min_cdclk);
>> > -
>> > -	/*
>> > -	 * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
>> > -	 * than 320000KHz.
>> > -	 */
>> > -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
>> > -	    IS_VALLEYVIEW(dev_priv))
>> > -		min_cdclk = max(320000, min_cdclk);
>> > -
>> > -	/*
>> > -	 * On Geminilake once the CDCLK gets as low as 79200
>> > -	 * picture gets unstable, despite that values are
>> > -	 * correct for DSI PLL and DE PLL.
>> > -	 */
>> > -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
>> > -	    IS_GEMINILAKE(dev_priv))
>> > -		min_cdclk = max(158400, min_cdclk);
>> > -
>> > -	/* Account for additional needs from the planes */
>> > +	min_cdclk = max(vlv_dsi_min_cdclk(crtc_state), min_cdclk);
>> >  	min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
>> >  
>> >  	if (crtc_state->dsc.compression_enable)
>> > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> > index 9383eedee2d4..49a895589150 100644
>> > --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
>> > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> > @@ -1760,6 +1760,31 @@ static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
>> >  	intel_dsi_log_params(intel_dsi);
>> >  }
>> >  
>> > +int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
>> > +{
>> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
>> > +	int min_cdclk = 0;
>> > +
>> > +	/*
>> > +	 * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
>> > +	 * than 320000KHz.
>> > +	 */
>> > +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
>> > +	    IS_VALLEYVIEW(dev_priv))
>> > +		min_cdclk = max(320000, min_cdclk);
>> > +
>> > +	/*
>> > +	 * On Geminilake once the CDCLK gets as low as 79200
>> > +	 * picture gets unstable, despite that values are
>> > +	 * correct for DSI PLL and DE PLL.
>> > +	 */
>> > +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
>> > +	    IS_GEMINILAKE(dev_priv))
>> > +		min_cdclk = max(158400, min_cdclk);
>> > +
>> > +	return min_cdclk;
>> > +}
>> > +
>> >  typedef void (*vlv_dsi_dmi_quirk_func)(struct intel_dsi *intel_dsi);
>> >  
>> >  /*
>> > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.h b/drivers/gpu/drm/i915/display/vlv_dsi.h
>> > index cf9d7b82f288..5f99059b4c48 100644
>> > --- a/drivers/gpu/drm/i915/display/vlv_dsi.h
>> > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.h
>> > @@ -8,13 +8,17 @@
>> >  
>> >  #include <linux/types.h>
>> >  
>> > +#include <drm/drm_mipi_dsi.h>
>> > +
>> 
>> Huh, why is this required? At least it's unrelated to the patch.
>
> Sorry meant to note that in the commit msg, but forgot.
>
> xe fails to build without this:
> ../drivers/gpu/drm/i915/display/vlv_dsi.h:28:42: error: return type is an incomplete type
>
> It looks like a forward declaration is not enough for
> return types of static inline functions. i915 on the other
> hand builds fine.
>
> I suppose one alternative would be to declare is as just
> 'int' for xe.

I think I've actually snuck in some of those int usages in the past,
because in the end it doesn't really matter for the inline stubs.

BR,
Jani.

>
>> 
>> Other than that,
>> 
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> 
>> >  enum port;
>> >  struct drm_i915_private;
>> > +struct intel_crtc_state;
>> >  struct intel_dsi;
>> >  
>> >  #ifdef I915
>> >  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
>> >  enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
>> > +int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state);
>> >  void vlv_dsi_init(struct drm_i915_private *dev_priv);
>> >  #else
>> >  static inline void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>> > @@ -24,6 +28,10 @@ static inline enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt
>> >  {
>> >  	return 0;
>> >  }
>> > +static inline int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
>> > +{
>> > +	return 0;
>> > +}
>> >  static inline void vlv_dsi_init(struct drm_i915_private *dev_priv)
>> >  {
>> >  }
>> 
>> -- 
>> Jani Nikula, Intel

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 89d12c521411..e10378744607 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -46,6 +46,7 @@ 
 #include "intel_vdsc.h"
 #include "skl_watermark.h"
 #include "skl_watermark_regs.h"
+#include "vlv_dsi.h"
 #include "vlv_sideband.h"
 
 /**
@@ -2849,8 +2850,6 @@  static int intel_vdsc_min_cdclk(const struct intel_crtc_state *crtc_state)
 
 int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 {
-	struct intel_display *display = to_intel_display(crtc_state);
-	struct drm_i915_private *dev_priv = to_i915(display->drm);
 	int min_cdclk;
 
 	if (!crtc_state->hw.enable)
@@ -2859,25 +2858,7 @@  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	min_cdclk = intel_pixel_rate_to_cdclk(crtc_state);
 	min_cdclk = max(hsw_ips_min_cdclk(crtc_state), min_cdclk);
 	min_cdclk = max(intel_audio_min_cdclk(crtc_state), min_cdclk);
-
-	/*
-	 * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
-	 * than 320000KHz.
-	 */
-	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
-	    IS_VALLEYVIEW(dev_priv))
-		min_cdclk = max(320000, min_cdclk);
-
-	/*
-	 * On Geminilake once the CDCLK gets as low as 79200
-	 * picture gets unstable, despite that values are
-	 * correct for DSI PLL and DE PLL.
-	 */
-	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
-	    IS_GEMINILAKE(dev_priv))
-		min_cdclk = max(158400, min_cdclk);
-
-	/* Account for additional needs from the planes */
+	min_cdclk = max(vlv_dsi_min_cdclk(crtc_state), min_cdclk);
 	min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
 
 	if (crtc_state->dsc.compression_enable)
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 9383eedee2d4..49a895589150 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -1760,6 +1760,31 @@  static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
 	intel_dsi_log_params(intel_dsi);
 }
 
+int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
+	int min_cdclk = 0;
+
+	/*
+	 * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
+	 * than 320000KHz.
+	 */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
+	    IS_VALLEYVIEW(dev_priv))
+		min_cdclk = max(320000, min_cdclk);
+
+	/*
+	 * On Geminilake once the CDCLK gets as low as 79200
+	 * picture gets unstable, despite that values are
+	 * correct for DSI PLL and DE PLL.
+	 */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
+	    IS_GEMINILAKE(dev_priv))
+		min_cdclk = max(158400, min_cdclk);
+
+	return min_cdclk;
+}
+
 typedef void (*vlv_dsi_dmi_quirk_func)(struct intel_dsi *intel_dsi);
 
 /*
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.h b/drivers/gpu/drm/i915/display/vlv_dsi.h
index cf9d7b82f288..5f99059b4c48 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.h
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.h
@@ -8,13 +8,17 @@ 
 
 #include <linux/types.h>
 
+#include <drm/drm_mipi_dsi.h>
+
 enum port;
 struct drm_i915_private;
+struct intel_crtc_state;
 struct intel_dsi;
 
 #ifdef I915
 void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
 enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
+int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state);
 void vlv_dsi_init(struct drm_i915_private *dev_priv);
 #else
 static inline void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
@@ -24,6 +28,10 @@  static inline enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt
 {
 	return 0;
 }
+static inline int vlv_dsi_min_cdclk(const struct intel_crtc_state *crtc_state)
+{
+	return 0;
+}
 static inline void vlv_dsi_init(struct drm_i915_private *dev_priv)
 {
 }

[06/11] drm/i915/cdclk: Extract vlv_dsi_min_cdclk()

Commit Message

Comments

Patch