Re: [PATCH 1/3] drm/i915: Add DPI FIFO empty status check

Re: [PATCH 1/3] drm/i915: Add DPI FIFO empty status check

[Ville Syrjälä]

On Fri, Dec 04, 2015 at 07:47:37PM +0530, Deepak M wrote:
> From: Gaurav K Singh <gaurav.k.singh@intel.com>
> 
> After sending SHUTDOWN or TURN ON packet,check the DPI
> FIFO empty status.
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 170ae6f..5c5b59a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -222,6 +222,12 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
>  	else
>  		cmd |= DPI_LP_MODE;
>  
> +	mask = DPI_FIFO_EMPTY;
> +
> +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & mask)
> +							== mask, 50))
> +		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
> +

This checks for it _before_ sending the command, but the commit message
says "after". Which is it supposed to be?

>  	/* clear bit */
>  	I915_WRITE(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT);
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 1/3] drm/i915: Add DPI FIFO empty status check

[Deepak, M]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Friday, December 4, 2015 3:20 PM
> To: Deepak, M <m.deepak@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Gaurav K Singh
> <gaurav.k.singh@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Add DPI FIFO empty status
> check
> 
> On Fri, Dec 04, 2015 at 07:47:37PM +0530, Deepak M wrote:
> > From: Gaurav K Singh <gaurav.k.singh@intel.com>
> >
> > After sending SHUTDOWN or TURN ON packet,check the DPI FIFO empty
> > status.
> >
> > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index 170ae6f..5c5b59a 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -222,6 +222,12 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi,
> u32 cmd, bool hs,
> >  	else
> >  		cmd |= DPI_LP_MODE;
> >
> > +	mask = DPI_FIFO_EMPTY;
> > +
> > +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & mask)
> > +							== mask, 50))
> > +		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
> > +
> 
> This checks for it _before_ sending the command, but the commit message
> says "after". Which is it supposed to be?
[Deepak, M] The check should be before, mistake in the commit message.
> 
> >  	/* clear bit */
> >  	I915_WRITE(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT);
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Correct the Ref clock value for BXT

[Ville Syrjälä]

On Fri, Dec 04, 2015 at 07:47:38PM +0530, Deepak M wrote:
> The reference clock for BXT is 19.2 MHz not 19.5 MHz, updating the
> correct value here.
> 
> Signed-off-by: Deepak M <m.deepak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8bd2699..009f474 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7676,7 +7676,7 @@ enum skl_disp_power_wells {
>  #define BXT_DSI_PLL_RATIO_MAX		0x7D
>  #define BXT_DSI_PLL_RATIO_MIN		0x22
>  #define BXT_DSI_PLL_RATIO_MASK		0xFF
> -#define BXT_REF_CLOCK_KHZ		19500
> +#define BXT_REF_CLOCK_KHZ		19200

No idea why we have this define in i915_reg.h. We don't have such
defines for other platforms (also my fix CHV refclk to 19.2MHz patch
never got reviewed by anyone either. Interedted?)

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  #define BXT_DSI_PLL_ENABLE		0x46080
>  #define  BXT_DSI_PLL_DO_ENABLE		(1 << 31)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915: Add DPI FIFO empty status check

[Ville Syrjälä]

On Fri, Dec 04, 2015 at 09:41:17PM +0530, Deepak M wrote:
> From: Gaurav K Singh <gaurav.k.singh@intel.com>
> 
> Before sending TURN ON packet,check the DPI
> FIFO empty status.
> 
> v2: Change in commit message
>     Checking for FIFO empty  only during TURN ON packet.
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> ---
> Got to know about the patch where they are removing this
> DPI FIFO empty check while sending the SHUTDOWN packet.
> 
> http://lists.freedesktop.org/archives/intel-gfx/2014-July/048401.html
> 
> According added the check and waiting for DPI FIFIO empty
> only when TURN_ON packet is sent.

Would seem cleaner to add a small function to do the wait, and
just call that before calling dpi_send_cmd(TURN_ON).

> 
>  drivers/gpu/drm/i915/intel_dsi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 170ae6f..495056f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -222,6 +222,12 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
>  	else
>  		cmd |= DPI_LP_MODE;
>  
> +	mask = DPI_FIFO_EMPTY;
> +
> +	if ((cmd & TURN_ON) && wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) &
> +			mask) == mask, 50))
> +		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
> +
>  	/* clear bit */
>  	I915_WRITE(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT);
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 3/3] drm/i915: Use the ceil value for the additional clk divider

[Ville Syrjälä]

On Fri, Dec 04, 2015 at 07:47:39PM +0530, Deepak M wrote:
> Additional clock value divider should use the ceil
> value of the calulation to get the correct divider value.
> 
> Signed-off-by: Deepak M <m.deepak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_pll.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index cb3cf39..1322a71 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -454,7 +454,7 @@ static void bxt_dsi_program_clocks(struct drm_device *dev, enum port port)
>  	dsi_rate = (BXT_REF_CLOCK_KHZ * pll_ratio) / 2;
>  
>  	/* Max possible output of clock is 39.5 MHz, program value -1 */
> -	divider = (dsi_rate / BXT_MAX_VAR_OUTPUT_KHZ) - 1;
> +	divider = DIV_ROUND_UP(dsi_rate, BXT_MAX_VAR_OUTPUT_KHZ) - 1;

I can't find anything to support the 39.5 MHz claim above. I do know the
tx escape clock should be <=20Mhz, so with the /2 extra divider it
seems we should aim for <=40Mhz here. So yes, round up does make sense,
but it seems to me that BXT_MAX_VAR_OUTPUT_KHZ should be 40 MHz.

>  	tmp |= BXT_MIPI_ESCLK_VAR_DIV(port, divider);
>  
>  	/*
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 1/3] drm/i915: Add DPI FIFO empty status check

[Deepak M]

From: Gaurav K Singh <gaurav.k.singh@intel.com>

After sending SHUTDOWN or TURN ON packet,check the DPI
FIFO empty status.

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

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 170ae6f..5c5b59a 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -222,6 +222,12 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
 	else
 		cmd |= DPI_LP_MODE;
 
+	mask = DPI_FIFO_EMPTY;
+
+	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & mask)
+							== mask, 50))
+		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
+
 	/* clear bit */
 	I915_WRITE(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT);
 
-- 
1.9.1

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

[PATCH 2/3] drm/i915: Correct the Ref clock value for BXT

[Deepak M]

The reference clock for BXT is 19.2 MHz not 19.5 MHz, updating the
correct value here.

Signed-off-by: Deepak M <m.deepak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8bd2699..009f474 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7676,7 +7676,7 @@ enum skl_disp_power_wells {
 #define BXT_DSI_PLL_RATIO_MAX		0x7D
 #define BXT_DSI_PLL_RATIO_MIN		0x22
 #define BXT_DSI_PLL_RATIO_MASK		0xFF
-#define BXT_REF_CLOCK_KHZ		19500
+#define BXT_REF_CLOCK_KHZ		19200
 
 #define BXT_DSI_PLL_ENABLE		0x46080
 #define  BXT_DSI_PLL_DO_ENABLE		(1 << 31)
-- 
1.9.1

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

[PATCH 3/3] drm/i915: Use the ceil value for the additional clk divider

[Deepak M]

Additional clock value divider should use the ceil
value of the calulation to get the correct divider value.

Signed-off-by: Deepak M <m.deepak@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index cb3cf39..1322a71 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -454,7 +454,7 @@ static void bxt_dsi_program_clocks(struct drm_device *dev, enum port port)
 	dsi_rate = (BXT_REF_CLOCK_KHZ * pll_ratio) / 2;
 
 	/* Max possible output of clock is 39.5 MHz, program value -1 */
-	divider = (dsi_rate / BXT_MAX_VAR_OUTPUT_KHZ) - 1;
+	divider = DIV_ROUND_UP(dsi_rate, BXT_MAX_VAR_OUTPUT_KHZ) - 1;
 	tmp |= BXT_MIPI_ESCLK_VAR_DIV(port, divider);
 
 	/*
-- 
1.9.1

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

[PATCH] drm/i915: Add DPI FIFO empty status check

[Deepak M]

From: Gaurav K Singh <gaurav.k.singh@intel.com>

Before sending TURN ON packet,check the DPI
FIFO empty status.

v2: Change in commit message
    Checking for FIFO empty  only during TURN ON packet.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Deepak M <m.deepak@intel.com>
---
Got to know about the patch where they are removing this
DPI FIFO empty check while sending the SHUTDOWN packet.

http://lists.freedesktop.org/archives/intel-gfx/2014-July/048401.html

According added the check and waiting for DPI FIFIO empty
only when TURN_ON packet is sent.

 drivers/gpu/drm/i915/intel_dsi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 170ae6f..495056f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -222,6 +222,12 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
 	else
 		cmd |= DPI_LP_MODE;
 
+	mask = DPI_FIFO_EMPTY;
+
+	if ((cmd & TURN_ON) && wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) &
+			mask) == mask, 50))
+		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
+
 	/* clear bit */
 	I915_WRITE(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT);
 
-- 
1.9.1

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

Re: [PATCH 2/3] drm/i915: Correct the Ref clock value for BXT

[Daniel Vetter]

On Fri, Dec 04, 2015 at 11:55:56AM +0200, Ville Syrjälä wrote:
> On Fri, Dec 04, 2015 at 07:47:38PM +0530, Deepak M wrote:
> > The reference clock for BXT is 19.2 MHz not 19.5 MHz, updating the
> > correct value here.
> > 
> > Signed-off-by: Deepak M <m.deepak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 8bd2699..009f474 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7676,7 +7676,7 @@ enum skl_disp_power_wells {
> >  #define BXT_DSI_PLL_RATIO_MAX		0x7D
> >  #define BXT_DSI_PLL_RATIO_MIN		0x22
> >  #define BXT_DSI_PLL_RATIO_MASK		0xFF
> > -#define BXT_REF_CLOCK_KHZ		19500
> > +#define BXT_REF_CLOCK_KHZ		19200
> 
> No idea why we have this define in i915_reg.h. We don't have such
> defines for other platforms (also my fix CHV refclk to 19.2MHz patch
> never got reviewed by anyone either. Interedted?)

Add link and I think Deepak is volunteered ;-)
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
> 
> >  
> >  #define BXT_DSI_PLL_ENABLE		0x46080
> >  #define  BXT_DSI_PLL_DO_ENABLE		(1 << 31)
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 2/3] drm/i915: Correct the Ref clock value for BXT

[Deepak, M]

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Friday, December 4, 2015 9:52 PM
> To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Deepak, M <m.deepak@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Correct the Ref clock value for
> BXT
> 
> On Fri, Dec 04, 2015 at 11:55:56AM +0200, Ville Syrjälä wrote:
> > On Fri, Dec 04, 2015 at 07:47:38PM +0530, Deepak M wrote:
> > > The reference clock for BXT is 19.2 MHz not 19.5 MHz, updating the
> > > correct value here.
> > >
> > > Signed-off-by: Deepak M <m.deepak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index 8bd2699..009f474 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7676,7 +7676,7 @@ enum skl_disp_power_wells {
> > >  #define BXT_DSI_PLL_RATIO_MAX		0x7D
> > >  #define BXT_DSI_PLL_RATIO_MIN		0x22
> > >  #define BXT_DSI_PLL_RATIO_MASK		0xFF
> > > -#define BXT_REF_CLOCK_KHZ		19500
> > > +#define BXT_REF_CLOCK_KHZ		19200
> >
> > No idea why we have this define in i915_reg.h. We don't have such
> > defines for other platforms (also my fix CHV refclk to 19.2MHz patch
> > never got reviewed by anyone either. Interedted?)
> 
> Add link and I think Deepak is volunteered ;-)
> >
[Deepak, M] Please share the link, I can go through the patch :)

> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Queued for -next, thanks for the patch.
> -Daniel
> >
> > >
> > >  #define BXT_DSI_PLL_ENABLE		0x46080
> > >  #define  BXT_DSI_PLL_DO_ENABLE		(1 << 31)
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Use the ceil value for the additional clk divider

[Deepak, M]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Friday, December 4, 2015 5:22 PM
> To: Deepak, M <m.deepak@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Use the ceil value for the
> additional clk divider
> 
> On Fri, Dec 04, 2015 at 07:47:39PM +0530, Deepak M wrote:
> > Additional clock value divider should use the ceil value of the
> > calulation to get the correct divider value.
> >
> > Signed-off-by: Deepak M <m.deepak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi_pll.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c
> > b/drivers/gpu/drm/i915/intel_dsi_pll.c
> > index cb3cf39..1322a71 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> > @@ -454,7 +454,7 @@ static void bxt_dsi_program_clocks(struct
> drm_device *dev, enum port port)
> >  	dsi_rate = (BXT_REF_CLOCK_KHZ * pll_ratio) / 2;
> >
> >  	/* Max possible output of clock is 39.5 MHz, program value -1 */
> > -	divider = (dsi_rate / BXT_MAX_VAR_OUTPUT_KHZ) - 1;
> > +	divider = DIV_ROUND_UP(dsi_rate, BXT_MAX_VAR_OUTPUT_KHZ) -
> 1;
> 
> I can't find anything to support the 39.5 MHz claim above. I do know the tx
> escape clock should be <=20Mhz, so with the /2 extra divider it seems we
> should aim for <=40Mhz here. So yes, round up does make sense, but it
> seems to me that BXT_MAX_VAR_OUTPUT_KHZ should be 40 MHz.
> 
[Deepak, M] Yes, thought about it and me too feel that it should be 40 MHz.  We locally have tried with 2 different MIPI panels with 39.5 Mhz and didn't see any issue. I will confirm with SV teams and will update the patch accordingly. Thanks for pointing it :)

> >  	tmp |= BXT_MIPI_ESCLK_VAR_DIV(port, divider);
> >
> >  	/*
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Correct the Ref clock value for BXT

[Ville Syrjälä]

On Fri, Dec 04, 2015 at 04:25:19PM +0000, Deepak, M wrote:
> 
> 
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Friday, December 4, 2015 9:52 PM
> > To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Deepak, M <m.deepak@intel.com>; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Correct the Ref clock value for
> > BXT
> > 
> > On Fri, Dec 04, 2015 at 11:55:56AM +0200, Ville Syrjälä wrote:
> > > On Fri, Dec 04, 2015 at 07:47:38PM +0530, Deepak M wrote:
> > > > The reference clock for BXT is 19.2 MHz not 19.5 MHz, updating the
> > > > correct value here.
> > > >
> > > > Signed-off-by: Deepak M <m.deepak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_reg.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h index 8bd2699..009f474 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -7676,7 +7676,7 @@ enum skl_disp_power_wells {
> > > >  #define BXT_DSI_PLL_RATIO_MAX		0x7D
> > > >  #define BXT_DSI_PLL_RATIO_MIN		0x22
> > > >  #define BXT_DSI_PLL_RATIO_MASK		0xFF
> > > > -#define BXT_REF_CLOCK_KHZ		19500
> > > > +#define BXT_REF_CLOCK_KHZ		19200
> > >
> > > No idea why we have this define in i915_reg.h. We don't have such
> > > defines for other platforms (also my fix CHV refclk to 19.2MHz patch
> > > never got reviewed by anyone either. Interedted?)
> > 
> > Add link and I think Deepak is volunteered ;-)
> > >
> [Deepak, M] Please share the link, I can go through the patch :)

This is the whole series:
http://lists.freedesktop.org/archives/intel-gfx/2015-September/075097.html
Some of it might have bitrotted a bit, but nothing a rebase wouldn't
fix I think.

There's also this other patch dealing with dual-link (which I've never
tested due to lack of hardware):
http://lists.freedesktop.org/archives/intel-gfx/2015-September/075568.html

> 
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Queued for -next, thanks for the patch.
> > -Daniel
> > >
> > > >
> > > >  #define BXT_DSI_PLL_ENABLE		0x46080
> > > >  #define  BXT_DSI_PLL_DO_ENABLE		(1 << 31)
> > > > --
> > > > 1.9.1
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

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

[PATCH] drm/i915: Add DPI FIFO empty status check

[Deepak M]

From: Gaurav K Singh <gaurav.k.singh@intel.com>

Before sending TURN ON packet,check the DPI
FIFO empty status.

v2: Change in commit message
    Checking for FIFO empty  only during TURN ON packet.
v3: Adding a new function for DPI FIFO empty check

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Deepak M <m.deepak@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 170ae6f..eff982b 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
 		DRM_ERROR("DPI FIFOs are not empty\n");
 }
 
+static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
+{
+	struct drm_encoder *encoder = &intel_dsi->base.base;
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY)
+					== DPI_FIFO_EMPTY, 50))
+		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
+}
+
 static void write_data(struct drm_i915_private *dev_priv, u32 reg,
 		       const u8 *data, u32 len)
 {
@@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 			I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4);
 	} else {
 		msleep(20); /* XXX */
-		for_each_dsi_port(port, intel_dsi->ports)
+		for_each_dsi_port(port, intel_dsi->ports) {
+			wait_for_dpi_fifo_empty(intel_dsi, port);
 			dpi_send_cmd(intel_dsi, TURN_ON, false, port);
+		}
 		msleep(100);
 
 		drm_panel_enable(intel_dsi->panel);
-- 
1.9.1

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

Re: [PATCH] drm/i915: Add DPI FIFO empty status check

[Ville Syrjälä]

On Wed, Dec 09, 2015 at 05:05:52PM +0530, Deepak M wrote:
> From: Gaurav K Singh <gaurav.k.singh@intel.com>
> 
> Before sending TURN ON packet,check the DPI
> FIFO empty status.
> 
> v2: Change in commit message
>     Checking for FIFO empty  only during TURN ON packet.
> v3: Adding a new function for DPI FIFO empty check
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 170ae6f..eff982b 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>  		DRM_ERROR("DPI FIFOs are not empty\n");
>  }
>  
> +static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
> +{
> +	struct drm_encoder *encoder = &intel_dsi->base.base;
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;

Passing in intel_dsi just to dig out the dev_priv again seems a bit
pointless. Could just pass dev_priv directly. But I guess it matches the
already existing functions. 

BTW for future reference, we would rather see to_i915(<whatever>)
instead of <whatever>->dev_private.

Anyway, the patch seems to do what it says. I can't really judge
whether it's the right thing to do though.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY)
> +					== DPI_FIFO_EMPTY, 50))
> +		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
> +}
> +
>  static void write_data(struct drm_i915_private *dev_priv, u32 reg,
>  		       const u8 *data, u32 len)
>  {
> @@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>  			I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4);
>  	} else {
>  		msleep(20); /* XXX */
> -		for_each_dsi_port(port, intel_dsi->ports)
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			wait_for_dpi_fifo_empty(intel_dsi, port);
>  			dpi_send_cmd(intel_dsi, TURN_ON, false, port);
> +		}
>  		msleep(100);
>  
>  		drm_panel_enable(intel_dsi->panel);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915: Add DPI FIFO empty status check

[Daniel Vetter]

On Wed, Dec 09, 2015 at 04:34:54PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 09, 2015 at 05:05:52PM +0530, Deepak M wrote:
> > From: Gaurav K Singh <gaurav.k.singh@intel.com>
> > 
> > Before sending TURN ON packet,check the DPI
> > FIFO empty status.
> > 
> > v2: Change in commit message
> >     Checking for FIFO empty  only during TURN ON packet.
> > v3: Adding a new function for DPI FIFO empty check
> > 
> > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> > Signed-off-by: Deepak M <m.deepak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 170ae6f..eff982b 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
> >  		DRM_ERROR("DPI FIFOs are not empty\n");
> >  }
> >  
> > +static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
> > +{
> > +	struct drm_encoder *encoder = &intel_dsi->base.base;
> > +	struct drm_device *dev = encoder->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> 
> Passing in intel_dsi just to dig out the dev_priv again seems a bit
> pointless. Could just pass dev_priv directly. But I guess it matches the
> already existing functions. 
> 
> BTW for future reference, we would rather see to_i915(<whatever>)
> instead of <whatever>->dev_private.

Yeah I think fixing up the above would be good. We're pretty close to
being able to embed our device structure.
-Daniel

> 
> Anyway, the patch seems to do what it says. I can't really judge
> whether it's the right thing to do though.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +
> > +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY)
> > +					== DPI_FIFO_EMPTY, 50))
> > +		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
> > +}
> > +
> >  static void write_data(struct drm_i915_private *dev_priv, u32 reg,
> >  		       const u8 *data, u32 len)
> >  {
> > @@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
> >  			I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4);
> >  	} else {
> >  		msleep(20); /* XXX */
> > -		for_each_dsi_port(port, intel_dsi->ports)
> > +		for_each_dsi_port(port, intel_dsi->ports) {
> > +			wait_for_dpi_fifo_empty(intel_dsi, port);
> >  			dpi_send_cmd(intel_dsi, TURN_ON, false, port);
> > +		}
> >  		msleep(100);
> >  
> >  		drm_panel_enable(intel_dsi->panel);
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 1/3] drm/i915: Add DPI FIFO empty status check

[Mika Kahola]

On Fri, 2015-12-04 at 19:47 +0530, Deepak M wrote:
> From: Gaurav K Singh <gaurav.k.singh@intel.com>
> 
> After sending SHUTDOWN or TURN ON packet,check the DPI
> FIFO empty status.
> 
Tested-by: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 170ae6f..5c5b59a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -222,6 +222,12 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
>  	else
>  		cmd |= DPI_LP_MODE;
>  
> +	mask = DPI_FIFO_EMPTY;
> +
> +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & mask)
> +							== mask, 50))
> +		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
> +
>  	/* clear bit */
>  	I915_WRITE(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT);
>  


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

Re: [PATCH] drm/i915: Add DPI FIFO empty status check

[Jani Nikula]

On Wed, 09 Dec 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Dec 09, 2015 at 05:05:52PM +0530, Deepak M wrote:
>> From: Gaurav K Singh <gaurav.k.singh@intel.com>
>> 
>> Before sending TURN ON packet,check the DPI
>> FIFO empty status.
>> 
>> v2: Change in commit message
>>     Checking for FIFO empty  only during TURN ON packet.
>> v3: Adding a new function for DPI FIFO empty check
>> 
>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>> Signed-off-by: Deepak M <m.deepak@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dsi.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 170ae6f..eff982b 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>>  		DRM_ERROR("DPI FIFOs are not empty\n");
>>  }
>>  
>> +static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>> +{
>> +	struct drm_encoder *encoder = &intel_dsi->base.base;
>> +	struct drm_device *dev = encoder->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>
> Passing in intel_dsi just to dig out the dev_priv again seems a bit
> pointless. Could just pass dev_priv directly. But I guess it matches the
> already existing functions. 
>
> BTW for future reference, we would rather see to_i915(<whatever>)
> instead of <whatever>->dev_private.
>
> Anyway, the patch seems to do what it says. I can't really judge
> whether it's the right thing to do though.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

OTOH,

commit a799a9780eb5c874d9d7ca0bbee66401ca98c013
Author: Shobhit Kumar <shobhit.kumar@intel.com>
Date:   Thu Jul 3 16:35:40 2014 +0530

    drm/i915/vlv: DPI FIFO empty check is not needed
    
    While sending DPI SHUTDOWN command, we cannot wait for FIFO empty as
    pipes are not disabled at that time. In case of MIPI we disable port
    first and send SHUTDOWN command while pipe is still running and FIFOs
    will not be empty, causing spurious error log
    
    Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
    Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
    Signed-off-by: Jani Nikula <jani.nikula@intel.com>


>
>> +
>> +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY)
>> +					== DPI_FIFO_EMPTY, 50))
>> +		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
>> +}
>> +
>>  static void write_data(struct drm_i915_private *dev_priv, u32 reg,
>>  		       const u8 *data, u32 len)
>>  {
>> @@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>>  			I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4);
>>  	} else {
>>  		msleep(20); /* XXX */
>> -		for_each_dsi_port(port, intel_dsi->ports)
>> +		for_each_dsi_port(port, intel_dsi->ports) {
>> +			wait_for_dpi_fifo_empty(intel_dsi, port);
>>  			dpi_send_cmd(intel_dsi, TURN_ON, false, port);
>> +		}
>>  		msleep(100);
>>  
>>  		drm_panel_enable(intel_dsi->panel);
>> -- 
>> 1.9.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx