Re: [PATCH v4 51/78] drm/vc4: hdmi: Implement a register layout abstraction

2020-07-28 Thread Dave Stevenson
Hi Maxime

On Wed, 8 Jul 2020 at 18:43, Maxime Ripard  wrote:
>
> The HDMI controllers found in the BCM2711 have most of the registers
> reorganized in multiple registers areas and at different offsets than
> previously found.
>
> The logic however remains pretty much the same, so it doesn't really make
> sense to create a whole new driver and we should share the code as much as
> possible.
>
> Let's implement some indirection to wrap around a register and depending on
> the variant will lookup the associated register on that particular variant.
>
> Signed-off-by: Maxime Ripard 

Reviewed-by: Dave Stevenson 

> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c  | 427 ++---
>  drivers/gpu/drm/vc4/vc4_hdmi.h  |  12 +-
>  drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 241 -
>  drivers/gpu/drm/vc4/vc4_regs.h  |  92 +--
>  4 files changed, 464 insertions(+), 308 deletions(-)
>  create mode 100644 drivers/gpu/drm/vc4/vc4_hdmi_regs.h
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index ac021e07a8cb..a4fed1439bf3 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -50,62 +50,13 @@
>  #include "media/cec.h"
>  #include "vc4_drv.h"
>  #include "vc4_hdmi.h"
> +#include "vc4_hdmi_regs.h"
>  #include "vc4_regs.h"
>
>  #define HSM_CLOCK_FREQ 163682864
>  #define CEC_CLOCK_FREQ 4
>  #define CEC_CLOCK_DIV  (HSM_CLOCK_FREQ / CEC_CLOCK_FREQ)
>
> -static const struct debugfs_reg32 hdmi_regs[] = {
> -   VC4_REG32(VC4_HDMI_CORE_REV),
> -   VC4_REG32(VC4_HDMI_SW_RESET_CONTROL),
> -   VC4_REG32(VC4_HDMI_HOTPLUG_INT),
> -   VC4_REG32(VC4_HDMI_HOTPLUG),
> -   VC4_REG32(VC4_HDMI_MAI_CHANNEL_MAP),
> -   VC4_REG32(VC4_HDMI_MAI_CONFIG),
> -   VC4_REG32(VC4_HDMI_MAI_FORMAT),
> -   VC4_REG32(VC4_HDMI_AUDIO_PACKET_CONFIG),
> -   VC4_REG32(VC4_HDMI_RAM_PACKET_CONFIG),
> -   VC4_REG32(VC4_HDMI_HORZA),
> -   VC4_REG32(VC4_HDMI_HORZB),
> -   VC4_REG32(VC4_HDMI_FIFO_CTL),
> -   VC4_REG32(VC4_HDMI_SCHEDULER_CONTROL),
> -   VC4_REG32(VC4_HDMI_VERTA0),
> -   VC4_REG32(VC4_HDMI_VERTA1),
> -   VC4_REG32(VC4_HDMI_VERTB0),
> -   VC4_REG32(VC4_HDMI_VERTB1),
> -   VC4_REG32(VC4_HDMI_TX_PHY_RESET_CTL),
> -   VC4_REG32(VC4_HDMI_TX_PHY_CTL0),
> -
> -   VC4_REG32(VC4_HDMI_CEC_CNTRL_1),
> -   VC4_REG32(VC4_HDMI_CEC_CNTRL_2),
> -   VC4_REG32(VC4_HDMI_CEC_CNTRL_3),
> -   VC4_REG32(VC4_HDMI_CEC_CNTRL_4),
> -   VC4_REG32(VC4_HDMI_CEC_CNTRL_5),
> -   VC4_REG32(VC4_HDMI_CPU_STATUS),
> -   VC4_REG32(VC4_HDMI_CPU_MASK_STATUS),
> -
> -   VC4_REG32(VC4_HDMI_CEC_RX_DATA_1),
> -   VC4_REG32(VC4_HDMI_CEC_RX_DATA_2),
> -   VC4_REG32(VC4_HDMI_CEC_RX_DATA_3),
> -   VC4_REG32(VC4_HDMI_CEC_RX_DATA_4),
> -   VC4_REG32(VC4_HDMI_CEC_TX_DATA_1),
> -   VC4_REG32(VC4_HDMI_CEC_TX_DATA_2),
> -   VC4_REG32(VC4_HDMI_CEC_TX_DATA_3),
> -   VC4_REG32(VC4_HDMI_CEC_TX_DATA_4),
> -};
> -
> -static const struct debugfs_reg32 hd_regs[] = {
> -   VC4_REG32(VC4_HD_M_CTL),
> -   VC4_REG32(VC4_HD_MAI_CTL),
> -   VC4_REG32(VC4_HD_MAI_THR),
> -   VC4_REG32(VC4_HD_MAI_FMT),
> -   VC4_REG32(VC4_HD_MAI_SMP),
> -   VC4_REG32(VC4_HD_VID_CTL),
> -   VC4_REG32(VC4_HD_CSC_CTL),
> -   VC4_REG32(VC4_HD_FRAME_COUNT),
> -};
> -
>  static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
>  {
> struct drm_info_node *node = (struct drm_info_node *)m->private;
> @@ -134,7 +85,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, 
> bool force)
> if (drm_probe_ddc(vc4_hdmi->ddc))
> return connector_status_connected;
>
> -   if (HDMI_READ(VC4_HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED)
> +   if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED)
> return connector_status_connected;
> cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
> return connector_status_disconnected;
> @@ -223,10 +174,10 @@ static int vc4_hdmi_stop_packet(struct drm_encoder 
> *encoder,
> struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> u32 packet_id = type - 0x80;
>
> -   HDMI_WRITE(VC4_HDMI_RAM_PACKET_CONFIG,
> -  HDMI_READ(VC4_HDMI_RAM_PACKET_CONFIG) & ~BIT(packet_id));
> +   HDMI_WRITE(HDMI_RAM_PACKET_CONFIG,
> +  HDMI_READ(HDMI_RAM_PACKET_CONFIG) & ~BIT(packet_id));
>
> -   return wait_for(!(HDMI_READ(VC4_HDMI_RAM_PACKET_STATUS) &
> +   return wait_for(!(HDMI_READ(HDMI_RAM_PACKET_STATUS) &
>   BIT(packet_id)), 100);
>  }
>
> @@ -235,12 +186,16 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder 
> *encoder,
>  {
> struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> u32 packet_id = frame->any.type - 0x80;
> -   u32 packet_reg = VC4_HDMI_RAM_PACKET(packet_id);
> +   const struct vc4_hdmi_register *ram_packet_start =
> +   &vc4_

[PATCH v4 51/78] drm/vc4: hdmi: Implement a register layout abstraction

2020-07-08 Thread Maxime Ripard
The HDMI controllers found in the BCM2711 have most of the registers
reorganized in multiple registers areas and at different offsets than
previously found.

The logic however remains pretty much the same, so it doesn't really make
sense to create a whole new driver and we should share the code as much as
possible.

Let's implement some indirection to wrap around a register and depending on
the variant will lookup the associated register on that particular variant.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c  | 427 ++---
 drivers/gpu/drm/vc4/vc4_hdmi.h  |  12 +-
 drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 241 -
 drivers/gpu/drm/vc4/vc4_regs.h  |  92 +--
 4 files changed, 464 insertions(+), 308 deletions(-)
 create mode 100644 drivers/gpu/drm/vc4/vc4_hdmi_regs.h

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index ac021e07a8cb..a4fed1439bf3 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -50,62 +50,13 @@
 #include "media/cec.h"
 #include "vc4_drv.h"
 #include "vc4_hdmi.h"
+#include "vc4_hdmi_regs.h"
 #include "vc4_regs.h"
 
 #define HSM_CLOCK_FREQ 163682864
 #define CEC_CLOCK_FREQ 4
 #define CEC_CLOCK_DIV  (HSM_CLOCK_FREQ / CEC_CLOCK_FREQ)
 
-static const struct debugfs_reg32 hdmi_regs[] = {
-   VC4_REG32(VC4_HDMI_CORE_REV),
-   VC4_REG32(VC4_HDMI_SW_RESET_CONTROL),
-   VC4_REG32(VC4_HDMI_HOTPLUG_INT),
-   VC4_REG32(VC4_HDMI_HOTPLUG),
-   VC4_REG32(VC4_HDMI_MAI_CHANNEL_MAP),
-   VC4_REG32(VC4_HDMI_MAI_CONFIG),
-   VC4_REG32(VC4_HDMI_MAI_FORMAT),
-   VC4_REG32(VC4_HDMI_AUDIO_PACKET_CONFIG),
-   VC4_REG32(VC4_HDMI_RAM_PACKET_CONFIG),
-   VC4_REG32(VC4_HDMI_HORZA),
-   VC4_REG32(VC4_HDMI_HORZB),
-   VC4_REG32(VC4_HDMI_FIFO_CTL),
-   VC4_REG32(VC4_HDMI_SCHEDULER_CONTROL),
-   VC4_REG32(VC4_HDMI_VERTA0),
-   VC4_REG32(VC4_HDMI_VERTA1),
-   VC4_REG32(VC4_HDMI_VERTB0),
-   VC4_REG32(VC4_HDMI_VERTB1),
-   VC4_REG32(VC4_HDMI_TX_PHY_RESET_CTL),
-   VC4_REG32(VC4_HDMI_TX_PHY_CTL0),
-
-   VC4_REG32(VC4_HDMI_CEC_CNTRL_1),
-   VC4_REG32(VC4_HDMI_CEC_CNTRL_2),
-   VC4_REG32(VC4_HDMI_CEC_CNTRL_3),
-   VC4_REG32(VC4_HDMI_CEC_CNTRL_4),
-   VC4_REG32(VC4_HDMI_CEC_CNTRL_5),
-   VC4_REG32(VC4_HDMI_CPU_STATUS),
-   VC4_REG32(VC4_HDMI_CPU_MASK_STATUS),
-
-   VC4_REG32(VC4_HDMI_CEC_RX_DATA_1),
-   VC4_REG32(VC4_HDMI_CEC_RX_DATA_2),
-   VC4_REG32(VC4_HDMI_CEC_RX_DATA_3),
-   VC4_REG32(VC4_HDMI_CEC_RX_DATA_4),
-   VC4_REG32(VC4_HDMI_CEC_TX_DATA_1),
-   VC4_REG32(VC4_HDMI_CEC_TX_DATA_2),
-   VC4_REG32(VC4_HDMI_CEC_TX_DATA_3),
-   VC4_REG32(VC4_HDMI_CEC_TX_DATA_4),
-};
-
-static const struct debugfs_reg32 hd_regs[] = {
-   VC4_REG32(VC4_HD_M_CTL),
-   VC4_REG32(VC4_HD_MAI_CTL),
-   VC4_REG32(VC4_HD_MAI_THR),
-   VC4_REG32(VC4_HD_MAI_FMT),
-   VC4_REG32(VC4_HD_MAI_SMP),
-   VC4_REG32(VC4_HD_VID_CTL),
-   VC4_REG32(VC4_HD_CSC_CTL),
-   VC4_REG32(VC4_HD_FRAME_COUNT),
-};
-
 static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
 {
struct drm_info_node *node = (struct drm_info_node *)m->private;
@@ -134,7 +85,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, 
bool force)
if (drm_probe_ddc(vc4_hdmi->ddc))
return connector_status_connected;
 
-   if (HDMI_READ(VC4_HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED)
+   if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED)
return connector_status_connected;
cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
return connector_status_disconnected;
@@ -223,10 +174,10 @@ static int vc4_hdmi_stop_packet(struct drm_encoder 
*encoder,
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
u32 packet_id = type - 0x80;
 
-   HDMI_WRITE(VC4_HDMI_RAM_PACKET_CONFIG,
-  HDMI_READ(VC4_HDMI_RAM_PACKET_CONFIG) & ~BIT(packet_id));
+   HDMI_WRITE(HDMI_RAM_PACKET_CONFIG,
+  HDMI_READ(HDMI_RAM_PACKET_CONFIG) & ~BIT(packet_id));
 
-   return wait_for(!(HDMI_READ(VC4_HDMI_RAM_PACKET_STATUS) &
+   return wait_for(!(HDMI_READ(HDMI_RAM_PACKET_STATUS) &
  BIT(packet_id)), 100);
 }
 
@@ -235,12 +186,16 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder 
*encoder,
 {
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
u32 packet_id = frame->any.type - 0x80;
-   u32 packet_reg = VC4_HDMI_RAM_PACKET(packet_id);
+   const struct vc4_hdmi_register *ram_packet_start =
+   &vc4_hdmi->variant->registers[HDMI_RAM_PACKET_START];
+   u32 packet_reg = ram_packet_start->offset + VC4_HDMI_PACKET_STRIDE * 
packet_id;
+   void __iomem *base = __vc4_hdmi_get_field_base(vc4_hdmi,
+  ram_packet_start->reg);
uint8_t buffer[VC4_HDMI_PACKET_