Re: [Intel-gfx] [BXT MIPI PATCH v3 06/14] drm/i915/bxt: DSI enable for BXT

[Shankar, Uma]

On 9/18/2015 6:47 PM, Jani Nikula wrote:

On Tue, 01 Sep 2015, Uma Shankar  wrote:

From: Shashank Sharma 

This patch contains following changes:
1. MIPI device ready changes to support dsi_pre_enable. Changes
are specific to BXT device ready sequence. Added check for
ULPS mode(No effects on VLV).
2. Changes in dsi_enable to pick BXT port control register.
3. Changes in dsi_pre_enable to restrict DPIO programming for VLV

v2: Fixed Jani's review comments. Removed the changes in VLV/CHV
 code. Fixed the macros to get proper port offsets.

v3: Rebased on latest drm-nightly branch. Fixed Jani's review comments.

Signed-off-by: Shashank Sharma 
Signed-off-by: Uma Shankar 

Reviewed-by: Jani Nikula 

When you look at the commits before sending, you should pay more
attention to what the diffs end up looking like. In this case, the
review becomes unnecessarily hard just because you've moved code
(intel_dsi_port_enable) around for no apparent reason, and the diff
seems like intel_dsi_port_enable is rewritten into
bxt_dsi_device_ready. It should be a hint to *not* combine code movement
into the same patch.

I understand the concern. Will be careful of this in future to avoid such 
issues.

---
  drivers/gpu/drm/i915/i915_reg.h  |7 ++
  drivers/gpu/drm/i915/intel_dsi.c |  165 ++
  2 files changed, 119 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 997a999..57c5dbf 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7403,6 +7403,13 @@ enum skl_disp_power_wells {
  #define _MIPIA_PORT_CTRL  (VLV_DISPLAY_BASE + 0x61190)
  #define _MIPIC_PORT_CTRL  (VLV_DISPLAY_BASE + 0x61700)
  #define MIPI_PORT_CTRL(port)  _MIPI_PORT(port, _MIPIA_PORT_CTRL, 
_MIPIC_PORT_CTRL)
+
+ /* BXT port control */
+#define _BXT_MIPIA_PORT_CTRL   0x6B0C0
+#define _BXT_MIPIC_PORT_CTRL   0x6B8C0
+#define BXT_MIPI_PORT_CTRL(tc) _MIPI_PORT(tc, _BXT_MIPIA_PORT_CTRL, \
+   _BXT_MIPIC_PORT_CTRL)
+
  #define  DPI_ENABLE   (1 << 31) /* A + C */
  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT27
  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 6d0c992..5a42f87 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -286,58 +286,46 @@ static bool intel_dsi_compute_config(struct intel_encoder 
*encoder,
return true;
  }
  
-static void intel_dsi_port_enable(struct intel_encoder *encoder)

+static void bxt_dsi_device_ready(struct intel_encoder *encoder)
  {
-   struct drm_device *dev = encoder->base.dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+   struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
enum port port;
-   u32 temp;
+   u32 val;
  
-	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {

-   temp = I915_READ(VLV_CHICKEN_3);
-   temp &= ~PIXEL_OVERLAP_CNT_MASK |
-   intel_dsi->pixel_overlap <<
-   PIXEL_OVERLAP_CNT_SHIFT;
-   I915_WRITE(VLV_CHICKEN_3, temp);
-   }
+   DRM_DEBUG_KMS("\n");
  
+	/* Exit Low power state in 4 steps*/




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

Re: [Intel-gfx] [BXT MIPI PATCH v3 06/14] drm/i915/bxt: DSI enable for BXT

[Jani Nikula]

On Tue, 01 Sep 2015, Uma Shankar  wrote:
> From: Shashank Sharma 
>
> This patch contains following changes:
> 1. MIPI device ready changes to support dsi_pre_enable. Changes
>are specific to BXT device ready sequence. Added check for
>ULPS mode(No effects on VLV).
> 2. Changes in dsi_enable to pick BXT port control register.
> 3. Changes in dsi_pre_enable to restrict DPIO programming for VLV
>
> v2: Fixed Jani's review comments. Removed the changes in VLV/CHV
> code. Fixed the macros to get proper port offsets.
>
> v3: Rebased on latest drm-nightly branch. Fixed Jani's review comments.
>
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Uma Shankar 

Reviewed-by: Jani Nikula 

When you look at the commits before sending, you should pay more
attention to what the diffs end up looking like. In this case, the
review becomes unnecessarily hard just because you've moved code
(intel_dsi_port_enable) around for no apparent reason, and the diff
seems like intel_dsi_port_enable is rewritten into
bxt_dsi_device_ready. It should be a hint to *not* combine code movement
into the same patch.


> ---
>  drivers/gpu/drm/i915/i915_reg.h  |7 ++
>  drivers/gpu/drm/i915/intel_dsi.c |  165 
> ++
>  2 files changed, 119 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 997a999..57c5dbf 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7403,6 +7403,13 @@ enum skl_disp_power_wells {
>  #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
>  #define _MIPIC_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
>  #define MIPI_PORT_CTRL(port) _MIPI_PORT(port, _MIPIA_PORT_CTRL, 
> _MIPIC_PORT_CTRL)
> +
> + /* BXT port control */
> +#define _BXT_MIPIA_PORT_CTRL 0x6B0C0
> +#define _BXT_MIPIC_PORT_CTRL 0x6B8C0
> +#define BXT_MIPI_PORT_CTRL(tc)   _MIPI_PORT(tc, _BXT_MIPIA_PORT_CTRL, \
> + _BXT_MIPIC_PORT_CTRL)
> +
>  #define  DPI_ENABLE  (1 << 31) /* A + C */
>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT   27
>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK(0xf << 27)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c 
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 6d0c992..5a42f87 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -286,58 +286,46 @@ static bool intel_dsi_compute_config(struct 
> intel_encoder *encoder,
>   return true;
>  }
>  
> -static void intel_dsi_port_enable(struct intel_encoder *encoder)
> +static void bxt_dsi_device_ready(struct intel_encoder *encoder)
>  {
> - struct drm_device *dev = encoder->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>   struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
>   enum port port;
> - u32 temp;
> + u32 val;
>  
> - if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> - temp = I915_READ(VLV_CHICKEN_3);
> - temp &= ~PIXEL_OVERLAP_CNT_MASK |
> - intel_dsi->pixel_overlap <<
> - PIXEL_OVERLAP_CNT_SHIFT;
> - I915_WRITE(VLV_CHICKEN_3, temp);
> - }
> + DRM_DEBUG_KMS("\n");
>  
> + /* Exit Low power state in 4 steps*/
>   for_each_dsi_port(port, intel_dsi->ports) {
> - temp = I915_READ(MIPI_PORT_CTRL(port));
> - temp &= ~LANE_CONFIGURATION_MASK;
> - temp &= ~DUAL_LINK_MODE_MASK;
>  
> - if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
> - temp |= (intel_dsi->dual_link - 1)
> - << DUAL_LINK_MODE_SHIFT;
> - temp |= intel_crtc->pipe ?
> - LANE_CONFIGURATION_DUAL_LINK_B :
> - LANE_CONFIGURATION_DUAL_LINK_A;
> - }
> - /* assert ip_tg_enable signal */
> - I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
> - POSTING_READ(MIPI_PORT_CTRL(port));
> - }
> -}
> + /* 1. Enable MIPI PHY transparent latch */
> + val = I915_READ(BXT_MIPI_PORT_CTRL(port));
> + I915_WRITE(BXT_MIPI_PORT_CTRL(port), val | LP_OUTPUT_HOLD);
> + usleep_range(2000, 2500);
>  
> -static void intel_dsi_port_disable(struct intel_encoder *encoder)
> -{
> - struct drm_device *dev = encoder->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_dsi 

[Intel-gfx] [BXT MIPI PATCH v3 06/14] drm/i915/bxt: DSI enable for BXT

[Uma Shankar]

From: Shashank Sharma 

This patch contains following changes:
1. MIPI device ready changes to support dsi_pre_enable. Changes
   are specific to BXT device ready sequence. Added check for
   ULPS mode(No effects on VLV).
2. Changes in dsi_enable to pick BXT port control register.
3. Changes in dsi_pre_enable to restrict DPIO programming for VLV

v2: Fixed Jani's review comments. Removed the changes in VLV/CHV
code. Fixed the macros to get proper port offsets.

v3: Rebased on latest drm-nightly branch. Fixed Jani's review comments.

Signed-off-by: Shashank Sharma 
Signed-off-by: Uma Shankar 
---
 drivers/gpu/drm/i915/i915_reg.h  |7 ++
 drivers/gpu/drm/i915/intel_dsi.c |  165 ++
 2 files changed, 119 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 997a999..57c5dbf 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7403,6 +7403,13 @@ enum skl_disp_power_wells {
 #define _MIPIA_PORT_CTRL   (VLV_DISPLAY_BASE + 0x61190)
 #define _MIPIC_PORT_CTRL   (VLV_DISPLAY_BASE + 0x61700)
 #define MIPI_PORT_CTRL(port)   _MIPI_PORT(port, _MIPIA_PORT_CTRL, 
_MIPIC_PORT_CTRL)
+
+ /* BXT port control */
+#define _BXT_MIPIA_PORT_CTRL   0x6B0C0
+#define _BXT_MIPIC_PORT_CTRL   0x6B8C0
+#define BXT_MIPI_PORT_CTRL(tc) _MIPI_PORT(tc, _BXT_MIPIA_PORT_CTRL, \
+   _BXT_MIPIC_PORT_CTRL)
+
 #define  DPI_ENABLE(1 << 31) /* A + C */
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK  (0xf << 27)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 6d0c992..5a42f87 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -286,58 +286,46 @@ static bool intel_dsi_compute_config(struct intel_encoder 
*encoder,
return true;
 }
 
-static void intel_dsi_port_enable(struct intel_encoder *encoder)
+static void bxt_dsi_device_ready(struct intel_encoder *encoder)
 {
-   struct drm_device *dev = encoder->base.dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+   struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
enum port port;
-   u32 temp;
+   u32 val;
 
-   if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-   temp = I915_READ(VLV_CHICKEN_3);
-   temp &= ~PIXEL_OVERLAP_CNT_MASK |
-   intel_dsi->pixel_overlap <<
-   PIXEL_OVERLAP_CNT_SHIFT;
-   I915_WRITE(VLV_CHICKEN_3, temp);
-   }
+   DRM_DEBUG_KMS("\n");
 
+   /* Exit Low power state in 4 steps*/
for_each_dsi_port(port, intel_dsi->ports) {
-   temp = I915_READ(MIPI_PORT_CTRL(port));
-   temp &= ~LANE_CONFIGURATION_MASK;
-   temp &= ~DUAL_LINK_MODE_MASK;
 
-   if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
-   temp |= (intel_dsi->dual_link - 1)
-   << DUAL_LINK_MODE_SHIFT;
-   temp |= intel_crtc->pipe ?
-   LANE_CONFIGURATION_DUAL_LINK_B :
-   LANE_CONFIGURATION_DUAL_LINK_A;
-   }
-   /* assert ip_tg_enable signal */
-   I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
-   POSTING_READ(MIPI_PORT_CTRL(port));
-   }
-}
+   /* 1. Enable MIPI PHY transparent latch */
+   val = I915_READ(BXT_MIPI_PORT_CTRL(port));
+   I915_WRITE(BXT_MIPI_PORT_CTRL(port), val | LP_OUTPUT_HOLD);
+   usleep_range(2000, 2500);
 
-static void intel_dsi_port_disable(struct intel_encoder *encoder)
-{
-   struct drm_device *dev = encoder->base.dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   struct intel_dsi *intel_dsi = enc_to_intel_dsi(>base);
-   enum port port;
-   u32 temp;
+   /* 2. Enter ULPS */
+   val = I915_READ(MIPI_DEVICE_READY(port));
+   val &= ~ULPS_STATE_MASK;
+   val |= (ULPS_STATE_ENTER | DEVICE_READY);
+   I915_WRITE(MIPI_DEVICE_READY(port), val);
+   usleep_range(2, 3);
+
+   /* 3. Exit ULPS */
+   val = I915_READ(MIPI_DEVICE_READY(port));
+   val &= ~ULPS_STATE_MASK;
+   val |= (ULPS_STATE_EXIT | DEVICE_READY);
+   I915_WRITE(MIPI_DEVICE_READY(port), val);
+