Re: [Intel-gfx] [PATCH v7 02/23] drm/i915/dsi: refactor bitrate calculations in intel_dsi_vbt_init()

2018-10-16 Thread Madhav Chauhan

On 10/16/2018 6:14 PM, Jani Nikula wrote:

On Tue, 16 Oct 2018, Madhav Chauhan  wrote:

On 10/15/2018 7:57 PM, Jani Nikula wrote:

Abstract bitrate calculation to a newly resurrected intel_dsi.c file
that will contain common code for VLV and ICL DSI.

No functional changes.

Cc: Madhav Chauhan 
Cc: Ville Syrjala 
Signed-off-by: Jani Nikula 
---
   drivers/gpu/drm/i915/Makefile|  1 +
   drivers/gpu/drm/i915/intel_dsi.c | 17 +
   drivers/gpu/drm/i915/intel_dsi.h |  3 +++
   drivers/gpu/drm/i915/intel_dsi_vbt.c | 28 ++--
   4 files changed, 31 insertions(+), 18 deletions(-)
   create mode 100644 drivers/gpu/drm/i915/intel_dsi.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 48cae0eae3f9..22cbf9c3bb0c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -143,6 +143,7 @@ i915-y += dvo_ch7017.o \
  intel_dp_link_training.o \
  intel_dp_mst.o \
  intel_dp.o \
+ intel_dsi.o \
  intel_dsi_dcs_backlight.o \
  intel_dsi_vbt.o \
  intel_dvo.o \
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
new file mode 100644
index ..4daa1da94047
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include 
+#include "intel_dsi.h"
+
+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi)
+{
+   int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
+
+   if (WARN_ON(bpp < 0))
+   bpp = 16;

Shouldn't we keep the default bpp to 24 here as in most of the cases bpp
is 24 for DSI or why 16??

*shrug*

Not sure it matters all that much really. We set the pixel format in
intel_dsi_vbt.c, and if that gives us something bogus, not much of a
chance any of it will work. More than anything I just wanted to avoid a
negative return from this function.


Agree,
Reviewed-by: Madhav Chauhan 

Regards,
Madhav



BR,
Jani.


Regards,
Madhav


+
+   return intel_dsi->pclk * bpp / intel_dsi->lane_count;
+}
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index ad7c1cb32983..68f14d8f1e18 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -129,6 +129,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct 
drm_encoder *encoder)
return container_of(encoder, struct intel_dsi, base.base);
   }
   
+/* intel_dsi.c */

+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
+
   /* vlv_dsi.c */
   void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port 
port);
   enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c 
b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index ac83d6b89ae0..6c4cc92f5947 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -506,14 +506,12 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 
panel_id)
struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
-   u32 bpp;
-   u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
+   u32 tlpx_ns, extra_byte_count, tlpx_ui;
u32 ui_num, ui_den;
u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
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;
u32 mul;
u16 burst_mode_ratio;
enum port port;
@@ -526,7 +524,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 
panel_id)
intel_dsi->pixel_format =
pixel_format_from_register_bits(
mipi_config->videomode_color_format << 7);
-   bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
   
   	intel_dsi->dual_link = mipi_config->dual_link;

intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
@@ -541,19 +538,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 
panel_id)
intel_dsi->video_frmt_cfg_bits =
mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
   
-	pclk = mode->clock;

+   /* Starting point, adjusted depending on dual link and burst mode */
+   intel_dsi->pclk = mode->clock;
   
   	/* In dual link mode each port needs half of pixel clock */

if (intel_dsi->dual_link) {
-   pclk = pclk / 2;
+   intel_dsi->pclk /= 2;
   
   		/* we can enable pixel_overlap if needed by panel. In this

 * case we need to increase the pixelclock for extra pixels
 */
if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-   pclk += DIV_ROUND_UP(mode->vtota

Re: [Intel-gfx] [PATCH v7 02/23] drm/i915/dsi: refactor bitrate calculations in intel_dsi_vbt_init()

2018-10-16 Thread Jani Nikula
On Tue, 16 Oct 2018, Madhav Chauhan  wrote:
> On 10/15/2018 7:57 PM, Jani Nikula wrote:
>> Abstract bitrate calculation to a newly resurrected intel_dsi.c file
>> that will contain common code for VLV and ICL DSI.
>>
>> No functional changes.
>>
>> Cc: Madhav Chauhan 
>> Cc: Ville Syrjala 
>> Signed-off-by: Jani Nikula 
>> ---
>>   drivers/gpu/drm/i915/Makefile|  1 +
>>   drivers/gpu/drm/i915/intel_dsi.c | 17 +
>>   drivers/gpu/drm/i915/intel_dsi.h |  3 +++
>>   drivers/gpu/drm/i915/intel_dsi_vbt.c | 28 ++--
>>   4 files changed, 31 insertions(+), 18 deletions(-)
>>   create mode 100644 drivers/gpu/drm/i915/intel_dsi.c
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 48cae0eae3f9..22cbf9c3bb0c 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -143,6 +143,7 @@ i915-y += dvo_ch7017.o \
>>intel_dp_link_training.o \
>>intel_dp_mst.o \
>>intel_dp.o \
>> +  intel_dsi.o \
>>intel_dsi_dcs_backlight.o \
>>intel_dsi_vbt.o \
>>intel_dvo.o \
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c 
>> b/drivers/gpu/drm/i915/intel_dsi.c
>> new file mode 100644
>> index ..4daa1da94047
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -0,0 +1,17 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2018 Intel Corporation
>> + */
>> +
>> +#include 
>> +#include "intel_dsi.h"
>> +
>> +int intel_dsi_bitrate(const struct intel_dsi *intel_dsi)
>> +{
>> +int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
>> +
>> +if (WARN_ON(bpp < 0))
>> +bpp = 16;
>
> Shouldn't we keep the default bpp to 24 here as in most of the cases bpp 
> is 24 for DSI or why 16??

*shrug*

Not sure it matters all that much really. We set the pixel format in
intel_dsi_vbt.c, and if that gives us something bogus, not much of a
chance any of it will work. More than anything I just wanted to avoid a
negative return from this function.

BR,
Jani.

>
> Regards,
> Madhav
>
>> +
>> +return intel_dsi->pclk * bpp / intel_dsi->lane_count;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h 
>> b/drivers/gpu/drm/i915/intel_dsi.h
>> index ad7c1cb32983..68f14d8f1e18 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> @@ -129,6 +129,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct 
>> drm_encoder *encoder)
>>  return container_of(encoder, struct intel_dsi, base.base);
>>   }
>>   
>> +/* intel_dsi.c */
>> +int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
>> +
>>   /* vlv_dsi.c */
>>   void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port 
>> port);
>>   enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c 
>> b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> index ac83d6b89ae0..6c4cc92f5947 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> @@ -506,14 +506,12 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, 
>> u16 panel_id)
>>  struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
>>  struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
>>  struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
>> -u32 bpp;
>> -u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
>> +u32 tlpx_ns, extra_byte_count, tlpx_ui;
>>  u32 ui_num, ui_den;
>>  u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
>>  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;
>>  u32 mul;
>>  u16 burst_mode_ratio;
>>  enum port port;
>> @@ -526,7 +524,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 
>> panel_id)
>>  intel_dsi->pixel_format =
>>  pixel_format_from_register_bits(
>>  mipi_config->videomode_color_format << 7);
>> -bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
>>   
>>  intel_dsi->dual_link = mipi_config->dual_link;
>>  intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
>> @@ -541,19 +538,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, 
>> u16 panel_id)
>>  intel_dsi->video_frmt_cfg_bits =
>>  mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
>>   
>> -pclk = mode->clock;
>> +/* Starting point, adjusted depending on dual link and burst mode */
>> +intel_dsi->pclk = mode->clock;
>>   
>>  /* In dual link mode each port needs half of pixel clock */
>>  if (intel_dsi->dual_link) {
>> -pclk = pclk / 2;
>> +intel_dsi->pclk /= 2;
>>   
>>  /* we can enable pixel_overlap if needed by panel. In this
>>   * case we need to increase the pixelclock for extra pixels
>>   *

Re: [Intel-gfx] [PATCH v7 02/23] drm/i915/dsi: refactor bitrate calculations in intel_dsi_vbt_init()

2018-10-16 Thread Madhav Chauhan

On 10/15/2018 7:57 PM, Jani Nikula wrote:

Abstract bitrate calculation to a newly resurrected intel_dsi.c file
that will contain common code for VLV and ICL DSI.

No functional changes.

Cc: Madhav Chauhan 
Cc: Ville Syrjala 
Signed-off-by: Jani Nikula 
---
  drivers/gpu/drm/i915/Makefile|  1 +
  drivers/gpu/drm/i915/intel_dsi.c | 17 +
  drivers/gpu/drm/i915/intel_dsi.h |  3 +++
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 28 ++--
  4 files changed, 31 insertions(+), 18 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/intel_dsi.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 48cae0eae3f9..22cbf9c3bb0c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -143,6 +143,7 @@ i915-y += dvo_ch7017.o \
  intel_dp_link_training.o \
  intel_dp_mst.o \
  intel_dp.o \
+ intel_dsi.o \
  intel_dsi_dcs_backlight.o \
  intel_dsi_vbt.o \
  intel_dvo.o \
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
new file mode 100644
index ..4daa1da94047
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include 
+#include "intel_dsi.h"
+
+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi)
+{
+   int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
+
+   if (WARN_ON(bpp < 0))
+   bpp = 16;


Shouldn't we keep the default bpp to 24 here as in most of the cases bpp 
is 24 for DSI or why 16??


Regards,
Madhav


+
+   return intel_dsi->pclk * bpp / intel_dsi->lane_count;
+}
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index ad7c1cb32983..68f14d8f1e18 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -129,6 +129,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct 
drm_encoder *encoder)
return container_of(encoder, struct intel_dsi, base.base);
  }
  
+/* intel_dsi.c */

+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
+
  /* vlv_dsi.c */
  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
  enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c 
b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index ac83d6b89ae0..6c4cc92f5947 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -506,14 +506,12 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 
panel_id)
struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
-   u32 bpp;
-   u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
+   u32 tlpx_ns, extra_byte_count, tlpx_ui;
u32 ui_num, ui_den;
u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
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;
u32 mul;
u16 burst_mode_ratio;
enum port port;
@@ -526,7 +524,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 
panel_id)
intel_dsi->pixel_format =
pixel_format_from_register_bits(
mipi_config->videomode_color_format << 7);
-   bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
  
  	intel_dsi->dual_link = mipi_config->dual_link;

intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
@@ -541,19 +538,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 
panel_id)
intel_dsi->video_frmt_cfg_bits =
mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
  
-	pclk = mode->clock;

+   /* Starting point, adjusted depending on dual link and burst mode */
+   intel_dsi->pclk = mode->clock;
  
  	/* In dual link mode each port needs half of pixel clock */

if (intel_dsi->dual_link) {
-   pclk = pclk / 2;
+   intel_dsi->pclk /= 2;
  
  		/* we can enable pixel_overlap if needed by panel. In this

 * case we need to increase the pixelclock for extra pixels
 */
if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-   pclk += DIV_ROUND_UP(mode->vtotal *
-   intel_dsi->pixel_overlap *
-   60, 1000);
+   intel_dsi->pclk += DIV_ROUND_UP(mode->vtotal * 
intel_dsi->pixel_overlap * 60, 1000);
}
}
  
@@ -563,19 +559,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)

 */
if (intel_dsi->video_mode_format == VIDE