Re: [PATCH v9 09/11] drm: bridge: dw-mipi-dsi: split low power cfg register into fields
Hi Adrian,
thanks for the pach: tested on stm32mp1.
Tested-by: Yannick Fertré
On 6/9/20 7:49 PM, Adrian Ratiu wrote:
> According to the Host Registers documentation for IMX, STM and RK
> the LP cfg register should not be written entirely in one go because
> some bits are reserved and should be kept to reset values, for eg.
> BIT(15) which is reserved in all versions.
>
> This also cleans up the code by removing the the ugly defines
> and making field ranges & values written more explicit.
>
> Tested-by: Adrian Pop
> Tested-by: Arnaud Ferraris
> Signed-off-by: Adrian Ratiu
> ---
> New in v6.
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 105 ++
> 1 file changed, 33 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 70df0578cbe7b..1e47d40b5becb 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -120,60 +120,6 @@
> #define DSI_TO_CNT_CFG_V101 0x40
> #define DSI_PCKHDL_CFG_V101 0x18
>
> -#define MAX_RD_PKT_SIZE_LP BIT(24)
> -#define DCS_LW_TX_LP BIT(19)
> -#define DCS_SR_0P_TX_LP BIT(18)
> -#define DCS_SW_1P_TX_LP BIT(17)
> -#define DCS_SW_0P_TX_LP BIT(16)
> -#define GEN_LW_TX_LP BIT(14)
> -#define GEN_SR_2P_TX_LP BIT(13)
> -#define GEN_SR_1P_TX_LP BIT(12)
> -#define GEN_SR_0P_TX_LP BIT(11)
> -#define GEN_SW_2P_TX_LP BIT(10)
> -#define GEN_SW_1P_TX_LP BIT(9)
> -#define GEN_SW_0P_TX_LP BIT(8)
> -#define TEAR_FX_EN BIT(0)
> -
> -#define CMD_MODE_ALL_LP (MAX_RD_PKT_SIZE_LP | \
> - DCS_LW_TX_LP | \
> - DCS_SR_0P_TX_LP | \
> - DCS_SW_1P_TX_LP | \
> - DCS_SW_0P_TX_LP | \
> - GEN_LW_TX_LP | \
> - GEN_SR_2P_TX_LP | \
> - GEN_SR_1P_TX_LP | \
> - GEN_SR_0P_TX_LP | \
> - GEN_SW_2P_TX_LP | \
> - GEN_SW_1P_TX_LP | \
> - GEN_SW_0P_TX_LP)
> -
> -#define EN_TEAR_FX_V101 BIT(14)
> -#define DCS_LW_TX_LP_V101BIT(12)
> -#define GEN_LW_TX_LP_V101BIT(11)
> -#define MAX_RD_PKT_SIZE_LP_V101 BIT(10)
> -#define DCS_SW_2P_TX_LP_V101 BIT(9)
> -#define DCS_SW_1P_TX_LP_V101 BIT(8)
> -#define DCS_SW_0P_TX_LP_V101 BIT(7)
> -#define GEN_SR_2P_TX_LP_V101 BIT(6)
> -#define GEN_SR_1P_TX_LP_V101 BIT(5)
> -#define GEN_SR_0P_TX_LP_V101 BIT(4)
> -#define GEN_SW_2P_TX_LP_V101 BIT(3)
> -#define GEN_SW_1P_TX_LP_V101 BIT(2)
> -#define GEN_SW_0P_TX_LP_V101 BIT(1)
> -
> -#define CMD_MODE_ALL_LP_V101 (DCS_LW_TX_LP_V101 | \
> - GEN_LW_TX_LP_V101 | \
> - MAX_RD_PKT_SIZE_LP_V101 | \
> - DCS_SW_2P_TX_LP_V101 | \
> - DCS_SW_1P_TX_LP_V101 | \
> - DCS_SW_0P_TX_LP_V101 | \
> - GEN_SR_2P_TX_LP_V101 | \
> - GEN_SR_1P_TX_LP_V101 | \
> - GEN_SR_0P_TX_LP_V101 | \
> - GEN_SW_2P_TX_LP_V101 | \
> - GEN_SW_1P_TX_LP_V101 | \
> - GEN_SW_0P_TX_LP_V101)
> -
> #define DSI_GEN_HDR 0x6c
> #define DSI_GEN_PLD_DATA0x70
>
> @@ -257,7 +203,11 @@ struct dw_mipi_dsi {
> struct regmap_field *field_dpi_vsync_active_low;
> struct regmap_field *field_dpi_hsync_active_low;
> struct regmap_field *field_cmd_mode_ack_rqst_en;
> - struct regmap_field *field_cmd_mode_all_lp_en;
> + struct regmap_field *field_cmd_mode_gen_sw_sr_en;
> + struct regmap_field *field_cmd_mode_dcs_sw_sr_en;
> + struct regmap_field *field_cmd_mode_gen_lw_en;
> + struct regmap_field *field_cmd_mode_dcs_lw_en;
> + struct regmap_field *field_cmd_mode_max_rd_pkt_size;
> struct regmap_field *field_cmd_mode_en;
> struct regmap_field *field_cmd_pkt_status;
> struct regmap_field *field_vid_mode_en;
> @@ -315,7 +265,11 @@ struct dw_mipi_dsi_variant {
> struct reg_fieldcfg_dpi_hsync_active_low;
> struct reg_fieldcfg_cmd_mode_en;
>
[PATCH v9 09/11] drm: bridge: dw-mipi-dsi: split low power cfg register into fields
According to the Host Registers documentation for IMX, STM and RK
the LP cfg register should not be written entirely in one go because
some bits are reserved and should be kept to reset values, for eg.
BIT(15) which is reserved in all versions.
This also cleans up the code by removing the the ugly defines
and making field ranges & values written more explicit.
Tested-by: Adrian Pop
Tested-by: Arnaud Ferraris
Signed-off-by: Adrian Ratiu
---
New in v6.
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 105 ++
1 file changed, 33 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 70df0578cbe7b..1e47d40b5becb 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -120,60 +120,6 @@
#define DSI_TO_CNT_CFG_V1010x40
#define DSI_PCKHDL_CFG_V1010x18
-#define MAX_RD_PKT_SIZE_LP BIT(24)
-#define DCS_LW_TX_LP BIT(19)
-#define DCS_SR_0P_TX_LPBIT(18)
-#define DCS_SW_1P_TX_LPBIT(17)
-#define DCS_SW_0P_TX_LPBIT(16)
-#define GEN_LW_TX_LP BIT(14)
-#define GEN_SR_2P_TX_LPBIT(13)
-#define GEN_SR_1P_TX_LPBIT(12)
-#define GEN_SR_0P_TX_LPBIT(11)
-#define GEN_SW_2P_TX_LPBIT(10)
-#define GEN_SW_1P_TX_LPBIT(9)
-#define GEN_SW_0P_TX_LPBIT(8)
-#define TEAR_FX_EN BIT(0)
-
-#define CMD_MODE_ALL_LP(MAX_RD_PKT_SIZE_LP | \
-DCS_LW_TX_LP | \
-DCS_SR_0P_TX_LP | \
-DCS_SW_1P_TX_LP | \
-DCS_SW_0P_TX_LP | \
-GEN_LW_TX_LP | \
-GEN_SR_2P_TX_LP | \
-GEN_SR_1P_TX_LP | \
-GEN_SR_0P_TX_LP | \
-GEN_SW_2P_TX_LP | \
-GEN_SW_1P_TX_LP | \
-GEN_SW_0P_TX_LP)
-
-#define EN_TEAR_FX_V101BIT(14)
-#define DCS_LW_TX_LP_V101 BIT(12)
-#define GEN_LW_TX_LP_V101 BIT(11)
-#define MAX_RD_PKT_SIZE_LP_V101BIT(10)
-#define DCS_SW_2P_TX_LP_V101 BIT(9)
-#define DCS_SW_1P_TX_LP_V101 BIT(8)
-#define DCS_SW_0P_TX_LP_V101 BIT(7)
-#define GEN_SR_2P_TX_LP_V101 BIT(6)
-#define GEN_SR_1P_TX_LP_V101 BIT(5)
-#define GEN_SR_0P_TX_LP_V101 BIT(4)
-#define GEN_SW_2P_TX_LP_V101 BIT(3)
-#define GEN_SW_1P_TX_LP_V101 BIT(2)
-#define GEN_SW_0P_TX_LP_V101 BIT(1)
-
-#define CMD_MODE_ALL_LP_V101 (DCS_LW_TX_LP_V101 | \
-GEN_LW_TX_LP_V101 | \
-MAX_RD_PKT_SIZE_LP_V101 | \
-DCS_SW_2P_TX_LP_V101 | \
-DCS_SW_1P_TX_LP_V101 | \
-DCS_SW_0P_TX_LP_V101 | \
-GEN_SR_2P_TX_LP_V101 | \
-GEN_SR_1P_TX_LP_V101 | \
-GEN_SR_0P_TX_LP_V101 | \
-GEN_SW_2P_TX_LP_V101 | \
-GEN_SW_1P_TX_LP_V101 | \
-GEN_SW_0P_TX_LP_V101)
-
#define DSI_GEN_HDR0x6c
#define DSI_GEN_PLD_DATA 0x70
@@ -257,7 +203,11 @@ struct dw_mipi_dsi {
struct regmap_field *field_dpi_vsync_active_low;
struct regmap_field *field_dpi_hsync_active_low;
struct regmap_field *field_cmd_mode_ack_rqst_en;
- struct regmap_field *field_cmd_mode_all_lp_en;
+ struct regmap_field *field_cmd_mode_gen_sw_sr_en;
+ struct regmap_field *field_cmd_mode_dcs_sw_sr_en;
+ struct regmap_field *field_cmd_mode_gen_lw_en;
+ struct regmap_field *field_cmd_mode_dcs_lw_en;
+ struct regmap_field *field_cmd_mode_max_rd_pkt_size;
struct regmap_field *field_cmd_mode_en;
struct regmap_field *field_cmd_pkt_status;
struct regmap_field *field_vid_mode_en;
@@ -315,7 +265,11 @@ struct dw_mipi_dsi_variant {
struct reg_fieldcfg_dpi_hsync_active_low;
struct reg_fieldcfg_cmd_mode_en;
struct reg_fieldcfg_cmd_mode_ack_rqst_en;
- struct reg_fieldcfg_cmd_mode_all_lp_en;
+ struct reg_fieldcfg_cmd_mode_gen_sw_sr_en;
+ struct
