Re: [PATCH 1/4] drm/i915: Add more dev ops for MIPI sub encoder

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

 



On 10/21/2013 6:57 PM, Jani Nikula wrote:

Hi Shobhit -

On Mon, 21 Oct 2013, Shobhit Kumar <shobhit.kumar@xxxxxxxxx> wrote:
Also add new fields in intel_dsi to have all dphy related parameters.
These will be useful even when we go for pure generic MIPI design

I feel like we have a different idea of what the ideal generic design
is. For me, the goal is that we change the struct intel_dsi_device to
struct drm_bridge, and those drm_bridge drivers are all about the panel,
and have as few details about i915 or our hardware as possible. Having
the bridge drivers fill in register values to be written by the core DSI
code does not fit that. Yes, I had some of those in my bridge conversion
patches too, but I did not intend we'd keep adding more.

I'd rather we provide generic helpers the bridge driver can call.

Yeah, look like our ideas are different. In your goal with drm_bridge architecture, we will still end up having multiple bridge drivers for each different panel. But my goal is to have a single driver which can work for multiple panels. Since we already have enabled some panels with sub-encoder architecture for completeness I was planning to maintain generic driver as one sub-encoder. But actually we can do away with all sub-encoder and do not need even drm_bridge and all implementation will be in intel_dsi.c. Panel specific configurations or sequences will come from VBT which I have tried to convert as parameters.

All the parameters DSI/DPHY spec specific and none of them particularly relate to our hardware.


Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@xxxxxxxxx>
Signed-off-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_dsi.c |    9 ++++++++-
  drivers/gpu/drm/i915/intel_dsi.h |   29 +++++++++++++++++++++++++++++
  2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 9a2fdd2..34e19b7 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -147,6 +147,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)

  	DRM_DEBUG_KMS("\n");

+	if (intel_dsi->dev.dev_ops->panel_reset)
+		intel_dsi->dev.dev_ops->panel_reset(&intel_dsi->dev);

Would this map to ->pre_enable in drm_bridge?

I have not yet migrated to drm_bridge and need to check thes call flows for drm_bridge


+
  	temp = I915_READ(MIPI_DEVICE_READY(pipe));
  	if ((temp & DEVICE_READY) == 0) {
  		temp &= ~ULPS_STATE_MASK;
@@ -162,6 +165,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
  		I915_WRITE(MIPI_DEVICE_READY(pipe), temp);
  	}

+	if (intel_dsi->dev.dev_ops->send_otp_cmds)
+		intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev);

What is otp? one time programming? Why not in ->enable?

Yes. OTP is done before sending pixel stream and enable is done after we start pixel stream


+
  	if (is_cmd_mode(intel_dsi))
  		I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(pipe), 8 * 4);

@@ -176,7 +182,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
  		POSTING_READ(MIPI_PORT_CTRL(pipe));
  	}

-	intel_dsi->dev.dev_ops->enable(&intel_dsi->dev);
+	if (intel_dsi->dev.dev_ops->enable)
+		intel_dsi->dev.dev_ops->enable(&intel_dsi->dev);
  }

  static void intel_dsi_disable(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index c7765f3..b71c9b3 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -39,6 +39,13 @@ struct intel_dsi_device {
  struct intel_dsi_dev_ops {
  	bool (*init)(struct intel_dsi_device *dsi);

+	void (*panel_reset)(struct intel_dsi_device *dsi);
+
+	void (*disable_panel_power)(struct intel_dsi_device *dsi);

What is the enabling counterpart to disable_panel_power? panel_reset?

Yes.


+
+	/* send one time programmable commands */
+	void (*send_otp_cmds)(struct intel_dsi_device *dsi);
+
  	/* This callback must be able to assume DSI commands can be sent */
  	void (*enable)(struct intel_dsi_device *dsi);

@@ -89,6 +96,28 @@ struct intel_dsi {

  	/* eot for MIPI_EOT_DISABLE register */
  	u32 eot_disable;
+
+	u16 dsi_clock_freq;
+	u8 video_mode_type;
+	u32 data_width;
+	u8 dither;
+	u32 port_bits;
+	u8 escape_clk_div;
+	u32 lp_rx_timeout;
+	u8 turn_arnd_val;
+	u16 init_count;
+	u16 rst_timer_val;
+	u16 hs_to_lp_count;
+	u16 lp_byte_clk;
+	u32 bw_timer;
+	u16 clk_lp_to_hs_count;
+	u16 clk_hs_to_lp_count;
+	u32 video_frmt_cfg_bits;
+	u32 dphy_reg;
+
+	u8 backlight_off_delay; /*in ms*/
+	bool send_shutdown;
+	u8 shutdown_pkt_delay; /*in ms*/
  };

  static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
--
1.7.9.5



Regards
Shobhit


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://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