diff mbox

[v2,3/7] drm/i915: Compute dsi_clk from pixel clock

Message ID 1383990548-30737-4-git-send-email-shobhit.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Shobhit Nov. 9, 2013, 9:49 a.m. UTC 
Pixel clock based calculation is recommended in the MIPI host controller
documentation

v2: Based on review comments from Jani and Ville
    - Use dsi_clk in KHz rather than converting in Hz and back to MHz
    - RR formula is retained though not used but return dsi_clk in KHz now
    - Moved the m-n-p changes into a separate patch
    - Removed the parameter check for intel_dsi->dsi_clock_freq. This will be
      bought back in if needed when appropriate panel drivers are done

Signed-off-by: Vijayakumar Balakrishnan <vijayakumar.balakrishnan@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_pll.c |   46 +++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 6 deletions(-)

Comments

Jani Nikula Nov. 15, 2013, 7:22 a.m. UTC | #1 
On Sat, 09 Nov 2013, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> Pixel clock based calculation is recommended in the MIPI host controller
> documentation
>
> v2: Based on review comments from Jani and Ville
>     - Use dsi_clk in KHz rather than converting in Hz and back to MHz
>     - RR formula is retained though not used but return dsi_clk in KHz now
>     - Moved the m-n-p changes into a separate patch
>     - Removed the parameter check for intel_dsi->dsi_clock_freq. This will be
>       bought back in if needed when appropriate panel drivers are done
>
> Signed-off-by: Vijayakumar Balakrishnan <vijayakumar.balakrishnan@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_pll.c |   46 +++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 44279b2..9f3e6b0 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 */
>  };
>  
> +#ifdef DSI_CLK_FROM_RR
> +
>  static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>  			  int pixel_format, int video_mode_format,
>  			  int lane_count, bool eotp)
> @@ -121,7 +123,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>  
>  	/* the dsi clock is divided by 2 in the hardware to get dsi ddr clock */
>  	dsi_bit_clock_hz = bytes_per_x_frames_x_lanes * 8;
> -	dsi_clk = dsi_bit_clock_hz / (1000 * 1000);
> +	dsi_clk = dsi_bit_clock_hz / 1000;
>  
>  	if (eotp && video_mode_format == VIDEO_MODE_BURST)
>  		dsi_clk *= 2;
> @@ -129,6 +131,38 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>  	return dsi_clk;
>  }
>  
> +#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)
> +{
> +	u32 dsi_clk_khz;
> +	u32 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;
> +	}
> +
> +	/* 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);
> +
> +	return dsi_clk_khz;
> +}
> +
> +#endif
> +
>  #ifdef MNP_FROM_TABLE
>  
>  struct dsi_clock_table {
> @@ -200,13 +234,14 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
>  	u32 calc_p;
>  	u32 m_seed;
>  
> -	if (dsi_clk < 300 || dsi_clk > 1150) {
> +	/* dsi_clk is expected in KHZ */
> +	if (dsi_clk < 300000 || dsi_clk > 1150000) {
>  		DRM_ERROR("DSI CLK Out of Range\n");
>  		return -ECHRNG;
>  	}
>  
>  	ref_clk = 25000;
> -	target_dsi_clk = dsi_clk * 1000;
> +	target_dsi_clk = dsi_clk;

The *other* dsi_calc_mnp() (with MNP_FROM_TABLE defined) remains
unchanged, and bitrots. Either /= 1000 the input there, or better, if
you don't see that the other function will be needed, just remove it.

BR,
Jani.


>  	error = 0xFFFFFFFF;
>  	calc_m = 0;
>  	calc_p = 0;
> @@ -251,9 +286,8 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
>  	struct dsi_mnp dsi_mnp;
>  	u32 dsi_clk;
>  
> -	dsi_clk = dsi_rr_formula(mode, intel_dsi->pixel_format,
> -				 intel_dsi->video_mode_format,
> -				 intel_dsi->lane_count, !intel_dsi->eot_disable);
> +	dsi_clk = dsi_clk_from_pclk(mode, intel_dsi->pixel_format,
> +						intel_dsi->lane_count);
>  
>  	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
>  	if (ret) {
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Nov. 15, 2013, 8:40 a.m. UTC | #2 
On Fri, 15 Nov 2013, Jani Nikula <jani.nikula@intel.com> wrote:
> On Sat, 09 Nov 2013, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>> Pixel clock based calculation is recommended in the MIPI host controller
>> documentation
>>
>> v2: Based on review comments from Jani and Ville
>>     - Use dsi_clk in KHz rather than converting in Hz and back to MHz
>>     - RR formula is retained though not used but return dsi_clk in KHz now
>>     - Moved the m-n-p changes into a separate patch
>>     - Removed the parameter check for intel_dsi->dsi_clock_freq. This will be
>>       bought back in if needed when appropriate panel drivers are done
>>
>> Signed-off-by: Vijayakumar Balakrishnan <vijayakumar.balakrishnan@intel.com>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dsi_pll.c |   46 +++++++++++++++++++++++++++++-----
>>  1 file changed, 40 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> index 44279b2..9f3e6b0 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 */
>>  };
>>  
>> +#ifdef DSI_CLK_FROM_RR
>> +
>>  static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>>  			  int pixel_format, int video_mode_format,
>>  			  int lane_count, bool eotp)
>> @@ -121,7 +123,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>>  
>>  	/* the dsi clock is divided by 2 in the hardware to get dsi ddr clock */
>>  	dsi_bit_clock_hz = bytes_per_x_frames_x_lanes * 8;
>> -	dsi_clk = dsi_bit_clock_hz / (1000 * 1000);
>> +	dsi_clk = dsi_bit_clock_hz / 1000;
>>  
>>  	if (eotp && video_mode_format == VIDEO_MODE_BURST)
>>  		dsi_clk *= 2;
>> @@ -129,6 +131,38 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>>  	return dsi_clk;
>>  }
>>  
>> +#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)
>> +{
>> +	u32 dsi_clk_khz;
>> +	u32 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;
>> +	}
>> +
>> +	/* 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);
>> +
>> +	return dsi_clk_khz;
>> +}
>> +
>> +#endif
>> +
>>  #ifdef MNP_FROM_TABLE
>>  
>>  struct dsi_clock_table {
>> @@ -200,13 +234,14 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
>>  	u32 calc_p;
>>  	u32 m_seed;
>>  
>> -	if (dsi_clk < 300 || dsi_clk > 1150) {
>> +	/* dsi_clk is expected in KHZ */
>> +	if (dsi_clk < 300000 || dsi_clk > 1150000) {
>>  		DRM_ERROR("DSI CLK Out of Range\n");
>>  		return -ECHRNG;
>>  	}
>>  
>>  	ref_clk = 25000;
>> -	target_dsi_clk = dsi_clk * 1000;
>> +	target_dsi_clk = dsi_clk;
>
> The *other* dsi_calc_mnp() (with MNP_FROM_TABLE defined) remains
> unchanged, and bitrots. Either /= 1000 the input there, or better, if
> you don't see that the other function will be needed, just remove it.

Other than that,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>




>
> BR,
> Jani.
>
>
>>  	error = 0xFFFFFFFF;
>>  	calc_m = 0;
>>  	calc_p = 0;
>> @@ -251,9 +286,8 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
>>  	struct dsi_mnp dsi_mnp;
>>  	u32 dsi_clk;
>>  
>> -	dsi_clk = dsi_rr_formula(mode, intel_dsi->pixel_format,
>> -				 intel_dsi->video_mode_format,
>> -				 intel_dsi->lane_count, !intel_dsi->eot_disable);
>> +	dsi_clk = dsi_clk_from_pclk(mode, intel_dsi->pixel_format,
>> +						intel_dsi->lane_count);
>>  
>>  	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
>>  	if (ret) {
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> 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
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 44279b2..9f3e6b0 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 */
 };
 
+#ifdef DSI_CLK_FROM_RR
+
 static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 			  int pixel_format, int video_mode_format,
 			  int lane_count, bool eotp)
@@ -121,7 +123,7 @@  static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 
 	/* the dsi clock is divided by 2 in the hardware to get dsi ddr clock */
 	dsi_bit_clock_hz = bytes_per_x_frames_x_lanes * 8;
-	dsi_clk = dsi_bit_clock_hz / (1000 * 1000);
+	dsi_clk = dsi_bit_clock_hz / 1000;
 
 	if (eotp && video_mode_format == VIDEO_MODE_BURST)
 		dsi_clk *= 2;
@@ -129,6 +131,38 @@  static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 	return dsi_clk;
 }
 
+#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)
+{
+	u32 dsi_clk_khz;
+	u32 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;
+	}
+
+	/* 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);
+
+	return dsi_clk_khz;
+}
+
+#endif
+
 #ifdef MNP_FROM_TABLE
 
 struct dsi_clock_table {
@@ -200,13 +234,14 @@  static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
 	u32 calc_p;
 	u32 m_seed;
 
-	if (dsi_clk < 300 || dsi_clk > 1150) {
+	/* dsi_clk is expected in KHZ */
+	if (dsi_clk < 300000 || dsi_clk > 1150000) {
 		DRM_ERROR("DSI CLK Out of Range\n");
 		return -ECHRNG;
 	}
 
 	ref_clk = 25000;
-	target_dsi_clk = dsi_clk * 1000;
+	target_dsi_clk = dsi_clk;
 	error = 0xFFFFFFFF;
 	calc_m = 0;
 	calc_p = 0;
@@ -251,9 +286,8 @@  static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
 	struct dsi_mnp dsi_mnp;
 	u32 dsi_clk;
 
-	dsi_clk = dsi_rr_formula(mode, intel_dsi->pixel_format,
-				 intel_dsi->video_mode_format,
-				 intel_dsi->lane_count, !intel_dsi->eot_disable);
+	dsi_clk = dsi_clk_from_pclk(mode, intel_dsi->pixel_format,
+						intel_dsi->lane_count);
 
 	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
 	if (ret) {