| Message ID | 20240730125023.710237-7-jbrunet@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | drm/meson: dw-hdmi: clean-up | expand |
On 30/07/2024 14:50, Jerome Brunet wrote: > The Amlogic mixes direct register access and regmap ones, with several > custom helpers. Using a single API makes rework and maintenance easier. > > Convert the Amlogic phy driver to regmap and use it to have more consistent > access to the registers in the driver, with less custom helpers. Add > register bit definitions when missing. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/gpu/drm/meson/meson_dw_hdmi.c | 475 ++++++++++++-------------- > drivers/gpu/drm/meson/meson_dw_hdmi.h | 49 +-- > 2 files changed, 239 insertions(+), 285 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c > index 47aa3e184e98..7c39e5c99043 100644 > --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c > +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c > @@ -90,16 +90,25 @@ > * - CEC Management > */ > > -/* TOP Block Communication Channel */ > -#define HDMITX_TOP_ADDR_REG 0x0 > -#define HDMITX_TOP_DATA_REG 0x4 > -#define HDMITX_TOP_CTRL_REG 0x8 > -#define HDMITX_TOP_G12A_OFFSET 0x8000 > +/* Indirect channel definition for GX */ > +#define HDMITX_TOP_REGS 0x0 > +#define HDMITX_DWC_REGS 0x10 > + > +#define GX_ADDR_OFFSET 0x0 > +#define GX_DATA_OFFSET 0x4 > +#define GX_CTRL_OFFSET 0x8 I don't see the point renaming thos defines > +#define GX_CTRL_APB3_ERRFAIL BIT(15) > > -/* Controller Communication Channel */ > -#define HDMITX_DWC_ADDR_REG 0x10 > -#define HDMITX_DWC_DATA_REG 0x14 > -#define HDMITX_DWC_CTRL_REG 0x18 > +/* > + * NOTE: G12 use direct addressing: > + * Ideally it should receive one memory region for each of the top > + * and dwc register regions but fixing this would require to change > + * the DT bindings. Doing so is a pain. Keep the region as it and work > + * around the problem, at least for now. > + * Future supported SoCs should properly describe the regions in the > + * DT bindings instead of using this trick. well I disagree here, there's a single memory region for the HDMITX module, and the DWC region is controlled by the TOP registers, so the DWC region _cannot_ be accessed without correctly configuring the TOP registers. So I disagree strongly with this comment. > + */ > +#define HDMITX_TOP_G12A_OFFSET 0x8000 > > /* HHI Registers */ > #define HHI_MEM_PD_REG0 0x100 /* 0x40 */ > @@ -108,28 +117,59 @@ > #define HHI_HDMI_PHY_CNTL1 0x3a4 /* 0xe9 */ > #define PHY_CNTL1_INIT 0x03900000 > #define PHY_INVERT BIT(17) > +#define PHY_FIFOS GENMASK(3, 2) > +#define PHY_CLOCK_EN BIT(1) > +#define PHY_SOFT_RST BIT(0) Please move those changes before or after this patch > #define HHI_HDMI_PHY_CNTL2 0x3a8 /* 0xea */ > #define HHI_HDMI_PHY_CNTL3 0x3ac /* 0xeb */ > #define HHI_HDMI_PHY_CNTL4 0x3b0 /* 0xec */ > #define HHI_HDMI_PHY_CNTL5 0x3b4 /* 0xed */ > > -static DEFINE_SPINLOCK(reg_lock); > - > -struct meson_dw_hdmi; > - > struct meson_dw_hdmi_data { > - unsigned int (*top_read)(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr); > - void (*top_write)(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr, unsigned int data); > - unsigned int (*dwc_read)(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr); > - void (*dwc_write)(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr, unsigned int data); > + int (*reg_init)(struct device *dev); > u32 cntl0_init; > u32 cntl1_init; > }; > > +static int hdmi_tx_indirect_reg_read(void *context, > + unsigned int reg, > + unsigned int *result) > +{ > + void __iomem *base = context; > + > + /* Must write the read address twice ... */ > + writel(reg, base + GX_ADDR_OFFSET); > + writel(reg, base + GX_ADDR_OFFSET); > + > + /* ... and read the data twice as well */ > + *result = readl(base + GX_DATA_OFFSET); > + *result = readl(base + GX_DATA_OFFSET); Why did you change the comments ? > + > + return 0; > +} > + > +static int hdmi_tx_indirect_reg_write(void *context, > + unsigned int reg, > + unsigned int val) > +{ > + void __iomem *base = context; > + > + /* Must write the read address twice ... */ > + writel(reg, base + GX_ADDR_OFFSET); > + writel(reg, base + GX_ADDR_OFFSET); > + > + /* ... but write the data only once */ > + writel(val, base + GX_DATA_OFFSET); Ditto ? were they wrong ? > + > + return 0; > +} > + > +static const struct regmap_bus hdmi_tx_indirect_mmio = { > + .fast_io = true, > + .reg_read = hdmi_tx_indirect_reg_read, > + .reg_write = hdmi_tx_indirect_reg_write, > +}; > + > struct meson_dw_hdmi { > struct dw_hdmi_plat_data dw_plat_data; > struct meson_drm *priv; > @@ -139,9 +179,10 @@ struct meson_dw_hdmi { > struct reset_control *hdmitx_apb; > struct reset_control *hdmitx_ctrl; > struct reset_control *hdmitx_phy; > - u32 irq_stat; > + unsigned int irq_stat; > struct dw_hdmi *hdmi; > struct drm_bridge *bridge; > + struct regmap *top; The name could be better, like top_regs or top_regmap > }; > > static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi, > @@ -150,136 +191,6 @@ static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi, > return of_device_is_compatible(dw_hdmi->dev->of_node, compat); > } > > -/* PHY (via TOP bridge) and Controller dedicated register interface */ > - > -static unsigned int dw_hdmi_top_read(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr) > -{ > - unsigned long flags; > - unsigned int data; > - > - spin_lock_irqsave(®_lock, flags); > - > - /* ADDR must be written twice */ > - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); > - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); > - > - /* Read needs a second DATA read */ > - data = readl(dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); > - data = readl(dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); > - > - spin_unlock_irqrestore(®_lock, flags); > - > - return data; > -} > - > -static unsigned int dw_hdmi_g12a_top_read(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr) > -{ > - return readl(dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET + (addr << 2)); > -} > - > -static inline void dw_hdmi_top_write(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr, unsigned int data) > -{ > - unsigned long flags; > - > - spin_lock_irqsave(®_lock, flags); > - > - /* ADDR must be written twice */ > - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); > - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); > - > - /* Write needs single DATA write */ > - writel(data, dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); > - > - spin_unlock_irqrestore(®_lock, flags); > -} > - > -static inline void dw_hdmi_g12a_top_write(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr, unsigned int data) > -{ > - writel(data, dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET + (addr << 2)); > -} > - > -/* Helper to change specific bits in PHY registers */ > -static inline void dw_hdmi_top_write_bits(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr, > - unsigned int mask, > - unsigned int val) > -{ > - unsigned int data = dw_hdmi->data->top_read(dw_hdmi, addr); > - > - data &= ~mask; > - data |= val; > - > - dw_hdmi->data->top_write(dw_hdmi, addr, data); > -} > - > -static unsigned int dw_hdmi_dwc_read(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr) > -{ > - unsigned long flags; > - unsigned int data; > - > - spin_lock_irqsave(®_lock, flags); > - > - /* ADDR must be written twice */ > - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); > - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); > - > - /* Read needs a second DATA read */ > - data = readl(dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); > - data = readl(dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); > - > - spin_unlock_irqrestore(®_lock, flags); > - > - return data; > -} > - > -static unsigned int dw_hdmi_g12a_dwc_read(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr) > -{ > - return readb(dw_hdmi->hdmitx + addr); > -} > - > -static inline void dw_hdmi_dwc_write(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr, unsigned int data) > -{ > - unsigned long flags; > - > - spin_lock_irqsave(®_lock, flags); > - > - /* ADDR must be written twice */ > - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); > - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); > - > - /* Write needs single DATA write */ > - writel(data, dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); > - > - spin_unlock_irqrestore(®_lock, flags); > -} > - > -static inline void dw_hdmi_g12a_dwc_write(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr, unsigned int data) > -{ > - writeb(data, dw_hdmi->hdmitx + addr); > -} > - > -/* Helper to change specific bits in controller registers */ > -static inline void dw_hdmi_dwc_write_bits(struct meson_dw_hdmi *dw_hdmi, > - unsigned int addr, > - unsigned int mask, > - unsigned int val) > -{ > - unsigned int data = dw_hdmi->data->dwc_read(dw_hdmi, addr); > - > - data &= ~mask; > - data |= val; > - > - dw_hdmi->data->dwc_write(dw_hdmi, addr, data); > -} > - > /* Bridge */ > > /* Setup PHY bandwidth modes */ > @@ -353,13 +264,15 @@ static inline void meson_dw_hdmi_phy_reset(struct meson_dw_hdmi *dw_hdmi) > struct meson_drm *priv = dw_hdmi->priv; > > /* Enable and software reset */ > - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0xf); > - > + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, > + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, > + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST); > mdelay(2); > > /* Enable and unreset */ > - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0xe); > - > + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, > + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, > + PHY_FIFOS | PHY_CLOCK_EN); > mdelay(2); > } > > @@ -382,27 +295,30 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data, > > /* TMDS pattern setup */ > if (mode->clock > 340000 && !mode_is_420) { > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01, > - 0); > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23, > - 0x03ff03ff); > + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_01, > + 0); > + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_23, > + 0x03ff03ff); > } else { > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01, > - 0x001f001f); > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23, > - 0x001f001f); > + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_01, > + 0x001f001f); > + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_23, > + 0x001f001f); > } > > /* Load TMDS pattern */ > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, 0x1); > + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, > + TOP_TDMS_CLK_PTTN_LOAD); > msleep(20); > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, 0x2); > + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, > + TOP_TDMS_CLK_PTTN_SHFT); > > /* Setup PHY parameters */ > meson_hdmi_phy_setup_mode(dw_hdmi, mode, mode_is_420); > > /* Disable clock, fifo, fifo_wr */ > - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0); > + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, > + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, 0); > > dw_hdmi_set_high_tmds_clock_ratio(hdmi, display); > > @@ -433,8 +349,11 @@ static enum drm_connector_status dw_hdmi_read_hpd(struct dw_hdmi *hdmi, > void *data) > { > struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data; > + unsigned int stat; > > - return !!dw_hdmi->data->top_read(dw_hdmi, HDMITX_TOP_STAT0) ? > + regmap_read(dw_hdmi->top, HDMITX_TOP_STAT0, &stat); > + > + return !!stat ? > connector_status_connected : connector_status_disconnected; > } > > @@ -444,17 +363,18 @@ static void dw_hdmi_setup_hpd(struct dw_hdmi *hdmi, > struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data; > > /* Setup HPD Filter */ > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_HPD_FILTER, > - (0xa << 12) | 0xa0); > + regmap_write(dw_hdmi->top, HDMITX_TOP_HPD_FILTER, > + FIELD_PREP(TOP_HPD_GLITCH_WIDTH, 10) | > + FIELD_PREP(TOP_HPD_VALID_WIDTH, 160)); > > /* Clear interrupts */ > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, > - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL); > + regmap_write(dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, > + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL); > > /* Unmask interrupts */ > - dw_hdmi_top_write_bits(dw_hdmi, HDMITX_TOP_INTR_MASKN, > - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL, > - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL); > + regmap_update_bits(dw_hdmi->top, HDMITX_TOP_INTR_MASKN, > + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL, > + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL); > } > > static const struct dw_hdmi_phy_ops meson_dw_hdmi_phy_ops = { > @@ -467,23 +387,22 @@ static const struct dw_hdmi_phy_ops meson_dw_hdmi_phy_ops = { > static irqreturn_t dw_hdmi_top_irq(int irq, void *dev_id) > { > struct meson_dw_hdmi *dw_hdmi = dev_id; > - u32 stat; > + unsigned int stat; > > - stat = dw_hdmi->data->top_read(dw_hdmi, HDMITX_TOP_INTR_STAT); > - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, stat); > + regmap_read(dw_hdmi->top, HDMITX_TOP_INTR_STAT, &stat); > + regmap_write(dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, stat); > > /* HPD Events, handle in the threaded interrupt handler */ > - if (stat & (HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL)) { > + if (stat & (TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL)) { > dw_hdmi->irq_stat = stat; > return IRQ_WAKE_THREAD; > } > > /* HDMI Controller Interrupt */ > - if (stat & 1) > + if (stat & TOP_INTR_CORE) > return IRQ_NONE; > > /* TOFIX Handle HDCP Interrupts */ > - > return IRQ_HANDLED; > } > > @@ -494,10 +413,10 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id) > u32 stat = dw_hdmi->irq_stat; > > /* HPD Events */ > - if (stat & (HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL)) { > + if (stat & (TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL)) { > bool hpd_connected = false; > > - if (stat & HDMITX_TOP_INTR_HPD_RISE) > + if (stat & TOP_INTR_HPD_RISE) > hpd_connected = true; > > dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected, > @@ -512,63 +431,25 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id) > return IRQ_HANDLED; > } > > -/* DW HDMI Regmap */ > - > -static int meson_dw_hdmi_reg_read(void *context, unsigned int reg, > - unsigned int *result) > -{ > - struct meson_dw_hdmi *dw_hdmi = context; > - > - *result = dw_hdmi->data->dwc_read(dw_hdmi, reg); > - > - return 0; > - > -} > - > -static int meson_dw_hdmi_reg_write(void *context, unsigned int reg, > - unsigned int val) > -{ > - struct meson_dw_hdmi *dw_hdmi = context; > - > - dw_hdmi->data->dwc_write(dw_hdmi, reg, val); > - > - return 0; > -} > - > -static const struct regmap_config meson_dw_hdmi_regmap_config = { > - .reg_bits = 32, > - .val_bits = 8, > - .reg_read = meson_dw_hdmi_reg_read, > - .reg_write = meson_dw_hdmi_reg_write, > - .max_register = 0x10000, > - .fast_io = true, > -}; > - > -static const struct meson_dw_hdmi_data meson_dw_hdmi_gxbb_data = { > - .top_read = dw_hdmi_top_read, > - .top_write = dw_hdmi_top_write, > - .dwc_read = dw_hdmi_dwc_read, > - .dwc_write = dw_hdmi_dwc_write, > - .cntl0_init = 0x0, > - .cntl1_init = PHY_CNTL1_INIT | PHY_INVERT, > +static const struct regmap_config top_gx_regmap_cfg = { > + .reg_bits = 32, > + .reg_stride = 4, > + .reg_shift = -2, > + .val_bits = 32, > + .max_register = 0x40, > }; > > -static const struct meson_dw_hdmi_data meson_dw_hdmi_gxl_data = { > - .top_read = dw_hdmi_top_read, > - .top_write = dw_hdmi_top_write, > - .dwc_read = dw_hdmi_dwc_read, > - .dwc_write = dw_hdmi_dwc_write, > - .cntl0_init = 0x0, > - .cntl1_init = PHY_CNTL1_INIT, > +static const struct regmap_config top_g12_regmap_cfg = { > + .reg_bits = 32, > + .reg_stride = 4, > + .val_bits = 32, > + .max_register = 0x40, > }; > > -static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = { > - .top_read = dw_hdmi_g12a_top_read, > - .top_write = dw_hdmi_g12a_top_write, > - .dwc_read = dw_hdmi_g12a_dwc_read, > - .dwc_write = dw_hdmi_g12a_dwc_write, > - .cntl0_init = 0x000b4242, /* Bandgap */ > - .cntl1_init = PHY_CNTL1_INIT, > +static const struct regmap_config dwc_regmap_cfg = { > + .reg_bits = 32, > + .val_bits = 8, > + .max_register = 0x8000, > }; > > static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) > @@ -581,41 +462,107 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) > /* Bring HDMITX MEM output of power down */ > regmap_update_bits(priv->hhi, HHI_MEM_PD_REG0, 0xff << 8, 0); > > - /* Enable APB3 fail on error */ > - if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) { > - writel_bits_relaxed(BIT(15), BIT(15), > - meson_dw_hdmi->hdmitx + HDMITX_TOP_CTRL_REG); > - writel_bits_relaxed(BIT(15), BIT(15), > - meson_dw_hdmi->hdmitx + HDMITX_DWC_CTRL_REG); > - } > - > /* Bring out of reset */ > - meson_dw_hdmi->data->top_write(meson_dw_hdmi, > - HDMITX_TOP_SW_RESET, 0); > - > + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_SW_RESET, 0); > msleep(20); > > - meson_dw_hdmi->data->top_write(meson_dw_hdmi, > - HDMITX_TOP_CLK_CNTL, 0xff); > + /* Enable clocks */ > + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_CLK_CNTL, > + TOP_CLK_EN); > > /* Enable normal output to PHY */ > - meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12)); > + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_BIST_CNTL, > + TOP_BIST_TMDS_EN); > > /* Setup PHY */ > - regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL1, meson_dw_hdmi->data->cntl1_init); > - regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, meson_dw_hdmi->data->cntl0_init); > + regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL1, > + meson_dw_hdmi->data->cntl1_init); > + regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, > + meson_dw_hdmi->data->cntl0_init); > > /* Enable HDMI-TX Interrupt */ > - meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, > - HDMITX_TOP_INTR_CORE); > + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, > + GENMASK(31, 0)); > + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_INTR_MASKN, > + TOP_INTR_CORE); > +} > + > +static int meson_dw_init_regmap_gx(struct device *dev) > +{ > + struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev); > + struct regmap *map; > > - meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_INTR_MASKN, > - HDMITX_TOP_INTR_CORE); > + /* Register TOP glue zone */ > + writel_bits_relaxed(GX_CTRL_APB3_ERRFAIL, GX_CTRL_APB3_ERRFAIL, > + meson_dw_hdmi->hdmitx + HDMITX_TOP_REGS + GX_CTRL_OFFSET); > > + map = devm_regmap_init(dev, &hdmi_tx_indirect_mmio, > + meson_dw_hdmi->hdmitx + HDMITX_TOP_REGS, > + &top_gx_regmap_cfg); > + if (IS_ERR(map)) > + return dev_err_probe(dev, PTR_ERR(map), "failed to init top regmap\n"); > + > + meson_dw_hdmi->top = map; > + > + /* Register DWC zone */ > + writel_bits_relaxed(GX_CTRL_APB3_ERRFAIL, GX_CTRL_APB3_ERRFAIL, > + meson_dw_hdmi->hdmitx + HDMITX_DWC_REGS + GX_CTRL_OFFSET); > + > + map = devm_regmap_init(dev, &hdmi_tx_indirect_mmio, > + meson_dw_hdmi->hdmitx + HDMITX_DWC_REGS, > + &dwc_regmap_cfg); > + if (IS_ERR(map)) > + return dev_err_probe(dev, PTR_ERR(map), "failed to init dwc regmap\n"); > + > + meson_dw_hdmi->dw_plat_data.regm = map; > + > + return 0; > +} > + > +static int meson_dw_init_regmap_g12(struct device *dev) > +{ > + struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev); > + struct regmap *map; > + > + /* Register TOP glue zone with the offset */ > + map = devm_regmap_init_mmio(dev, meson_dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET, > + &top_g12_regmap_cfg); > + if (IS_ERR(map)) > + dev_err_probe(dev, PTR_ERR(map), "failed to init top regmap\n"); > + > + meson_dw_hdmi->top = map; > + > + /* Register DWC zone */ > + map = devm_regmap_init_mmio(dev, meson_dw_hdmi->hdmitx, > + &dwc_regmap_cfg); > + if (IS_ERR(map)) > + dev_err_probe(dev, PTR_ERR(map), "failed to init dwc regmap\n"); > + > + meson_dw_hdmi->dw_plat_data.regm = map; > + > + return 0; > } > > +static const struct meson_dw_hdmi_data meson_dw_hdmi_gxbb_data = { > + .reg_init = meson_dw_init_regmap_gx, > + .cntl0_init = 0x0, > + .cntl1_init = PHY_CNTL1_INIT | PHY_INVERT, > +}; > + > +static const struct meson_dw_hdmi_data meson_dw_hdmi_gxl_data = { > + .reg_init = meson_dw_init_regmap_gx, > + .cntl0_init = 0x0, > + .cntl1_init = PHY_CNTL1_INIT, > +}; > + > +static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = { > + .reg_init = meson_dw_init_regmap_g12, > + .cntl0_init = 0x000b4242, /* Bandgap */ > + .cntl1_init = PHY_CNTL1_INIT, > +}; > + > static int meson_dw_hdmi_bind(struct device *dev, struct device *master, > - void *data) > + void *data) > { > struct platform_device *pdev = to_platform_device(dev); > const struct meson_dw_hdmi_data *match; > @@ -640,6 +587,8 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, > if (!meson_dw_hdmi) > return -ENOMEM; > > + platform_set_drvdata(pdev, meson_dw_hdmi); > + > meson_dw_hdmi->priv = priv; > meson_dw_hdmi->dev = dev; > meson_dw_hdmi->data = match; > @@ -682,10 +631,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, > if (ret) > return dev_err_probe(dev, ret, "Failed to enable all clocks\n"); > > - dw_plat_data->regm = devm_regmap_init(dev, NULL, meson_dw_hdmi, > - &meson_dw_hdmi_regmap_config); > - if (IS_ERR(dw_plat_data->regm)) > - return PTR_ERR(dw_plat_data->regm); > + ret = meson_dw_hdmi->data->reg_init(dev); > + if (ret) > + return ret; > > irq = platform_get_irq(pdev, 0); > if (irq < 0) > @@ -717,8 +665,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, > dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-g12a-dw-hdmi")) > dw_plat_data->use_drm_infoframe = true; > > - platform_set_drvdata(pdev, meson_dw_hdmi); > - > meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data); > if (IS_ERR(meson_dw_hdmi->hdmi)) > return PTR_ERR(meson_dw_hdmi->hdmi); > @@ -751,8 +697,7 @@ static int __maybe_unused meson_dw_hdmi_pm_suspend(struct device *dev) > return 0; > > /* FIXME: This actually bring top out reset on suspend, why ? */ > - meson_dw_hdmi->data->top_write(meson_dw_hdmi, > - HDMITX_TOP_SW_RESET, 0); > + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_SW_RESET, 0); > > return 0; > } > diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.h b/drivers/gpu/drm/meson/meson_dw_hdmi.h > index 08e1c14e4ea0..3ab8c56d5fe1 100644 > --- a/drivers/gpu/drm/meson/meson_dw_hdmi.h > +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.h > @@ -28,6 +28,7 @@ > * 0=Release from reset. Default 1. > */ > #define HDMITX_TOP_SW_RESET (0x000) > +#define TOP_RST_EN GENMASK(4, 0) Well NAK, the registers are named HDMITX_TOP_XXXX in the datasheet, and TOP_XXXX defines could collide with other too generic defines, and in any case don't mix defines renaming with other changes. Just move the GENMASK in a new patch and leave the HDMITX_ prefix alone. > > /* > * Bit 31 RW free_clk_en: 0=Enable clock gating for power saving; 1= Disable > @@ -45,7 +46,8 @@ > * Bit 1 RW tmds_clk_en: 1=enable tmds_clk; 0=disable. Default 0. > * Bit 0 RW pixel_clk_en: 1=enable pixel_clk; 0=disable. Default 0. > */ > -#define HDMITX_TOP_CLK_CNTL (0x001) > +#define HDMITX_TOP_CLK_CNTL (0x004) > +#define TOP_CLK_EN GENMASK(7, 0) > > /* > * Bit 31:28 RW rxsense_glitch_width: starting from G12A > @@ -53,7 +55,9 @@ > * Bit 11: 0 RW hpd_valid_width: filter out width <= M*1024. Default 0. > * Bit 15:12 RW hpd_glitch_width: filter out glitch <= N. Default 0. > */ > -#define HDMITX_TOP_HPD_FILTER (0x002) > +#define HDMITX_TOP_HPD_FILTER (0x008) > +#define TOP_HPD_GLITCH_WIDTH GENMASK(15, 12) > +#define TOP_HPD_VALID_WIDTH GENMASK(11, 0) > > /* > * intr_maskn: MASK_N, one bit per interrupt source. > @@ -67,7 +71,7 @@ > * [ 1] hpd_rise_intr > * [ 0] core_intr > */ > -#define HDMITX_TOP_INTR_MASKN (0x003) > +#define HDMITX_TOP_INTR_MASKN (0x00c) > > /* > * Bit 30: 0 RW intr_stat: For each bit, write 1 to manually set the interrupt > @@ -80,7 +84,7 @@ > * Bit 1 RW hpd_rise > * Bit 0 RW IP interrupt > */ > -#define HDMITX_TOP_INTR_STAT (0x004) > +#define HDMITX_TOP_INTR_STAT (0x010) > > /* > * [7] rxsense_fall starting from G12A > @@ -92,13 +96,12 @@ > * [1] hpd_rise > * [0] core_intr_rise > */ > -#define HDMITX_TOP_INTR_STAT_CLR (0x005) > - > -#define HDMITX_TOP_INTR_CORE BIT(0) > -#define HDMITX_TOP_INTR_HPD_RISE BIT(1) > -#define HDMITX_TOP_INTR_HPD_FALL BIT(2) > -#define HDMITX_TOP_INTR_RXSENSE_RISE BIT(6) > -#define HDMITX_TOP_INTR_RXSENSE_FALL BIT(7) > +#define HDMITX_TOP_INTR_STAT_CLR (0x014) > +#define TOP_INTR_CORE BIT(0) > +#define TOP_INTR_HPD_RISE BIT(1) > +#define TOP_INTR_HPD_FALL BIT(2) > +#define TOP_INTR_RXSENSE_RISE BIT(6) > +#define TOP_INTR_RXSENSE_FALL BIT(7) > > /* > * Bit 14:12 RW tmds_sel: 3'b000=Output zero; 3'b001=Output normal TMDS data; > @@ -112,29 +115,31 @@ > * 2=Output 1-bit pattern; 3=output 10-bit pattern. Default 0. > * Bit 0 RW prbs_pttn_en: 1=Enable PRBS generator; 0=Disable. Default 0. > */ > -#define HDMITX_TOP_BIST_CNTL (0x006) > +#define HDMITX_TOP_BIST_CNTL (0x018) > +#define TOP_BIST_OUT_MASK GENMASK(14, 12) > +#define TOP_BIST_TMDS_EN BIT(12) > > /* Bit 29:20 RW shift_pttn_data[59:50]. Default 0. */ > /* Bit 19:10 RW shift_pttn_data[69:60]. Default 0. */ > /* Bit 9: 0 RW shift_pttn_data[79:70]. Default 0. */ > -#define HDMITX_TOP_SHIFT_PTTN_012 (0x007) > +#define HDMITX_TOP_SHIFT_PTTN_012 (0x01c) > > /* Bit 29:20 RW shift_pttn_data[29:20]. Default 0. */ > /* Bit 19:10 RW shift_pttn_data[39:30]. Default 0. */ > /* Bit 9: 0 RW shift_pttn_data[49:40]. Default 0. */ > -#define HDMITX_TOP_SHIFT_PTTN_345 (0x008) > +#define HDMITX_TOP_SHIFT_PTTN_345 (0x020) > > /* Bit 19:10 RW shift_pttn_data[ 9: 0]. Default 0. */ > /* Bit 9: 0 RW shift_pttn_data[19:10]. Default 0. */ > -#define HDMITX_TOP_SHIFT_PTTN_67 (0x009) > +#define HDMITX_TOP_SHIFT_PTTN_67 (0x024) > > /* Bit 25:16 RW tmds_clk_pttn[19:10]. Default 0. */ > /* Bit 9: 0 RW tmds_clk_pttn[ 9: 0]. Default 0. */ > -#define HDMITX_TOP_TMDS_CLK_PTTN_01 (0x00A) > +#define HDMITX_TOP_TMDS_CLK_PTTN_01 (0x028) > > /* Bit 25:16 RW tmds_clk_pttn[39:30]. Default 0. */ > /* Bit 9: 0 RW tmds_clk_pttn[29:20]. Default 0. */ > -#define HDMITX_TOP_TMDS_CLK_PTTN_23 (0x00B) > +#define HDMITX_TOP_TMDS_CLK_PTTN_23 (0x02c) > > /* > * Bit 1 RW shift_tmds_clk_pttn:1=Enable shifting clk pattern, > @@ -143,18 +148,22 @@ > * [ 1] shift_tmds_clk_pttn > * [ 0] load_tmds_clk_pttn > */ > -#define HDMITX_TOP_TMDS_CLK_PTTN_CNTL (0x00C) > +#define HDMITX_TOP_TMDS_CLK_PTTN_CNTL (0x030) > +#define TOP_TDMS_CLK_PTTN_LOAD BIT(0) > +#define TOP_TDMS_CLK_PTTN_SHFT BIT(1) > > /* > * Bit 0 RW revocmem_wr_fail: Read back 1 to indicate Host write REVOC MEM > * failure, write 1 to clear the failure flag. Default 0. > */ > -#define HDMITX_TOP_REVOCMEM_STAT (0x00D) > +#define HDMITX_TOP_REVOCMEM_STAT (0x034) > > /* > * Bit 1 R filtered RxSense status > * Bit 0 R filtered HPD status. > */ > -#define HDMITX_TOP_STAT0 (0x00E) > +#define HDMITX_TOP_STAT0 (0x038) > +#define TOP_STAT0_HPD BIT(0) > +#define TOP_STAT0_RXSENSE BIT(1) > > #endif /* __MESON_DW_HDMI_H */
On Mon 19 Aug 2024 at 18:22, Neil Armstrong <neil.armstrong@linaro.org> wrote: > On 30/07/2024 14:50, Jerome Brunet wrote: >> The Amlogic mixes direct register access and regmap ones, with several >> custom helpers. Using a single API makes rework and maintenance easier. >> Convert the Amlogic phy driver to regmap and use it to have more >> consistent >> access to the registers in the driver, with less custom helpers. Add >> register bit definitions when missing. >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> >> --- >> drivers/gpu/drm/meson/meson_dw_hdmi.c | 475 ++++++++++++-------------- >> drivers/gpu/drm/meson/meson_dw_hdmi.h | 49 +-- >> 2 files changed, 239 insertions(+), 285 deletions(-) >> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c >> b/drivers/gpu/drm/meson/meson_dw_hdmi.c >> index 47aa3e184e98..7c39e5c99043 100644 >> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c >> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c >> @@ -90,16 +90,25 @@ >> * - CEC Management >> */ >> -/* TOP Block Communication Channel */ >> -#define HDMITX_TOP_ADDR_REG 0x0 >> -#define HDMITX_TOP_DATA_REG 0x4 >> -#define HDMITX_TOP_CTRL_REG 0x8 >> -#define HDMITX_TOP_G12A_OFFSET 0x8000 >> +/* Indirect channel definition for GX */ >> +#define HDMITX_TOP_REGS 0x0 >> +#define HDMITX_DWC_REGS 0x10 >> + >> +#define GX_ADDR_OFFSET 0x0 >> +#define GX_DATA_OFFSET 0x4 >> +#define GX_CTRL_OFFSET 0x8 > > I don't see the point renaming thos defines It makes clear that indirect addressing offset are for GX SoC only, which the original define did not do > >> +#define GX_CTRL_APB3_ERRFAIL BIT(15) >> -/* Controller Communication Channel */ >> -#define HDMITX_DWC_ADDR_REG 0x10 >> -#define HDMITX_DWC_DATA_REG 0x14 >> -#define HDMITX_DWC_CTRL_REG 0x18 >> +/* >> + * NOTE: G12 use direct addressing: >> + * Ideally it should receive one memory region for each of the top >> + * and dwc register regions but fixing this would require to change >> + * the DT bindings. Doing so is a pain. Keep the region as it and work >> + * around the problem, at least for now. >> + * Future supported SoCs should properly describe the regions in the >> + * DT bindings instead of using this trick. > > well I disagree here, there's a single memory region for the HDMITX module, > and the DWC region is controlled by the TOP registers, so the DWC region > _cannot_ be accessed without correctly configuring the TOP registers. > > So I disagree strongly with this comment. Even if one cannot be accessed without configuring the other, it does not make it a single region. The regions do not even use the same register value width. That's a clear indication they are separate. Anyway, I'll remove the comment > >> + */ >> +#define HDMITX_TOP_G12A_OFFSET 0x8000 >> /* HHI Registers */ >> #define HHI_MEM_PD_REG0 0x100 /* 0x40 */ >> @@ -108,28 +117,59 @@ >> #define HHI_HDMI_PHY_CNTL1 0x3a4 /* 0xe9 */ >> #define PHY_CNTL1_INIT 0x03900000 >> #define PHY_INVERT BIT(17) >> +#define PHY_FIFOS GENMASK(3, 2) >> +#define PHY_CLOCK_EN BIT(1) >> +#define PHY_SOFT_RST BIT(0) > > Please move those changes before or after this patch > >> #define HHI_HDMI_PHY_CNTL2 0x3a8 /* 0xea */ >> #define HHI_HDMI_PHY_CNTL3 0x3ac /* 0xeb */ >> #define HHI_HDMI_PHY_CNTL4 0x3b0 /* 0xec */ >> #define HHI_HDMI_PHY_CNTL5 0x3b4 /* 0xed */ >> -static DEFINE_SPINLOCK(reg_lock); >> - >> -struct meson_dw_hdmi; >> - >> struct meson_dw_hdmi_data { >> - unsigned int (*top_read)(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr); >> - void (*top_write)(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr, unsigned int data); >> - unsigned int (*dwc_read)(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr); >> - void (*dwc_write)(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr, unsigned int data); >> + int (*reg_init)(struct device *dev); >> u32 cntl0_init; >> u32 cntl1_init; >> }; >> +static int hdmi_tx_indirect_reg_read(void *context, >> + unsigned int reg, >> + unsigned int *result) >> +{ >> + void __iomem *base = context; >> + >> + /* Must write the read address twice ... */ >> + writel(reg, base + GX_ADDR_OFFSET); >> + writel(reg, base + GX_ADDR_OFFSET); >> + >> + /* ... and read the data twice as well */ >> + *result = readl(base + GX_DATA_OFFSET); >> + *result = readl(base + GX_DATA_OFFSET); > > Why did you change the comments ? > >> + >> + return 0; >> +} >> + >> +static int hdmi_tx_indirect_reg_write(void *context, >> + unsigned int reg, >> + unsigned int val) >> +{ >> + void __iomem *base = context; >> + >> + /* Must write the read address twice ... */ >> + writel(reg, base + GX_ADDR_OFFSET); >> + writel(reg, base + GX_ADDR_OFFSET); >> + >> + /* ... but write the data only once */ >> + writel(val, base + GX_DATA_OFFSET); > > Ditto ? were they wrong ? > >> + >> + return 0; >> +} >> + >> +static const struct regmap_bus hdmi_tx_indirect_mmio = { >> + .fast_io = true, >> + .reg_read = hdmi_tx_indirect_reg_read, >> + .reg_write = hdmi_tx_indirect_reg_write, >> +}; >> + >> struct meson_dw_hdmi { >> struct dw_hdmi_plat_data dw_plat_data; >> struct meson_drm *priv; >> @@ -139,9 +179,10 @@ struct meson_dw_hdmi { >> struct reset_control *hdmitx_apb; >> struct reset_control *hdmitx_ctrl; >> struct reset_control *hdmitx_phy; >> - u32 irq_stat; >> + unsigned int irq_stat; >> struct dw_hdmi *hdmi; >> struct drm_bridge *bridge; >> + struct regmap *top; > > The name could be better, like top_regs or top_regmap That's not really consistent with priv->hhi, is it ? > >> }; >> static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi >> *dw_hdmi, >> @@ -150,136 +191,6 @@ static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi, >> return of_device_is_compatible(dw_hdmi->dev->of_node, compat); >> } >> -/* PHY (via TOP bridge) and Controller dedicated register interface */ >> - >> -static unsigned int dw_hdmi_top_read(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr) >> -{ >> - unsigned long flags; >> - unsigned int data; >> - >> - spin_lock_irqsave(®_lock, flags); >> - >> - /* ADDR must be written twice */ >> - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); >> - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); >> - >> - /* Read needs a second DATA read */ >> - data = readl(dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); >> - data = readl(dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); >> - >> - spin_unlock_irqrestore(®_lock, flags); >> - >> - return data; >> -} >> - >> -static unsigned int dw_hdmi_g12a_top_read(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr) >> -{ >> - return readl(dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET + (addr << 2)); >> -} >> - >> -static inline void dw_hdmi_top_write(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr, unsigned int data) >> -{ >> - unsigned long flags; >> - >> - spin_lock_irqsave(®_lock, flags); >> - >> - /* ADDR must be written twice */ >> - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); >> - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); >> - >> - /* Write needs single DATA write */ >> - writel(data, dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); >> - >> - spin_unlock_irqrestore(®_lock, flags); >> -} >> - >> -static inline void dw_hdmi_g12a_top_write(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr, unsigned int data) >> -{ >> - writel(data, dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET + (addr << 2)); >> -} >> - >> -/* Helper to change specific bits in PHY registers */ >> -static inline void dw_hdmi_top_write_bits(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr, >> - unsigned int mask, >> - unsigned int val) >> -{ >> - unsigned int data = dw_hdmi->data->top_read(dw_hdmi, addr); >> - >> - data &= ~mask; >> - data |= val; >> - >> - dw_hdmi->data->top_write(dw_hdmi, addr, data); >> -} >> - >> -static unsigned int dw_hdmi_dwc_read(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr) >> -{ >> - unsigned long flags; >> - unsigned int data; >> - >> - spin_lock_irqsave(®_lock, flags); >> - >> - /* ADDR must be written twice */ >> - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); >> - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); >> - >> - /* Read needs a second DATA read */ >> - data = readl(dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); >> - data = readl(dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); >> - >> - spin_unlock_irqrestore(®_lock, flags); >> - >> - return data; >> -} >> - >> -static unsigned int dw_hdmi_g12a_dwc_read(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr) >> -{ >> - return readb(dw_hdmi->hdmitx + addr); >> -} >> - >> -static inline void dw_hdmi_dwc_write(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr, unsigned int data) >> -{ >> - unsigned long flags; >> - >> - spin_lock_irqsave(®_lock, flags); >> - >> - /* ADDR must be written twice */ >> - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); >> - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); >> - >> - /* Write needs single DATA write */ >> - writel(data, dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); >> - >> - spin_unlock_irqrestore(®_lock, flags); >> -} >> - >> -static inline void dw_hdmi_g12a_dwc_write(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr, unsigned int data) >> -{ >> - writeb(data, dw_hdmi->hdmitx + addr); >> -} >> - >> -/* Helper to change specific bits in controller registers */ >> -static inline void dw_hdmi_dwc_write_bits(struct meson_dw_hdmi *dw_hdmi, >> - unsigned int addr, >> - unsigned int mask, >> - unsigned int val) >> -{ >> - unsigned int data = dw_hdmi->data->dwc_read(dw_hdmi, addr); >> - >> - data &= ~mask; >> - data |= val; >> - >> - dw_hdmi->data->dwc_write(dw_hdmi, addr, data); >> -} >> - >> /* Bridge */ >> /* Setup PHY bandwidth modes */ >> @@ -353,13 +264,15 @@ static inline void meson_dw_hdmi_phy_reset(struct meson_dw_hdmi *dw_hdmi) >> struct meson_drm *priv = dw_hdmi->priv; >> /* Enable and software reset */ >> - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0xf); >> - >> + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, >> + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, >> + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST); >> mdelay(2); >> /* Enable and unreset */ >> - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0xe); >> - >> + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, >> + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, >> + PHY_FIFOS | PHY_CLOCK_EN); >> mdelay(2); >> } >> @@ -382,27 +295,30 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, >> void *data, >> /* TMDS pattern setup */ >> if (mode->clock > 340000 && !mode_is_420) { >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01, >> - 0); >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23, >> - 0x03ff03ff); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_01, >> + 0); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_23, >> + 0x03ff03ff); >> } else { >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01, >> - 0x001f001f); >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23, >> - 0x001f001f); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_01, >> + 0x001f001f); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_23, >> + 0x001f001f); >> } >> /* Load TMDS pattern */ >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, 0x1); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, >> + TOP_TDMS_CLK_PTTN_LOAD); >> msleep(20); >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, 0x2); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, >> + TOP_TDMS_CLK_PTTN_SHFT); >> /* Setup PHY parameters */ >> meson_hdmi_phy_setup_mode(dw_hdmi, mode, mode_is_420); >> /* Disable clock, fifo, fifo_wr */ >> - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0); >> + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, >> + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, 0); >> dw_hdmi_set_high_tmds_clock_ratio(hdmi, display); >> @@ -433,8 +349,11 @@ static enum drm_connector_status >> dw_hdmi_read_hpd(struct dw_hdmi *hdmi, >> void *data) >> { >> struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data; >> + unsigned int stat; >> - return !!dw_hdmi->data->top_read(dw_hdmi, HDMITX_TOP_STAT0) ? >> + regmap_read(dw_hdmi->top, HDMITX_TOP_STAT0, &stat); >> + >> + return !!stat ? >> connector_status_connected : connector_status_disconnected; >> } >> @@ -444,17 +363,18 @@ static void dw_hdmi_setup_hpd(struct dw_hdmi >> *hdmi, >> struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data; >> /* Setup HPD Filter */ >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_HPD_FILTER, >> - (0xa << 12) | 0xa0); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_HPD_FILTER, >> + FIELD_PREP(TOP_HPD_GLITCH_WIDTH, 10) | >> + FIELD_PREP(TOP_HPD_VALID_WIDTH, 160)); >> /* Clear interrupts */ >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, >> - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, >> + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL); >> /* Unmask interrupts */ >> - dw_hdmi_top_write_bits(dw_hdmi, HDMITX_TOP_INTR_MASKN, >> - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL, >> - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL); >> + regmap_update_bits(dw_hdmi->top, HDMITX_TOP_INTR_MASKN, >> + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL, >> + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL); >> } >> static const struct dw_hdmi_phy_ops meson_dw_hdmi_phy_ops = { >> @@ -467,23 +387,22 @@ static const struct dw_hdmi_phy_ops meson_dw_hdmi_phy_ops = { >> static irqreturn_t dw_hdmi_top_irq(int irq, void *dev_id) >> { >> struct meson_dw_hdmi *dw_hdmi = dev_id; >> - u32 stat; >> + unsigned int stat; >> - stat = dw_hdmi->data->top_read(dw_hdmi, HDMITX_TOP_INTR_STAT); >> - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, stat); >> + regmap_read(dw_hdmi->top, HDMITX_TOP_INTR_STAT, &stat); >> + regmap_write(dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, stat); >> /* HPD Events, handle in the threaded interrupt handler */ >> - if (stat & (HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL)) { >> + if (stat & (TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL)) { >> dw_hdmi->irq_stat = stat; >> return IRQ_WAKE_THREAD; >> } >> /* HDMI Controller Interrupt */ >> - if (stat & 1) >> + if (stat & TOP_INTR_CORE) >> return IRQ_NONE; >> /* TOFIX Handle HDCP Interrupts */ >> - >> return IRQ_HANDLED; >> } >> @@ -494,10 +413,10 @@ static irqreturn_t dw_hdmi_top_thread_irq(int >> irq, void *dev_id) >> u32 stat = dw_hdmi->irq_stat; >> /* HPD Events */ >> - if (stat & (HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL)) { >> + if (stat & (TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL)) { >> bool hpd_connected = false; >> - if (stat & HDMITX_TOP_INTR_HPD_RISE) >> + if (stat & TOP_INTR_HPD_RISE) >> hpd_connected = true; >> dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected, >> @@ -512,63 +431,25 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id) >> return IRQ_HANDLED; >> } >> -/* DW HDMI Regmap */ >> - >> -static int meson_dw_hdmi_reg_read(void *context, unsigned int reg, >> - unsigned int *result) >> -{ >> - struct meson_dw_hdmi *dw_hdmi = context; >> - >> - *result = dw_hdmi->data->dwc_read(dw_hdmi, reg); >> - >> - return 0; >> - >> -} >> - >> -static int meson_dw_hdmi_reg_write(void *context, unsigned int reg, >> - unsigned int val) >> -{ >> - struct meson_dw_hdmi *dw_hdmi = context; >> - >> - dw_hdmi->data->dwc_write(dw_hdmi, reg, val); >> - >> - return 0; >> -} >> - >> -static const struct regmap_config meson_dw_hdmi_regmap_config = { >> - .reg_bits = 32, >> - .val_bits = 8, >> - .reg_read = meson_dw_hdmi_reg_read, >> - .reg_write = meson_dw_hdmi_reg_write, >> - .max_register = 0x10000, >> - .fast_io = true, >> -}; >> - >> -static const struct meson_dw_hdmi_data meson_dw_hdmi_gxbb_data = { >> - .top_read = dw_hdmi_top_read, >> - .top_write = dw_hdmi_top_write, >> - .dwc_read = dw_hdmi_dwc_read, >> - .dwc_write = dw_hdmi_dwc_write, >> - .cntl0_init = 0x0, >> - .cntl1_init = PHY_CNTL1_INIT | PHY_INVERT, >> +static const struct regmap_config top_gx_regmap_cfg = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .reg_shift = -2, >> + .val_bits = 32, >> + .max_register = 0x40, >> }; >> -static const struct meson_dw_hdmi_data meson_dw_hdmi_gxl_data = { >> - .top_read = dw_hdmi_top_read, >> - .top_write = dw_hdmi_top_write, >> - .dwc_read = dw_hdmi_dwc_read, >> - .dwc_write = dw_hdmi_dwc_write, >> - .cntl0_init = 0x0, >> - .cntl1_init = PHY_CNTL1_INIT, >> +static const struct regmap_config top_g12_regmap_cfg = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .val_bits = 32, >> + .max_register = 0x40, >> }; >> -static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = { >> - .top_read = dw_hdmi_g12a_top_read, >> - .top_write = dw_hdmi_g12a_top_write, >> - .dwc_read = dw_hdmi_g12a_dwc_read, >> - .dwc_write = dw_hdmi_g12a_dwc_write, >> - .cntl0_init = 0x000b4242, /* Bandgap */ >> - .cntl1_init = PHY_CNTL1_INIT, >> +static const struct regmap_config dwc_regmap_cfg = { >> + .reg_bits = 32, >> + .val_bits = 8, >> + .max_register = 0x8000, >> }; >> static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) >> @@ -581,41 +462,107 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) >> /* Bring HDMITX MEM output of power down */ >> regmap_update_bits(priv->hhi, HHI_MEM_PD_REG0, 0xff << 8, 0); >> - /* Enable APB3 fail on error */ >> - if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) { >> - writel_bits_relaxed(BIT(15), BIT(15), >> - meson_dw_hdmi->hdmitx + HDMITX_TOP_CTRL_REG); >> - writel_bits_relaxed(BIT(15), BIT(15), >> - meson_dw_hdmi->hdmitx + HDMITX_DWC_CTRL_REG); >> - } >> - >> /* Bring out of reset */ >> - meson_dw_hdmi->data->top_write(meson_dw_hdmi, >> - HDMITX_TOP_SW_RESET, 0); >> - >> + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_SW_RESET, 0); >> msleep(20); >> - meson_dw_hdmi->data->top_write(meson_dw_hdmi, >> - HDMITX_TOP_CLK_CNTL, 0xff); >> + /* Enable clocks */ >> + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_CLK_CNTL, >> + TOP_CLK_EN); >> /* Enable normal output to PHY */ >> - meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12)); >> + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_BIST_CNTL, >> + TOP_BIST_TMDS_EN); >> /* Setup PHY */ >> - regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL1, meson_dw_hdmi->data->cntl1_init); >> - regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, meson_dw_hdmi->data->cntl0_init); >> + regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL1, >> + meson_dw_hdmi->data->cntl1_init); >> + regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, >> + meson_dw_hdmi->data->cntl0_init); >> /* Enable HDMI-TX Interrupt */ >> - meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, >> - HDMITX_TOP_INTR_CORE); >> + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, >> + GENMASK(31, 0)); >> + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_INTR_MASKN, >> + TOP_INTR_CORE); >> +} >> + >> +static int meson_dw_init_regmap_gx(struct device *dev) >> +{ >> + struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev); >> + struct regmap *map; >> - meson_dw_hdmi->data->top_write(meson_dw_hdmi, >> HDMITX_TOP_INTR_MASKN, >> - HDMITX_TOP_INTR_CORE); >> + /* Register TOP glue zone */ >> + writel_bits_relaxed(GX_CTRL_APB3_ERRFAIL, GX_CTRL_APB3_ERRFAIL, >> + meson_dw_hdmi->hdmitx + HDMITX_TOP_REGS + GX_CTRL_OFFSET); >> + map = devm_regmap_init(dev, &hdmi_tx_indirect_mmio, >> + meson_dw_hdmi->hdmitx + HDMITX_TOP_REGS, >> + &top_gx_regmap_cfg); >> + if (IS_ERR(map)) >> + return dev_err_probe(dev, PTR_ERR(map), "failed to init top regmap\n"); >> + >> + meson_dw_hdmi->top = map; >> + >> + /* Register DWC zone */ >> + writel_bits_relaxed(GX_CTRL_APB3_ERRFAIL, GX_CTRL_APB3_ERRFAIL, >> + meson_dw_hdmi->hdmitx + HDMITX_DWC_REGS + GX_CTRL_OFFSET); >> + >> + map = devm_regmap_init(dev, &hdmi_tx_indirect_mmio, >> + meson_dw_hdmi->hdmitx + HDMITX_DWC_REGS, >> + &dwc_regmap_cfg); >> + if (IS_ERR(map)) >> + return dev_err_probe(dev, PTR_ERR(map), "failed to init dwc regmap\n"); >> + >> + meson_dw_hdmi->dw_plat_data.regm = map; >> + >> + return 0; >> +} >> + >> +static int meson_dw_init_regmap_g12(struct device *dev) >> +{ >> + struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev); >> + struct regmap *map; >> + >> + /* Register TOP glue zone with the offset */ >> + map = devm_regmap_init_mmio(dev, meson_dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET, >> + &top_g12_regmap_cfg); >> + if (IS_ERR(map)) >> + dev_err_probe(dev, PTR_ERR(map), "failed to init top regmap\n"); >> + >> + meson_dw_hdmi->top = map; >> + >> + /* Register DWC zone */ >> + map = devm_regmap_init_mmio(dev, meson_dw_hdmi->hdmitx, >> + &dwc_regmap_cfg); >> + if (IS_ERR(map)) >> + dev_err_probe(dev, PTR_ERR(map), "failed to init dwc regmap\n"); >> + >> + meson_dw_hdmi->dw_plat_data.regm = map; >> + >> + return 0; >> } >> +static const struct meson_dw_hdmi_data meson_dw_hdmi_gxbb_data = { >> + .reg_init = meson_dw_init_regmap_gx, >> + .cntl0_init = 0x0, >> + .cntl1_init = PHY_CNTL1_INIT | PHY_INVERT, >> +}; >> + >> +static const struct meson_dw_hdmi_data meson_dw_hdmi_gxl_data = { >> + .reg_init = meson_dw_init_regmap_gx, >> + .cntl0_init = 0x0, >> + .cntl1_init = PHY_CNTL1_INIT, >> +}; >> + >> +static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = { >> + .reg_init = meson_dw_init_regmap_g12, >> + .cntl0_init = 0x000b4242, /* Bandgap */ >> + .cntl1_init = PHY_CNTL1_INIT, >> +}; >> + >> static int meson_dw_hdmi_bind(struct device *dev, struct device *master, >> - void *data) >> + void *data) >> { >> struct platform_device *pdev = to_platform_device(dev); >> const struct meson_dw_hdmi_data *match; >> @@ -640,6 +587,8 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, >> if (!meson_dw_hdmi) >> return -ENOMEM; >> + platform_set_drvdata(pdev, meson_dw_hdmi); >> + >> meson_dw_hdmi->priv = priv; >> meson_dw_hdmi->dev = dev; >> meson_dw_hdmi->data = match; >> @@ -682,10 +631,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, >> if (ret) >> return dev_err_probe(dev, ret, "Failed to enable all clocks\n"); >> - dw_plat_data->regm = devm_regmap_init(dev, NULL, meson_dw_hdmi, >> - &meson_dw_hdmi_regmap_config); >> - if (IS_ERR(dw_plat_data->regm)) >> - return PTR_ERR(dw_plat_data->regm); >> + ret = meson_dw_hdmi->data->reg_init(dev); >> + if (ret) >> + return ret; >> irq = platform_get_irq(pdev, 0); >> if (irq < 0) >> @@ -717,8 +665,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, >> dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-g12a-dw-hdmi")) >> dw_plat_data->use_drm_infoframe = true; >> - platform_set_drvdata(pdev, meson_dw_hdmi); >> - >> meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data); >> if (IS_ERR(meson_dw_hdmi->hdmi)) >> return PTR_ERR(meson_dw_hdmi->hdmi); >> @@ -751,8 +697,7 @@ static int __maybe_unused meson_dw_hdmi_pm_suspend(struct device *dev) >> return 0; >> /* FIXME: This actually bring top out reset on suspend, why ? */ >> - meson_dw_hdmi->data->top_write(meson_dw_hdmi, >> - HDMITX_TOP_SW_RESET, 0); >> + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_SW_RESET, 0); >> return 0; >> } >> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.h b/drivers/gpu/drm/meson/meson_dw_hdmi.h >> index 08e1c14e4ea0..3ab8c56d5fe1 100644 >> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.h >> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.h >> @@ -28,6 +28,7 @@ >> * 0=Release from reset. Default 1. >> */ >> #define HDMITX_TOP_SW_RESET (0x000) >> +#define TOP_RST_EN GENMASK(4, 0) > > Well NAK, the registers are named HDMITX_TOP_XXXX in the datasheet, > and TOP_XXXX defines could collide with other too generic defines, > and in any case don't mix defines renaming with other changes. > > Just move the GENMASK in a new patch and leave the HDMITX_ prefix alone. > >> /* >> * Bit 31 RW free_clk_en: 0=Enable clock gating for power saving; 1= Disable >> @@ -45,7 +46,8 @@ >> * Bit 1 RW tmds_clk_en: 1=enable tmds_clk; 0=disable. Default 0. >> * Bit 0 RW pixel_clk_en: 1=enable pixel_clk; 0=disable. Default 0. >> */ >> -#define HDMITX_TOP_CLK_CNTL (0x001) >> +#define HDMITX_TOP_CLK_CNTL (0x004) >> +#define TOP_CLK_EN GENMASK(7, 0) >> /* >> * Bit 31:28 RW rxsense_glitch_width: starting from G12A >> @@ -53,7 +55,9 @@ >> * Bit 11: 0 RW hpd_valid_width: filter out width <= M*1024. Default 0. >> * Bit 15:12 RW hpd_glitch_width: filter out glitch <= N. Default 0. >> */ >> -#define HDMITX_TOP_HPD_FILTER (0x002) >> +#define HDMITX_TOP_HPD_FILTER (0x008) >> +#define TOP_HPD_GLITCH_WIDTH GENMASK(15, 12) >> +#define TOP_HPD_VALID_WIDTH GENMASK(11, 0) >> /* >> * intr_maskn: MASK_N, one bit per interrupt source. >> @@ -67,7 +71,7 @@ >> * [ 1] hpd_rise_intr >> * [ 0] core_intr >> */ >> -#define HDMITX_TOP_INTR_MASKN (0x003) >> +#define HDMITX_TOP_INTR_MASKN (0x00c) >> /* >> * Bit 30: 0 RW intr_stat: For each bit, write 1 to manually set the interrupt >> @@ -80,7 +84,7 @@ >> * Bit 1 RW hpd_rise >> * Bit 0 RW IP interrupt >> */ >> -#define HDMITX_TOP_INTR_STAT (0x004) >> +#define HDMITX_TOP_INTR_STAT (0x010) >> /* >> * [7] rxsense_fall starting from G12A >> @@ -92,13 +96,12 @@ >> * [1] hpd_rise >> * [0] core_intr_rise >> */ >> -#define HDMITX_TOP_INTR_STAT_CLR (0x005) >> - >> -#define HDMITX_TOP_INTR_CORE BIT(0) >> -#define HDMITX_TOP_INTR_HPD_RISE BIT(1) >> -#define HDMITX_TOP_INTR_HPD_FALL BIT(2) >> -#define HDMITX_TOP_INTR_RXSENSE_RISE BIT(6) >> -#define HDMITX_TOP_INTR_RXSENSE_FALL BIT(7) >> +#define HDMITX_TOP_INTR_STAT_CLR (0x014) >> +#define TOP_INTR_CORE BIT(0) >> +#define TOP_INTR_HPD_RISE BIT(1) >> +#define TOP_INTR_HPD_FALL BIT(2) >> +#define TOP_INTR_RXSENSE_RISE BIT(6) >> +#define TOP_INTR_RXSENSE_FALL BIT(7) >> /* >> * Bit 14:12 RW tmds_sel: 3'b000=Output zero; 3'b001=Output normal TMDS data; >> @@ -112,29 +115,31 @@ >> * 2=Output 1-bit pattern; 3=output 10-bit pattern. Default 0. >> * Bit 0 RW prbs_pttn_en: 1=Enable PRBS generator; 0=Disable. Default 0. >> */ >> -#define HDMITX_TOP_BIST_CNTL (0x006) >> +#define HDMITX_TOP_BIST_CNTL (0x018) >> +#define TOP_BIST_OUT_MASK GENMASK(14, 12) >> +#define TOP_BIST_TMDS_EN BIT(12) >> /* Bit 29:20 RW shift_pttn_data[59:50]. Default 0. */ >> /* Bit 19:10 RW shift_pttn_data[69:60]. Default 0. */ >> /* Bit 9: 0 RW shift_pttn_data[79:70]. Default 0. */ >> -#define HDMITX_TOP_SHIFT_PTTN_012 (0x007) >> +#define HDMITX_TOP_SHIFT_PTTN_012 (0x01c) >> /* Bit 29:20 RW shift_pttn_data[29:20]. Default 0. */ >> /* Bit 19:10 RW shift_pttn_data[39:30]. Default 0. */ >> /* Bit 9: 0 RW shift_pttn_data[49:40]. Default 0. */ >> -#define HDMITX_TOP_SHIFT_PTTN_345 (0x008) >> +#define HDMITX_TOP_SHIFT_PTTN_345 (0x020) >> /* Bit 19:10 RW shift_pttn_data[ 9: 0]. Default 0. */ >> /* Bit 9: 0 RW shift_pttn_data[19:10]. Default 0. */ >> -#define HDMITX_TOP_SHIFT_PTTN_67 (0x009) >> +#define HDMITX_TOP_SHIFT_PTTN_67 (0x024) >> /* Bit 25:16 RW tmds_clk_pttn[19:10]. Default 0. */ >> /* Bit 9: 0 RW tmds_clk_pttn[ 9: 0]. Default 0. */ >> -#define HDMITX_TOP_TMDS_CLK_PTTN_01 (0x00A) >> +#define HDMITX_TOP_TMDS_CLK_PTTN_01 (0x028) >> /* Bit 25:16 RW tmds_clk_pttn[39:30]. Default 0. */ >> /* Bit 9: 0 RW tmds_clk_pttn[29:20]. Default 0. */ >> -#define HDMITX_TOP_TMDS_CLK_PTTN_23 (0x00B) >> +#define HDMITX_TOP_TMDS_CLK_PTTN_23 (0x02c) >> /* >> * Bit 1 RW shift_tmds_clk_pttn:1=Enable shifting clk pattern, >> @@ -143,18 +148,22 @@ >> * [ 1] shift_tmds_clk_pttn >> * [ 0] load_tmds_clk_pttn >> */ >> -#define HDMITX_TOP_TMDS_CLK_PTTN_CNTL (0x00C) >> +#define HDMITX_TOP_TMDS_CLK_PTTN_CNTL (0x030) >> +#define TOP_TDMS_CLK_PTTN_LOAD BIT(0) >> +#define TOP_TDMS_CLK_PTTN_SHFT BIT(1) >> /* >> * Bit 0 RW revocmem_wr_fail: Read back 1 to indicate Host write REVOC MEM >> * failure, write 1 to clear the failure flag. Default 0. >> */ >> -#define HDMITX_TOP_REVOCMEM_STAT (0x00D) >> +#define HDMITX_TOP_REVOCMEM_STAT (0x034) >> /* >> * Bit 1 R filtered RxSense status >> * Bit 0 R filtered HPD status. >> */ >> -#define HDMITX_TOP_STAT0 (0x00E) >> +#define HDMITX_TOP_STAT0 (0x038) >> +#define TOP_STAT0_HPD BIT(0) >> +#define TOP_STAT0_RXSENSE BIT(1) >> #endif /* __MESON_DW_HDMI_H */
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c index 47aa3e184e98..7c39e5c99043 100644 --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c @@ -90,16 +90,25 @@ * - CEC Management */ -/* TOP Block Communication Channel */ -#define HDMITX_TOP_ADDR_REG 0x0 -#define HDMITX_TOP_DATA_REG 0x4 -#define HDMITX_TOP_CTRL_REG 0x8 -#define HDMITX_TOP_G12A_OFFSET 0x8000 +/* Indirect channel definition for GX */ +#define HDMITX_TOP_REGS 0x0 +#define HDMITX_DWC_REGS 0x10 + +#define GX_ADDR_OFFSET 0x0 +#define GX_DATA_OFFSET 0x4 +#define GX_CTRL_OFFSET 0x8 +#define GX_CTRL_APB3_ERRFAIL BIT(15) -/* Controller Communication Channel */ -#define HDMITX_DWC_ADDR_REG 0x10 -#define HDMITX_DWC_DATA_REG 0x14 -#define HDMITX_DWC_CTRL_REG 0x18 +/* + * NOTE: G12 use direct addressing: + * Ideally it should receive one memory region for each of the top + * and dwc register regions but fixing this would require to change + * the DT bindings. Doing so is a pain. Keep the region as it and work + * around the problem, at least for now. + * Future supported SoCs should properly describe the regions in the + * DT bindings instead of using this trick. + */ +#define HDMITX_TOP_G12A_OFFSET 0x8000 /* HHI Registers */ #define HHI_MEM_PD_REG0 0x100 /* 0x40 */ @@ -108,28 +117,59 @@ #define HHI_HDMI_PHY_CNTL1 0x3a4 /* 0xe9 */ #define PHY_CNTL1_INIT 0x03900000 #define PHY_INVERT BIT(17) +#define PHY_FIFOS GENMASK(3, 2) +#define PHY_CLOCK_EN BIT(1) +#define PHY_SOFT_RST BIT(0) #define HHI_HDMI_PHY_CNTL2 0x3a8 /* 0xea */ #define HHI_HDMI_PHY_CNTL3 0x3ac /* 0xeb */ #define HHI_HDMI_PHY_CNTL4 0x3b0 /* 0xec */ #define HHI_HDMI_PHY_CNTL5 0x3b4 /* 0xed */ -static DEFINE_SPINLOCK(reg_lock); - -struct meson_dw_hdmi; - struct meson_dw_hdmi_data { - unsigned int (*top_read)(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr); - void (*top_write)(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr, unsigned int data); - unsigned int (*dwc_read)(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr); - void (*dwc_write)(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr, unsigned int data); + int (*reg_init)(struct device *dev); u32 cntl0_init; u32 cntl1_init; }; +static int hdmi_tx_indirect_reg_read(void *context, + unsigned int reg, + unsigned int *result) +{ + void __iomem *base = context; + + /* Must write the read address twice ... */ + writel(reg, base + GX_ADDR_OFFSET); + writel(reg, base + GX_ADDR_OFFSET); + + /* ... and read the data twice as well */ + *result = readl(base + GX_DATA_OFFSET); + *result = readl(base + GX_DATA_OFFSET); + + return 0; +} + +static int hdmi_tx_indirect_reg_write(void *context, + unsigned int reg, + unsigned int val) +{ + void __iomem *base = context; + + /* Must write the read address twice ... */ + writel(reg, base + GX_ADDR_OFFSET); + writel(reg, base + GX_ADDR_OFFSET); + + /* ... but write the data only once */ + writel(val, base + GX_DATA_OFFSET); + + return 0; +} + +static const struct regmap_bus hdmi_tx_indirect_mmio = { + .fast_io = true, + .reg_read = hdmi_tx_indirect_reg_read, + .reg_write = hdmi_tx_indirect_reg_write, +}; + struct meson_dw_hdmi { struct dw_hdmi_plat_data dw_plat_data; struct meson_drm *priv; @@ -139,9 +179,10 @@ struct meson_dw_hdmi { struct reset_control *hdmitx_apb; struct reset_control *hdmitx_ctrl; struct reset_control *hdmitx_phy; - u32 irq_stat; + unsigned int irq_stat; struct dw_hdmi *hdmi; struct drm_bridge *bridge; + struct regmap *top; }; static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi, @@ -150,136 +191,6 @@ static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi, return of_device_is_compatible(dw_hdmi->dev->of_node, compat); } -/* PHY (via TOP bridge) and Controller dedicated register interface */ - -static unsigned int dw_hdmi_top_read(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr) -{ - unsigned long flags; - unsigned int data; - - spin_lock_irqsave(®_lock, flags); - - /* ADDR must be written twice */ - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); - - /* Read needs a second DATA read */ - data = readl(dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); - data = readl(dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); - - spin_unlock_irqrestore(®_lock, flags); - - return data; -} - -static unsigned int dw_hdmi_g12a_top_read(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr) -{ - return readl(dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET + (addr << 2)); -} - -static inline void dw_hdmi_top_write(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr, unsigned int data) -{ - unsigned long flags; - - spin_lock_irqsave(®_lock, flags); - - /* ADDR must be written twice */ - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_TOP_ADDR_REG); - - /* Write needs single DATA write */ - writel(data, dw_hdmi->hdmitx + HDMITX_TOP_DATA_REG); - - spin_unlock_irqrestore(®_lock, flags); -} - -static inline void dw_hdmi_g12a_top_write(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr, unsigned int data) -{ - writel(data, dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET + (addr << 2)); -} - -/* Helper to change specific bits in PHY registers */ -static inline void dw_hdmi_top_write_bits(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr, - unsigned int mask, - unsigned int val) -{ - unsigned int data = dw_hdmi->data->top_read(dw_hdmi, addr); - - data &= ~mask; - data |= val; - - dw_hdmi->data->top_write(dw_hdmi, addr, data); -} - -static unsigned int dw_hdmi_dwc_read(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr) -{ - unsigned long flags; - unsigned int data; - - spin_lock_irqsave(®_lock, flags); - - /* ADDR must be written twice */ - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); - - /* Read needs a second DATA read */ - data = readl(dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); - data = readl(dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); - - spin_unlock_irqrestore(®_lock, flags); - - return data; -} - -static unsigned int dw_hdmi_g12a_dwc_read(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr) -{ - return readb(dw_hdmi->hdmitx + addr); -} - -static inline void dw_hdmi_dwc_write(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr, unsigned int data) -{ - unsigned long flags; - - spin_lock_irqsave(®_lock, flags); - - /* ADDR must be written twice */ - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); - writel(addr & 0xffff, dw_hdmi->hdmitx + HDMITX_DWC_ADDR_REG); - - /* Write needs single DATA write */ - writel(data, dw_hdmi->hdmitx + HDMITX_DWC_DATA_REG); - - spin_unlock_irqrestore(®_lock, flags); -} - -static inline void dw_hdmi_g12a_dwc_write(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr, unsigned int data) -{ - writeb(data, dw_hdmi->hdmitx + addr); -} - -/* Helper to change specific bits in controller registers */ -static inline void dw_hdmi_dwc_write_bits(struct meson_dw_hdmi *dw_hdmi, - unsigned int addr, - unsigned int mask, - unsigned int val) -{ - unsigned int data = dw_hdmi->data->dwc_read(dw_hdmi, addr); - - data &= ~mask; - data |= val; - - dw_hdmi->data->dwc_write(dw_hdmi, addr, data); -} - /* Bridge */ /* Setup PHY bandwidth modes */ @@ -353,13 +264,15 @@ static inline void meson_dw_hdmi_phy_reset(struct meson_dw_hdmi *dw_hdmi) struct meson_drm *priv = dw_hdmi->priv; /* Enable and software reset */ - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0xf); - + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST); mdelay(2); /* Enable and unreset */ - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0xe); - + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, + PHY_FIFOS | PHY_CLOCK_EN); mdelay(2); } @@ -382,27 +295,30 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data, /* TMDS pattern setup */ if (mode->clock > 340000 && !mode_is_420) { - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01, - 0); - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23, - 0x03ff03ff); + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_01, + 0); + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_23, + 0x03ff03ff); } else { - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01, - 0x001f001f); - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23, - 0x001f001f); + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_01, + 0x001f001f); + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_23, + 0x001f001f); } /* Load TMDS pattern */ - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, 0x1); + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, + TOP_TDMS_CLK_PTTN_LOAD); msleep(20); - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, 0x2); + regmap_write(dw_hdmi->top, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, + TOP_TDMS_CLK_PTTN_SHFT); /* Setup PHY parameters */ meson_hdmi_phy_setup_mode(dw_hdmi, mode, mode_is_420); /* Disable clock, fifo, fifo_wr */ - regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, 0xf, 0); + regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1, + PHY_FIFOS | PHY_CLOCK_EN | PHY_SOFT_RST, 0); dw_hdmi_set_high_tmds_clock_ratio(hdmi, display); @@ -433,8 +349,11 @@ static enum drm_connector_status dw_hdmi_read_hpd(struct dw_hdmi *hdmi, void *data) { struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data; + unsigned int stat; - return !!dw_hdmi->data->top_read(dw_hdmi, HDMITX_TOP_STAT0) ? + regmap_read(dw_hdmi->top, HDMITX_TOP_STAT0, &stat); + + return !!stat ? connector_status_connected : connector_status_disconnected; } @@ -444,17 +363,18 @@ static void dw_hdmi_setup_hpd(struct dw_hdmi *hdmi, struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data; /* Setup HPD Filter */ - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_HPD_FILTER, - (0xa << 12) | 0xa0); + regmap_write(dw_hdmi->top, HDMITX_TOP_HPD_FILTER, + FIELD_PREP(TOP_HPD_GLITCH_WIDTH, 10) | + FIELD_PREP(TOP_HPD_VALID_WIDTH, 160)); /* Clear interrupts */ - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL); + regmap_write(dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL); /* Unmask interrupts */ - dw_hdmi_top_write_bits(dw_hdmi, HDMITX_TOP_INTR_MASKN, - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL, - HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL); + regmap_update_bits(dw_hdmi->top, HDMITX_TOP_INTR_MASKN, + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL, + TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL); } static const struct dw_hdmi_phy_ops meson_dw_hdmi_phy_ops = { @@ -467,23 +387,22 @@ static const struct dw_hdmi_phy_ops meson_dw_hdmi_phy_ops = { static irqreturn_t dw_hdmi_top_irq(int irq, void *dev_id) { struct meson_dw_hdmi *dw_hdmi = dev_id; - u32 stat; + unsigned int stat; - stat = dw_hdmi->data->top_read(dw_hdmi, HDMITX_TOP_INTR_STAT); - dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, stat); + regmap_read(dw_hdmi->top, HDMITX_TOP_INTR_STAT, &stat); + regmap_write(dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, stat); /* HPD Events, handle in the threaded interrupt handler */ - if (stat & (HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL)) { + if (stat & (TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL)) { dw_hdmi->irq_stat = stat; return IRQ_WAKE_THREAD; } /* HDMI Controller Interrupt */ - if (stat & 1) + if (stat & TOP_INTR_CORE) return IRQ_NONE; /* TOFIX Handle HDCP Interrupts */ - return IRQ_HANDLED; } @@ -494,10 +413,10 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id) u32 stat = dw_hdmi->irq_stat; /* HPD Events */ - if (stat & (HDMITX_TOP_INTR_HPD_RISE | HDMITX_TOP_INTR_HPD_FALL)) { + if (stat & (TOP_INTR_HPD_RISE | TOP_INTR_HPD_FALL)) { bool hpd_connected = false; - if (stat & HDMITX_TOP_INTR_HPD_RISE) + if (stat & TOP_INTR_HPD_RISE) hpd_connected = true; dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected, @@ -512,63 +431,25 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id) return IRQ_HANDLED; } -/* DW HDMI Regmap */ - -static int meson_dw_hdmi_reg_read(void *context, unsigned int reg, - unsigned int *result) -{ - struct meson_dw_hdmi *dw_hdmi = context; - - *result = dw_hdmi->data->dwc_read(dw_hdmi, reg); - - return 0; - -} - -static int meson_dw_hdmi_reg_write(void *context, unsigned int reg, - unsigned int val) -{ - struct meson_dw_hdmi *dw_hdmi = context; - - dw_hdmi->data->dwc_write(dw_hdmi, reg, val); - - return 0; -} - -static const struct regmap_config meson_dw_hdmi_regmap_config = { - .reg_bits = 32, - .val_bits = 8, - .reg_read = meson_dw_hdmi_reg_read, - .reg_write = meson_dw_hdmi_reg_write, - .max_register = 0x10000, - .fast_io = true, -}; - -static const struct meson_dw_hdmi_data meson_dw_hdmi_gxbb_data = { - .top_read = dw_hdmi_top_read, - .top_write = dw_hdmi_top_write, - .dwc_read = dw_hdmi_dwc_read, - .dwc_write = dw_hdmi_dwc_write, - .cntl0_init = 0x0, - .cntl1_init = PHY_CNTL1_INIT | PHY_INVERT, +static const struct regmap_config top_gx_regmap_cfg = { + .reg_bits = 32, + .reg_stride = 4, + .reg_shift = -2, + .val_bits = 32, + .max_register = 0x40, }; -static const struct meson_dw_hdmi_data meson_dw_hdmi_gxl_data = { - .top_read = dw_hdmi_top_read, - .top_write = dw_hdmi_top_write, - .dwc_read = dw_hdmi_dwc_read, - .dwc_write = dw_hdmi_dwc_write, - .cntl0_init = 0x0, - .cntl1_init = PHY_CNTL1_INIT, +static const struct regmap_config top_g12_regmap_cfg = { + .reg_bits = 32, + .reg_stride = 4, + .val_bits = 32, + .max_register = 0x40, }; -static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = { - .top_read = dw_hdmi_g12a_top_read, - .top_write = dw_hdmi_g12a_top_write, - .dwc_read = dw_hdmi_g12a_dwc_read, - .dwc_write = dw_hdmi_g12a_dwc_write, - .cntl0_init = 0x000b4242, /* Bandgap */ - .cntl1_init = PHY_CNTL1_INIT, +static const struct regmap_config dwc_regmap_cfg = { + .reg_bits = 32, + .val_bits = 8, + .max_register = 0x8000, }; static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) @@ -581,41 +462,107 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) /* Bring HDMITX MEM output of power down */ regmap_update_bits(priv->hhi, HHI_MEM_PD_REG0, 0xff << 8, 0); - /* Enable APB3 fail on error */ - if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) { - writel_bits_relaxed(BIT(15), BIT(15), - meson_dw_hdmi->hdmitx + HDMITX_TOP_CTRL_REG); - writel_bits_relaxed(BIT(15), BIT(15), - meson_dw_hdmi->hdmitx + HDMITX_DWC_CTRL_REG); - } - /* Bring out of reset */ - meson_dw_hdmi->data->top_write(meson_dw_hdmi, - HDMITX_TOP_SW_RESET, 0); - + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_SW_RESET, 0); msleep(20); - meson_dw_hdmi->data->top_write(meson_dw_hdmi, - HDMITX_TOP_CLK_CNTL, 0xff); + /* Enable clocks */ + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_CLK_CNTL, + TOP_CLK_EN); /* Enable normal output to PHY */ - meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12)); + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_BIST_CNTL, + TOP_BIST_TMDS_EN); /* Setup PHY */ - regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL1, meson_dw_hdmi->data->cntl1_init); - regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, meson_dw_hdmi->data->cntl0_init); + regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL1, + meson_dw_hdmi->data->cntl1_init); + regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, + meson_dw_hdmi->data->cntl0_init); /* Enable HDMI-TX Interrupt */ - meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_INTR_STAT_CLR, - HDMITX_TOP_INTR_CORE); + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_INTR_STAT_CLR, + GENMASK(31, 0)); + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_INTR_MASKN, + TOP_INTR_CORE); +} + +static int meson_dw_init_regmap_gx(struct device *dev) +{ + struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev); + struct regmap *map; - meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_INTR_MASKN, - HDMITX_TOP_INTR_CORE); + /* Register TOP glue zone */ + writel_bits_relaxed(GX_CTRL_APB3_ERRFAIL, GX_CTRL_APB3_ERRFAIL, + meson_dw_hdmi->hdmitx + HDMITX_TOP_REGS + GX_CTRL_OFFSET); + map = devm_regmap_init(dev, &hdmi_tx_indirect_mmio, + meson_dw_hdmi->hdmitx + HDMITX_TOP_REGS, + &top_gx_regmap_cfg); + if (IS_ERR(map)) + return dev_err_probe(dev, PTR_ERR(map), "failed to init top regmap\n"); + + meson_dw_hdmi->top = map; + + /* Register DWC zone */ + writel_bits_relaxed(GX_CTRL_APB3_ERRFAIL, GX_CTRL_APB3_ERRFAIL, + meson_dw_hdmi->hdmitx + HDMITX_DWC_REGS + GX_CTRL_OFFSET); + + map = devm_regmap_init(dev, &hdmi_tx_indirect_mmio, + meson_dw_hdmi->hdmitx + HDMITX_DWC_REGS, + &dwc_regmap_cfg); + if (IS_ERR(map)) + return dev_err_probe(dev, PTR_ERR(map), "failed to init dwc regmap\n"); + + meson_dw_hdmi->dw_plat_data.regm = map; + + return 0; +} + +static int meson_dw_init_regmap_g12(struct device *dev) +{ + struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev); + struct regmap *map; + + /* Register TOP glue zone with the offset */ + map = devm_regmap_init_mmio(dev, meson_dw_hdmi->hdmitx + HDMITX_TOP_G12A_OFFSET, + &top_g12_regmap_cfg); + if (IS_ERR(map)) + dev_err_probe(dev, PTR_ERR(map), "failed to init top regmap\n"); + + meson_dw_hdmi->top = map; + + /* Register DWC zone */ + map = devm_regmap_init_mmio(dev, meson_dw_hdmi->hdmitx, + &dwc_regmap_cfg); + if (IS_ERR(map)) + dev_err_probe(dev, PTR_ERR(map), "failed to init dwc regmap\n"); + + meson_dw_hdmi->dw_plat_data.regm = map; + + return 0; } +static const struct meson_dw_hdmi_data meson_dw_hdmi_gxbb_data = { + .reg_init = meson_dw_init_regmap_gx, + .cntl0_init = 0x0, + .cntl1_init = PHY_CNTL1_INIT | PHY_INVERT, +}; + +static const struct meson_dw_hdmi_data meson_dw_hdmi_gxl_data = { + .reg_init = meson_dw_init_regmap_gx, + .cntl0_init = 0x0, + .cntl1_init = PHY_CNTL1_INIT, +}; + +static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = { + .reg_init = meson_dw_init_regmap_g12, + .cntl0_init = 0x000b4242, /* Bandgap */ + .cntl1_init = PHY_CNTL1_INIT, +}; + static int meson_dw_hdmi_bind(struct device *dev, struct device *master, - void *data) + void *data) { struct platform_device *pdev = to_platform_device(dev); const struct meson_dw_hdmi_data *match; @@ -640,6 +587,8 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, if (!meson_dw_hdmi) return -ENOMEM; + platform_set_drvdata(pdev, meson_dw_hdmi); + meson_dw_hdmi->priv = priv; meson_dw_hdmi->dev = dev; meson_dw_hdmi->data = match; @@ -682,10 +631,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, if (ret) return dev_err_probe(dev, ret, "Failed to enable all clocks\n"); - dw_plat_data->regm = devm_regmap_init(dev, NULL, meson_dw_hdmi, - &meson_dw_hdmi_regmap_config); - if (IS_ERR(dw_plat_data->regm)) - return PTR_ERR(dw_plat_data->regm); + ret = meson_dw_hdmi->data->reg_init(dev); + if (ret) + return ret; irq = platform_get_irq(pdev, 0); if (irq < 0) @@ -717,8 +665,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-g12a-dw-hdmi")) dw_plat_data->use_drm_infoframe = true; - platform_set_drvdata(pdev, meson_dw_hdmi); - meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data); if (IS_ERR(meson_dw_hdmi->hdmi)) return PTR_ERR(meson_dw_hdmi->hdmi); @@ -751,8 +697,7 @@ static int __maybe_unused meson_dw_hdmi_pm_suspend(struct device *dev) return 0; /* FIXME: This actually bring top out reset on suspend, why ? */ - meson_dw_hdmi->data->top_write(meson_dw_hdmi, - HDMITX_TOP_SW_RESET, 0); + regmap_write(meson_dw_hdmi->top, HDMITX_TOP_SW_RESET, 0); return 0; } diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.h b/drivers/gpu/drm/meson/meson_dw_hdmi.h index 08e1c14e4ea0..3ab8c56d5fe1 100644 --- a/drivers/gpu/drm/meson/meson_dw_hdmi.h +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.h @@ -28,6 +28,7 @@ * 0=Release from reset. Default 1. */ #define HDMITX_TOP_SW_RESET (0x000) +#define TOP_RST_EN GENMASK(4, 0) /* * Bit 31 RW free_clk_en: 0=Enable clock gating for power saving; 1= Disable @@ -45,7 +46,8 @@ * Bit 1 RW tmds_clk_en: 1=enable tmds_clk; 0=disable. Default 0. * Bit 0 RW pixel_clk_en: 1=enable pixel_clk; 0=disable. Default 0. */ -#define HDMITX_TOP_CLK_CNTL (0x001) +#define HDMITX_TOP_CLK_CNTL (0x004) +#define TOP_CLK_EN GENMASK(7, 0) /* * Bit 31:28 RW rxsense_glitch_width: starting from G12A @@ -53,7 +55,9 @@ * Bit 11: 0 RW hpd_valid_width: filter out width <= M*1024. Default 0. * Bit 15:12 RW hpd_glitch_width: filter out glitch <= N. Default 0. */ -#define HDMITX_TOP_HPD_FILTER (0x002) +#define HDMITX_TOP_HPD_FILTER (0x008) +#define TOP_HPD_GLITCH_WIDTH GENMASK(15, 12) +#define TOP_HPD_VALID_WIDTH GENMASK(11, 0) /* * intr_maskn: MASK_N, one bit per interrupt source. @@ -67,7 +71,7 @@ * [ 1] hpd_rise_intr * [ 0] core_intr */ -#define HDMITX_TOP_INTR_MASKN (0x003) +#define HDMITX_TOP_INTR_MASKN (0x00c) /* * Bit 30: 0 RW intr_stat: For each bit, write 1 to manually set the interrupt @@ -80,7 +84,7 @@ * Bit 1 RW hpd_rise * Bit 0 RW IP interrupt */ -#define HDMITX_TOP_INTR_STAT (0x004) +#define HDMITX_TOP_INTR_STAT (0x010) /* * [7] rxsense_fall starting from G12A @@ -92,13 +96,12 @@ * [1] hpd_rise * [0] core_intr_rise */ -#define HDMITX_TOP_INTR_STAT_CLR (0x005) - -#define HDMITX_TOP_INTR_CORE BIT(0) -#define HDMITX_TOP_INTR_HPD_RISE BIT(1) -#define HDMITX_TOP_INTR_HPD_FALL BIT(2) -#define HDMITX_TOP_INTR_RXSENSE_RISE BIT(6) -#define HDMITX_TOP_INTR_RXSENSE_FALL BIT(7) +#define HDMITX_TOP_INTR_STAT_CLR (0x014) +#define TOP_INTR_CORE BIT(0) +#define TOP_INTR_HPD_RISE BIT(1) +#define TOP_INTR_HPD_FALL BIT(2) +#define TOP_INTR_RXSENSE_RISE BIT(6) +#define TOP_INTR_RXSENSE_FALL BIT(7) /* * Bit 14:12 RW tmds_sel: 3'b000=Output zero; 3'b001=Output normal TMDS data; @@ -112,29 +115,31 @@ * 2=Output 1-bit pattern; 3=output 10-bit pattern. Default 0. * Bit 0 RW prbs_pttn_en: 1=Enable PRBS generator; 0=Disable. Default 0. */ -#define HDMITX_TOP_BIST_CNTL (0x006) +#define HDMITX_TOP_BIST_CNTL (0x018) +#define TOP_BIST_OUT_MASK GENMASK(14, 12) +#define TOP_BIST_TMDS_EN BIT(12) /* Bit 29:20 RW shift_pttn_data[59:50]. Default 0. */ /* Bit 19:10 RW shift_pttn_data[69:60]. Default 0. */ /* Bit 9: 0 RW shift_pttn_data[79:70]. Default 0. */ -#define HDMITX_TOP_SHIFT_PTTN_012 (0x007) +#define HDMITX_TOP_SHIFT_PTTN_012 (0x01c) /* Bit 29:20 RW shift_pttn_data[29:20]. Default 0. */ /* Bit 19:10 RW shift_pttn_data[39:30]. Default 0. */ /* Bit 9: 0 RW shift_pttn_data[49:40]. Default 0. */ -#define HDMITX_TOP_SHIFT_PTTN_345 (0x008) +#define HDMITX_TOP_SHIFT_PTTN_345 (0x020) /* Bit 19:10 RW shift_pttn_data[ 9: 0]. Default 0. */ /* Bit 9: 0 RW shift_pttn_data[19:10]. Default 0. */ -#define HDMITX_TOP_SHIFT_PTTN_67 (0x009) +#define HDMITX_TOP_SHIFT_PTTN_67 (0x024) /* Bit 25:16 RW tmds_clk_pttn[19:10]. Default 0. */ /* Bit 9: 0 RW tmds_clk_pttn[ 9: 0]. Default 0. */ -#define HDMITX_TOP_TMDS_CLK_PTTN_01 (0x00A) +#define HDMITX_TOP_TMDS_CLK_PTTN_01 (0x028) /* Bit 25:16 RW tmds_clk_pttn[39:30]. Default 0. */ /* Bit 9: 0 RW tmds_clk_pttn[29:20]. Default 0. */ -#define HDMITX_TOP_TMDS_CLK_PTTN_23 (0x00B) +#define HDMITX_TOP_TMDS_CLK_PTTN_23 (0x02c) /* * Bit 1 RW shift_tmds_clk_pttn:1=Enable shifting clk pattern, @@ -143,18 +148,22 @@ * [ 1] shift_tmds_clk_pttn * [ 0] load_tmds_clk_pttn */ -#define HDMITX_TOP_TMDS_CLK_PTTN_CNTL (0x00C) +#define HDMITX_TOP_TMDS_CLK_PTTN_CNTL (0x030) +#define TOP_TDMS_CLK_PTTN_LOAD BIT(0) +#define TOP_TDMS_CLK_PTTN_SHFT BIT(1) /* * Bit 0 RW revocmem_wr_fail: Read back 1 to indicate Host write REVOC MEM * failure, write 1 to clear the failure flag. Default 0. */ -#define HDMITX_TOP_REVOCMEM_STAT (0x00D) +#define HDMITX_TOP_REVOCMEM_STAT (0x034) /* * Bit 1 R filtered RxSense status * Bit 0 R filtered HPD status. */ -#define HDMITX_TOP_STAT0 (0x00E) +#define HDMITX_TOP_STAT0 (0x038) +#define TOP_STAT0_HPD BIT(0) +#define TOP_STAT0_RXSENSE BIT(1) #endif /* __MESON_DW_HDMI_H */
The Amlogic mixes direct register access and regmap ones, with several custom helpers. Using a single API makes rework and maintenance easier. Convert the Amlogic phy driver to regmap and use it to have more consistent access to the registers in the driver, with less custom helpers. Add register bit definitions when missing. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/gpu/drm/meson/meson_dw_hdmi.c | 475 ++++++++++++-------------- drivers/gpu/drm/meson/meson_dw_hdmi.h | 49 +-- 2 files changed, 239 insertions(+), 285 deletions(-)