Re: [PATCH v4 1/2] drm/msm/dp: revise timing engine programming to support widebus feature
On 15/02/2022 07:12, Bjorn Andersson wrote:
On Mon 14 Feb 16:39 CST 2022, Kuogee Hsieh wrote:
Widebus feature will transmit two pixel data per pixel clock to interface.
Timing engine provides driving force for this purpose. This patch base
on HPG (Hardware Programming Guide) to revise timing engine register
setting to accommodate both widebus and non widebus application. Also
horizontal width parameters need to be reduced by half since two pixel
data are clocked out per pixel clock when widebus feature enabled.
Widebus can be enabled individually at DP. However at DSI, widebus have
to be enabled along with DSC enabled to achieve pixel clock rate be
scaled down with same ratio as compression ratio when 10 bits per source
component. Therefore this patch have both widebus and compression covered
together so tat less efforts will be required when DSC enabled later.
Please split compression away. It's has hanging loose ends, which are
not tied anywhere. It can be sent as a part of this patch series, but
will be merged later, together with the DP DSC support.
In general, sending a patch that does several items at once is not the
best idea. Usually it's better to send two separate patches.
Changes in v2:
-- remove compression related code from timing
-- remove op_info from struct msm_drm_private
-- remove unnecessary wide_bus_en variables
-- pass wide_bus_en into timing configuration by struct msm_dp
Changes in v3:
-- split patch into 3 patches
Changes in v4:
-- rework timing engine to not interfere with dsi/hdmi
Thanks a lot, this is much cleaner now!
-- cover both widebus and compression
Even though the change relates to DP, I think it would be appropriate to
change the $subject prefix to "drm/msm/dpu".
When booting sc8180x the bootloader leaves widebus enabled in the eDP
controller, and the two patches takes care of this problem for me. I
also checked the DP still works.
Tested-by: Bjorn Andersson
Thanks,
Bjorn
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 +
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 99 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 6 ++
5 files changed, 115 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0d315b4..0c22839 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -208,6 +208,8 @@ struct dpu_encoder_virt {
u32 idle_timeout;
+ bool wide_bus_en;
+
struct msm_dp *dp;
};
@@ -217,6 +219,14 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+
+bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc)
+{
+ struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ return dpu_enc->wide_bus_en;
+}
+
static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, unsigned
bpc)
{
struct dpu_hw_dither_cfg dither_cfg = { 0 };
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 99a5d73..893d74d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -168,4 +168,6 @@ int dpu_encoder_get_linecount(struct drm_encoder *drm_enc);
*/
int dpu_encoder_get_frame_count(struct drm_encoder *drm_enc);
+bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc);
+
#endif /* __DPU_ENCODER_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 185379b..2af2bb7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -110,6 +110,20 @@ static void drm_mode_to_intf_timing_params(
timing->v_back_porch += timing->v_front_porch;
timing->v_front_porch = 0;
}
+
+ timing->wide_bus_en = dpu_encoder_is_widebus_enabled(phys_enc->parent);
+
+ /*
+* for DP, divide the horizonal parameters by 2 when
+* widebus is enabled
+*/
+ if (phys_enc->hw_intf->cap->type == INTF_DP && timing->wide_bus_en) {
+ timing->width = timing->width >> 1;
+ timing->xres = timing->xres >> 1;
+ timing->h_back_porch = timing->h_back_porch >> 1;
+ timing->h_front_porch = timing->h_front_porch >> 1;
+ timing->hsync_pulse_width = timing->hsync_pulse_width >> 1;
+ }
}
static u32 get_horizontal_total(const struct intf_timing_params *timing)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 116e2b5..3b9273e 100644
---
Re: [PATCH v4 1/2] drm/msm/dp: revise timing engine programming to support widebus feature
On Mon 14 Feb 16:39 CST 2022, Kuogee Hsieh wrote:
> Widebus feature will transmit two pixel data per pixel clock to interface.
> Timing engine provides driving force for this purpose. This patch base
> on HPG (Hardware Programming Guide) to revise timing engine register
> setting to accommodate both widebus and non widebus application. Also
> horizontal width parameters need to be reduced by half since two pixel
> data are clocked out per pixel clock when widebus feature enabled.
>
> Widebus can be enabled individually at DP. However at DSI, widebus have
> to be enabled along with DSC enabled to achieve pixel clock rate be
> scaled down with same ratio as compression ratio when 10 bits per source
> component. Therefore this patch have both widebus and compression covered
> together so tat less efforts will be required when DSC enabled later.
>
> Changes in v2:
> -- remove compression related code from timing
> -- remove op_info from struct msm_drm_private
> -- remove unnecessary wide_bus_en variables
> -- pass wide_bus_en into timing configuration by struct msm_dp
>
> Changes in v3:
> -- split patch into 3 patches
>
> Changes in v4:
> -- rework timing engine to not interfere with dsi/hdmi
> -- cover both widebus and compression
>
Even though the change relates to DP, I think it would be appropriate to
change the $subject prefix to "drm/msm/dpu".
When booting sc8180x the bootloader leaves widebus enabled in the eDP
controller, and the two patches takes care of this problem for me. I
also checked the DP still works.
Tested-by: Bjorn Andersson
Thanks,
Bjorn
> Signed-off-by: Kuogee Hsieh
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 +++
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 +
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 99
> ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 6 ++
> 5 files changed, 115 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0d315b4..0c22839 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -208,6 +208,8 @@ struct dpu_encoder_virt {
>
> u32 idle_timeout;
>
> + bool wide_bus_en;
> +
> struct msm_dp *dp;
> };
>
> @@ -217,6 +219,14 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
> };
>
> +
> +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc)
> +{
> + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> +
> + return dpu_enc->wide_bus_en;
> +}
> +
> static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp,
> unsigned bpc)
> {
> struct dpu_hw_dither_cfg dither_cfg = { 0 };
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index 99a5d73..893d74d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -168,4 +168,6 @@ int dpu_encoder_get_linecount(struct drm_encoder
> *drm_enc);
> */
> int dpu_encoder_get_frame_count(struct drm_encoder *drm_enc);
>
> +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc);
> +
> #endif /* __DPU_ENCODER_H__ */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 185379b..2af2bb7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -110,6 +110,20 @@ static void drm_mode_to_intf_timing_params(
> timing->v_back_porch += timing->v_front_porch;
> timing->v_front_porch = 0;
> }
> +
> + timing->wide_bus_en = dpu_encoder_is_widebus_enabled(phys_enc->parent);
> +
> + /*
> + * for DP, divide the horizonal parameters by 2 when
> + * widebus is enabled
> + */
> + if (phys_enc->hw_intf->cap->type == INTF_DP && timing->wide_bus_en) {
> + timing->width = timing->width >> 1;
> + timing->xres = timing->xres >> 1;
> + timing->h_back_porch = timing->h_back_porch >> 1;
> + timing->h_front_porch = timing->h_front_porch >> 1;
> + timing->hsync_pulse_width = timing->hsync_pulse_width >> 1;
> + }
> }
>
> static u32 get_horizontal_total(const struct intf_timing_params *timing)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 116e2b5..3b9273e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -33,6 +33,7 @@
> #define INTF_TP_COLOR1 0x05C
> #define INTF_CONFIG20x060
> #define INTF_DISPLAY_DATA_HCTL 0x064
> +#define
[PATCH v4 1/2] drm/msm/dp: revise timing engine programming to support widebus feature
Widebus feature will transmit two pixel data per pixel clock to interface.
Timing engine provides driving force for this purpose. This patch base
on HPG (Hardware Programming Guide) to revise timing engine register
setting to accommodate both widebus and non widebus application. Also
horizontal width parameters need to be reduced by half since two pixel
data are clocked out per pixel clock when widebus feature enabled.
Widebus can be enabled individually at DP. However at DSI, widebus have
to be enabled along with DSC enabled to achieve pixel clock rate be
scaled down with same ratio as compression ratio when 10 bits per source
component. Therefore this patch have both widebus and compression covered
together so tat less efforts will be required when DSC enabled later.
Changes in v2:
-- remove compression related code from timing
-- remove op_info from struct msm_drm_private
-- remove unnecessary wide_bus_en variables
-- pass wide_bus_en into timing configuration by struct msm_dp
Changes in v3:
-- split patch into 3 patches
Changes in v4:
-- rework timing engine to not interfere with dsi/hdmi
-- cover both widebus and compression
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 +
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 99 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 6 ++
5 files changed, 115 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0d315b4..0c22839 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -208,6 +208,8 @@ struct dpu_encoder_virt {
u32 idle_timeout;
+ bool wide_bus_en;
+
struct msm_dp *dp;
};
@@ -217,6 +219,14 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+
+bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc)
+{
+ struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ return dpu_enc->wide_bus_en;
+}
+
static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, unsigned
bpc)
{
struct dpu_hw_dither_cfg dither_cfg = { 0 };
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 99a5d73..893d74d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -168,4 +168,6 @@ int dpu_encoder_get_linecount(struct drm_encoder *drm_enc);
*/
int dpu_encoder_get_frame_count(struct drm_encoder *drm_enc);
+bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc);
+
#endif /* __DPU_ENCODER_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 185379b..2af2bb7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -110,6 +110,20 @@ static void drm_mode_to_intf_timing_params(
timing->v_back_porch += timing->v_front_porch;
timing->v_front_porch = 0;
}
+
+ timing->wide_bus_en = dpu_encoder_is_widebus_enabled(phys_enc->parent);
+
+ /*
+* for DP, divide the horizonal parameters by 2 when
+* widebus is enabled
+*/
+ if (phys_enc->hw_intf->cap->type == INTF_DP && timing->wide_bus_en) {
+ timing->width = timing->width >> 1;
+ timing->xres = timing->xres >> 1;
+ timing->h_back_porch = timing->h_back_porch >> 1;
+ timing->h_front_porch = timing->h_front_porch >> 1;
+ timing->hsync_pulse_width = timing->hsync_pulse_width >> 1;
+ }
}
static u32 get_horizontal_total(const struct intf_timing_params *timing)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 116e2b5..3b9273e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -33,6 +33,7 @@
#define INTF_TP_COLOR1 0x05C
#define INTF_CONFIG20x060
#define INTF_DISPLAY_DATA_HCTL 0x064
+#define INTF_ACTIVE_DATA_HCTL 0x068
#define INTF_FRAME_LINE_COUNT_EN0x0A8
#define INTF_FRAME_COUNT0x0AC
#define INTF_LINE_COUNT 0x0B0
@@ -60,6 +61,14 @@
#define INTF_MUX 0x25C
+#define BIT_INTF_CFG_ACTIVE_H_EN BIT(29)
+#define BIT_INTF_CFG_ACTIVE_V_EN BIT(30)
+
+#define BIT_INTF_CFG2_DATABUS_WIDENBIT(0)
+#define BIT_INTF_CFG2_DATA_HCTL_EN BIT(4)
+#define BIT_INTF_CFG2_DCE_DATA_COMPRESSBIT(12)
+
+
static const struct dpu_intf_cfg *_intf_offset(enum dpu_intf intf,
