[PATCH 0/3] Fixing last of few known issues in DSI and Burst mode Support

[PATCH 0/3] Fixing last of few known issues in DSI and Burst mode Support

[Shobhit Kumar]

Hi,
This pacth set addresses a couple WARN dumps as DSI encoder did not yet implement the
->get_config for state tracking. This is tried to be addressed here. Most likely I have
missed few things but atleast all WARNs are taken care of.

Also better check is added to ensure all data has been sent to the panel before going
ahead in the sequence of enabling/disabling of the panel by waiting for all FIFOs to
be empty. 

Another new feature is DSI BURST mode support. With this single link DSI Video mode
support in our driver will be complete. Next plan is Dual link video mode support.

Regards
Shobhit

Shobhit Kumar (3):
  drm/i915: Add get_config implementation for DSI encoder
  drm/i915: wait for all DSI FIFOs to be empty
  drm/i915: Add support for Video Burst Mode for MIPI DSI

 drivers/gpu/drm/i915/intel_bios.h          |  3 +-
 drivers/gpu/drm/i915/intel_display.c       |  7 +--
 drivers/gpu/drm/i915/intel_dsi.c           | 75 ++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_dsi.h           |  5 ++
 drivers/gpu/drm/i915/intel_dsi_cmd.c       | 16 +++++++
 drivers/gpu/drm/i915/intel_dsi_cmd.h       |  1 +
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 38 ++++++++++++++-
 drivers/gpu/drm/i915/intel_dsi_pll.c       | 13 +++---
 8 files changed, 137 insertions(+), 21 deletions(-)

-- 
1.9.1

[PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder

[Shobhit Kumar]

Call to vlv_crtc_clock_get is not needed for DSI and was causing dpio
read WARN dumps as well. Absence of ->get_config was casuing othet WARN
dumps as well. With this the last of the known WARN dumps for DSI should
be fixed.

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  7 +++---
 drivers/gpu/drm/i915/intel_dsi.c     | 45 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi.h     |  3 +++
 drivers/gpu/drm/i915/intel_dsi_pll.c |  4 +++-
 4 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fe6f1db..3d0ea7c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6309,9 +6309,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 
 	if (IS_CHERRYVIEW(dev))
 		chv_crtc_clock_get(crtc, pipe_config);
-	else if (IS_VALLEYVIEW(dev))
-		vlv_crtc_clock_get(crtc, pipe_config);
-	else
+	else if (IS_VALLEYVIEW(dev)) {
+		if (!intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DSI))
+			vlv_crtc_clock_get(crtc, pipe_config);
+	} else
 		i9xx_crtc_clock_get(crtc, pipe_config);
 
 	return true;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index bfcefbf..61da0e5 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -351,9 +351,54 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 static void intel_dsi_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_config *pipe_config)
 {
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 dsi_clock, pclk;
+	u32 pll_ctl, pll_div;
+	u32 m = 0, p = 0;
+	int refclk = 25000;
+	int i;
+
 	DRM_DEBUG_KMS("\n");
 
 	/* XXX: read flags, set to adjusted_mode */
+	pipe_config->quirks = 1;
+
+	memset(&pipe_config->dpll_hw_state, 0,
+				sizeof(pipe_config->dpll_hw_state));
+
+	mutex_lock(&dev_priv->dpio_lock);
+	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
+	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
+	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
+
+	while (pll_ctl) {
+		pll_ctl = pll_ctl >> 1;
+		p++;
+	}
+	p--;
+
+	for (i = 0; i < num_lfsr_converts; i++) {
+		if (lfsr_converts[i] == pll_div)
+			break;
+	}
+
+	if (i == num_lfsr_converts) {
+		DRM_ERROR("wrong m_seed programmed\n");
+		return;
+	}
+
+	m = i + 62;
+
+	dsi_clock = (m * refclk) / p;
+	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count,
+			         pipe_config->pipe_bpp);
+
+	pipe_config->adjusted_mode.crtc_clock = pclk;
+	pipe_config->port_clock = pclk;
 }
 
 static enum drm_mode_status
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 31db33d..e0c16b0 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -130,6 +130,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 	return container_of(encoder, struct intel_dsi, base.base);
 }
 
+extern const u32 lfsr_converts[];
+extern const int num_lfsr_converts;
+
 extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
 extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index ba79ec1..78449ea 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -43,13 +43,15 @@ struct dsi_mnp {
 	u32 dsi_pll_div;
 };
 
-static const u32 lfsr_converts[] = {
+const u32 lfsr_converts[] = {
 	426, 469, 234, 373, 442, 221, 110, 311, 411,		/* 62 - 70 */
 	461, 486, 243, 377, 188, 350, 175, 343, 427, 213,	/* 71 - 80 */
 	106, 53, 282, 397, 354, 227, 113, 56, 284, 142,		/* 81 - 90 */
 	71, 35							/* 91 - 92 */
 };
 
+const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);
+
 #ifdef DSI_CLK_FROM_RR
 
 static u32 dsi_rr_formula(const struct drm_display_mode *mode,
-- 
1.9.1

Re: [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder

[Daniel Vetter]

On Sat, Jul 12, 2014 at 1:47 PM, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> Call to vlv_crtc_clock_get is not needed for DSI and was causing dpio
> read WARN dumps as well. Absence of ->get_config was casuing othet WARN
> dumps as well. With this the last of the known WARN dumps for DSI should
> be fixed.
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  7 +++---
>  drivers/gpu/drm/i915/intel_dsi.c     | 45 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.h     |  3 +++
>  drivers/gpu/drm/i915/intel_dsi_pll.c |  4 +++-
>  4 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fe6f1db..3d0ea7c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6309,9 +6309,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>
>         if (IS_CHERRYVIEW(dev))
>                 chv_crtc_clock_get(crtc, pipe_config);
> -       else if (IS_VALLEYVIEW(dev))
> -               vlv_crtc_clock_get(crtc, pipe_config);
> -       else
> +       else if (IS_VALLEYVIEW(dev)) {
> +               if (!intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DSI))
> +                       vlv_crtc_clock_get(crtc, pipe_config);

If I understand the logic correctly we don't even enable the DPLL for
dsi, i.e. bit31 is clear. So instead of this sw-side check (which is
fragile since it depends upon corrected encoder->pipe links in our
data structures) we should instead check bit 31 in vlv_crtc_clock_get
and just bail out without reading out hw settings.

The goal of the hw state cross-check is to check the hw state, so
wherever possible we should rely 100% on available information in the
hw and not on software tracking.

> +       } else
>                 i9xx_crtc_clock_get(crtc, pipe_config);
>
>         return true;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index bfcefbf..61da0e5 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -351,9 +351,54 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  static void intel_dsi_get_config(struct intel_encoder *encoder,
>                                  struct intel_crtc_config *pipe_config)
>  {
> +       struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +       struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +       u32 dsi_clock, pclk;
> +       u32 pll_ctl, pll_div;
> +       u32 m = 0, p = 0;
> +       int refclk = 25000;
> +       int i;
> +
>         DRM_DEBUG_KMS("\n");
>
>         /* XXX: read flags, set to adjusted_mode */
> +       pipe_config->quirks = 1;

Nack. First you need to use one of the symbolic quirk definitions
(there's a bunch of them). Second this needs a comment why exactly we
need the quirk (which really only should be used if there's no way to
read a given piece of state back from the hw).

> +
> +       memset(&pipe_config->dpll_hw_state, 0,
> +                               sizeof(pipe_config->dpll_hw_state));
> +
> +       mutex_lock(&dev_priv->dpio_lock);
> +       pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
> +       pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
> +       mutex_unlock(&dev_priv->dpio_lock);
> +
> +       pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
> +       pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
> +
> +       while (pll_ctl) {
> +               pll_ctl = pll_ctl >> 1;
> +               p++;
> +       }
> +       p--;
> +
> +       for (i = 0; i < num_lfsr_converts; i++) {
> +               if (lfsr_converts[i] == pll_div)
> +                       break;
> +       }
> +
> +       if (i == num_lfsr_converts) {
> +               DRM_ERROR("wrong m_seed programmed\n");
> +               return;
> +       }
> +
> +       m = i + 62;
> +
> +       dsi_clock = (m * refclk) / p;
> +       pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count,
> +                                pipe_config->pipe_bpp);
> +
> +       pipe_config->adjusted_mode.crtc_clock = pclk;
> +       pipe_config->port_clock = pclk;
>  }
>
>  static enum drm_mode_status
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 31db33d..e0c16b0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -130,6 +130,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>         return container_of(encoder, struct intel_dsi, base.base);
>  }
>
> +extern const u32 lfsr_converts[];
> +extern const int num_lfsr_converts;
> +
>  extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
>  extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index ba79ec1..78449ea 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -43,13 +43,15 @@ struct dsi_mnp {
>         u32 dsi_pll_div;
>  };
>
> -static const u32 lfsr_converts[] = {
> +const u32 lfsr_converts[] = {
>         426, 469, 234, 373, 442, 221, 110, 311, 411,            /* 62 - 70 */
>         461, 486, 243, 377, 188, 350, 175, 343, 427, 213,       /* 71 - 80 */
>         106, 53, 282, 397, 354, 227, 113, 56, 284, 142,         /* 81 - 90 */
>         71, 35                                                  /* 91 - 92 */
>  };

Optional bikeshed: I'd extract the dsi pll read-out code into a helper
function so that all the pll code is in this file and we don't need to
export internal details. The get_hw_state function in intel_dsi.c
would then use that to compute pclk and just store that at the right
places in the pipe config.
-Daniel

>
> +const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);
> +
>  #ifdef DSI_CLK_FROM_RR
>
>  static u32 dsi_rr_formula(const struct drm_display_mode *mode,
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder

[Kumar, Shobhit]

On 7/12/2014 5:28 PM, Daniel Vetter wrote:
> On Sat, Jul 12, 2014 at 1:47 PM, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>> Call to vlv_crtc_clock_get is not needed for DSI and was causing dpio
>> read WARN dumps as well. Absence of ->get_config was casuing othet WARN
>> dumps as well. With this the last of the known WARN dumps for DSI should
>> be fixed.
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |  7 +++---
>>   drivers/gpu/drm/i915/intel_dsi.c     | 45 ++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_dsi.h     |  3 +++
>>   drivers/gpu/drm/i915/intel_dsi_pll.c |  4 +++-
>>   4 files changed, 55 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index fe6f1db..3d0ea7c 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6309,9 +6309,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>>
>>          if (IS_CHERRYVIEW(dev))
>>                  chv_crtc_clock_get(crtc, pipe_config);
>> -       else if (IS_VALLEYVIEW(dev))
>> -               vlv_crtc_clock_get(crtc, pipe_config);
>> -       else
>> +       else if (IS_VALLEYVIEW(dev)) {
>> +               if (!intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DSI))
>> +                       vlv_crtc_clock_get(crtc, pipe_config);
>
> If I understand the logic correctly we don't even enable the DPLL for
> dsi, i.e. bit31 is clear. So instead of this sw-side check (which is
> fragile since it depends upon corrected encoder->pipe links in our
> data structures) we should instead check bit 31 in vlv_crtc_clock_get
> and just bail out without reading out hw settings.

Yeah, sounds better will do this.

>
> The goal of the hw state cross-check is to check the hw state, so
> wherever possible we should rely 100% on available information in the
> hw and not on software tracking.

Got it.

>
>> +       } else
>>                  i9xx_crtc_clock_get(crtc, pipe_config);
>>
>>          return true;
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index bfcefbf..61da0e5 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -351,9 +351,54 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>>   static void intel_dsi_get_config(struct intel_encoder *encoder,
>>                                   struct intel_crtc_config *pipe_config)
>>   {
>> +       struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>> +       struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>> +       u32 dsi_clock, pclk;
>> +       u32 pll_ctl, pll_div;
>> +       u32 m = 0, p = 0;
>> +       int refclk = 25000;
>> +       int i;
>> +
>>          DRM_DEBUG_KMS("\n");
>>
>>          /* XXX: read flags, set to adjusted_mode */
>> +       pipe_config->quirks = 1;
>
> Nack. First you need to use one of the symbolic quirk definitions
> (there's a bunch of them). Second this needs a comment why exactly we
> need the quirk (which really only should be used if there's no way to
> read a given piece of state back from the hw).

Okay, in MIPI we have sync events going as short packets. In that case I 
think it should be okay to use PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS ?

>
>> +
>> +       memset(&pipe_config->dpll_hw_state, 0,
>> +                               sizeof(pipe_config->dpll_hw_state));
>> +
>> +       mutex_lock(&dev_priv->dpio_lock);
>> +       pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
>> +       pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
>> +       mutex_unlock(&dev_priv->dpio_lock);
>> +
>> +       pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
>> +       pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
>> +
>> +       while (pll_ctl) {
>> +               pll_ctl = pll_ctl >> 1;
>> +               p++;
>> +       }
>> +       p--;
>> +
>> +       for (i = 0; i < num_lfsr_converts; i++) {
>> +               if (lfsr_converts[i] == pll_div)
>> +                       break;
>> +       }
>> +
>> +       if (i == num_lfsr_converts) {
>> +               DRM_ERROR("wrong m_seed programmed\n");
>> +               return;
>> +       }
>> +
>> +       m = i + 62;
>> +
>> +       dsi_clock = (m * refclk) / p;
>> +       pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count,
>> +                                pipe_config->pipe_bpp);
>> +
>> +       pipe_config->adjusted_mode.crtc_clock = pclk;
>> +       pipe_config->port_clock = pclk;
>>   }
>>
>>   static enum drm_mode_status
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>> index 31db33d..e0c16b0 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> @@ -130,6 +130,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>>          return container_of(encoder, struct intel_dsi, base.base);
>>   }
>>
>> +extern const u32 lfsr_converts[];
>> +extern const int num_lfsr_converts;
>> +
>>   extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
>>   extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> index ba79ec1..78449ea 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> @@ -43,13 +43,15 @@ struct dsi_mnp {
>>          u32 dsi_pll_div;
>>   };
>>
>> -static const u32 lfsr_converts[] = {
>> +const u32 lfsr_converts[] = {
>>          426, 469, 234, 373, 442, 221, 110, 311, 411,            /* 62 - 70 */
>>          461, 486, 243, 377, 188, 350, 175, 343, 427, 213,       /* 71 - 80 */
>>          106, 53, 282, 397, 354, 227, 113, 56, 284, 142,         /* 81 - 90 */
>>          71, 35                                                  /* 91 - 92 */
>>   };
>
> Optional bikeshed: I'd extract the dsi pll read-out code into a helper
> function so that all the pll code is in this file and we don't need to
> export internal details. The get_hw_state function in intel_dsi.c
> would then use that to compute pclk and just store that at the right
> places in the pipe config.

Yeah will correct this as well.

Regards
Shobhit

Re: [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder

[Daniel Vetter]

On Mon, Jul 14, 2014 at 4:36 PM, Kumar, Shobhit <shobhit.kumar@intel.com> wrote:
>>>          /* XXX: read flags, set to adjusted_mode */
>>> +       pipe_config->quirks = 1;
>>
>>
>> Nack. First you need to use one of the symbolic quirk definitions
>> (there's a bunch of them). Second this needs a comment why exactly we
>> need the quirk (which really only should be used if there's no way to
>> read a given piece of state back from the hw).
>
>
> Okay, in MIPI we have sync events going as short packets. In that case I
> think it should be okay to use PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS ?

Well it depends. From a quick look it seems like the current dsi code
doesn't care at all about sync flags. In that case you should
normalize the sync flags of the adjusted mode in the compute_config
callback to 0 and not set them in the get_hw_state function. We do
that already for e.g. tv encoder outputs.

The quirk flag should only be used if we do set the sync modes but
somehow can't read it back. The only case is sdvo where some encoders
(in violation of the spec) don't support the flag readback. But that
case needs a big comment explaining why.

The goal here isn't to shut up the hw cross checker but to actually
make it useful ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder

[Kumar, Shobhit]

On 7/14/2014 9:20 PM, Daniel Vetter wrote:
> On Mon, Jul 14, 2014 at 4:36 PM, Kumar, Shobhit <shobhit.kumar@intel.com> wrote:
>>>>           /* XXX: read flags, set to adjusted_mode */
>>>> +       pipe_config->quirks = 1;
>>>
>>>
>>> Nack. First you need to use one of the symbolic quirk definitions
>>> (there's a bunch of them). Second this needs a comment why exactly we
>>> need the quirk (which really only should be used if there's no way to
>>> read a given piece of state back from the hw).
>>
>>
>> Okay, in MIPI we have sync events going as short packets. In that case I
>> think it should be okay to use PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS ?
>
> Well it depends. From a quick look it seems like the current dsi code
> doesn't care at all about sync flags. In that case you should
> normalize the sync flags of the adjusted mode in the compute_config
> callback to 0 and not set them in the get_hw_state function. We do
> that already for e.g. tv encoder outputs.

I just assumed that as we don't care about sync flags just suppress 
their check :) Thanks for pointing correct way of doing this. I will 
send the corrected patch

>
> The quirk flag should only be used if we do set the sync modes but
> somehow can't read it back. The only case is sdvo where some encoders
> (in violation of the spec) don't support the flag readback. But that
> case needs a big comment explaining why.
>
> The goal here isn't to shut up the hw cross checker but to actually
> make it useful ;-)

Of-course. Thanks for clarifying.

Regards
Shobhit

[v2] drm/i915: Add correct hw/sw config check for DSI encoder

[Shobhit Kumar]

Check in vlv_crtc_clock_get if DPLL is enabled before calling dpio read.
It will not be enabled for DSI and avoid dpio read WARN dumps.

Absence of ->get_config was causing other WARN dumps as well. Update
dpll_hw_state as well correctly

v2: Address review comments by Daniel
    - Check if DPLL is enabled rather than checking pipe output type
    - set adjusted_mode->flags to 0 in compute_config rather than using
      pipe_config->quirks
    - Add helper function in intel_dsi_pll.c and use that in intel_dsi.c
    - updated dpll_hw_state correctly
    - Updated commit message and title

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  4 ++++
 drivers/gpu/drm/i915/intel_dsi.c     | 21 +++++++++++++++-
 drivers/gpu/drm/i915/intel_dsi.h     |  1 +
 drivers/gpu/drm/i915/intel_dsi_pll.c | 46 ++++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c89b4ac..d9c34e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6132,6 +6132,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 	u32 mdiv;
 	int refclk = 100000;
 
+	/* In case of MIPI DPLL will not even be used */
+	if (!(pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE))
+		return;
+
 	mutex_lock(&dev_priv->dpio_lock);
 	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
 	mutex_unlock(&dev_priv->dpio_lock);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index bfcefbf..43be71bf 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -92,6 +92,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 	if (fixed_mode)
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
+	/* DSI uses short packets for sync events, so clear mode flags for DSI */
+	adjusted_mode->flags = 0;
+
 	if (intel_dsi->dev.dev_ops->mode_fixup)
 		return intel_dsi->dev.dev_ops->mode_fixup(&intel_dsi->dev,
 							  mode, adjusted_mode);
@@ -177,6 +180,10 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 	tmp |= DPLL_REFA_CLK_ENABLE_VLV;
 	I915_WRITE(DPLL(pipe), tmp);
 
+	/* update the hw state for DPLL */
+	intel_crtc->config.dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
+						DPLL_REFA_CLK_ENABLE_VLV;
+
 	tmp = I915_READ(DSPCLK_GATE_D);
 	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
 	I915_WRITE(DSPCLK_GATE_D, tmp);
@@ -351,9 +358,21 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 static void intel_dsi_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_config *pipe_config)
 {
+	u32 pclk;
 	DRM_DEBUG_KMS("\n");
 
-	/* XXX: read flags, set to adjusted_mode */
+	/*
+	 * DPLL_MD is not used in case of DSI, reading will get some default value
+	 * set dpll_md = 0
+	 */
+	pipe_config->dpll_hw_state.dpll_md = 0;
+
+	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
+	if (!pclk)
+		return;
+
+	pipe_config->adjusted_mode.crtc_clock = pclk;
+	pipe_config->port_clock = pclk;
 }
 
 static enum drm_mode_status
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 31db33d..fd51867 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -132,6 +132,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 
 extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
 extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
+extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
 
 extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index ba79ec1..8085afe 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -50,6 +50,8 @@ static const u32 lfsr_converts[] = {
 	71, 35							/* 91 - 92 */
 };
 
+static const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);
+
 #ifdef DSI_CLK_FROM_RR
 
 static u32 dsi_rr_formula(const struct drm_display_mode *mode,
@@ -298,3 +300,47 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 
 	mutex_unlock(&dev_priv->dpio_lock);
 }
+
+u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 dsi_clock, pclk;
+	u32 pll_ctl, pll_div;
+	u32 m = 0, p = 0;
+	int refclk = 25000;
+	int i;
+
+	DRM_DEBUG_KMS("\n");
+
+	mutex_lock(&dev_priv->dpio_lock);
+	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
+	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
+	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
+
+	while (pll_ctl) {
+		pll_ctl = pll_ctl >> 1;
+		p++;
+	}
+	p--;
+
+	for (i = 0; i < num_lfsr_converts; i++) {
+		if (lfsr_converts[i] == pll_div)
+			break;
+	}
+
+	if (i == num_lfsr_converts) {
+		DRM_ERROR("wrong m_seed programmed\n");
+		return 0;
+	}
+
+	m = i + 62;
+
+	dsi_clock = (m * refclk) / p;
+	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, pipe_bpp);
+
+	return pclk;
+}
-- 
1.9.1

Re: [v2] drm/i915: Add correct hw/sw config check for DSI encoder

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 6125 bytes --]

On Tue, 2014-07-15 at 18:15 +0530, Shobhit Kumar wrote:
> Check in vlv_crtc_clock_get if DPLL is enabled before calling dpio read.
> It will not be enabled for DSI and avoid dpio read WARN dumps.
> 
> Absence of ->get_config was causing other WARN dumps as well. Update
> dpll_hw_state as well correctly
> 
> v2: Address review comments by Daniel
>     - Check if DPLL is enabled rather than checking pipe output type
>     - set adjusted_mode->flags to 0 in compute_config rather than using
>       pipe_config->quirks
>     - Add helper function in intel_dsi_pll.c and use that in intel_dsi.c
>     - updated dpll_hw_state correctly
>     - Updated commit message and title
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

Ok, reviewing now the latest version after Daniel pointed me to it.

> ---
>  drivers/gpu/drm/i915/intel_display.c |  4 ++++
>  drivers/gpu/drm/i915/intel_dsi.c     | 21 +++++++++++++++-
>  drivers/gpu/drm/i915/intel_dsi.h     |  1 +
>  drivers/gpu/drm/i915/intel_dsi_pll.c | 46 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c89b4ac..d9c34e4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6132,6 +6132,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
>  	u32 mdiv;
>  	int refclk = 100000;
>  
> +	/* In case of MIPI DPLL will not even be used */
> +	if (!(pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE))
> +		return;
> +
>  	mutex_lock(&dev_priv->dpio_lock);
>  	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
>  	mutex_unlock(&dev_priv->dpio_lock);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index bfcefbf..43be71bf 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -92,6 +92,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  	if (fixed_mode)
>  		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
>  
> +	/* DSI uses short packets for sync events, so clear mode flags for DSI */
> +	adjusted_mode->flags = 0;
> +
>  	if (intel_dsi->dev.dev_ops->mode_fixup)
>  		return intel_dsi->dev.dev_ops->mode_fixup(&intel_dsi->dev,
>  							  mode, adjusted_mode);
> @@ -177,6 +180,10 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  	tmp |= DPLL_REFA_CLK_ENABLE_VLV;
>  	I915_WRITE(DPLL(pipe), tmp);
>  
> +	/* update the hw state for DPLL */
> +	intel_crtc->config.dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> +						DPLL_REFA_CLK_ENABLE_VLV;
> +
>  	tmp = I915_READ(DSPCLK_GATE_D);
>  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
>  	I915_WRITE(DSPCLK_GATE_D, tmp);
> @@ -351,9 +358,21 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  static void intel_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_config *pipe_config)
>  {
> +	u32 pclk;
>  	DRM_DEBUG_KMS("\n");
>  
> -	/* XXX: read flags, set to adjusted_mode */
> +	/*
> +	 * DPLL_MD is not used in case of DSI, reading will get some default value
> +	 * set dpll_md = 0
> +	 */
> +	pipe_config->dpll_hw_state.dpll_md = 0;
> +
> +	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> +	if (!pclk)
> +		return;
> +
> +	pipe_config->adjusted_mode.crtc_clock = pclk;
> +	pipe_config->port_clock = pclk;
>  }
>  
>  static enum drm_mode_status
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 31db33d..fd51867 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -132,6 +132,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>  
>  extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
>  extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
> +extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
>  
>  extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index ba79ec1..8085afe 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -50,6 +50,8 @@ static const u32 lfsr_converts[] = {
>  	71, 35							/* 91 - 92 */
>  };
>  
> +static const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);
> +

This could be just inlined using ARRAY_SIZE.

>  #ifdef DSI_CLK_FROM_RR
>  
>  static u32 dsi_rr_formula(const struct drm_display_mode *mode,
> @@ -298,3 +300,47 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder)
>  
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
> +
> +u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
> +{
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	u32 dsi_clock, pclk;
> +	u32 pll_ctl, pll_div;
> +	u32 m = 0, p = 0;
> +	int refclk = 25000;
> +	int i;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	mutex_lock(&dev_priv->dpio_lock);
> +	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
> +	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
> +	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
> +
> +	while (pll_ctl) {
> +		pll_ctl = pll_ctl >> 1;
> +		p++;
> +	}
> +	p--;
> +
> +	for (i = 0; i < num_lfsr_converts; i++) {
> +		if (lfsr_converts[i] == pll_div)
> +			break;
> +	}
> +
> +	if (i == num_lfsr_converts) {
> +		DRM_ERROR("wrong m_seed programmed\n");
> +		return 0;
> +	}
> +
> +	m = i + 62;
> +
> +	dsi_clock = (m * refclk) / p;
> +	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, pipe_bpp);
> +
> +	return pclk;
> +}

Please see my comments for intel_dsi_get_config() in my v1 review where
they apply in vlv_get_dsi_pclk().

--Imre

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[v3] drm/i915: Add correct hw/sw config check for DSI encoder

[Shobhit Kumar]

Check in vlv_crtc_clock_get if DPLL is enabled before calling dpio read.
It will not be enabled for DSI and avoid dpio read WARN dumps.

Absence of ->get_config was causing other WARN dumps as well. Update
dpll_hw_state as well correctly

v2: Address review comments by Daniel
    - Check if DPLL is enabled rather than checking pipe output type
    - set adjusted_mode->flags to 0 in compute_config rather than using
      pipe_config->quirks
    - Add helper function in intel_dsi_pll.c and use that in intel_dsi.c
    - updated dpll_hw_state correctly
    - Updated commit message and title

v3: Address review comments by Imre
    - Proper masking of P1, M1 fields while computing divisors
    - assert in case of bpp mismatch
    - guard for divide by 0 while computing pclk
    - Use ARRAY_SIZE instead of direct calculation

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  4 ++
 drivers/gpu/drm/i915/intel_dsi.c     | 21 +++++++++-
 drivers/gpu/drm/i915/intel_dsi.h     |  1 +
 drivers/gpu/drm/i915/intel_dsi_pll.c | 81 ++++++++++++++++++++++++++++++++++++
 4 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f3d6cc3..883af0b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6195,6 +6195,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 	u32 mdiv;
 	int refclk = 100000;
 
+	/* In case of MIPI DPLL will not even be used */
+	if (!(pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE))
+		return;
+
 	mutex_lock(&dev_priv->dpio_lock);
 	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
 	mutex_unlock(&dev_priv->dpio_lock);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 09e1caf..670c29a 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -92,6 +92,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 	if (fixed_mode)
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
+	/* DSI uses short packets for sync events, so clear mode flags for DSI */
+	adjusted_mode->flags = 0;
+
 	if (intel_dsi->dev.dev_ops->mode_fixup)
 		return intel_dsi->dev.dev_ops->mode_fixup(&intel_dsi->dev,
 							  mode, adjusted_mode);
@@ -179,6 +182,10 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 	tmp |= DPLL_REFA_CLK_ENABLE_VLV;
 	I915_WRITE(DPLL(pipe), tmp);
 
+	/* update the hw state for DPLL */
+	intel_crtc->config.dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
+						DPLL_REFA_CLK_ENABLE_VLV;
+
 	tmp = I915_READ(DSPCLK_GATE_D);
 	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
 	I915_WRITE(DSPCLK_GATE_D, tmp);
@@ -359,9 +366,21 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 static void intel_dsi_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_config *pipe_config)
 {
+	u32 pclk;
 	DRM_DEBUG_KMS("\n");
 
-	/* XXX: read flags, set to adjusted_mode */
+	/*
+	 * DPLL_MD is not used in case of DSI, reading will get some default value
+	 * set dpll_md = 0
+	 */
+	pipe_config->dpll_hw_state.dpll_md = 0;
+
+	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
+	if (!pclk)
+		return;
+
+	pipe_config->adjusted_mode.crtc_clock = pclk;
+	pipe_config->port_clock = pclk;
 }
 
 static enum drm_mode_status
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 31db33d..fd51867 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -132,6 +132,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 
 extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
 extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
+extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
 
 extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index ba79ec1..d8bb1ea 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -298,3 +298,84 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 
 	mutex_unlock(&dev_priv->dpio_lock);
 }
+
+static void assert_bpp_mismatch(int pixel_format, int pipe_bpp)
+{
+	int bpp;
+
+	switch (pixel_format) {
+	default:
+	case VID_MODE_FORMAT_RGB888:
+	case VID_MODE_FORMAT_RGB666_LOOSE:
+		bpp = 24;
+		break;
+	case VID_MODE_FORMAT_RGB666:
+		bpp = 18;
+		break;
+	case VID_MODE_FORMAT_RGB565:
+		bpp = 16;
+		break;
+	}
+
+	WARN(bpp != pipe_bpp,
+		"bpp match assertion failure (expected %d, current %d)\n",
+		bpp, pipe_bpp);
+}
+
+u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 dsi_clock, pclk;
+	u32 pll_ctl, pll_div;
+	u32 m = 0, p = 0;
+	int refclk = 25000;
+	int i;
+
+	DRM_DEBUG_KMS("\n");
+
+	mutex_lock(&dev_priv->dpio_lock);
+	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
+	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	/* mask out other bits and extract the P1 divisor */
+	pll_ctl &= DSI_PLL_P1_POST_DIV_MASK;
+	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
+
+	/* mask out the other bits and extract the M1 divisor */
+	pll_div &= DSI_PLL_M1_DIV_MASK;
+	pll_div = pll_div >> DSI_PLL_M1_DIV_SHIFT;
+
+	while (pll_ctl) {
+		pll_ctl = pll_ctl >> 1;
+		p++;
+	}
+	p--;
+
+	if (!p) {
+		DRM_ERROR("wrong P1 divisor\n");
+		return 0;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(lfsr_converts); i++) {
+		if (lfsr_converts[i] == pll_div)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(lfsr_converts)) {
+		DRM_ERROR("wrong m_seed programmed\n");
+		return 0;
+	}
+
+	m = i + 62;
+
+	dsi_clock = (m * refclk) / p;
+
+	/* pixel_format and pipe_bpp should agree */
+	assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
+
+	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, pipe_bpp);
+
+	return pclk;
+}
-- 
1.9.1

Re: [v3] drm/i915: Add correct hw/sw config check for DSI encoder

[Imre Deak]

On Wed, 2014-07-30 at 20:32 +0530, Shobhit Kumar wrote:
> Check in vlv_crtc_clock_get if DPLL is enabled before calling dpio read.
> It will not be enabled for DSI and avoid dpio read WARN dumps.
> 
> Absence of ->get_config was causing other WARN dumps as well. Update
> dpll_hw_state as well correctly
> 
> v2: Address review comments by Daniel
>     - Check if DPLL is enabled rather than checking pipe output type
>     - set adjusted_mode->flags to 0 in compute_config rather than using
>       pipe_config->quirks
>     - Add helper function in intel_dsi_pll.c and use that in intel_dsi.c
>     - updated dpll_hw_state correctly
>     - Updated commit message and title
> 
> v3: Address review comments by Imre
>     - Proper masking of P1, M1 fields while computing divisors
>     - assert in case of bpp mismatch
>     - guard for divide by 0 while computing pclk
>     - Use ARRAY_SIZE instead of direct calculation
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c |  4 ++
>  drivers/gpu/drm/i915/intel_dsi.c     | 21 +++++++++-
>  drivers/gpu/drm/i915/intel_dsi.h     |  1 +
>  drivers/gpu/drm/i915/intel_dsi_pll.c | 81 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f3d6cc3..883af0b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6195,6 +6195,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
>  	u32 mdiv;
>  	int refclk = 100000;
>  
> +	/* In case of MIPI DPLL will not even be used */
> +	if (!(pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE))
> +		return;
> +
>  	mutex_lock(&dev_priv->dpio_lock);
>  	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
>  	mutex_unlock(&dev_priv->dpio_lock);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 09e1caf..670c29a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -92,6 +92,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  	if (fixed_mode)
>  		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
>  
> +	/* DSI uses short packets for sync events, so clear mode flags for DSI */
> +	adjusted_mode->flags = 0;
> +
>  	if (intel_dsi->dev.dev_ops->mode_fixup)
>  		return intel_dsi->dev.dev_ops->mode_fixup(&intel_dsi->dev,
>  							  mode, adjusted_mode);
> @@ -179,6 +182,10 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  	tmp |= DPLL_REFA_CLK_ENABLE_VLV;
>  	I915_WRITE(DPLL(pipe), tmp);
>  
> +	/* update the hw state for DPLL */
> +	intel_crtc->config.dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> +						DPLL_REFA_CLK_ENABLE_VLV;
> +
>  	tmp = I915_READ(DSPCLK_GATE_D);
>  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
>  	I915_WRITE(DSPCLK_GATE_D, tmp);
> @@ -359,9 +366,21 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  static void intel_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_config *pipe_config)
>  {
> +	u32 pclk;
>  	DRM_DEBUG_KMS("\n");
>  
> -	/* XXX: read flags, set to adjusted_mode */
> +	/*
> +	 * DPLL_MD is not used in case of DSI, reading will get some default value
> +	 * set dpll_md = 0
> +	 */
> +	pipe_config->dpll_hw_state.dpll_md = 0;
> +
> +	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> +	if (!pclk)
> +		return;
> +
> +	pipe_config->adjusted_mode.crtc_clock = pclk;
> +	pipe_config->port_clock = pclk;
>  }
>  
>  static enum drm_mode_status
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 31db33d..fd51867 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -132,6 +132,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>  
>  extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
>  extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
> +extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
>  
>  extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index ba79ec1..d8bb1ea 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -298,3 +298,84 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder)
>  
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
> +
> +static void assert_bpp_mismatch(int pixel_format, int pipe_bpp)
> +{
> +	int bpp;
> +
> +	switch (pixel_format) {
> +	default:
> +	case VID_MODE_FORMAT_RGB888:
> +	case VID_MODE_FORMAT_RGB666_LOOSE:
> +		bpp = 24;
> +		break;
> +	case VID_MODE_FORMAT_RGB666:
> +		bpp = 18;
> +		break;
> +	case VID_MODE_FORMAT_RGB565:
> +		bpp = 16;
> +		break;
> +	}
> +
> +	WARN(bpp != pipe_bpp,
> +		"bpp match assertion failure (expected %d, current %d)\n",
> +		bpp, pipe_bpp);
> +}
> +
> +u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
> +{
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	u32 dsi_clock, pclk;
> +	u32 pll_ctl, pll_div;
> +	u32 m = 0, p = 0;
> +	int refclk = 25000;
> +	int i;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	mutex_lock(&dev_priv->dpio_lock);
> +	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
> +	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	/* mask out other bits and extract the P1 divisor */
> +	pll_ctl &= DSI_PLL_P1_POST_DIV_MASK;
> +	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
> +
> +	/* mask out the other bits and extract the M1 divisor */
> +	pll_div &= DSI_PLL_M1_DIV_MASK;
> +	pll_div = pll_div >> DSI_PLL_M1_DIV_SHIFT;
> +
> +	while (pll_ctl) {
> +		pll_ctl = pll_ctl >> 1;
> +		p++;
> +	}
> +	p--;
> +
> +	if (!p) {
> +		DRM_ERROR("wrong P1 divisor\n");
> +		return 0;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(lfsr_converts); i++) {
> +		if (lfsr_converts[i] == pll_div)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(lfsr_converts)) {
> +		DRM_ERROR("wrong m_seed programmed\n");
> +		return 0;
> +	}
> +
> +	m = i + 62;
> +
> +	dsi_clock = (m * refclk) / p;
> +
> +	/* pixel_format and pipe_bpp should agree */
> +	assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
> +
> +	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, pipe_bpp);
> +
> +	return pclk;
> +}

Re: [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 4994 bytes --]

On Sat, 2014-07-12 at 17:17 +0530, Shobhit Kumar wrote:
> Call to vlv_crtc_clock_get is not needed for DSI and was causing dpio
> read WARN dumps as well. Absence of ->get_config was casuing othet WARN
> dumps as well. With this the last of the known WARN dumps for DSI should
> be fixed.
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  7 +++---
>  drivers/gpu/drm/i915/intel_dsi.c     | 45 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.h     |  3 +++
>  drivers/gpu/drm/i915/intel_dsi_pll.c |  4 +++-
>  4 files changed, 55 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fe6f1db..3d0ea7c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6309,9 +6309,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  
>  	if (IS_CHERRYVIEW(dev))
>  		chv_crtc_clock_get(crtc, pipe_config);
> -	else if (IS_VALLEYVIEW(dev))
> -		vlv_crtc_clock_get(crtc, pipe_config);
> -	else
> +	else if (IS_VALLEYVIEW(dev)) {
> +		if (!intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DSI))
> +			vlv_crtc_clock_get(crtc, pipe_config);
> +	} else
>  		i9xx_crtc_clock_get(crtc, pipe_config);
>  
>  	return true;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index bfcefbf..61da0e5 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -351,9 +351,54 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  static void intel_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_config *pipe_config)
>  {
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	u32 dsi_clock, pclk;
> +	u32 pll_ctl, pll_div;
> +	u32 m = 0, p = 0;
> +	int refclk = 25000;
> +	int i;
> +
>  	DRM_DEBUG_KMS("\n");
>  
>  	/* XXX: read flags, set to adjusted_mode */

This comment can be removed.

> +	pipe_config->quirks = 1;

The proper macro should be used.

> +
> +	memset(&pipe_config->dpll_hw_state, 0,
> +				sizeof(pipe_config->dpll_hw_state));
> +
> +	mutex_lock(&dev_priv->dpio_lock);
> +	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
> +	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
> +	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);

I'd prefer a proper masking of the P1 field instead of depending on
other register fields being 0. The same goes for pll_div[M1].

> +
> +	while (pll_ctl) {
> +		pll_ctl = pll_ctl >> 1;
> +		p++;
> +	}
> +	p--;
> +
> +	for (i = 0; i < num_lfsr_converts; i++) {
> +		if (lfsr_converts[i] == pll_div)
> +			break;
> +	}
> +
> +	if (i == num_lfsr_converts) {
> +		DRM_ERROR("wrong m_seed programmed\n");
> +		return;
> +	}
> +
> +	m = i + 62;
> +
> +	dsi_clock = (m * refclk) / p;

Should guard against div-by-zero.

> +	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count,
> +			         pipe_config->pipe_bpp);

dsi_clk_from_pclk() uses dsi->pixel_format in place of pipe_bpp, so an
assert here that the two values agree would be nice.

> +
> +	pipe_config->adjusted_mode.crtc_clock = pclk;
> +	pipe_config->port_clock = pclk;
>  }
>  
>  static enum drm_mode_status
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 31db33d..e0c16b0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -130,6 +130,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>  	return container_of(encoder, struct intel_dsi, base.base);
>  }
>  
> +extern const u32 lfsr_converts[];
> +extern const int num_lfsr_converts;
> +
>  extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
>  extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index ba79ec1..78449ea 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -43,13 +43,15 @@ struct dsi_mnp {
>  	u32 dsi_pll_div;
>  };
>  
> -static const u32 lfsr_converts[] = {
> +const u32 lfsr_converts[] = {
>  	426, 469, 234, 373, 442, 221, 110, 311, 411,		/* 62 - 70 */
>  	461, 486, 243, 377, 188, 350, 175, 343, 427, 213,	/* 71 - 80 */
>  	106, 53, 282, 397, 354, 227, 113, 56, 284, 142,		/* 81 - 90 */
>  	71, 35							/* 91 - 92 */
>  };
>  
> +const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);

You can use ARRAY_SIZE here.

> +
>  #ifdef DSI_CLK_FROM_RR
>  
>  static u32 dsi_rr_formula(const struct drm_display_mode *mode,


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder

[Daniel Vetter]

On Tue, Jul 29, 2014 at 02:38:36PM +0300, Imre Deak wrote:
> On Sat, 2014-07-12 at 17:17 +0530, Shobhit Kumar wrote:
> > Call to vlv_crtc_clock_get is not needed for DSI and was causing dpio
> > read WARN dumps as well. Absence of ->get_config was casuing othet WARN
> > dumps as well. With this the last of the known WARN dumps for DSI should
> > be fixed.
> > 
> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |  7 +++---
> >  drivers/gpu/drm/i915/intel_dsi.c     | 45 ++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dsi.h     |  3 +++
> >  drivers/gpu/drm/i915/intel_dsi_pll.c |  4 +++-
> >  4 files changed, 55 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index fe6f1db..3d0ea7c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6309,9 +6309,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >  
> >  	if (IS_CHERRYVIEW(dev))
> >  		chv_crtc_clock_get(crtc, pipe_config);
> > -	else if (IS_VALLEYVIEW(dev))
> > -		vlv_crtc_clock_get(crtc, pipe_config);
> > -	else
> > +	else if (IS_VALLEYVIEW(dev)) {
> > +		if (!intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DSI))
> > +			vlv_crtc_clock_get(crtc, pipe_config);
> > +	} else
> >  		i9xx_crtc_clock_get(crtc, pipe_config);
> >  
> >  	return true;
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index bfcefbf..61da0e5 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -351,9 +351,54 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> >  static void intel_dsi_get_config(struct intel_encoder *encoder,
> >  				 struct intel_crtc_config *pipe_config)
> >  {
> > +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	u32 dsi_clock, pclk;
> > +	u32 pll_ctl, pll_div;
> > +	u32 m = 0, p = 0;
> > +	int refclk = 25000;
> > +	int i;
> > +
> >  	DRM_DEBUG_KMS("\n");
> >  
> >  	/* XXX: read flags, set to adjusted_mode */
> 
> This comment can be removed.

There's a v2 with a lot of this addressed already.
-Daniel

> 
> > +	pipe_config->quirks = 1;
> 
> The proper macro should be used.
> 
> > +
> > +	memset(&pipe_config->dpll_hw_state, 0,
> > +				sizeof(pipe_config->dpll_hw_state));
> > +
> > +	mutex_lock(&dev_priv->dpio_lock);
> > +	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
> > +	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
> > +	mutex_unlock(&dev_priv->dpio_lock);
> > +
> > +	pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
> > +	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
> 
> I'd prefer a proper masking of the P1 field instead of depending on
> other register fields being 0. The same goes for pll_div[M1].
> 
> > +
> > +	while (pll_ctl) {
> > +		pll_ctl = pll_ctl >> 1;
> > +		p++;
> > +	}
> > +	p--;
> > +
> > +	for (i = 0; i < num_lfsr_converts; i++) {
> > +		if (lfsr_converts[i] == pll_div)
> > +			break;
> > +	}
> > +
> > +	if (i == num_lfsr_converts) {
> > +		DRM_ERROR("wrong m_seed programmed\n");
> > +		return;
> > +	}
> > +
> > +	m = i + 62;
> > +
> > +	dsi_clock = (m * refclk) / p;
> 
> Should guard against div-by-zero.
> 
> > +	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count,
> > +			         pipe_config->pipe_bpp);
> 
> dsi_clk_from_pclk() uses dsi->pixel_format in place of pipe_bpp, so an
> assert here that the two values agree would be nice.
> 
> > +
> > +	pipe_config->adjusted_mode.crtc_clock = pclk;
> > +	pipe_config->port_clock = pclk;
> >  }
> >  
> >  static enum drm_mode_status
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> > index 31db33d..e0c16b0 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.h
> > +++ b/drivers/gpu/drm/i915/intel_dsi.h
> > @@ -130,6 +130,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
> >  	return container_of(encoder, struct intel_dsi, base.base);
> >  }
> >  
> > +extern const u32 lfsr_converts[];
> > +extern const int num_lfsr_converts;
> > +
> >  extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
> >  extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> > index ba79ec1..78449ea 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> > @@ -43,13 +43,15 @@ struct dsi_mnp {
> >  	u32 dsi_pll_div;
> >  };
> >  
> > -static const u32 lfsr_converts[] = {
> > +const u32 lfsr_converts[] = {
> >  	426, 469, 234, 373, 442, 221, 110, 311, 411,		/* 62 - 70 */
> >  	461, 486, 243, 377, 188, 350, 175, 343, 427, 213,	/* 71 - 80 */
> >  	106, 53, 282, 397, 354, 227, 113, 56, 284, 142,		/* 81 - 90 */
> >  	71, 35							/* 91 - 92 */
> >  };
> >  
> > +const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);
> 
> You can use ARRAY_SIZE here.
> 
> > +
> >  #ifdef DSI_CLK_FROM_RR
> >  
> >  static u32 dsi_rr_formula(const struct drm_display_mode *mode,
> 



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


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 2/3] drm/i915: wait for all DSI FIFOs to be empty

[Shobhit Kumar]

Ensure that the DSI packets for a particular sequence are completely
sent before going ahead in the enabling or disabling of the panel

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c     |  8 ++++++++
 drivers/gpu/drm/i915/intel_dsi_cmd.c | 16 ++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi_cmd.h |  1 +
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 61da0e5..98c78ab 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -152,6 +152,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 		if (intel_dsi->dev.dev_ops->enable)
 			intel_dsi->dev.dev_ops->enable(&intel_dsi->dev);
 
+		wait_for_dsi_fifo_empty(intel_dsi);
+
 		/* assert ip_tg_enable signal */
 		temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
 		temp = temp | intel_dsi->port_bits;
@@ -192,6 +194,8 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 	if (intel_dsi->dev.dev_ops->send_otp_cmds)
 		intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev);
 
+	wait_for_dsi_fifo_empty(intel_dsi);
+
 	/* Enable port in pre-enable phase itself because as per hw team
 	 * recommendation, port should be enabled befor plane & pipe */
 	intel_dsi_enable(encoder);
@@ -232,6 +236,8 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
 	DRM_DEBUG_KMS("\n");
 
 	if (is_vid_mode(intel_dsi)) {
+		wait_for_dsi_fifo_empty(intel_dsi);
+
 		/* de-assert ip_tg_enable signal */
 		temp = I915_READ(MIPI_PORT_CTRL(pipe));
 		I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
@@ -261,6 +267,8 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
 	 * some next enable sequence send turn on packet error is observed */
 	if (intel_dsi->dev.dev_ops->disable)
 		intel_dsi->dev.dev_ops->disable(&intel_dsi->dev);
+
+	wait_for_dsi_fifo_empty(intel_dsi);
 }
 
 static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c b/drivers/gpu/drm/i915/intel_dsi_cmd.c
index 933c863..7f1430a 100644
--- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
+++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
@@ -419,3 +419,19 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs)
 
 	return 0;
 }
+
+void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi)
+{
+	struct drm_encoder *encoder = &intel_dsi->base.base;
+	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);
+	enum pipe pipe = intel_crtc->pipe;
+	u32 mask;
+
+	mask = LP_CTRL_FIFO_EMPTY | HS_CTRL_FIFO_EMPTY |
+					LP_DATA_FIFO_EMPTY | HS_DATA_FIFO_EMPTY;
+
+	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(pipe)) & mask) == mask, 100))
+		DRM_ERROR("DPI FIFOs are not empty\n");
+}
diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.h b/drivers/gpu/drm/i915/intel_dsi_cmd.h
index 9a18cbf..46aa1ac 100644
--- a/drivers/gpu/drm/i915/intel_dsi_cmd.h
+++ b/drivers/gpu/drm/i915/intel_dsi_cmd.h
@@ -51,6 +51,7 @@ int dsi_vc_generic_read(struct intel_dsi *intel_dsi, int channel,
 			u8 *reqdata, int reqlen, u8 *buf, int buflen);
 
 int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs);
+void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi);
 
 /* XXX: questionable write helpers */
 static inline int dsi_vc_dcs_write_0(struct intel_dsi *intel_dsi,
-- 
1.9.1

Re: [PATCH 2/3] drm/i915: wait for all DSI FIFOs to be empty

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 3812 bytes --]

On Sat, 2014-07-12 at 17:17 +0530, Shobhit Kumar wrote:
> Ensure that the DSI packets for a particular sequence are completely
> sent before going ahead in the enabling or disabling of the panel
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dsi.c     |  8 ++++++++
>  drivers/gpu/drm/i915/intel_dsi_cmd.c | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi_cmd.h |  1 +
>  3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 61da0e5..98c78ab 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -152,6 +152,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>  		if (intel_dsi->dev.dev_ops->enable)
>  			intel_dsi->dev.dev_ops->enable(&intel_dsi->dev);
>  
> +		wait_for_dsi_fifo_empty(intel_dsi);
> +
>  		/* assert ip_tg_enable signal */
>  		temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
>  		temp = temp | intel_dsi->port_bits;
> @@ -192,6 +194,8 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  	if (intel_dsi->dev.dev_ops->send_otp_cmds)
>  		intel_dsi->dev.dev_ops->send_otp_cmds(&intel_dsi->dev);
>  
> +	wait_for_dsi_fifo_empty(intel_dsi);
> +
>  	/* Enable port in pre-enable phase itself because as per hw team
>  	 * recommendation, port should be enabled befor plane & pipe */
>  	intel_dsi_enable(encoder);
> @@ -232,6 +236,8 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
>  	DRM_DEBUG_KMS("\n");
>  
>  	if (is_vid_mode(intel_dsi)) {
> +		wait_for_dsi_fifo_empty(intel_dsi);
> +
>  		/* de-assert ip_tg_enable signal */
>  		temp = I915_READ(MIPI_PORT_CTRL(pipe));
>  		I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
> @@ -261,6 +267,8 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
>  	 * some next enable sequence send turn on packet error is observed */
>  	if (intel_dsi->dev.dev_ops->disable)
>  		intel_dsi->dev.dev_ops->disable(&intel_dsi->dev);
> +
> +	wait_for_dsi_fifo_empty(intel_dsi);
>  }
>  
>  static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c b/drivers/gpu/drm/i915/intel_dsi_cmd.c
> index 933c863..7f1430a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
> @@ -419,3 +419,19 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs)
>  
>  	return 0;
>  }
> +
> +void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi)
> +{
> +	struct drm_encoder *encoder = &intel_dsi->base.base;
> +	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);
> +	enum pipe pipe = intel_crtc->pipe;
> +	u32 mask;
> +
> +	mask = LP_CTRL_FIFO_EMPTY | HS_CTRL_FIFO_EMPTY |
> +					LP_DATA_FIFO_EMPTY | HS_DATA_FIFO_EMPTY;
> +
> +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(pipe)) & mask) == mask, 100))
> +		DRM_ERROR("DPI FIFOs are not empty\n");
> +}
> diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.h b/drivers/gpu/drm/i915/intel_dsi_cmd.h
> index 9a18cbf..46aa1ac 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_cmd.h
> +++ b/drivers/gpu/drm/i915/intel_dsi_cmd.h
> @@ -51,6 +51,7 @@ int dsi_vc_generic_read(struct intel_dsi *intel_dsi, int channel,
>  			u8 *reqdata, int reqlen, u8 *buf, int buflen);
>  
>  int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs);
> +void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi);
>  
>  /* XXX: questionable write helpers */
>  static inline int dsi_vc_dcs_write_0(struct intel_dsi *intel_dsi,


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 3/3] drm/i915: Add support for Video Burst Mode for MIPI DSI

[Shobhit Kumar]

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.h          |  3 ++-
 drivers/gpu/drm/i915/intel_dsi.c           | 22 ++++++++++-------
 drivers/gpu/drm/i915/intel_dsi.h           |  2 ++
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 38 ++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_dsi_pll.c       |  9 +++----
 5 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index b986677..905999b 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -802,7 +802,8 @@ struct mipi_config {
 
 	u16 rsvd4;
 
-	u8 rsvd5[5];
+	u8 rsvd5;
+	u32 target_burst_mode_freq;
 	u32 dsi_ddr_clk;
 	u32 bridge_ref_clk;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 98c78ab..732d96b 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -449,9 +449,11 @@ static u16 txclkesc(u32 divider, unsigned int us)
 }
 
 /* return pixels in terms of txbyteclkhs */
-static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count)
+static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count,
+		       u16 burst_mode_ratio)
 {
-	return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp, 8), lane_count);
+	return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio,
+							8 * 100), lane_count);
 }
 
 static void set_dsi_timings(struct drm_encoder *encoder,
@@ -477,10 +479,12 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 	vbp = mode->vtotal - mode->vsync_end;
 
 	/* horizontal values are in terms of high speed byte clock */
-	hactive = txbyteclkhs(hactive, bpp, lane_count);
-	hfp = txbyteclkhs(hfp, bpp, lane_count);
-	hsync = txbyteclkhs(hsync, bpp, lane_count);
-	hbp = txbyteclkhs(hbp, bpp, lane_count);
+	hactive = txbyteclkhs(hactive, bpp, lane_count,
+						intel_dsi->burst_mode_ratio);
+	hfp = txbyteclkhs(hfp, bpp, lane_count, intel_dsi->burst_mode_ratio);
+	hsync = txbyteclkhs(hsync, bpp, lane_count,
+						intel_dsi->burst_mode_ratio);
+	hbp = txbyteclkhs(hbp, bpp, lane_count, intel_dsi->burst_mode_ratio);
 
 	I915_WRITE(MIPI_HACTIVE_AREA_COUNT(pipe), hactive);
 	I915_WRITE(MIPI_HFP_COUNT(pipe), hfp);
@@ -567,12 +571,14 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 	    intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
 		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
 			   txbyteclkhs(adjusted_mode->htotal, bpp,
-				       intel_dsi->lane_count) + 1);
+				       intel_dsi->lane_count,
+				       intel_dsi->burst_mode_ratio) + 1);
 	} else {
 		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
 			   txbyteclkhs(adjusted_mode->vtotal *
 				       adjusted_mode->htotal,
-				       bpp, intel_dsi->lane_count) + 1);
+				       bpp, intel_dsi->lane_count,
+				       intel_dsi->burst_mode_ratio) + 1);
 	}
 	I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout);
 	I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), intel_dsi->turn_arnd_val);
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index e0c16b0..a34ffa4 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -116,6 +116,8 @@ struct intel_dsi {
 	u16 clk_hs_to_lp_count;
 
 	u16 init_count;
+	u32 pclk;
+	u16 burst_mode_ratio;
 
 	/* all delays in ms */
 	u16 backlight_off_delay;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 47c7584..1f5abb4 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -271,6 +271,8 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	u32 ths_prepare_ns, tclk_trail_ns;
 	u32 tclk_prepare_clkzero, ths_prepare_hszero;
 	u32 lp_to_hs_switch, hs_to_lp_switch;
+	u32 pclk, computed_ddr;
+	u16 burst_mode_ratio;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -284,8 +286,6 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
 		bits_per_pixel = 16;
 
-	bitrate = (mode->clock * bits_per_pixel) / intel_dsi->lane_count;
-
 	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
 	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
 	intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
@@ -297,6 +297,40 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	intel_dsi->video_frmt_cfg_bits =
 		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
 
+	pclk = mode->clock;
+
+	/* Burst Mode Ratio
+	 * Target ddr frequency from VBT / non burst ddr freq
+	 * multiply by 100 to preserve remainder
+	 */
+	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
+		if (mipi_config->target_burst_mode_freq) {
+			computed_ddr =
+				(pclk * bits_per_pixel) / intel_dsi->lane_count;
+
+			if (mipi_config->target_burst_mode_freq <
+								computed_ddr) {
+				DRM_ERROR("DDR clock is less than computed\n");
+				return false;
+			}
+
+			burst_mode_ratio = DIV_ROUND_UP(
+				mipi_config->target_burst_mode_freq * 100,
+				computed_ddr);
+
+			pclk = DIV_ROUND_UP(pclk * burst_mode_ratio, 100);
+		} else {
+			DRM_ERROR("Burst mode target is not set\n");
+			return false;
+		}
+	} else
+		burst_mode_ratio = 100;
+
+	intel_dsi->burst_mode_ratio = burst_mode_ratio;
+	intel_dsi->pclk = pclk;
+
+	bitrate = (pclk * bits_per_pixel) / intel_dsi->lane_count;
+
 	switch (intel_dsi->escape_clk_div) {
 	case 0:
 		tlpx_ns = 50;
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 78449ea..20ed460 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -136,8 +136,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 #else
 
 /* Get DSI clock from pixel clock */
-static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
-			  int pixel_format, int lane_count)
+static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 {
 	u32 dsi_clk_khz;
 	u32 bpp;
@@ -158,7 +157,7 @@ static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
 
 	/* DSI data rate = pixel clock * bits per pixel / lane count
 	   pixel clock is converted from KHz to Hz */
-	dsi_clk_khz = DIV_ROUND_CLOSEST(mode->clock * bpp, lane_count);
+	dsi_clk_khz = DIV_ROUND_CLOSEST(pclk * bpp, lane_count);
 
 	return dsi_clk_khz;
 }
@@ -230,14 +229,12 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
 static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
-	const struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	int ret;
 	struct dsi_mnp dsi_mnp;
 	u32 dsi_clk;
 
-	dsi_clk = dsi_clk_from_pclk(mode, intel_dsi->pixel_format,
+	dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
 						intel_dsi->lane_count);
 
 	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
-- 
1.9.1

Re: [PATCH 3/3] drm/i915: Add support for Video Burst Mode for MIPI DSI

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 7818 bytes --]

On Sat, 2014-07-12 at 17:17 +0530, Shobhit Kumar wrote:
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.h          |  3 ++-
>  drivers/gpu/drm/i915/intel_dsi.c           | 22 ++++++++++-------
>  drivers/gpu/drm/i915/intel_dsi.h           |  2 ++
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 38 ++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_dsi_pll.c       |  9 +++----
>  5 files changed, 57 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index b986677..905999b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -802,7 +802,8 @@ struct mipi_config {
>  
>  	u16 rsvd4;
>  
> -	u8 rsvd5[5];
> +	u8 rsvd5;
> +	u32 target_burst_mode_freq;
>  	u32 dsi_ddr_clk;
>  	u32 bridge_ref_clk;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 98c78ab..732d96b 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -449,9 +449,11 @@ static u16 txclkesc(u32 divider, unsigned int us)
>  }
>  
>  /* return pixels in terms of txbyteclkhs */
> -static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count)
> +static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count,
> +		       u16 burst_mode_ratio)
>  {
> -	return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp, 8), lane_count);
> +	return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio,
> +							8 * 100), lane_count);
>  }
>  
>  static void set_dsi_timings(struct drm_encoder *encoder,
> @@ -477,10 +479,12 @@ static void set_dsi_timings(struct drm_encoder *encoder,
>  	vbp = mode->vtotal - mode->vsync_end;
>  
>  	/* horizontal values are in terms of high speed byte clock */
> -	hactive = txbyteclkhs(hactive, bpp, lane_count);
> -	hfp = txbyteclkhs(hfp, bpp, lane_count);
> -	hsync = txbyteclkhs(hsync, bpp, lane_count);
> -	hbp = txbyteclkhs(hbp, bpp, lane_count);
> +	hactive = txbyteclkhs(hactive, bpp, lane_count,
> +						intel_dsi->burst_mode_ratio);
> +	hfp = txbyteclkhs(hfp, bpp, lane_count, intel_dsi->burst_mode_ratio);
> +	hsync = txbyteclkhs(hsync, bpp, lane_count,
> +						intel_dsi->burst_mode_ratio);
> +	hbp = txbyteclkhs(hbp, bpp, lane_count, intel_dsi->burst_mode_ratio);
>  
>  	I915_WRITE(MIPI_HACTIVE_AREA_COUNT(pipe), hactive);
>  	I915_WRITE(MIPI_HFP_COUNT(pipe), hfp);
> @@ -567,12 +571,14 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>  	    intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
>  		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
>  			   txbyteclkhs(adjusted_mode->htotal, bpp,
> -				       intel_dsi->lane_count) + 1);
> +				       intel_dsi->lane_count,
> +				       intel_dsi->burst_mode_ratio) + 1);
>  	} else {
>  		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
>  			   txbyteclkhs(adjusted_mode->vtotal *
>  				       adjusted_mode->htotal,
> -				       bpp, intel_dsi->lane_count) + 1);
> +				       bpp, intel_dsi->lane_count,
> +				       intel_dsi->burst_mode_ratio) + 1);
>  	}
>  	I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout);
>  	I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), intel_dsi->turn_arnd_val);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index e0c16b0..a34ffa4 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -116,6 +116,8 @@ struct intel_dsi {
>  	u16 clk_hs_to_lp_count;
>  
>  	u16 init_count;
> +	u32 pclk;
> +	u16 burst_mode_ratio;
>  
>  	/* all delays in ms */
>  	u16 backlight_off_delay;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 47c7584..1f5abb4 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -271,6 +271,8 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	u32 ths_prepare_ns, tclk_trail_ns;
>  	u32 tclk_prepare_clkzero, ths_prepare_hszero;
>  	u32 lp_to_hs_switch, hs_to_lp_switch;
> +	u32 pclk, computed_ddr;
> +	u16 burst_mode_ratio;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> @@ -284,8 +286,6 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
>  		bits_per_pixel = 16;
>  
> -	bitrate = (mode->clock * bits_per_pixel) / intel_dsi->lane_count;
> -
>  	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
>  	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
>  	intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
> @@ -297,6 +297,40 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	intel_dsi->video_frmt_cfg_bits =
>  		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
>  
> +	pclk = mode->clock;
> +
> +	/* Burst Mode Ratio
> +	 * Target ddr frequency from VBT / non burst ddr freq
> +	 * multiply by 100 to preserve remainder
> +	 */
> +	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
> +		if (mipi_config->target_burst_mode_freq) {
> +			computed_ddr =
> +				(pclk * bits_per_pixel) / intel_dsi->lane_count;
> +
> +			if (mipi_config->target_burst_mode_freq <
> +								computed_ddr) {
> +				DRM_ERROR("DDR clock is less than computed\n");

Bikeshed: "Burst mode freq is less than computed" makes more sense to
me. In any case the patch looks ok to me:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> +				return false;
> +			}
> +
> +			burst_mode_ratio = DIV_ROUND_UP(
> +				mipi_config->target_burst_mode_freq * 100,
> +				computed_ddr);
> +
> +			pclk = DIV_ROUND_UP(pclk * burst_mode_ratio, 100);
> +		} else {
> +			DRM_ERROR("Burst mode target is not set\n");
> +			return false;
> +		}
> +	} else
> +		burst_mode_ratio = 100;
> +
> +	intel_dsi->burst_mode_ratio = burst_mode_ratio;
> +	intel_dsi->pclk = pclk;
> +
> +	bitrate = (pclk * bits_per_pixel) / intel_dsi->lane_count;
> +
>  	switch (intel_dsi->escape_clk_div) {
>  	case 0:
>  		tlpx_ns = 50;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 78449ea..20ed460 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -136,8 +136,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>  #else
>  
>  /* Get DSI clock from pixel clock */
> -static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
> -			  int pixel_format, int lane_count)
> +static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
>  {
>  	u32 dsi_clk_khz;
>  	u32 bpp;
> @@ -158,7 +157,7 @@ static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
>  
>  	/* DSI data rate = pixel clock * bits per pixel / lane count
>  	   pixel clock is converted from KHz to Hz */
> -	dsi_clk_khz = DIV_ROUND_CLOSEST(mode->clock * bpp, lane_count);
> +	dsi_clk_khz = DIV_ROUND_CLOSEST(pclk * bpp, lane_count);
>  
>  	return dsi_clk_khz;
>  }
> @@ -230,14 +229,12 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
>  static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> -	const struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	int ret;
>  	struct dsi_mnp dsi_mnp;
>  	u32 dsi_clk;
>  
> -	dsi_clk = dsi_clk_from_pclk(mode, intel_dsi->pixel_format,
> +	dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
>  						intel_dsi->lane_count);
>  
>  	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[v2] drm/i915: Add support for Video Burst Mode for MIPI DSI

[Shobhit Kumar]

v2: Updated the error log as suggested by Imre

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.h          |  3 ++-
 drivers/gpu/drm/i915/intel_dsi.c           | 22 ++++++++++-------
 drivers/gpu/drm/i915/intel_dsi.h           |  2 ++
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 38 ++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_dsi_pll.c       |  9 +++----
 5 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index b986677..905999b 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -802,7 +802,8 @@ struct mipi_config {
 
 	u16 rsvd4;
 
-	u8 rsvd5[5];
+	u8 rsvd5;
+	u32 target_burst_mode_freq;
 	u32 dsi_ddr_clk;
 	u32 bridge_ref_clk;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 670c29a..aea8f33 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -423,9 +423,11 @@ static u16 txclkesc(u32 divider, unsigned int us)
 }
 
 /* return pixels in terms of txbyteclkhs */
-static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count)
+static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count,
+		       u16 burst_mode_ratio)
 {
-	return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp, 8), lane_count);
+	return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio,
+							8 * 100), lane_count);
 }
 
 static void set_dsi_timings(struct drm_encoder *encoder,
@@ -451,10 +453,12 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 	vbp = mode->vtotal - mode->vsync_end;
 
 	/* horizontal values are in terms of high speed byte clock */
-	hactive = txbyteclkhs(hactive, bpp, lane_count);
-	hfp = txbyteclkhs(hfp, bpp, lane_count);
-	hsync = txbyteclkhs(hsync, bpp, lane_count);
-	hbp = txbyteclkhs(hbp, bpp, lane_count);
+	hactive = txbyteclkhs(hactive, bpp, lane_count,
+						intel_dsi->burst_mode_ratio);
+	hfp = txbyteclkhs(hfp, bpp, lane_count, intel_dsi->burst_mode_ratio);
+	hsync = txbyteclkhs(hsync, bpp, lane_count,
+						intel_dsi->burst_mode_ratio);
+	hbp = txbyteclkhs(hbp, bpp, lane_count, intel_dsi->burst_mode_ratio);
 
 	I915_WRITE(MIPI_HACTIVE_AREA_COUNT(pipe), hactive);
 	I915_WRITE(MIPI_HFP_COUNT(pipe), hfp);
@@ -541,12 +545,14 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 	    intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
 		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
 			   txbyteclkhs(adjusted_mode->htotal, bpp,
-				       intel_dsi->lane_count) + 1);
+				       intel_dsi->lane_count,
+				       intel_dsi->burst_mode_ratio) + 1);
 	} else {
 		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
 			   txbyteclkhs(adjusted_mode->vtotal *
 				       adjusted_mode->htotal,
-				       bpp, intel_dsi->lane_count) + 1);
+				       bpp, intel_dsi->lane_count,
+				       intel_dsi->burst_mode_ratio) + 1);
 	}
 	I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout);
 	I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), intel_dsi->turn_arnd_val);
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index fd51867..657eb5c 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -116,6 +116,8 @@ struct intel_dsi {
 	u16 clk_hs_to_lp_count;
 
 	u16 init_count;
+	u32 pclk;
+	u16 burst_mode_ratio;
 
 	/* all delays in ms */
 	u16 backlight_off_delay;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 47c7584..f6bdd44 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -271,6 +271,8 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	u32 ths_prepare_ns, tclk_trail_ns;
 	u32 tclk_prepare_clkzero, ths_prepare_hszero;
 	u32 lp_to_hs_switch, hs_to_lp_switch;
+	u32 pclk, computed_ddr;
+	u16 burst_mode_ratio;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -284,8 +286,6 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
 		bits_per_pixel = 16;
 
-	bitrate = (mode->clock * bits_per_pixel) / intel_dsi->lane_count;
-
 	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
 	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
 	intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
@@ -297,6 +297,40 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	intel_dsi->video_frmt_cfg_bits =
 		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
 
+	pclk = mode->clock;
+
+	/* Burst Mode Ratio
+	 * Target ddr frequency from VBT / non burst ddr freq
+	 * multiply by 100 to preserve remainder
+	 */
+	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
+		if (mipi_config->target_burst_mode_freq) {
+			computed_ddr =
+				(pclk * bits_per_pixel) / intel_dsi->lane_count;
+
+			if (mipi_config->target_burst_mode_freq <
+								computed_ddr) {
+				DRM_ERROR("Burst mode freq is less than computed\n");
+				return false;
+			}
+
+			burst_mode_ratio = DIV_ROUND_UP(
+				mipi_config->target_burst_mode_freq * 100,
+				computed_ddr);
+
+			pclk = DIV_ROUND_UP(pclk * burst_mode_ratio, 100);
+		} else {
+			DRM_ERROR("Burst mode target is not set\n");
+			return false;
+		}
+	} else
+		burst_mode_ratio = 100;
+
+	intel_dsi->burst_mode_ratio = burst_mode_ratio;
+	intel_dsi->pclk = pclk;
+
+	bitrate = (pclk * bits_per_pixel) / intel_dsi->lane_count;
+
 	switch (intel_dsi->escape_clk_div) {
 	case 0:
 		tlpx_ns = 50;
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index d8bb1ea..06fad93 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -134,8 +134,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 #else
 
 /* Get DSI clock from pixel clock */
-static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
-			  int pixel_format, int lane_count)
+static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 {
 	u32 dsi_clk_khz;
 	u32 bpp;
@@ -156,7 +155,7 @@ static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
 
 	/* DSI data rate = pixel clock * bits per pixel / lane count
 	   pixel clock is converted from KHz to Hz */
-	dsi_clk_khz = DIV_ROUND_CLOSEST(mode->clock * bpp, lane_count);
+	dsi_clk_khz = DIV_ROUND_CLOSEST(pclk * bpp, lane_count);
 
 	return dsi_clk_khz;
 }
@@ -228,14 +227,12 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
 static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
-	const struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	int ret;
 	struct dsi_mnp dsi_mnp;
 	u32 dsi_clk;
 
-	dsi_clk = dsi_clk_from_pclk(mode, intel_dsi->pixel_format,
+	dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
 						intel_dsi->lane_count);
 
 	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
-- 
1.9.1

Re: [v2] drm/i915: Add support for Video Burst Mode for MIPI DSI

[Daniel Vetter]

On Wed, Jul 30, 2014 at 08:34:57PM +0530, Shobhit Kumar wrote:
> v2: Updated the error log as suggested by Imre
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Remaining patches merged, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_bios.h          |  3 ++-
>  drivers/gpu/drm/i915/intel_dsi.c           | 22 ++++++++++-------
>  drivers/gpu/drm/i915/intel_dsi.h           |  2 ++
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 38 ++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_dsi_pll.c       |  9 +++----
>  5 files changed, 57 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index b986677..905999b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -802,7 +802,8 @@ struct mipi_config {
>  
>  	u16 rsvd4;
>  
> -	u8 rsvd5[5];
> +	u8 rsvd5;
> +	u32 target_burst_mode_freq;
>  	u32 dsi_ddr_clk;
>  	u32 bridge_ref_clk;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 670c29a..aea8f33 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -423,9 +423,11 @@ static u16 txclkesc(u32 divider, unsigned int us)
>  }
>  
>  /* return pixels in terms of txbyteclkhs */
> -static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count)
> +static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count,
> +		       u16 burst_mode_ratio)
>  {
> -	return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp, 8), lane_count);
> +	return DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio,
> +							8 * 100), lane_count);
>  }
>  
>  static void set_dsi_timings(struct drm_encoder *encoder,
> @@ -451,10 +453,12 @@ static void set_dsi_timings(struct drm_encoder *encoder,
>  	vbp = mode->vtotal - mode->vsync_end;
>  
>  	/* horizontal values are in terms of high speed byte clock */
> -	hactive = txbyteclkhs(hactive, bpp, lane_count);
> -	hfp = txbyteclkhs(hfp, bpp, lane_count);
> -	hsync = txbyteclkhs(hsync, bpp, lane_count);
> -	hbp = txbyteclkhs(hbp, bpp, lane_count);
> +	hactive = txbyteclkhs(hactive, bpp, lane_count,
> +						intel_dsi->burst_mode_ratio);
> +	hfp = txbyteclkhs(hfp, bpp, lane_count, intel_dsi->burst_mode_ratio);
> +	hsync = txbyteclkhs(hsync, bpp, lane_count,
> +						intel_dsi->burst_mode_ratio);
> +	hbp = txbyteclkhs(hbp, bpp, lane_count, intel_dsi->burst_mode_ratio);
>  
>  	I915_WRITE(MIPI_HACTIVE_AREA_COUNT(pipe), hactive);
>  	I915_WRITE(MIPI_HFP_COUNT(pipe), hfp);
> @@ -541,12 +545,14 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>  	    intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
>  		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
>  			   txbyteclkhs(adjusted_mode->htotal, bpp,
> -				       intel_dsi->lane_count) + 1);
> +				       intel_dsi->lane_count,
> +				       intel_dsi->burst_mode_ratio) + 1);
>  	} else {
>  		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
>  			   txbyteclkhs(adjusted_mode->vtotal *
>  				       adjusted_mode->htotal,
> -				       bpp, intel_dsi->lane_count) + 1);
> +				       bpp, intel_dsi->lane_count,
> +				       intel_dsi->burst_mode_ratio) + 1);
>  	}
>  	I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout);
>  	I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), intel_dsi->turn_arnd_val);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index fd51867..657eb5c 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -116,6 +116,8 @@ struct intel_dsi {
>  	u16 clk_hs_to_lp_count;
>  
>  	u16 init_count;
> +	u32 pclk;
> +	u16 burst_mode_ratio;
>  
>  	/* all delays in ms */
>  	u16 backlight_off_delay;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 47c7584..f6bdd44 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -271,6 +271,8 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	u32 ths_prepare_ns, tclk_trail_ns;
>  	u32 tclk_prepare_clkzero, ths_prepare_hszero;
>  	u32 lp_to_hs_switch, hs_to_lp_switch;
> +	u32 pclk, computed_ddr;
> +	u16 burst_mode_ratio;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> @@ -284,8 +286,6 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
>  		bits_per_pixel = 16;
>  
> -	bitrate = (mode->clock * bits_per_pixel) / intel_dsi->lane_count;
> -
>  	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
>  	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
>  	intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
> @@ -297,6 +297,40 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	intel_dsi->video_frmt_cfg_bits =
>  		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
>  
> +	pclk = mode->clock;
> +
> +	/* Burst Mode Ratio
> +	 * Target ddr frequency from VBT / non burst ddr freq
> +	 * multiply by 100 to preserve remainder
> +	 */
> +	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
> +		if (mipi_config->target_burst_mode_freq) {
> +			computed_ddr =
> +				(pclk * bits_per_pixel) / intel_dsi->lane_count;
> +
> +			if (mipi_config->target_burst_mode_freq <
> +								computed_ddr) {
> +				DRM_ERROR("Burst mode freq is less than computed\n");
> +				return false;
> +			}
> +
> +			burst_mode_ratio = DIV_ROUND_UP(
> +				mipi_config->target_burst_mode_freq * 100,
> +				computed_ddr);
> +
> +			pclk = DIV_ROUND_UP(pclk * burst_mode_ratio, 100);
> +		} else {
> +			DRM_ERROR("Burst mode target is not set\n");
> +			return false;
> +		}
> +	} else
> +		burst_mode_ratio = 100;
> +
> +	intel_dsi->burst_mode_ratio = burst_mode_ratio;
> +	intel_dsi->pclk = pclk;
> +
> +	bitrate = (pclk * bits_per_pixel) / intel_dsi->lane_count;
> +
>  	switch (intel_dsi->escape_clk_div) {
>  	case 0:
>  		tlpx_ns = 50;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index d8bb1ea..06fad93 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -134,8 +134,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>  #else
>  
>  /* Get DSI clock from pixel clock */
> -static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
> -			  int pixel_format, int lane_count)
> +static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
>  {
>  	u32 dsi_clk_khz;
>  	u32 bpp;
> @@ -156,7 +155,7 @@ static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
>  
>  	/* DSI data rate = pixel clock * bits per pixel / lane count
>  	   pixel clock is converted from KHz to Hz */
> -	dsi_clk_khz = DIV_ROUND_CLOSEST(mode->clock * bpp, lane_count);
> +	dsi_clk_khz = DIV_ROUND_CLOSEST(pclk * bpp, lane_count);
>  
>  	return dsi_clk_khz;
>  }
> @@ -228,14 +227,12 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
>  static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> -	const struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	int ret;
>  	struct dsi_mnp dsi_mnp;
>  	u32 dsi_clk;
>  
> -	dsi_clk = dsi_clk_from_pclk(mode, intel_dsi->pixel_format,
> +	dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
>  						intel_dsi->lane_count);
>  
>  	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch