Re: [Intel-gfx] [PATCH v7 02/23] drm/i915/dsi: refactor bitrate calculations in intel_dsi_vbt_init()
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()
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()
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