[RFC 00/14] DSI Command mode(DBI mode) enabling on CHT

[RFC 00/14] DSI Command mode(DBI mode) enabling on CHT

[Gaurav K Singh]

Hi,

These set of patches are for enabling DSI command mode. Command Mode refers to
an operation in which transactions primarily take the form of sending commands
and data to a peripheral that incorporates a display controller. The display
controller may include local registers and a frame buffer. The host processor
indirectly controls activity at the peripheral by sending commands, parameters
and data to the display controller.

Sink refreshes from its local frame buffer. Image updates require Source to write
new data into the frame buffer which Source sends in the form of command + payload.

The command mode panel we have here is 1080 x 1920. For this panel, features like support of
i2c transactions for sending backlight on sequence, VBT version 3 patches, GPIO configuration
changes for CHT are required. The same will be added as part of different patch series.
Floating the command mode patches separately to start with the initial review.

Regards
Gaurav

Gaurav K Singh (14):
  drm/i915: allocate gem memory for mipi dbi cmd buffer
  drm/i915: Add support for TEAR ON Sequence
  drm/i915: Add functions for dcs memory write cmd
  drm/i915: Calculate bw timer for mipi DBI interface
  drm/i915: Use the bpp value wrt the pixel format
  drm/i915: Disable vlank interrupt for disabling MIPI cmd mode
  drm/i915: Disable MIPI display self refresh mode
  drm/i915: Disable Tearing effect trigger by GPIO pin
  drm/i915: Changes for command mode preparation
  drm/i915: Enable Tearing effect trigger by GPIO pin
  drm/i915: Enable MIPI display self refresh mode
  drm/i915: Generalize DSI enable function
  drm/i915: Reset the display hw if vid mode to cmd mode
  drm/i915: send one frame after enabling mipi cmd mode

 drivers/gpu/drm/i915/i915_drv.h            |    1 +
 drivers/gpu/drm/i915/i915_reg.h            |    1 +
 drivers/gpu/drm/i915/intel_bios.h          |    4 +
 drivers/gpu/drm/i915/intel_display.c       |  155 ++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h           |    7 ++
 drivers/gpu/drm/i915/intel_dsi.c           |  136 +++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_dsi.h           |    5 +
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   36 ++++++-
 8 files changed, 311 insertions(+), 34 deletions(-)

-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer

[Gaurav K Singh]

Allocate gem memory for MIPI DBI command buffer. This memory
will be used when sending command via DBI interface.

Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   31 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi.h |    4 ++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 5196642..2e3c801 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -415,6 +415,27 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 
 	DRM_DEBUG_KMS("\n");
 
+	if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) {
+		intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096);
+		if (!intel_dsi->gem_obj) {
+			DRM_ERROR("Failed to allocate seqno page\n");
+			return;
+		}
+
+		i915_gem_object_set_cache_level(intel_dsi->gem_obj,
+						I915_CACHE_LLC);
+
+		if (i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0)) {
+			DRM_ERROR("MIPI command buffer GTT pin failed");
+			return;
+		}
+
+		intel_dsi->cmd_buff =
+				kmap(sg_page(intel_dsi->gem_obj->pages->sgl));
+		intel_dsi->cmd_buff_phy_addr = page_to_phys(
+				sg_page(intel_dsi->gem_obj->pages->sgl));
+	}
+
 	/* Disable DPOunit clock gating, can stall pipe
 	 * and we need DPLL REFA always enabled */
 	tmp = I915_READ(DPLL(pipe));
@@ -576,6 +597,12 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
 
 	msleep(intel_dsi->panel_off_delay);
 	msleep(intel_dsi->panel_pwr_cycle_delay);
+
+	if (intel_dsi->gem_obj) {
+		kunmap(intel_dsi->cmd_buff);
+		i915_gem_object_ggtt_unpin(intel_dsi->gem_obj);
+		drm_gem_object_unreference(&intel_dsi->gem_obj->base);
+	}
 }
 
 static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
@@ -1048,6 +1075,10 @@ void intel_dsi_init(struct drm_device *dev)
 		intel_dsi->ports = (1 << PORT_C);
 	}
 
+	intel_dsi->cmd_buff = NULL;
+	intel_dsi->cmd_buff_phy_addr = 0;
+	intel_dsi->gem_obj = NULL;
+
 	/* Create a DSI host (and a device) for each port. */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		struct intel_dsi_host *host;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 2784ac4..36ca3cc 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -44,6 +44,10 @@ struct intel_dsi {
 
 	struct intel_connector *attached_connector;
 
+	struct drm_i915_gem_object *gem_obj;
+	void *cmd_buff;
+	dma_addr_t cmd_buff_phy_addr;
+
 	/* bit mask of ports being driven */
 	u16 ports;
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer

[Ville Syrjälä]

On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote:
> Allocate gem memory for MIPI DBI command buffer. This memory
> will be used when sending command via DBI interface.

Why would you allocate this via gem? AFAICS you only feed the bus
address to the hardware. Using the dma-api would seem like the right
choice here, but I'm not sure how to deal with the dma mask.

> 
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c |   31 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.h |    4 ++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 5196642..2e3c801 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -415,6 +415,27 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) {
> +		intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096);
> +		if (!intel_dsi->gem_obj) {
> +			DRM_ERROR("Failed to allocate seqno page\n");
> +			return;
> +		}
> +
> +		i915_gem_object_set_cache_level(intel_dsi->gem_obj,
> +						I915_CACHE_LLC);
> +
> +		if (i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0)) {
> +			DRM_ERROR("MIPI command buffer GTT pin failed");
> +			return;
> +		}
> +
> +		intel_dsi->cmd_buff =
> +				kmap(sg_page(intel_dsi->gem_obj->pages->sgl));
> +		intel_dsi->cmd_buff_phy_addr = page_to_phys(
> +				sg_page(intel_dsi->gem_obj->pages->sgl));
> +	}
> +
>  	/* Disable DPOunit clock gating, can stall pipe
>  	 * and we need DPLL REFA always enabled */
>  	tmp = I915_READ(DPLL(pipe));
> @@ -576,6 +597,12 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>  
>  	msleep(intel_dsi->panel_off_delay);
>  	msleep(intel_dsi->panel_pwr_cycle_delay);
> +
> +	if (intel_dsi->gem_obj) {
> +		kunmap(intel_dsi->cmd_buff);
> +		i915_gem_object_ggtt_unpin(intel_dsi->gem_obj);
> +		drm_gem_object_unreference(&intel_dsi->gem_obj->base);
> +	}
>  }
>  
>  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> @@ -1048,6 +1075,10 @@ void intel_dsi_init(struct drm_device *dev)
>  		intel_dsi->ports = (1 << PORT_C);
>  	}
>  
> +	intel_dsi->cmd_buff = NULL;
> +	intel_dsi->cmd_buff_phy_addr = 0;
> +	intel_dsi->gem_obj = NULL;
> +
>  	/* Create a DSI host (and a device) for each port. */
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		struct intel_dsi_host *host;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 2784ac4..36ca3cc 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -44,6 +44,10 @@ struct intel_dsi {
>  
>  	struct intel_connector *attached_connector;
>  
> +	struct drm_i915_gem_object *gem_obj;
> +	void *cmd_buff;
> +	dma_addr_t cmd_buff_phy_addr;
> +
>  	/* bit mask of ports being driven */
>  	u16 ports;
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer

[Daniel Vetter]

On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote:
> On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote:
> > Allocate gem memory for MIPI DBI command buffer. This memory
> > will be used when sending command via DBI interface.
> 
> Why would you allocate this via gem? AFAICS you only feed the bus
> address to the hardware. Using the dma-api would seem like the right
> choice here, but I'm not sure how to deal with the dma mask.

Yeah dma_alloc_coherent is what you want here. The mask can be ignored,
it should be suitable already.
-Daniel

> 
> > 
> > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c |   31 +++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dsi.h |    4 ++++
> >  2 files changed, 35 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 5196642..2e3c801 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -415,6 +415,27 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >  
> >  	DRM_DEBUG_KMS("\n");
> >  
> > +	if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) {
> > +		intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096);
> > +		if (!intel_dsi->gem_obj) {
> > +			DRM_ERROR("Failed to allocate seqno page\n");
> > +			return;
> > +		}
> > +
> > +		i915_gem_object_set_cache_level(intel_dsi->gem_obj,
> > +						I915_CACHE_LLC);
> > +
> > +		if (i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0)) {
> > +			DRM_ERROR("MIPI command buffer GTT pin failed");
> > +			return;
> > +		}
> > +
> > +		intel_dsi->cmd_buff =
> > +				kmap(sg_page(intel_dsi->gem_obj->pages->sgl));
> > +		intel_dsi->cmd_buff_phy_addr = page_to_phys(
> > +				sg_page(intel_dsi->gem_obj->pages->sgl));
> > +	}
> > +
> >  	/* Disable DPOunit clock gating, can stall pipe
> >  	 * and we need DPLL REFA always enabled */
> >  	tmp = I915_READ(DPLL(pipe));
> > @@ -576,6 +597,12 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
> >  
> >  	msleep(intel_dsi->panel_off_delay);
> >  	msleep(intel_dsi->panel_pwr_cycle_delay);
> > +
> > +	if (intel_dsi->gem_obj) {
> > +		kunmap(intel_dsi->cmd_buff);
> > +		i915_gem_object_ggtt_unpin(intel_dsi->gem_obj);
> > +		drm_gem_object_unreference(&intel_dsi->gem_obj->base);
> > +	}
> >  }
> >  
> >  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> > @@ -1048,6 +1075,10 @@ void intel_dsi_init(struct drm_device *dev)
> >  		intel_dsi->ports = (1 << PORT_C);
> >  	}
> >  
> > +	intel_dsi->cmd_buff = NULL;
> > +	intel_dsi->cmd_buff_phy_addr = 0;
> > +	intel_dsi->gem_obj = NULL;
> > +
> >  	/* Create a DSI host (and a device) for each port. */
> >  	for_each_dsi_port(port, intel_dsi->ports) {
> >  		struct intel_dsi_host *host;
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> > index 2784ac4..36ca3cc 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.h
> > +++ b/drivers/gpu/drm/i915/intel_dsi.h
> > @@ -44,6 +44,10 @@ struct intel_dsi {
> >  
> >  	struct intel_connector *attached_connector;
> >  
> > +	struct drm_i915_gem_object *gem_obj;
> > +	void *cmd_buff;
> > +	dma_addr_t cmd_buff_phy_addr;
> > +
> >  	/* bit mask of ports being driven */
> >  	u16 ports;
> >  
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer

[Ville Syrjälä]

On Fri, May 29, 2015 at 07:10:53PM +0200, Daniel Vetter wrote:
> On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote:
> > On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote:
> > > Allocate gem memory for MIPI DBI command buffer. This memory
> > > will be used when sending command via DBI interface.
> > 
> > Why would you allocate this via gem? AFAICS you only feed the bus
> > address to the hardware. Using the dma-api would seem like the right
> > choice here, but I'm not sure how to deal with the dma mask.
> 
> Yeah dma_alloc_coherent is what you want here. The mask can be ignored,
> it should be suitable already.

Umm, this thing seems to limited to 32bit addresses. And we set the mask
to 39 or 40 bits depending on the gen.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer

[Daniel Vetter]

On Mon, Jun 01, 2015 at 02:03:15PM +0300, Ville Syrjälä wrote:
> On Fri, May 29, 2015 at 07:10:53PM +0200, Daniel Vetter wrote:
> > On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote:
> > > On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote:
> > > > Allocate gem memory for MIPI DBI command buffer. This memory
> > > > will be used when sending command via DBI interface.
> > > 
> > > Why would you allocate this via gem? AFAICS you only feed the bus
> > > address to the hardware. Using the dma-api would seem like the right
> > > choice here, but I'm not sure how to deal with the dma mask.
> > 
> > Yeah dma_alloc_coherent is what you want here. The mask can be ignored,
> > it should be suitable already.
> 
> Umm, this thing seems to limited to 32bit addresses. And we set the mask
> to 39 or 40 bits depending on the gen.

Well that's what we have dma_set_coherent_mask for, hooray. Not the first
one, see the other hacks with comments in i915_driver_load.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer

[Singh, Gaurav K]

On 6/15/2015 4:00 PM, Daniel Vetter wrote:
> On Mon, Jun 01, 2015 at 02:03:15PM +0300, Ville Syrjälä wrote:
>> On Fri, May 29, 2015 at 07:10:53PM +0200, Daniel Vetter wrote:
>>> On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote:
>>>> On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote:
>>>>> Allocate gem memory for MIPI DBI command buffer. This memory
>>>>> will be used when sending command via DBI interface.
>>>> Why would you allocate this via gem? AFAICS you only feed the bus
>>>> address to the hardware. Using the dma-api would seem like the right
>>>> choice here, but I'm not sure how to deal with the dma mask.
>>> Yeah dma_alloc_coherent is what you want here. The mask can be ignored,
>>> it should be suitable already.
>> Umm, this thing seems to limited to 32bit addresses. And we set the mask
>> to 39 or 40 bits depending on the gen.
> Well that's what we have dma_set_coherent_mask for, hooray. Not the first
> one, see the other hacks with comments in i915_driver_load.
> -Daniel
checking on the usage of dma_alloc_coherent. Will come back on this.

With regards,
Gaurav
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

{Intel-gfx] [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer

[Gaurav K Singh]

Allocate gem memory for MIPI DBI command buffer. This memory
will be used when sending command via DBI interface.

v2: lock mutex before gem object unreference and later set gem obj ptr to NULL (Gaurav)

Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   40 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi.h |    4 ++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 98998e9..011fef2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -407,9 +407,35 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 	enum pipe pipe = intel_crtc->pipe;
 	enum port port;
 	u32 tmp;
+	int ret;
 
 	DRM_DEBUG_KMS("\n");
 
+	if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) {
+		intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096);
+		if (!intel_dsi->gem_obj) {
+			DRM_ERROR("Failed to allocate seqno page\n");
+			return;
+		}
+
+		ret = i915_gem_object_set_cache_level(intel_dsi->gem_obj,
+						      I915_CACHE_LLC);
+		if (ret)
+			goto err_unref;
+
+		ret = i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0);
+		if (ret) {
+err_unref:
+			drm_gem_object_unreference(&intel_dsi->gem_obj->base);
+			return;
+		}
+
+		intel_dsi->cmd_buff =
+				kmap(sg_page(intel_dsi->gem_obj->pages->sgl));
+		intel_dsi->cmd_buff_phy_addr = page_to_phys(
+				sg_page(intel_dsi->gem_obj->pages->sgl));
+	}
+
 	/* Disable DPOunit clock gating, can stall pipe
 	 * and we need DPLL REFA always enabled */
 	tmp = I915_READ(DPLL(pipe));
@@ -555,6 +581,7 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	struct drm_device *dev = encoder->base.dev;
 	u32 val;
 
 	DRM_DEBUG_KMS("\n");
@@ -571,6 +598,15 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
 
 	msleep(intel_dsi->panel_off_delay);
 	msleep(intel_dsi->panel_pwr_cycle_delay);
+
+	if (intel_dsi->gem_obj) {
+		kunmap(intel_dsi->cmd_buff);
+		i915_gem_object_ggtt_unpin(intel_dsi->gem_obj);
+		mutex_lock(&dev->struct_mutex);
+		drm_gem_object_unreference(&intel_dsi->gem_obj->base);
+		mutex_unlock(&dev->struct_mutex);
+	}
+	intel_dsi->gem_obj = NULL;
 }
 
 static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
@@ -1042,6 +1078,10 @@ void intel_dsi_init(struct drm_device *dev)
 		intel_dsi->ports = (1 << PORT_C);
 	}
 
+	intel_dsi->cmd_buff = NULL;
+	intel_dsi->cmd_buff_phy_addr = 0;
+	intel_dsi->gem_obj = NULL;
+
 	/* Create a DSI host (and a device) for each port. */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		struct intel_dsi_host *host;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 2784ac4..36ca3cc 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -44,6 +44,10 @@ struct intel_dsi {
 
 	struct intel_connector *attached_connector;
 
+	struct drm_i915_gem_object *gem_obj;
+	void *cmd_buff;
+	dma_addr_t cmd_buff_phy_addr;
+
 	/* bit mask of ports being driven */
 	u16 ports;
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: {Intel-gfx] [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer

[Singh, Gaurav K]

On 6/19/2015 3:32 AM, Gaurav K Singh wrote:
> Allocate gem memory for MIPI DBI command buffer. This memory
> will be used when sending command via DBI interface.
>
> v2: lock mutex before gem object unreference and later set gem obj ptr to NULL (Gaurav)
>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dsi.c |   40 ++++++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_dsi.h |    4 ++++
>   2 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 98998e9..011fef2 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -407,9 +407,35 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>   	enum pipe pipe = intel_crtc->pipe;
>   	enum port port;
>   	u32 tmp;
> +	int ret;
>   
>   	DRM_DEBUG_KMS("\n");
>   
> +	if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) {
> +		intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096);
> +		if (!intel_dsi->gem_obj) {
> +			DRM_ERROR("Failed to allocate seqno page\n");
> +			return;
> +		}
> +
> +		ret = i915_gem_object_set_cache_level(intel_dsi->gem_obj,
> +						      I915_CACHE_LLC);
> +		if (ret)
> +			goto err_unref;
> +
> +		ret = i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0);
> +		if (ret) {
> +err_unref:
> +			drm_gem_object_unreference(&intel_dsi->gem_obj->base);
> +			return;
> +		}
> +
> +		intel_dsi->cmd_buff =
> +				kmap(sg_page(intel_dsi->gem_obj->pages->sgl));
> +		intel_dsi->cmd_buff_phy_addr = page_to_phys(
> +				sg_page(intel_dsi->gem_obj->pages->sgl));
> +	}
> +
>   	/* Disable DPOunit clock gating, can stall pipe
>   	 * and we need DPLL REFA always enabled */
>   	tmp = I915_READ(DPLL(pipe));
> @@ -555,6 +581,7 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>   {
>   	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	struct drm_device *dev = encoder->base.dev;
>   	u32 val;
>   
>   	DRM_DEBUG_KMS("\n");
> @@ -571,6 +598,15 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>   
>   	msleep(intel_dsi->panel_off_delay);
>   	msleep(intel_dsi->panel_pwr_cycle_delay);
> +
> +	if (intel_dsi->gem_obj) {
> +		kunmap(intel_dsi->cmd_buff);
> +		i915_gem_object_ggtt_unpin(intel_dsi->gem_obj);
> +		mutex_lock(&dev->struct_mutex);
> +		drm_gem_object_unreference(&intel_dsi->gem_obj->base);
> +		mutex_unlock(&dev->struct_mutex);
> +	}
> +	intel_dsi->gem_obj = NULL;
>   }
>   
>   static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> @@ -1042,6 +1078,10 @@ void intel_dsi_init(struct drm_device *dev)
>   		intel_dsi->ports = (1 << PORT_C);
>   	}
>   
> +	intel_dsi->cmd_buff = NULL;
> +	intel_dsi->cmd_buff_phy_addr = 0;
> +	intel_dsi->gem_obj = NULL;
> +
>   	/* Create a DSI host (and a device) for each port. */
>   	for_each_dsi_port(port, intel_dsi->ports) {
>   		struct intel_dsi_host *host;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 2784ac4..36ca3cc 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -44,6 +44,10 @@ struct intel_dsi {
>   
>   	struct intel_connector *attached_connector;
>   
> +	struct drm_i915_gem_object *gem_obj;
> +	void *cmd_buff;
> +	dma_addr_t cmd_buff_phy_addr;
> +
>   	/* bit mask of ports being driven */
>   	u16 ports;
>   
Corrected the initial patch. Working on the dma_alloc_coherent patch , 
will update soon.

With regards,
Gaurav
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 02/14] drm/i915: Add support for TEAR ON Sequence

[Gaurav K Singh]

For command mode panel, panel's fb enabling and tearing configuration
is done as part of TEAR ON sequence. This patch parses and executes
TEAR ON sequence for MIPI command mode.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.h          |    4 ++++
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index af0b476..be4eaab 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -934,6 +934,10 @@ enum mipi_seq {
 	MIPI_SEQ_DISPLAY_ON,
 	MIPI_SEQ_DISPLAY_OFF,
 	MIPI_SEQ_DEASSERT_RESET,
+	MIPI_SEQ_BACKLIGHT_ON,
+	MIPI_SEQ_BACKLIGHT_OFF,
+	MIPI_SEQ_TEAR_ON,
+	MIPI_SEQ_TEAR_OFF,
 	MIPI_SEQ_MAX
 };
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index d2cd8d5..9deaec3 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -251,7 +251,11 @@ static const char * const seq_name[] = {
 	"MIPI_SEQ_INIT_OTP",
 	"MIPI_SEQ_DISPLAY_ON",
 	"MIPI_SEQ_DISPLAY_OFF",
-	"MIPI_SEQ_DEASSERT_RESET"
+	"MIPI_SEQ_DEASSERT_RESET",
+	"MIPI_SEQ_BACKLIGHT_ON",
+	"MIPI_SEQ_BACKLIGHT_OFF",
+	"MIPI_SEQ_TEAR_ON",
+	"MIPI_SEQ_TEAR_OFF"
 };
 
 static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data)
@@ -306,6 +310,11 @@ static int vbt_panel_prepare(struct drm_panel *panel)
 	sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
 	generic_exec_sequence(intel_dsi, sequence);
 
+	if (intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE) {
+		sequence = dev_priv->vbt.dsi.sequence[MIPI_SEQ_TEAR_ON];
+		generic_exec_sequence(intel_dsi, sequence);
+	}
+
 	return 0;
 }
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 03/14] drm/i915: Add functions for dcs memory write cmd

[Gaurav K Singh]

Add functions for DCS memory write command. The mem write
command to send fb data to panel is sent using this function.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |    1 +
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 drivers/gpu/drm/i915/intel_dsi.c |   44 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 77055b9..464719b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4231,6 +4231,7 @@ enum skl_disp_power_wells {
 #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
 #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
 #define   PIPECONF_INTERLACE_MODE_MASK		(7 << 21)
+#define   PIPECONF_MIPI_DSR_ENABLE		(1 << 20)
 #define   PIPECONF_EDP_RR_MODE_SWITCH		(1 << 20)
 #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
 #define   PIPECONF_EDP_RR_MODE_SWITCH_VLV	(1 << 14)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47bc729..7c59862 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1197,6 +1197,7 @@ void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
 /* intel_dsi.c */
 void intel_dsi_init(struct drm_device *dev);
 
+void intel_dsi_update_panel_fb(struct intel_encoder *encoder);
 
 /* intel_dvo.c */
 void intel_dvo_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 2e3c801..7cedd63 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -31,6 +31,7 @@
 #include <drm/drm_panel.h>
 #include <drm/drm_mipi_dsi.h>
 #include <linux/slab.h>
+#include <video/mipi_display.h>
 #include "i915_drv.h"
 #include "intel_drv.h"
 #include "intel_dsi.h"
@@ -202,6 +203,41 @@ static struct intel_dsi_host *intel_dsi_host_init(struct intel_dsi *intel_dsi,
 	return host;
 }
 
+int dsi_send_dcs_cmd(struct intel_dsi *intel_dsi, int channel, const u8 *data,
+		     int len, bool pipe_render)
+{
+	struct drm_encoder *encoder = &intel_dsi->base.base;
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum port port;
+	u32 cmd_addr;
+
+	for_each_dsi_port(port, intel_dsi->ports) {
+		if (I915_READ(MIPI_COMMAND_ADDRESS(port)) & COMMAND_VALID)
+			return -EBUSY;
+
+		if ((I915_READ(PIPECONF(port)) & PIPECONF_MIPI_DSR_ENABLE) == 0)
+			return -EBUSY;
+
+		if (!intel_dsi->cmd_buff)
+			return -ENOMEM;
+
+		memcpy(intel_dsi->cmd_buff, data, len);
+
+		cmd_addr = intel_dsi->cmd_buff_phy_addr &
+						COMMAND_MEM_ADDRESS_MASK;
+		cmd_addr |= COMMAND_VALID;
+
+		if (pipe_render)
+			cmd_addr |= MEMORY_WRITE_DATA_FROM_PIPE_RENDERING;
+
+		I915_WRITE(MIPI_COMMAND_LENGTH(port), len);
+		I915_WRITE(MIPI_COMMAND_ADDRESS(port), cmd_addr);
+	}
+
+	return 0;
+}
+
 /*
  * send a video mode command
  *
@@ -667,6 +703,14 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 	pipe_config->port_clock = pclk;
 }
 
+void intel_dsi_update_panel_fb(struct intel_encoder *encoder)
+{
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	unsigned char uc_data[] = {MIPI_DCS_WRITE_MEMORY_START};
+
+	dsi_send_dcs_cmd(intel_dsi, 0, uc_data, sizeof(uc_data), true);
+}
+
 static enum drm_mode_status
 intel_dsi_mode_valid(struct drm_connector *connector,
 		     struct drm_display_mode *mode)
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 04/14] drm/i915: Calculate bw timer for mipi DBI interface

[Gaurav K Singh]

This patch will calculate the bandwidth timer for MIPI DBI interface.
If the BW timer value is available from VBT, then value from VBT
will be used.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 9deaec3..38de166 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -56,6 +56,11 @@ static inline struct vbt_panel *to_vbt_panel(struct drm_panel *panel)
 #define CLK_ZERO_CNT_MAX	0xFF
 #define TRAIL_CNT_MAX		0x1F
 
+#define LP_HDR_FOOT_SIZE	6
+#define BW_LP_NUM_OF_PKT	16
+#define BW_LP_LOAD_SIZE		252
+#define EXTRA_ONE_BYTE		1
+
 #define NS_KHZ_RATIO 1000000
 
 #define GPI0_NC_0_HV_DDI0_HPD           0x4130
@@ -430,7 +435,6 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	intel_dsi->turn_arnd_val = mipi_config->turn_around_timeout;
 	intel_dsi->rst_timer_val = mipi_config->device_reset_timer;
 	intel_dsi->init_count = mipi_config->master_init_timer;
-	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
 	intel_dsi->video_frmt_cfg_bits =
 		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
 
@@ -588,6 +592,24 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	intel_dsi->dphy_reg = exit_zero_cnt << 24 | trail_cnt << 16 |
 						clk_zero_cnt << 8 | prepare_cnt;
 
+	if (mipi_config->dbi_bw_timer) {
+		intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
+	} else {
+		/*
+		 * bw timer should be more than 16 longs packets containing
+		 * 252 bytes + 2 blanking packets.
+		 * bw timer = 16 long packets * (252 bytes payload for each
+		 *            long packet + 6 bytes for long packet header and
+		 *            footer) + 12 bytes for 2 blanking packets + 1
+		 *            byte for having more of the above.
+		 */
+		intel_dsi->bw_timer = DIV_ROUND_UP(BW_LP_NUM_OF_PKT *
+					(BW_LP_LOAD_SIZE + LP_HDR_FOOT_SIZE),
+					intel_dsi->lane_count);
+
+		intel_dsi->bw_timer += (extra_byte_count + EXTRA_ONE_BYTE);
+	}
+
 	/*
 	 * LP to HS switch count = 4TLPX + PREP_COUNT * 2 + EXIT_ZERO_COUNT * 2
 	 *					+ 10UI + Extra Byte Count
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 05/14] drm/i915: Use the bpp value wrt the pixel format

[Gaurav K Singh]

The bpp value which is used while calulating the txbyteclkhs values
should be wrt the pixel format value. Currently bpp is coming
from pipe config to calculate txbyteclkhs.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c           |    5 ++---
 drivers/gpu/drm/i915/intel_dsi.h           |    1 +
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 7cedd63..04d8ce0 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -762,10 +762,9 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
-	unsigned int bpp = intel_crtc->config->pipe_bpp;
+	unsigned int bpp = intel_dsi->dsi_bpp;
 	unsigned int lane_count = intel_dsi->lane_count;
 
 	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
@@ -822,7 +821,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 	struct drm_display_mode *adjusted_mode =
 		&intel_crtc->config->base.adjusted_mode;
 	enum port port;
-	unsigned int bpp = intel_crtc->config->pipe_bpp;
+	unsigned int bpp = intel_dsi->dsi_bpp;
 	u32 val, tmp;
 	u16 mode_hdisplay;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 36ca3cc..6b53b1f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -65,6 +65,7 @@ struct intel_dsi {
 
 	/* video mode pixel format for MIPI_DSI_FUNC_PRG register */
 	u32 pixel_format;
+	u32 dsi_bpp;
 
 	/* video mode format for MIPI_VIDEO_MODE_FORMAT register */
 	u32 video_mode_format;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 38de166..6774726 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -437,6 +437,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	intel_dsi->init_count = mipi_config->master_init_timer;
 	intel_dsi->video_frmt_cfg_bits =
 		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
+	intel_dsi->dsi_bpp = bits_per_pixel;
 
 	pclk = mode->clock;
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 06/14] drm/i915: Disable vlank interrupt for disabling MIPI cmd mode

[Gaurav K Singh]

vblank interrupt should be disabled before starting the disable
sequence for MIPI command mode. Otherwise when pipe is disabled
TE interurpt will be still handled and one memory write command
will be sent with pipe disabled. This makes the pipe hw to get
stuck and it doesn't recover in the next enable sequence causing
display blank out.

Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 04d8ce0..aeea289 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -513,11 +513,25 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
 
 static void intel_dsi_pre_disable(struct intel_encoder *encoder)
 {
+	struct drm_device *dev = encoder->base.dev;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	int pipe = intel_crtc->pipe;
 	enum port port;
 
 	DRM_DEBUG_KMS("\n");
 
+	if (is_cmd_mode(intel_dsi)) {
+		dev->driver->disable_vblank(dev, pipe);
+
+		/*
+		 * Make sure that the last frame is sent otherwise pipe can get
+		 * stuck. Currently providing delay time for ~2 vblanks
+		 * assuming 60fps.
+		 */
+		mdelay(40);
+	}
+
 	if (is_vid_mode(intel_dsi)) {
 		/* Send Shutdown command to the panel in LP mode */
 		for_each_dsi_port(port, intel_dsi->ports)
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 06/14] drm/i915: Disable vlank interrupt for disabling MIPI cmd mode

[Daniel Vetter]

On Fri, May 29, 2015 at 04:06:58PM +0530, Gaurav K Singh wrote:
> vblank interrupt should be disabled before starting the disable
> sequence for MIPI command mode. Otherwise when pipe is disabled
> TE interurpt will be still handled and one memory write command
> will be sent with pipe disabled. This makes the pipe hw to get
> stuck and it doesn't recover in the next enable sequence causing
> display blank out.
> 
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 04d8ce0..aeea289 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -513,11 +513,25 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>  
>  static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>  {
> +	struct drm_device *dev = encoder->base.dev;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	int pipe = intel_crtc->pipe;
>  	enum port port;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	if (is_cmd_mode(intel_dsi)) {
> +		dev->driver->disable_vblank(dev, pipe);
> +
> +		/*
> +		 * Make sure that the last frame is sent otherwise pipe can get
> +		 * stuck. Currently providing delay time for ~2 vblanks
> +		 * assuming 60fps.
> +		 */
> +		mdelay(40);
> +	}

Nope. You need to move around the drm_vblank_off suitably, only that
function correctly handles all the book-keeping around vblank interrupts.
If this doesn't work out because of ordering we need to dig into this and
figure out something. Worst case we need to push the drm_vblank_off call
into encoder callbacks for everyone. That's something we already discussed
but then decided against.
-Daniel

> +
>  	if (is_vid_mode(intel_dsi)) {
>  		/* Send Shutdown command to the panel in LP mode */
>  		for_each_dsi_port(port, intel_dsi->ports)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 06/14] drm/i915: Disable vlank interrupt for disabling MIPI cmd mode

[Daniel Vetter]

On Fri, May 29, 2015 at 07:14:43PM +0200, Daniel Vetter wrote:
> On Fri, May 29, 2015 at 04:06:58PM +0530, Gaurav K Singh wrote:
> > vblank interrupt should be disabled before starting the disable
> > sequence for MIPI command mode. Otherwise when pipe is disabled
> > TE interurpt will be still handled and one memory write command
> > will be sent with pipe disabled. This makes the pipe hw to get
> > stuck and it doesn't recover in the next enable sequence causing
> > display blank out.
> > 
> > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c |   14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 04d8ce0..aeea289 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -513,11 +513,25 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
> >  
> >  static void intel_dsi_pre_disable(struct intel_encoder *encoder)
> >  {
> > +	struct drm_device *dev = encoder->base.dev;
> >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > +	int pipe = intel_crtc->pipe;
> >  	enum port port;
> >  
> >  	DRM_DEBUG_KMS("\n");
> >  
> > +	if (is_cmd_mode(intel_dsi)) {
> > +		dev->driver->disable_vblank(dev, pipe);
> > +
> > +		/*
> > +		 * Make sure that the last frame is sent otherwise pipe can get
> > +		 * stuck. Currently providing delay time for ~2 vblanks
> > +		 * assuming 60fps.
> > +		 */
> > +		mdelay(40);
> > +	}
> 
> Nope. You need to move around the drm_vblank_off suitably, only that
> function correctly handles all the book-keeping around vblank interrupts.
> If this doesn't work out because of ordering we need to dig into this and
> figure out something. Worst case we need to push the drm_vblank_off call
> into encoder callbacks for everyone. That's something we already discussed
> but then decided against.

I seem to be blind, but where exactly is that vblank-driven upload code?
-Daniel

> -Daniel
> 
> > +
> >  	if (is_vid_mode(intel_dsi)) {
> >  		/* Send Shutdown command to the panel in LP mode */
> >  		for_each_dsi_port(port, intel_dsi->ports)
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 06/14] drm/i915: Disable vlank interrupt for disabling MIPI cmd mode

[Singh, Gaurav K]

On 5/29/2015 10:53 PM, Daniel Vetter wrote:
> On Fri, May 29, 2015 at 07:14:43PM +0200, Daniel Vetter wrote:
>> On Fri, May 29, 2015 at 04:06:58PM +0530, Gaurav K Singh wrote:
>>> vblank interrupt should be disabled before starting the disable
>>> sequence for MIPI command mode. Otherwise when pipe is disabled
>>> TE interurpt will be still handled and one memory write command
>>> will be sent with pipe disabled. This makes the pipe hw to get
>>> stuck and it doesn't recover in the next enable sequence causing
>>> display blank out.
>>>
>>> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
>>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_dsi.c |   14 ++++++++++++++
>>>   1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>> index 04d8ce0..aeea289 100644
>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>> @@ -513,11 +513,25 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>>>   
>>>   static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>>>   {
>>> +	struct drm_device *dev = encoder->base.dev;
>>>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>> +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>>> +	int pipe = intel_crtc->pipe;
>>>   	enum port port;
>>>   
>>>   	DRM_DEBUG_KMS("\n");
>>>   
>>> +	if (is_cmd_mode(intel_dsi)) {
>>> +		dev->driver->disable_vblank(dev, pipe);
>>> +
>>> +		/*
>>> +		 * Make sure that the last frame is sent otherwise pipe can get
>>> +		 * stuck. Currently providing delay time for ~2 vblanks
>>> +		 * assuming 60fps.
>>> +		 */
>>> +		mdelay(40);
>>> +	}
>> Nope. You need to move around the drm_vblank_off suitably, only that
>> function correctly handles all the book-keeping around vblank interrupts.
>> If this doesn't work out because of ordering we need to dig into this and
>> figure out something. Worst case we need to push the drm_vblank_off call
>> into encoder callbacks for everyone. That's something we already discussed
>> but then decided against.
> I seem to be blind, but where exactly is that vblank-driven upload code?
> -Daniel
Hi Daniel,

dev->driver->disable_vblank calls valleyview_disable_vblank which already exists. But I did check with drm_vblank_off, it seems to work fine.

I will float the updated patch shortly.

With regards,
Gaurav


>> -Daniel
>>
>>> +
>>>   	if (is_vid_mode(intel_dsi)) {
>>>   		/* Send Shutdown command to the panel in LP mode */
>>>   		for_each_dsi_port(port, intel_dsi->ports)
>>> -- 
>>> 1.7.9.5
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> -- 
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 06/14] drm/i915: Disable vlank interrupt for disabling MIPI cmd mode

[Daniel Vetter]

On Tue, Jun 16, 2015 at 10:24:57PM +0530, Singh, Gaurav K wrote:
> 
> 
> On 5/29/2015 10:53 PM, Daniel Vetter wrote:
> >On Fri, May 29, 2015 at 07:14:43PM +0200, Daniel Vetter wrote:
> >>On Fri, May 29, 2015 at 04:06:58PM +0530, Gaurav K Singh wrote:
> >>>vblank interrupt should be disabled before starting the disable
> >>>sequence for MIPI command mode. Otherwise when pipe is disabled
> >>>TE interurpt will be still handled and one memory write command
> >>>will be sent with pipe disabled. This makes the pipe hw to get
> >>>stuck and it doesn't recover in the next enable sequence causing
> >>>display blank out.
> >>>
> >>>Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> >>>Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> >>>---
> >>>  drivers/gpu/drm/i915/intel_dsi.c |   14 ++++++++++++++
> >>>  1 file changed, 14 insertions(+)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> >>>index 04d8ce0..aeea289 100644
> >>>--- a/drivers/gpu/drm/i915/intel_dsi.c
> >>>+++ b/drivers/gpu/drm/i915/intel_dsi.c
> >>>@@ -513,11 +513,25 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
> >>>  static void intel_dsi_pre_disable(struct intel_encoder *encoder)
> >>>  {
> >>>+	struct drm_device *dev = encoder->base.dev;
> >>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> >>>+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >>>+	int pipe = intel_crtc->pipe;
> >>>  	enum port port;
> >>>  	DRM_DEBUG_KMS("\n");
> >>>+	if (is_cmd_mode(intel_dsi)) {
> >>>+		dev->driver->disable_vblank(dev, pipe);
> >>>+
> >>>+		/*
> >>>+		 * Make sure that the last frame is sent otherwise pipe can get
> >>>+		 * stuck. Currently providing delay time for ~2 vblanks
> >>>+		 * assuming 60fps.
> >>>+		 */
> >>>+		mdelay(40);
> >>>+	}
> >>Nope. You need to move around the drm_vblank_off suitably, only that
> >>function correctly handles all the book-keeping around vblank interrupts.
> >>If this doesn't work out because of ordering we need to dig into this and
> >>figure out something. Worst case we need to push the drm_vblank_off call
> >>into encoder callbacks for everyone. That's something we already discussed
> >>but then decided against.
> >I seem to be blind, but where exactly is that vblank-driven upload code?

This question is still open for me. Why exactly do we have to disable
vblanks here already? I feel like this is hiding some deeper
synchronization issue - just disabling the vblank source doesn't mean they
have all be processed correctly by the irq handler. If this is really
required then you still have a big race here, even when using
drm_vblank_off.
-Daneil
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 06/14] drm/i915: Disable vlank interrupt for disabling MIPI cmd mode

[Gaurav K Singh]

vblank interrupt should be disabled before starting the disable
sequence for MIPI command mode. Otherwise when pipe is disabled
TE interurpt will be still handled and one memory write command
will be sent with pipe disabled. This makes the pipe hw to get
stuck and it doesn't recover in the next enable sequence causing
display blank out.

v2: Use drm_blank_off instead of platform specific disable vblank functions (Daniel)

Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index d378246..7021591 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -513,11 +513,25 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
 
 static void intel_dsi_pre_disable(struct intel_encoder *encoder)
 {
+	struct drm_device *dev = encoder->base.dev;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	int pipe = intel_crtc->pipe;
 	enum port port;
 
 	DRM_DEBUG_KMS("\n");
 
+	if (is_cmd_mode(intel_dsi)) {
+		drm_vblank_off(dev, pipe);
+
+		/*
+		 * Make sure that the last frame is sent otherwise pipe can get
+		 * stuck. Currently providing delay time for ~2 vblanks
+		 * assuming 60fps.
+		 */
+		mdelay(40);
+	}
+
 	if (is_vid_mode(intel_dsi)) {
 		/* Send Shutdown command to the panel in LP mode */
 		for_each_dsi_port(port, intel_dsi->ports)
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 06/14] drm/i915: Disable vlank interrupt for disabling MIPI cmd mode

[Daniel Vetter]

On Fri, Jun 19, 2015 at 03:19:47AM +0530, Gaurav K Singh wrote:
> vblank interrupt should be disabled before starting the disable
> sequence for MIPI command mode. Otherwise when pipe is disabled
> TE interurpt will be still handled and one memory write command
> will be sent with pipe disabled. This makes the pipe hw to get
> stuck and it doesn't recover in the next enable sequence causing
> display blank out.
> 
> v2: Use drm_blank_off instead of platform specific disable vblank functions (Daniel)
> 
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index d378246..7021591 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -513,11 +513,25 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>  
>  static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>  {
> +	struct drm_device *dev = encoder->base.dev;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	int pipe = intel_crtc->pipe;
>  	enum port port;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	if (is_cmd_mode(intel_dsi)) {
> +		drm_vblank_off(dev, pipe);

Now you'll have 2 drm_vblank_off in the crtc disable path, which should
result in some pretty loud warning. If it doesn't we need to fix up
drm_vblank_off to WARN, and you also need to move around the existing
drm_vblank_off in the vlv/chv code into encoder callbacks for _all_
encoders.

Hm, looking at drm_irq.c we indeed seem to be missing these crucial
WARN_ONs. I'll float a patch to address this asap.
-Daniel

> +
> +		/*
> +		 * Make sure that the last frame is sent otherwise pipe can get
> +		 * stuck. Currently providing delay time for ~2 vblanks
> +		 * assuming 60fps.
> +		 */
> +		mdelay(40);
> +	}
> +
>  	if (is_vid_mode(intel_dsi)) {
>  		/* Send Shutdown command to the panel in LP mode */
>  		for_each_dsi_port(port, intel_dsi->ports)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 07/14] drm/i915: Disable MIPI display self refresh mode

[Gaurav K Singh]

During disable sequence for MIPI encoder in command mode, disable
MIPI display self-refresh mode bit in Pipe Ctrl reg.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 895d7c7..cab2ac8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2171,6 +2171,9 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 static void intel_disable_pipe(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct intel_encoder *encoder;
+	struct intel_dsi *intel_dsi;
+	struct drm_device *dev = crtc->base.dev;
 	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
 	enum pipe pipe = crtc->pipe;
 	int reg;
@@ -2189,6 +2192,16 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
 	if ((val & PIPECONF_ENABLE) == 0)
 		return;
 
+	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+		if (encoder->type == INTEL_OUTPUT_DSI) {
+			intel_dsi = enc_to_intel_dsi(&encoder->base);
+			if (intel_dsi && (intel_dsi->operation_mode ==
+						INTEL_DSI_COMMAND_MODE))
+				val = val & ~PIPECONF_MIPI_DSR_ENABLE;
+			break;
+		}
+	}
+
 	/*
 	 * Double wide has implications for planes
 	 * so best keep it disabled when not needed.
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 07/14] drm/i915: Disable MIPI display self refresh mode

[Daniel Vetter]

On Fri, May 29, 2015 at 04:06:59PM +0530, Gaurav K Singh wrote:
> During disable sequence for MIPI encoder in command mode, disable
> MIPI display self-refresh mode bit in Pipe Ctrl reg.
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 895d7c7..cab2ac8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2171,6 +2171,9 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>  static void intel_disable_pipe(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +	struct intel_encoder *encoder;
> +	struct intel_dsi *intel_dsi;
> +	struct drm_device *dev = crtc->base.dev;
>  	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
>  	enum pipe pipe = crtc->pipe;
>  	int reg;
> @@ -2189,6 +2192,16 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>  	if ((val & PIPECONF_ENABLE) == 0)
>  		return;
>  
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +		if (encoder->type == INTEL_OUTPUT_DSI) {
> +			intel_dsi = enc_to_intel_dsi(&encoder->base);
> +			if (intel_dsi && (intel_dsi->operation_mode ==
> +						INTEL_DSI_COMMAND_MODE))
> +				val = val & ~PIPECONF_MIPI_DSR_ENABLE;
> +			break;
> +		}
> +	}

This must be moved into a suitable encoder callback. Yes the ddi code is
full of cases where encoder stuff is done from the generic crtc code. But
we now have 2 completely different kinds of ports on bxt (ddi and dsi),
and we need to get some solid structure into the code again.
-Daniel

> +
>  	/*
>  	 * Double wide has implications for planes
>  	 * so best keep it disabled when not needed.
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 07/14] drm/i915: Disable MIPI display self refresh mode

[Daniel Vetter]

On Fri, May 29, 2015 at 07:16:36PM +0200, Daniel Vetter wrote:
> On Fri, May 29, 2015 at 04:06:59PM +0530, Gaurav K Singh wrote:
> > During disable sequence for MIPI encoder in command mode, disable
> > MIPI display self-refresh mode bit in Pipe Ctrl reg.
> > 
> > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 895d7c7..cab2ac8 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2171,6 +2171,9 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> >  static void intel_disable_pipe(struct intel_crtc *crtc)
> >  {
> >  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> > +	struct intel_encoder *encoder;
> > +	struct intel_dsi *intel_dsi;
> > +	struct drm_device *dev = crtc->base.dev;
> >  	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> >  	enum pipe pipe = crtc->pipe;
> >  	int reg;
> > @@ -2189,6 +2192,16 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
> >  	if ((val & PIPECONF_ENABLE) == 0)
> >  		return;
> >  
> > +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> > +		if (encoder->type == INTEL_OUTPUT_DSI) {
> > +			intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +			if (intel_dsi && (intel_dsi->operation_mode ==
> > +						INTEL_DSI_COMMAND_MODE))
> > +				val = val & ~PIPECONF_MIPI_DSR_ENABLE;
> > +			break;
> > +		}
> > +	}
> 
> This must be moved into a suitable encoder callback. Yes the ddi code is
> full of cases where encoder stuff is done from the generic crtc code. But
> we now have 2 completely different kinds of ports on bxt (ddi and dsi),
> and we need to get some solid structure into the code again.

Ah I missed that this is a pipeconf thing. Unconditionally clearing this
bit will achieve the same, without the need to loop over connectors.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 07/14] drm/i915: Disable MIPI display self refresh mode

[Singh, Gaurav K]

On 5/29/2015 10:50 PM, Daniel Vetter wrote:
> On Fri, May 29, 2015 at 07:16:36PM +0200, Daniel Vetter wrote:
>> On Fri, May 29, 2015 at 04:06:59PM +0530, Gaurav K Singh wrote:
>>> During disable sequence for MIPI encoder in command mode, disable
>>> MIPI display self-refresh mode bit in Pipe Ctrl reg.
>>>
>>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>>> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++++
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 895d7c7..cab2ac8 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -2171,6 +2171,9 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>>>   static void intel_disable_pipe(struct intel_crtc *crtc)
>>>   {
>>>   	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>>> +	struct intel_encoder *encoder;
>>> +	struct intel_dsi *intel_dsi;
>>> +	struct drm_device *dev = crtc->base.dev;
>>>   	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
>>>   	enum pipe pipe = crtc->pipe;
>>>   	int reg;
>>> @@ -2189,6 +2192,16 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>>>   	if ((val & PIPECONF_ENABLE) == 0)
>>>   		return;
>>>   
>>> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>>> +		if (encoder->type == INTEL_OUTPUT_DSI) {
>>> +			intel_dsi = enc_to_intel_dsi(&encoder->base);
>>> +			if (intel_dsi && (intel_dsi->operation_mode ==
>>> +						INTEL_DSI_COMMAND_MODE))
>>> +				val = val & ~PIPECONF_MIPI_DSR_ENABLE;
>>> +			break;
>>> +		}
>>> +	}
>> This must be moved into a suitable encoder callback. Yes the ddi code is
>> full of cases where encoder stuff is done from the generic crtc code. But
>> we now have 2 completely different kinds of ports on bxt (ddi and dsi),
>> and we need to get some solid structure into the code again.
> Ah I missed that this is a pipeconf thing. Unconditionally clearing this
> bit will achieve the same, without the need to loop over connectors.
> -Daniel
Agree with you. Instead of loop over the encoders, will set up the state 
flag and will use it here.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 07/14] drm/i915: Disable MIPI display self refresh mode

[Gaurav K Singh]

During disable sequence for MIPI encoder in command mode, disable
MIPI display self-refresh mode bit in Pipe Ctrl reg.

v2: Use crtc state flag instead of loop over encoders (Daniel)

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    3 +++
 drivers/gpu/drm/i915/intel_drv.h     |    3 +++
 drivers/gpu/drm/i915/intel_dsi.c     |    3 +++
 3 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 067b1de..dd518d6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2193,6 +2193,9 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
 	if ((val & PIPECONF_ENABLE) == 0)
 		return;
 
+	if (crtc->config->dsi_self_refresh)
+		val = val & ~PIPECONF_MIPI_DSR_ENABLE;
+
 	/*
 	 * Double wide has implications for planes
 	 * so best keep it disabled when not needed.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 14562c6..4298a00 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -444,6 +444,9 @@ struct intel_crtc_state {
 	bool double_wide;
 
 	bool dp_encoder_is_mst;
+
+	bool dsi_self_refresh;
+
 	int pbn;
 
 	struct intel_crtc_scaler_state scaler_state;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 7021591..36d8ad6 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -308,6 +308,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("\n");
 
+	if (is_cmd_mode(intel_dsi))
+		config->dsi_self_refresh = true;
+
 	if (fixed_mode)
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 07/14] drm/i915: Disable MIPI display self refresh mode

[Daniel Vetter]

On Fri, Jun 19, 2015 at 03:23:29AM +0530, Gaurav K Singh wrote:
> During disable sequence for MIPI encoder in command mode, disable
> MIPI display self-refresh mode bit in Pipe Ctrl reg.
> 
> v2: Use crtc state flag instead of loop over encoders (Daniel)
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    3 +++
>  drivers/gpu/drm/i915/intel_drv.h     |    3 +++
>  drivers/gpu/drm/i915/intel_dsi.c     |    3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 067b1de..dd518d6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2193,6 +2193,9 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>  	if ((val & PIPECONF_ENABLE) == 0)
>  		return;
>  
> +	if (crtc->config->dsi_self_refresh)
> +		val = val & ~PIPECONF_MIPI_DSR_ENABLE;

This shouldn't be required since we just clear all of PIPECONF when
disabling (well when re-enabling too) the crtc.

What's missing though is the hw state readout & cross-check support for
this new state bit. Also the code to set this bit seems to be missing.
-Daniel

> +
>  	/*
>  	 * Double wide has implications for planes
>  	 * so best keep it disabled when not needed.
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 14562c6..4298a00 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -444,6 +444,9 @@ struct intel_crtc_state {
>  	bool double_wide;
>  
>  	bool dp_encoder_is_mst;
> +
> +	bool dsi_self_refresh;
> +
>  	int pbn;
>  
>  	struct intel_crtc_scaler_state scaler_state;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 7021591..36d8ad6 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -308,6 +308,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	if (is_cmd_mode(intel_dsi))
> +		config->dsi_self_refresh = true;
> +
>  	if (fixed_mode)
>  		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 08/14] drm/i915: Disable Tearing effect trigger by GPIO pin

[Gaurav K Singh]

While disabling MIPI Port in command mode, disable TE trigger by GPIO pin.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index aeea289..91ed2c2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -367,8 +367,12 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
 
 	for_each_dsi_port(port, intel_dsi->ports) {
 		/* de-assert ip_tg_enable signal */
-		temp = I915_READ(MIPI_PORT_CTRL(port));
-		I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
+		if (is_cmd_mode(intel_dsi)) {
+			I915_WRITE(MIPI_PORT_CTRL(port), 0);
+		} else {
+			temp = I915_READ(MIPI_PORT_CTRL(port));
+			I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
+		}
 		POSTING_READ(MIPI_PORT_CTRL(port));
 	}
 }
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 09/14] drm/i915: Changes for command mode preparation

[Gaurav K Singh]

Changes done in preparation of command mode-

1. Set DBI HS LS Switch bit for DBI packets to be tramitted in HS mode.
2. Set DBI FIFO watermark.
3. Timing regs need not be programmmed for command mode.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 91ed2c2..f2fb3fc 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -876,12 +876,16 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 			mode_hdisplay << HORIZONTAL_ADDRESS_SHIFT);
 	}
 
-	set_dsi_timings(encoder, adjusted_mode);
+	if (is_vid_mode(intel_dsi))
+		set_dsi_timings(encoder, adjusted_mode);
 
 	val = intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT;
 	if (is_cmd_mode(intel_dsi)) {
 		val |= intel_dsi->channel << CMD_MODE_CHANNEL_NUMBER_SHIFT;
-		val |= CMD_MODE_DATA_WIDTH_8_BIT; /* XXX */
+		val |= CMD_MODE_DATA_WIDTH_OPTION2;
+		I915_WRITE(MIPI_DBI_FIFO_THROTTLE(port),
+			   DBI_FIFO_EMPTY_QUARTER);
+		I915_WRITE(MIPI_HS_LP_DBI_ENABLE(port), 0);
 	} else {
 		val |= intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT;
 
@@ -983,6 +987,11 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 				intel_dsi->video_mode_format |
 				IP_TG_CONFIG |
 				RANDOM_DPI_DISPLAY_RESOLUTION);
+		else
+			I915_WRITE(MIPI_VIDEO_MODE_FORMAT(port),
+				   intel_dsi->video_frmt_cfg_bits |
+				   IP_TG_CONFIG |
+				   RANDOM_DPI_DISPLAY_RESOLUTION);
 	}
 }
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 10/14] drm/i915: Enable Tearing effect trigger by GPIO pin

[Gaurav K Singh]

While enabling MIPI Port in command mode, enable tearing effect
by GPIO pin.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index f2fb3fc..484ed38 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -352,7 +352,11 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
 					LANE_CONFIGURATION_DUAL_LINK_A;
 		}
 		/* assert ip_tg_enable signal */
-		I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
+		if (is_cmd_mode(intel_dsi))
+			I915_WRITE(MIPI_PORT_CTRL(port),
+				   temp | TEARING_EFFECT_GPIO);
+		else
+			I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
 		POSTING_READ(MIPI_PORT_CTRL(port));
 	}
 }
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Gaurav K Singh]

During enable sequence for MIPI encoder in command mode, enable
MIPI display self-refresh mode bit in Pipe Ctrl reg.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cab2ac8..fc84313 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -44,6 +44,7 @@
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_rect.h>
 #include <linux/dma_remapping.h>
+#include "intel_dsi.h"
 
 /* Primary plane formats supported by all gen */
 #define COMMON_PRIMARY_FORMATS \
@@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_encoder *encoder;
+	struct intel_dsi *intel_dsi;
 	enum pipe pipe = crtc->pipe;
 	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
 								      pipe);
@@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 		return;
 	}
 
+	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+		if (encoder->type == INTEL_OUTPUT_DSI) {
+			intel_dsi = enc_to_intel_dsi(&encoder->base);
+			if (intel_dsi && (intel_dsi->operation_mode ==
+			    INTEL_DSI_COMMAND_MODE)) {
+				val = val | PIPECONF_MIPI_DSR_ENABLE;
+				I915_WRITE(reg, val);
+			}
+			break;
+		}
+	}
+
 	I915_WRITE(reg, val | PIPECONF_ENABLE);
 	POSTING_READ(reg);
 }
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Daniel Vetter]

On Fri, May 29, 2015 at 04:07:03PM +0530, Gaurav K Singh wrote:
> During enable sequence for MIPI encoder in command mode, enable
> MIPI display self-refresh mode bit in Pipe Ctrl reg.
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cab2ac8..fc84313 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -44,6 +44,7 @@
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_rect.h>
>  #include <linux/dma_remapping.h>
> +#include "intel_dsi.h"
>  
>  /* Primary plane formats supported by all gen */
>  #define COMMON_PRIMARY_FORMATS \
> @@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_encoder *encoder;
> +	struct intel_dsi *intel_dsi;
>  	enum pipe pipe = crtc->pipe;
>  	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
>  								      pipe);
> @@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>  		return;
>  	}
>  
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +		if (encoder->type == INTEL_OUTPUT_DSI) {
> +			intel_dsi = enc_to_intel_dsi(&encoder->base);
> +			if (intel_dsi && (intel_dsi->operation_mode ==
> +			    INTEL_DSI_COMMAND_MODE)) {
> +				val = val | PIPECONF_MIPI_DSR_ENABLE;
> +				I915_WRITE(reg, val);
> +			}
> +			break;
> +		}
> +	}

When we have these kind of encoder/crtc state depencies we resolve them by
adding a bit of state to intel_crtc_state which is set as needed in the
encoder's compute_config callback. Then all you need here is

	if (intel_state->dsi_self_refresh)
		val |= PIPECONF_MIPI_DSR_ENABLE;

Also is that additional write really required?
-Daniel

> +
>  	I915_WRITE(reg, val | PIPECONF_ENABLE);
>  	POSTING_READ(reg);
>  }
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Mohan Marimuthu, Yogesh]

On 5/29/2015 10:51 PM, Daniel Vetter wrote:
> On Fri, May 29, 2015 at 04:07:03PM +0530, Gaurav K Singh wrote:
>> During enable sequence for MIPI encoder in command mode, enable
>> MIPI display self-refresh mode bit in Pipe Ctrl reg.
>>
>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |   15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index cab2ac8..fc84313 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -44,6 +44,7 @@
>>   #include <drm/drm_plane_helper.h>
>>   #include <drm/drm_rect.h>
>>   #include <linux/dma_remapping.h>
>> +#include "intel_dsi.h"
>>   
>>   /* Primary plane formats supported by all gen */
>>   #define COMMON_PRIMARY_FORMATS \
>> @@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>>   {
>>   	struct drm_device *dev = crtc->base.dev;
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_encoder *encoder;
>> +	struct intel_dsi *intel_dsi;
>>   	enum pipe pipe = crtc->pipe;
>>   	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
>>   								      pipe);
>> @@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>>   		return;
>>   	}
>>   
>> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>> +		if (encoder->type == INTEL_OUTPUT_DSI) {
>> +			intel_dsi = enc_to_intel_dsi(&encoder->base);
>> +			if (intel_dsi && (intel_dsi->operation_mode ==
>> +			    INTEL_DSI_COMMAND_MODE)) {
>> +				val = val | PIPECONF_MIPI_DSR_ENABLE;
>> +				I915_WRITE(reg, val);
>> +			}
>> +			break;
>> +		}
>> +	}
> When we have these kind of encoder/crtc state depencies we resolve them by
> adding a bit of state to intel_crtc_state which is set as needed in the
> encoder's compute_config callback. Then all you need here is
>
> 	if (intel_state->dsi_self_refresh)
> 		val |= PIPECONF_MIPI_DSR_ENABLE;
>
> Also is that additional write really required?
> -Daniel
Yes additional write is required. MIPI_DSR_ENABLE has to be written 
first then followed
by pipe enable. if MIPI_DSR_ENABLE and pipe enable is done in same 
write, observed
that the image from pipe is not sent to panel when issued mem write command.

Having a state variable instead of looping through the encoders 
definitely looks good.
Need to find a place to update the state variable. I will get back on this.
>> +
>>   	I915_WRITE(reg, val | PIPECONF_ENABLE);
>>   	POSTING_READ(reg);
>>   }
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Daniel Vetter]

On Sat, Jun 13, 2015 at 12:24:57PM +0530, Mohan Marimuthu, Yogesh wrote:
> 
> 
> On 5/29/2015 10:51 PM, Daniel Vetter wrote:
> >On Fri, May 29, 2015 at 04:07:03PM +0530, Gaurav K Singh wrote:
> >>During enable sequence for MIPI encoder in command mode, enable
> >>MIPI display self-refresh mode bit in Pipe Ctrl reg.
> >>
> >>Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> >>Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> >>Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/intel_display.c |   15 +++++++++++++++
> >>  1 file changed, 15 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index cab2ac8..fc84313 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -44,6 +44,7 @@
> >>  #include <drm/drm_plane_helper.h>
> >>  #include <drm/drm_rect.h>
> >>  #include <linux/dma_remapping.h>
> >>+#include "intel_dsi.h"
> >>  /* Primary plane formats supported by all gen */
> >>  #define COMMON_PRIMARY_FORMATS \
> >>@@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> >>  {
> >>  	struct drm_device *dev = crtc->base.dev;
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>+	struct intel_encoder *encoder;
> >>+	struct intel_dsi *intel_dsi;
> >>  	enum pipe pipe = crtc->pipe;
> >>  	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> >>  								      pipe);
> >>@@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> >>  		return;
> >>  	}
> >>+	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> >>+		if (encoder->type == INTEL_OUTPUT_DSI) {
> >>+			intel_dsi = enc_to_intel_dsi(&encoder->base);
> >>+			if (intel_dsi && (intel_dsi->operation_mode ==
> >>+			    INTEL_DSI_COMMAND_MODE)) {
> >>+				val = val | PIPECONF_MIPI_DSR_ENABLE;
> >>+				I915_WRITE(reg, val);
> >>+			}
> >>+			break;
> >>+		}
> >>+	}
> >When we have these kind of encoder/crtc state depencies we resolve them by
> >adding a bit of state to intel_crtc_state which is set as needed in the
> >encoder's compute_config callback. Then all you need here is
> >
> >	if (intel_state->dsi_self_refresh)
> >		val |= PIPECONF_MIPI_DSR_ENABLE;
> >
> >Also is that additional write really required?
> >-Daniel
> Yes additional write is required. MIPI_DSR_ENABLE has to be written first
> then followed
> by pipe enable. if MIPI_DSR_ENABLE and pipe enable is done in same write,
> observed
> that the image from pipe is not sent to panel when issued mem write command.
> 
> Having a state variable instead of looping through the encoders definitely
> looks good.
> Need to find a place to update the state variable. I will get back on this.

Like I said such state is precomputed in the encoder callbacks, in this
case intel_dsi_compute_config.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Singh, Gaurav K]

On 6/15/2015 4:03 PM, Daniel Vetter wrote:
> On Sat, Jun 13, 2015 at 12:24:57PM +0530, Mohan Marimuthu, Yogesh wrote:
>>
>> On 5/29/2015 10:51 PM, Daniel Vetter wrote:
>>> On Fri, May 29, 2015 at 04:07:03PM +0530, Gaurav K Singh wrote:
>>>> During enable sequence for MIPI encoder in command mode, enable
>>>> MIPI display self-refresh mode bit in Pipe Ctrl reg.
>>>>
>>>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>>>> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
>>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/intel_display.c |   15 +++++++++++++++
>>>>   1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index cab2ac8..fc84313 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -44,6 +44,7 @@
>>>>   #include <drm/drm_plane_helper.h>
>>>>   #include <drm/drm_rect.h>
>>>>   #include <linux/dma_remapping.h>
>>>> +#include "intel_dsi.h"
>>>>   /* Primary plane formats supported by all gen */
>>>>   #define COMMON_PRIMARY_FORMATS \
>>>> @@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>>>>   {
>>>>   	struct drm_device *dev = crtc->base.dev;
>>>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>>> +	struct intel_encoder *encoder;
>>>> +	struct intel_dsi *intel_dsi;
>>>>   	enum pipe pipe = crtc->pipe;
>>>>   	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
>>>>   								      pipe);
>>>> @@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>>>>   		return;
>>>>   	}
>>>> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>>>> +		if (encoder->type == INTEL_OUTPUT_DSI) {
>>>> +			intel_dsi = enc_to_intel_dsi(&encoder->base);
>>>> +			if (intel_dsi && (intel_dsi->operation_mode ==
>>>> +			    INTEL_DSI_COMMAND_MODE)) {
>>>> +				val = val | PIPECONF_MIPI_DSR_ENABLE;
>>>> +				I915_WRITE(reg, val);
>>>> +			}
>>>> +			break;
>>>> +		}
>>>> +	}
>>> When we have these kind of encoder/crtc state depencies we resolve them by
>>> adding a bit of state to intel_crtc_state which is set as needed in the
>>> encoder's compute_config callback. Then all you need here is
>>>
>>> 	if (intel_state->dsi_self_refresh)
>>> 		val |= PIPECONF_MIPI_DSR_ENABLE;
>>>
>>> Also is that additional write really required?
>>> -Daniel
>> Yes additional write is required. MIPI_DSR_ENABLE has to be written first
>> then followed
>> by pipe enable. if MIPI_DSR_ENABLE and pipe enable is done in same write,
>> observed
>> that the image from pipe is not sent to panel when issued mem write command.
>>
>> Having a state variable instead of looping through the encoders definitely
>> looks good.
>> Need to find a place to update the state variable. I will get back on this.
> Like I said such state is precomputed in the encoder callbacks, in this
> case intel_dsi_compute_config.
>
> Cheers, Daniel
Agree with you daniel regarding state flag. Updated patch is ready, will 
upload shortly.

Regarding additional write, as Yogesh confirmed, both the writes are 
required.

With regards,
Gaurav
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Daniel Vetter]

On Tue, Jun 16, 2015 at 10:33:35PM +0530, Singh, Gaurav K wrote:
> 
> 
> On 6/15/2015 4:03 PM, Daniel Vetter wrote:
> >On Sat, Jun 13, 2015 at 12:24:57PM +0530, Mohan Marimuthu, Yogesh wrote:
> >>
> >>On 5/29/2015 10:51 PM, Daniel Vetter wrote:
> >>>On Fri, May 29, 2015 at 04:07:03PM +0530, Gaurav K Singh wrote:
> >>>>During enable sequence for MIPI encoder in command mode, enable
> >>>>MIPI display self-refresh mode bit in Pipe Ctrl reg.
> >>>>
> >>>>Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> >>>>Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> >>>>Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> >>>>---
> >>>>  drivers/gpu/drm/i915/intel_display.c |   15 +++++++++++++++
> >>>>  1 file changed, 15 insertions(+)
> >>>>
> >>>>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>>index cab2ac8..fc84313 100644
> >>>>--- a/drivers/gpu/drm/i915/intel_display.c
> >>>>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>>>@@ -44,6 +44,7 @@
> >>>>  #include <drm/drm_plane_helper.h>
> >>>>  #include <drm/drm_rect.h>
> >>>>  #include <linux/dma_remapping.h>
> >>>>+#include "intel_dsi.h"
> >>>>  /* Primary plane formats supported by all gen */
> >>>>  #define COMMON_PRIMARY_FORMATS \
> >>>>@@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> >>>>  {
> >>>>  	struct drm_device *dev = crtc->base.dev;
> >>>>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>>>+	struct intel_encoder *encoder;
> >>>>+	struct intel_dsi *intel_dsi;
> >>>>  	enum pipe pipe = crtc->pipe;
> >>>>  	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> >>>>  								      pipe);
> >>>>@@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> >>>>  		return;
> >>>>  	}
> >>>>+	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> >>>>+		if (encoder->type == INTEL_OUTPUT_DSI) {
> >>>>+			intel_dsi = enc_to_intel_dsi(&encoder->base);
> >>>>+			if (intel_dsi && (intel_dsi->operation_mode ==
> >>>>+			    INTEL_DSI_COMMAND_MODE)) {
> >>>>+				val = val | PIPECONF_MIPI_DSR_ENABLE;
> >>>>+				I915_WRITE(reg, val);
> >>>>+			}
> >>>>+			break;
> >>>>+		}
> >>>>+	}
> >>>When we have these kind of encoder/crtc state depencies we resolve them by
> >>>adding a bit of state to intel_crtc_state which is set as needed in the
> >>>encoder's compute_config callback. Then all you need here is
> >>>
> >>>	if (intel_state->dsi_self_refresh)
> >>>		val |= PIPECONF_MIPI_DSR_ENABLE;
> >>>
> >>>Also is that additional write really required?
> >>>-Daniel
> >>Yes additional write is required. MIPI_DSR_ENABLE has to be written first
> >>then followed
> >>by pipe enable. if MIPI_DSR_ENABLE and pipe enable is done in same write,
> >>observed
> >>that the image from pipe is not sent to panel when issued mem write command.
> >>
> >>Having a state variable instead of looping through the encoders definitely
> >>looks good.
> >>Need to find a place to update the state variable. I will get back on this.
> >Like I said such state is precomputed in the encoder callbacks, in this
> >case intel_dsi_compute_config.
> >
> >Cheers, Daniel
> Agree with you daniel regarding state flag. Updated patch is ready, will
> upload shortly.
> 
> Regarding additional write, as Yogesh confirmed, both the writes are
> required.

If we _must_ have that second write, then you need a POSTING_READ and a
comment to explain this. Otherwise this extra write will get refactoring
into just one eventually. Also if you really need a 2nd write I wonder why
we don't have to wait for hw somehow to process the first one?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Gaurav K Singh]

During enable sequence for MIPI encoder in command mode, enable
MIPI display self-refresh mode bit in Pipe Ctrl reg.

v2: Use crtc state flag instead of loop over encoders (Daniel)

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dd518d6..c53f66d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2158,6 +2158,11 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 		return;
 	}
 
+	if (crtc->config->dsi_self_refresh) {
+		val = val | PIPECONF_MIPI_DSR_ENABLE;
+		I915_WRITE(reg, val);
+	}
+
 	I915_WRITE(reg, val | PIPECONF_ENABLE);
 	POSTING_READ(reg);
 }
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Daniel Vetter]

On Fri, Jun 19, 2015 at 03:26:45AM +0530, Gaurav K Singh wrote:
> During enable sequence for MIPI encoder in command mode, enable
> MIPI display self-refresh mode bit in Pipe Ctrl reg.
> 
> v2: Use crtc state flag instead of loop over encoders (Daniel)
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dd518d6..c53f66d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2158,6 +2158,11 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>  		return;
>  	}
>  
> +	if (crtc->config->dsi_self_refresh) {
> +		val = val | PIPECONF_MIPI_DSR_ENABLE;
> +		I915_WRITE(reg, val);
> +	}

Ah here it is. Please squash this patch with patch 7 so that you introduce
the state tracking and the user for the new dsi_self_refresh bit in one
patch. Makes reviewing a lot easier. Also please add a comment here that
the additional write is required, and enforce ordering with a
POSTING_READ.

Thanks, Daniel

> +
>  	I915_WRITE(reg, val | PIPECONF_ENABLE);
>  	POSTING_READ(reg);
>  }
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Enable MIPI display self refresh mode

[Daniel Vetter]

On Mon, Jun 22, 2015 at 02:05:51PM +0200, Daniel Vetter wrote:
> On Fri, Jun 19, 2015 at 03:26:45AM +0530, Gaurav K Singh wrote:
> > During enable sequence for MIPI encoder in command mode, enable
> > MIPI display self-refresh mode bit in Pipe Ctrl reg.
> > 
> > v2: Use crtc state flag instead of loop over encoders (Daniel)
> > 
> > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |    5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index dd518d6..c53f66d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2158,6 +2158,11 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
> >  		return;
> >  	}
> >  
> > +	if (crtc->config->dsi_self_refresh) {
> > +		val = val | PIPECONF_MIPI_DSR_ENABLE;
> > +		I915_WRITE(reg, val);
> > +	}
> 
> Ah here it is. Please squash this patch with patch 7 so that you introduce
> the state tracking and the user for the new dsi_self_refresh bit in one
> patch. Makes reviewing a lot easier. Also please add a comment here that
> the additional write is required, and enforce ordering with a
> POSTING_READ.

And I just realized that I've written the same review in the discussion of
the previous patch. Please make sure next time around you address all the
outstanding review, since if you don't the reviewer has to paintstakingly
check _everything_ every time around, which is a massive waste of
everyone's time.

Also can you pls change the subject to include DSI (and drop the MIPI
part, that's just the standard's group and not the standard itself)?
Applies to a few other patches in this series too.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 12/14] drm/i915: Generalize DSI enable function

[Gaurav K Singh]

For command mode and video mode, panel prepare, wait for FIFO
checks are required. Making these changes generic across command
mode and video mode.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 484ed38..fc552f1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -437,14 +437,15 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 		for_each_dsi_port(port, intel_dsi->ports)
 			dpi_send_cmd(intel_dsi, TURN_ON, false, port);
 		msleep(100);
+	}
 
-		drm_panel_enable(intel_dsi->panel);
+	drm_panel_enable(intel_dsi->panel);
 
-		for_each_dsi_port(port, intel_dsi->ports)
-			wait_for_dsi_fifo_empty(intel_dsi, port);
+	for_each_dsi_port(port, intel_dsi->ports)
+		wait_for_dsi_fifo_empty(intel_dsi, port);
+
+	intel_dsi_port_enable(encoder);
 
-		intel_dsi_port_enable(encoder);
-	}
 }
 
 static void intel_dsi_pre_enable(struct intel_encoder *encoder)
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 13/14] drm/i915: Reset the display hw if vid mode to cmd mode

[Gaurav K Singh]

Reset the display hardware if video mode to command mode transition
has to be done in MIPI display. otherwise command mode will not work.

Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   81 ++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_dsi.c     |    4 ++
 3 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6a66d6b..8c06377 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1854,6 +1854,7 @@ struct drm_i915_private {
 	} gt;
 
 	bool edp_low_vswing;
+	bool video_disabled;
 
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fc84313..8dd0066 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6025,6 +6025,25 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 		encoder->enable(encoder);
 }
 
+/* Disable the VGA plane that we never use */
+static void i915_disable_vga(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u8 sr1;
+	u32 vga_reg = i915_vgacntrl_reg(dev);
+
+	/* WaEnableVGAAccessThroughIOPort:ctg,elk,ilk,snb,ivb,vlv,hsw */
+	vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
+	outb(SR01, VGA_SR_INDEX);
+	sr1 = inb(VGA_SR_DATA);
+	outb(sr1 | 1<<5, VGA_SR_DATA);
+	vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
+	udelay(300);
+
+	I915_WRITE(vga_reg, VGA_DISP_DISABLE);
+	POSTING_READ(vga_reg);
+}
+
 static void i9xx_pfit_disable(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
@@ -6047,6 +6066,8 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
+	bool all_pipe_disabled;
+	u32 val;
 
 	if (!intel_crtc->active)
 		return;
@@ -6091,6 +6112,47 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	mutex_lock(&dev->struct_mutex);
 	intel_fbc_update(dev);
 	mutex_unlock(&dev->struct_mutex);
+
+	all_pipe_disabled = true;
+	for_each_pipe(dev_priv, pipe) {
+		if ((I915_READ(PIPECONF(pipe)) &
+				PIPECONF_ENABLE) == PIPECONF_ENABLE)
+			all_pipe_disabled = false;
+	}
+
+	if ((all_pipe_disabled == true) &&
+				(dev_priv->video_disabled == true)) {
+
+		/*
+		 * to switch from video mode to command mode, need to reset
+		 * the display.
+		 * FIXME: Even after resetting the display, the first modeset
+		 * works sporadically(2 out of 3 times). Need to fix this.
+		 * FIXME: Need to find a better way of doing this, because
+		 * resetting the display resets all the registers in the
+		 * display controller. Need to save and restore some of these
+		 * required registers.
+		 */
+		DRM_DEBUG_KMS("vid mode to cmd mode, reset display\n");
+		if (IS_CHERRYVIEW(dev)) {
+			val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
+			val = val | DP_SSC_PWR_GATE(0);
+			vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
+
+			/* delay to power gate display controller */
+			mdelay(5);
+
+			val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
+			val = val & ~((u32)DP_SSC_MASK(0));
+			vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
+
+			/* delay to power on display controller */
+			mdelay(10);
+		} else
+			DRM_ERROR("vid mode to cmd mode reset is not done.\n");
+
+		i915_disable_vga(dev_priv->dev);
+	}
 }
 
 static void i9xx_crtc_off(struct drm_crtc *crtc)
@@ -14405,25 +14467,6 @@ static void intel_init_quirks(struct drm_device *dev)
 	}
 }
 
-/* Disable the VGA plane that we never use */
-static void i915_disable_vga(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	u8 sr1;
-	u32 vga_reg = i915_vgacntrl_reg(dev);
-
-	/* WaEnableVGAAccessThroughIOPort:ctg,elk,ilk,snb,ivb,vlv,hsw */
-	vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
-	outb(SR01, VGA_SR_INDEX);
-	sr1 = inb(VGA_SR_DATA);
-	outb(sr1 | 1<<5, VGA_SR_DATA);
-	vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
-	udelay(300);
-
-	I915_WRITE(vga_reg, VGA_DISP_DISABLE);
-	POSTING_READ(vga_reg);
-}
-
 void intel_modeset_init_hw(struct drm_device *dev)
 {
 	intel_prepare_ddi(dev);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index fc552f1..e22c7ae 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -369,9 +369,13 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
 	enum port port;
 	u32 temp;
 
+	dev_priv->video_disabled = false;
+
 	for_each_dsi_port(port, intel_dsi->ports) {
 		/* de-assert ip_tg_enable signal */
 		if (is_cmd_mode(intel_dsi)) {
+			if (I915_READ(MIPI_PORT_CTRL(port)) & DPI_ENABLE)
+				dev_priv->video_disabled = true;
 			I915_WRITE(MIPI_PORT_CTRL(port), 0);
 		} else {
 			temp = I915_READ(MIPI_PORT_CTRL(port));
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 14/14] drm/i915: send one frame after enabling mipi cmd mode

[Gaurav K Singh]

If MIPI is operated in command mode, and after display reset. if
not even one frame is sent after enabling the pipe and then if it
is disabled, pipe is getting stuck. This patch will fix this issue
by sending one frame after enabling the pipe.

Ideally,there should not be a case where there is mode set and no frames
are sent.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   46 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |    6 +++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8dd0066..46e7f0b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5905,6 +5905,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
+	struct intel_dsi *intel_dsi;
 	int pipe = intel_crtc->pipe;
 	bool is_dsi;
 
@@ -5967,6 +5968,25 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
+
+	for_each_encoder_on_crtc(dev, crtc, encoder) {
+		if (encoder->type != INTEL_OUTPUT_DSI)
+			continue;
+
+		intel_dsi = enc_to_intel_dsi(&encoder->base);
+		if (intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE) {
+			/*
+			 * save the current pipe counter. During disable use
+			 * this variable to check if at least one frame has
+			 * been sent. If no frame is sent and MIPI is disabled
+			 * in command mode, then pipe gets stuck.
+			 */
+			intel_crtc->hw_frm_cnt_at_enable =
+						I915_READ(PIPEFRAME(pipe));
+		}
+		break;
+	}
+
 }
 
 static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
@@ -6065,6 +6085,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
+	struct intel_dsi *intel_dsi;
 	int pipe = intel_crtc->pipe;
 	bool all_pipe_disabled;
 	u32 val;
@@ -6072,6 +6093,31 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
+	for_each_encoder_on_crtc(dev, crtc, encoder) {
+		if (encoder->type != INTEL_OUTPUT_DSI)
+			continue;
+
+		intel_dsi = enc_to_intel_dsi(&encoder->base);
+		if ((intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE) &&
+				(intel_crtc->hw_frm_cnt_at_enable ==
+						I915_READ(PIPEFRAME(pipe)))) {
+
+			intel_dsi_update_panel_fb(encoder);
+
+			/*
+			 * wait for ~2 frames for TE interrupt and sending one
+			 * frame.
+			 */
+			msleep(40);
+
+			if (intel_crtc->hw_frm_cnt_at_enable ==
+						I915_READ(PIPEFRAME(pipe)))
+				DRM_ERROR("Pipe is stuck for DSI cmd mode.");
+		}
+
+		break;
+	}
+
 	/*
 	 * On gen2 planes are double buffered but the pipe isn't, so we must
 	 * wait for planes to fully turn off before disabling the pipe.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7c59862..e453934 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -550,6 +550,12 @@ struct intel_crtc {
 
 	/* scalers available on this crtc */
 	int num_scalers;
+
+	/*
+	 * save the frame counter at enable sequence to make sure one frame has
+	 * been sent before disable sequence.
+	 */
+	u32 hw_frm_cnt_at_enable;
 };
 
 struct intel_plane_wm_parameters {
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx