Re: [PATCH v2 2/2] drm/i915/dsi: use gen7 prefix for the global DSI functions

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

 



> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, July 5, 2018 3:31 PM
> To: Chauhan, Madhav <madhav.chauhan@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Daniel Vetter <daniel@xxxxxxxx>; Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Subject: RE: [PATCH v2 2/2] drm/i915/dsi: use gen7 prefix for the global DSI
> functions
> 
> On Thu, 05 Jul 2018, "Chauhan, Madhav" <madhav.chauhan@xxxxxxxxx>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Tuesday, July 3, 2018 7:23 PM
> >> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> >> Cc: Nikula, Jani <jani.nikula@xxxxxxxxx>; Chauhan, Madhav
> >> <madhav.chauhan@xxxxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; Chris
> >> Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >> Subject: [PATCH v2 2/2] drm/i915/dsi: use gen7 prefix for the global
> >> DSI functions
> >>
> >> Avoid confusion with the functions to be added for the new gen 11 DSI
> >> implementation by renaming the current DSI functions. While at it,
> >> permutate the words in the function names to make them all start with
> >> "gen7_dsi" or "gen7_dsi_pll".
> >>
> >> Leave the static functions as-is for now; they could be renamed later
> >> if needed.
> >>
> >> No functional changes.
> >>
> >> References: https://patchwork.freedesktop.org/series/44823/
> >> Cc: Madhav Chauhan <madhav.chauhan@xxxxxxxxx>
> >> Cc: Daniel Vetter <daniel@xxxxxxxx>
> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> >> ---
> >>  drivers/gpu/drm/i915/gen7_dsi.c      | 21 ++++++++++-----------
> >>  drivers/gpu/drm/i915/gen7_dsi_pll.c  | 18 +++++++++---------
> >> drivers/gpu/drm/i915/intel_display.c |  6 +++---
> >>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
> >>  drivers/gpu/drm/i915/intel_dsi.h     | 21 ++++++++++-----------
> >>  drivers/gpu/drm/i915/intel_dsi_vbt.c |  2 +-
> >>  6 files changed, 34 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gen7_dsi.c
> >> b/drivers/gpu/drm/i915/gen7_dsi.c index 3b7acb5a70b3..6dd5e9988221
> >> 100644
> >> --- a/drivers/gpu/drm/i915/gen7_dsi.c
> >> +++ b/drivers/gpu/drm/i915/gen7_dsi.c
> >> @@ -69,7 +69,7 @@ enum mipi_dsi_pixel_format
> >> pixel_format_from_register_bits(u32 fmt)
> >
> > What about this function i.e. enum mipi_dsi_pixel_format
> pixel_format_from_register_bits()??
> > This will be used by gen11_dsi.c and currently getting used inside
> intel_dsi_vbt.c.
> 
> Let it be for now. If we ever reach an agreement over what the filenames
> should be (argh!) we can afterwards repurpose intel_dsi.c for any common
> stuff.

Agree.

Reviewed-by: Madhav Chauhan <madhav.chauhan@xxxxxxxxx>

Regards,
Madhav
> 
> BR,
> Jani.
> 
> >
> > Regards,
> > Madhav
> >
> >>  	}
> >>  }
> >>
> >> -void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port
> >> port)
> >> +void gen7_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum
> >> +port port)
> >>  {
> >>  	struct drm_encoder *encoder = &intel_dsi->base.base;
> >>  	struct drm_device *dev = encoder->dev; @@ -344,7 +344,7 @@
> static
> >> bool intel_dsi_compute_config(struct intel_encoder *encoder,
> >>  			pipe_config->cpu_transcoder = TRANSCODER_DSI_A;
> >>  	}
> >>
> >> -	ret = intel_compute_dsi_pll(encoder, pipe_config);
> >> +	ret = gen7_dsi_pll_compute(encoder, pipe_config);
> >>  	if (ret)
> >>  		return false;
> >>
> >> @@ -810,8 +810,8 @@ static void intel_dsi_pre_enable(struct
> >> intel_encoder *encoder,
> >>  	 * The BIOS may leave the PLL in a wonky state where it doesn't
> >>  	 * lock. It needs to be fully powered down to fix it.
> >>  	 */
> >> -	intel_disable_dsi_pll(encoder);
> >> -	intel_enable_dsi_pll(encoder, pipe_config);
> >> +	gen7_dsi_pll_disable(encoder);
> >> +	gen7_dsi_pll_enable(encoder, pipe_config);
> >>
> >>  	if (IS_BROXTON(dev_priv)) {
> >>  		/* Add MIPI IO reset programming for modeset */ @@ -
> >> 949,7 +949,7 @@ static void intel_dsi_post_disable(struct
> >> intel_encoder *encoder,
> >>
> >>  	if (is_vid_mode(intel_dsi)) {
> >>  		for_each_dsi_port(port, intel_dsi->ports)
> >> -			wait_for_dsi_fifo_empty(intel_dsi, port);
> >> +			gen7_dsi_wait_for_fifo_empty(intel_dsi, port);
> >>
> >>  		intel_dsi_port_disable(encoder);
> >>  		usleep_range(2000, 5000);
> >> @@ -979,7 +979,7 @@ static void intel_dsi_post_disable(struct
> >> intel_encoder *encoder,
> >>  				val & ~MIPIO_RST_CTRL);
> >>  	}
> >>
> >> -	intel_disable_dsi_pll(encoder);
> >> +	gen7_dsi_pll_disable(encoder);
> >>
> >>  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> >>  		u32 val;
> >> @@ -1024,7 +1024,7 @@ static bool intel_dsi_get_hw_state(struct
> >> intel_encoder *encoder,
> >>  	 * configuration, otherwise accessing DSI registers will hang the
> >>  	 * machine. See BSpec North Display Engine registers/MIPI[BXT].
> >>  	 */
> >> -	if (IS_GEN9_LP(dev_priv) && !intel_dsi_pll_is_enabled(dev_priv))
> >> +	if (IS_GEN9_LP(dev_priv) && !gen7_dsi_pll_is_enabled(dev_priv))
> >>  		goto out_put_power;
> >>
> >>  	/* XXX: this only works for one DSI output */ @@ -1250,8 +1250,7
> @@
> >> static void intel_dsi_get_config(struct intel_encoder *encoder,
> >>  	if (IS_GEN9_LP(dev_priv))
> >>  		bxt_dsi_get_pipe_config(encoder, pipe_config);
> >>
> >> -	pclk = intel_dsi_get_pclk(encoder, pipe_config->pipe_bpp,
> >> -				  pipe_config);
> >> +	pclk = gen7_dsi_get_pclk(encoder, pipe_config->pipe_bpp,
> >> pipe_config);
> >>  	if (!pclk)
> >>  		return;
> >>
> >> @@ -1590,7 +1589,7 @@ static void intel_dsi_unprepare(struct
> >> intel_encoder *encoder)
> >>  			/* Panel commands can be sent when clock is in LP11
> */
> >>  			I915_WRITE(MIPI_DEVICE_READY(port), 0x0);
> >>
> >> -			intel_dsi_reset_clocks(encoder, port);
> >> +			gen7_dsi_reset_clocks(encoder, port);
> >>  			I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP);
> >>
> >>  			val = I915_READ(MIPI_DSI_FUNC_PRG(port));
> >> @@ -1713,7 +1712,7 @@ static void intel_dsi_add_properties(struct
> >> intel_connector *connector)
> >>  	}
> >>  }
> >>
> >> -void intel_dsi_init(struct drm_i915_private *dev_priv)
> >> +void gen7_dsi_init(struct drm_i915_private *dev_priv)
> >>  {
> >>  	struct drm_device *dev = &dev_priv->drm;
> >>  	struct intel_dsi *intel_dsi;
> >> diff --git a/drivers/gpu/drm/i915/gen7_dsi_pll.c
> >> b/drivers/gpu/drm/i915/gen7_dsi_pll.c
> >> index 2ff2ee7f3b78..bb46996a5843 100644
> >> --- a/drivers/gpu/drm/i915/gen7_dsi_pll.c
> >> +++ b/drivers/gpu/drm/i915/gen7_dsi_pll.c
> >> @@ -357,8 +357,8 @@ static u32 bxt_dsi_get_pclk(struct intel_encoder
> >> *encoder, int pipe_bpp,
> >>  	return pclk;
> >>  }
> >>
> >> -u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
> >> -		       struct intel_crtc_state *config)
> >> +u32 gen7_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
> >> +		      struct intel_crtc_state *config)
> >>  {
> >>  	if (IS_GEN9_LP(to_i915(encoder->base.dev)))
> >>  		return bxt_dsi_get_pclk(encoder, pipe_bpp, config); @@ -
> >> 568,7 +568,7 @@ static void gen9lp_enable_dsi_pll(struct
> >> intel_encoder *encoder,
> >>  	DRM_DEBUG_KMS("DSI PLL locked\n");
> >>  }
> >>
> >> -bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
> >> +bool gen7_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
> >>  {
> >>  	if (IS_GEN9_LP(dev_priv))
> >>  		return bxt_dsi_pll_is_enabled(dev_priv); @@ -578,8 +578,8
> @@ bool
> >> intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
> >>  	return false;
> >>  }
> >>
> >> -int intel_compute_dsi_pll(struct intel_encoder *encoder,
> >> -			  struct intel_crtc_state *config)
> >> +int gen7_dsi_pll_compute(struct intel_encoder *encoder,
> >> +			 struct intel_crtc_state *config)
> >>  {
> >>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>
> >> @@ -591,8 +591,8 @@ int intel_compute_dsi_pll(struct intel_encoder
> >> *encoder,
> >>  	return -ENODEV;
> >>  }
> >>
> >> -void intel_enable_dsi_pll(struct intel_encoder *encoder,
> >> -			  const struct intel_crtc_state *config)
> >> +void gen7_dsi_pll_enable(struct intel_encoder *encoder,
> >> +			 const struct intel_crtc_state *config)
> >>  {
> >>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>
> >> @@ -602,7 +602,7 @@ void intel_enable_dsi_pll(struct intel_encoder
> >> *encoder,
> >>  		gen9lp_enable_dsi_pll(encoder, config);  }
> >>
> >> -void intel_disable_dsi_pll(struct intel_encoder *encoder)
> >> +void gen7_dsi_pll_disable(struct intel_encoder *encoder)
> >>  {
> >>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>
> >> @@ -639,7 +639,7 @@ static void gen9lp_dsi_reset_clocks(struct
> >> intel_encoder *encoder,
> >>  	I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP);  }
> >>
> >> -void intel_dsi_reset_clocks(struct intel_encoder *encoder, enum port
> >> port)
> >> +void gen7_dsi_reset_clocks(struct intel_encoder *encoder, enum port
> >> +port)
> >>  {
> >>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 681e0710a467..e94a2d65b4a4 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -9414,7 +9414,7 @@ static bool bxt_get_dsi_transcoder_state(struct
> >> intel_crtc *crtc,
> >>  		 * registers/MIPI[BXT]. We can break out here early, since we
> >>  		 * need the same DSI PLL to be enabled for both DSI ports.
> >>  		 */
> >> -		if (!intel_dsi_pll_is_enabled(dev_priv))
> >> +		if (!gen7_dsi_pll_is_enabled(dev_priv))
> >>  			break;
> >>
> >>  		/* XXX: this works for video mode only */ @@ -14133,7
> >> +14133,7 @@ static void intel_setup_outputs(struct drm_i915_private
> >> *dev_priv)
> >>  		intel_ddi_init(dev_priv, PORT_B);
> >>  		intel_ddi_init(dev_priv, PORT_C);
> >>
> >> -		intel_dsi_init(dev_priv);
> >> +		gen7_dsi_init(dev_priv);
> >>  	} else if (HAS_DDI(dev_priv)) {
> >>  		int found;
> >>
> >> @@ -14239,7 +14239,7 @@ static void intel_setup_outputs(struct
> >> drm_i915_private *dev_priv)
> >>  				intel_hdmi_init(dev_priv, CHV_HDMID,
> PORT_D);
> >>  		}
> >>
> >> -		intel_dsi_init(dev_priv);
> >> +		gen7_dsi_init(dev_priv);
> >>  	} else if (!IS_GEN2(dev_priv) && !IS_PINEVIEW(dev_priv)) {
> >>  		bool found = false;
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >> b/drivers/gpu/drm/i915/intel_drv.h
> >> index 888a85dc3856..7fd0bb25bf84 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -1731,7 +1731,7 @@ int intel_dp_aux_init_backlight_funcs(struct
> >> intel_connector *intel_connector);  int
> >> intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port,
> >> int conn_id);  void intel_dp_mst_encoder_cleanup(struct
> >> intel_digital_port *intel_dig_port);
> >>  /* gen7_dsi.c */
> >> -void intel_dsi_init(struct drm_i915_private *dev_priv);
> >> +void gen7_dsi_init(struct drm_i915_private *dev_priv);
> >>
> >>  /* intel_dsi_dcs_backlight.c */
> >>  int intel_dsi_dcs_init_backlight_funcs(struct intel_connector
> >> *intel_connector); diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> >> b/drivers/gpu/drm/i915/intel_dsi.h
> >> index 1b5c2c167472..b87f2531aeea 100644
> >> --- a/drivers/gpu/drm/i915/intel_dsi.h
> >> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> >> @@ -130,20 +130,19 @@ static inline struct intel_dsi
> >> *enc_to_intel_dsi(struct drm_encoder *encoder)  }
> >>
> >>  /* gen7_dsi.c */
> >> -void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port
> >> port);
> >> +void gen7_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);
> >>
> >>  /* gen7_dsi_pll.c */
> >> -bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
> >> -int intel_compute_dsi_pll(struct intel_encoder *encoder,
> >> -			  struct intel_crtc_state *config);
> >> -void intel_enable_dsi_pll(struct intel_encoder *encoder,
> >> -			  const struct intel_crtc_state *config);
> >> -void intel_disable_dsi_pll(struct intel_encoder *encoder);
> >> -u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
> >> -		       struct intel_crtc_state *config);
> >> -void intel_dsi_reset_clocks(struct intel_encoder *encoder,
> >> -			    enum port port);
> >> +bool gen7_dsi_pll_is_enabled(struct drm_i915_private *dev_priv); int
> >> +gen7_dsi_pll_compute(struct intel_encoder *encoder,
> >> +			 struct intel_crtc_state *config); void
> >> +gen7_dsi_pll_enable(struct intel_encoder *encoder,
> >> +			 const struct intel_crtc_state *config); void
> >> +gen7_dsi_pll_disable(struct intel_encoder *encoder);
> >> +u32 gen7_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
> >> +		      struct intel_crtc_state *config); void
> >> +gen7_dsi_reset_clocks(struct intel_encoder *encoder, enum port
> >> +port);
> >>
> >>  /* intel_dsi_vbt.c */
> >>  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
> >> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> >> b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> >> index 4d6ffa7b3e7b..0f4bf9541e84 100644
> >> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> >> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> >> @@ -181,7 +181,7 @@ static const u8 *mipi_exec_send_packet(struct
> >> intel_dsi *intel_dsi,
> >>  		break;
> >>  	}
> >>
> >> -	wait_for_dsi_fifo_empty(intel_dsi, port);
> >> +	gen7_dsi_wait_for_fifo_empty(intel_dsi, port);
> >>
> >>  out:
> >>  	data += len;
> >> --
> >> 2.11.0
> >
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




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

  Powered by Linux