| Message ID | 20241225103741.364597-2-andyshrk@163.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add HDMI support for rk3576 | expand |
Hi Andy, Am Mittwoch, 25. Dezember 2024, 11:37:29 CET schrieb Andy Yan: > From: Andy Yan <andy.yan@rock-chips.com> > > There are some control bits for IO and interrupts status scattered > across different GRF on differt SOC. > Add platform callback for this IO setting and interrupts status > handling. > > Signed-off-by: Andy Yan <andy.yan@rock-chips.com> > > --- > > .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 81 ++++++++++++------- > 1 file changed, 54 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > index c36fc130b734..b21e868e7c16 100644 > --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > @@ -62,6 +62,12 @@ struct rockchip_hdmi_qp { > int port_id; > }; > > +struct rockchip_hdmi_qp_ctrl_ops { > + void (*io_init)(struct rockchip_hdmi_qp *hdmi); > + irqreturn_t (*irq_callback)(int irq, void *dev_id); > + irqreturn_t (*hardirq_callback)(int irq, void *dev_id); > +}; > + > static struct rockchip_hdmi_qp *to_rockchip_hdmi_qp(struct drm_encoder *encoder) > { > struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder); > @@ -226,9 +232,47 @@ static irqreturn_t dw_hdmi_qp_rk3588_irq(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +static void dw_hdmi_qp_rk3588_io_init(struct rockchip_hdmi_qp *hdmi) > +{ > + u32 val; > + > + val = HIWORD_UPDATE(RK3588_SCLIN_MASK, RK3588_SCLIN_MASK) | > + HIWORD_UPDATE(RK3588_SDAIN_MASK, RK3588_SDAIN_MASK) | > + HIWORD_UPDATE(RK3588_MODE_MASK, RK3588_MODE_MASK) | > + HIWORD_UPDATE(RK3588_I2S_SEL_MASK, RK3588_I2S_SEL_MASK); > + nit: below val = ... and regmap_write don't have a blank line between them which makes sense to show that they belong together. So the blank above can probably also go away. > + regmap_write(hdmi->vo_regmap, > + hdmi->port_id ? RK3588_GRF_VO1_CON6 : RK3588_GRF_VO1_CON3, > + val); > + > + val = HIWORD_UPDATE(RK3588_SET_HPD_PATH_MASK, RK3588_SET_HPD_PATH_MASK); > + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON7, val); > + > + if (hdmi->port_id) > + val = HIWORD_UPDATE(RK3588_HDMI1_GRANT_SEL, > + RK3588_HDMI1_GRANT_SEL); > + else > + val = HIWORD_UPDATE(RK3588_HDMI0_GRANT_SEL, > + RK3588_HDMI0_GRANT_SEL); > + regmap_write(hdmi->vo_regmap, RK3588_GRF_VO1_CON9, val); > + > + if (hdmi->port_id) > + val = HIWORD_UPDATE(RK3588_HDMI1_HPD_INT_MSK, RK3588_HDMI1_HPD_INT_MSK); > + else > + val = HIWORD_UPDATE(RK3588_HDMI0_HPD_INT_MSK, RK3588_HDMI0_HPD_INT_MSK); > + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val); > +} > + > +static const struct rockchip_hdmi_qp_ctrl_ops rk3588_hdmi_ctrl_ops = { > + .io_init = dw_hdmi_qp_rk3588_io_init, > + .irq_callback = dw_hdmi_qp_rk3588_irq, > + .hardirq_callback = dw_hdmi_qp_rk3588_hardirq, > +}; > + > struct rockchip_hdmi_qp_cfg { > unsigned int num_ports; > unsigned int port_ids[MAX_HDMI_PORT_NUM]; > + const struct rockchip_hdmi_qp_ctrl_ops *ctrl_ops; > const struct dw_hdmi_qp_phy_ops *phy_ops; > }; > > @@ -238,6 +282,7 @@ static const struct rockchip_hdmi_qp_cfg rk3588_hdmi_cfg = { > 0xfde80000, > 0xfdea0000, > }, > + .ctrl_ops = &rk3588_hdmi_ctrl_ops, > .phy_ops = &rk3588_hdmi_phy_ops, > }; > > @@ -282,6 +327,12 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master, > if (!cfg) > return -ENODEV; > > + if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init || > + !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->irq_callback) { cfg->ctrl_ops->irq_callback is checked twice, I guess one of them should've been ctrl_ops->hardirq_callback? Patch looks good otherwise. Thanks Heiko
Hi Heiko, At 2025-01-02 19:51:58, "Heiko Stübner" <heiko@sntech.de> wrote: >Hi Andy, > >Am Mittwoch, 25. Dezember 2024, 11:37:29 CET schrieb Andy Yan: >> From: Andy Yan <andy.yan@rock-chips.com> >> >> There are some control bits for IO and interrupts status scattered >> across different GRF on differt SOC. >> Add platform callback for this IO setting and interrupts status >> handling. >> >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> >> >> --- >> >> .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 81 ++++++++++++------- >> 1 file changed, 54 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c >> index c36fc130b734..b21e868e7c16 100644 >> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c >> @@ -62,6 +62,12 @@ struct rockchip_hdmi_qp { >> int port_id; >> }; >> >> +struct rockchip_hdmi_qp_ctrl_ops { >> + void (*io_init)(struct rockchip_hdmi_qp *hdmi); >> + irqreturn_t (*irq_callback)(int irq, void *dev_id); >> + irqreturn_t (*hardirq_callback)(int irq, void *dev_id); >> +}; >> + >> static struct rockchip_hdmi_qp *to_rockchip_hdmi_qp(struct drm_encoder *encoder) >> { >> struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder); >> @@ -226,9 +232,47 @@ static irqreturn_t dw_hdmi_qp_rk3588_irq(int irq, void *dev_id) >> return IRQ_HANDLED; >> } >> >> +static void dw_hdmi_qp_rk3588_io_init(struct rockchip_hdmi_qp *hdmi) >> +{ >> + u32 val; >> + >> + val = HIWORD_UPDATE(RK3588_SCLIN_MASK, RK3588_SCLIN_MASK) | >> + HIWORD_UPDATE(RK3588_SDAIN_MASK, RK3588_SDAIN_MASK) | >> + HIWORD_UPDATE(RK3588_MODE_MASK, RK3588_MODE_MASK) | >> + HIWORD_UPDATE(RK3588_I2S_SEL_MASK, RK3588_I2S_SEL_MASK); >> + > >nit: below val = ... and regmap_write don't have a blank line between them >which makes sense to show that they belong together. So the blank above >can probably also go away. Okay, it will be removed in V2. > >> + regmap_write(hdmi->vo_regmap, >> + hdmi->port_id ? RK3588_GRF_VO1_CON6 : RK3588_GRF_VO1_CON3, >> + val); >> + >> + val = HIWORD_UPDATE(RK3588_SET_HPD_PATH_MASK, RK3588_SET_HPD_PATH_MASK); >> + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON7, val); >> + >> + if (hdmi->port_id) >> + val = HIWORD_UPDATE(RK3588_HDMI1_GRANT_SEL, >> + RK3588_HDMI1_GRANT_SEL); >> + else >> + val = HIWORD_UPDATE(RK3588_HDMI0_GRANT_SEL, >> + RK3588_HDMI0_GRANT_SEL); >> + regmap_write(hdmi->vo_regmap, RK3588_GRF_VO1_CON9, val); >> + >> + if (hdmi->port_id) >> + val = HIWORD_UPDATE(RK3588_HDMI1_HPD_INT_MSK, RK3588_HDMI1_HPD_INT_MSK); >> + else >> + val = HIWORD_UPDATE(RK3588_HDMI0_HPD_INT_MSK, RK3588_HDMI0_HPD_INT_MSK); >> + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val); >> +} >> + >> +static const struct rockchip_hdmi_qp_ctrl_ops rk3588_hdmi_ctrl_ops = { >> + .io_init = dw_hdmi_qp_rk3588_io_init, >> + .irq_callback = dw_hdmi_qp_rk3588_irq, >> + .hardirq_callback = dw_hdmi_qp_rk3588_hardirq, >> +}; >> + >> struct rockchip_hdmi_qp_cfg { >> unsigned int num_ports; >> unsigned int port_ids[MAX_HDMI_PORT_NUM]; >> + const struct rockchip_hdmi_qp_ctrl_ops *ctrl_ops; >> const struct dw_hdmi_qp_phy_ops *phy_ops; >> }; >> >> @@ -238,6 +282,7 @@ static const struct rockchip_hdmi_qp_cfg rk3588_hdmi_cfg = { >> 0xfde80000, >> 0xfdea0000, >> }, >> + .ctrl_ops = &rk3588_hdmi_ctrl_ops, >> .phy_ops = &rk3588_hdmi_phy_ops, >> }; >> >> @@ -282,6 +327,12 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master, >> if (!cfg) >> return -ENODEV; >> >> + if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init || >> + !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->irq_callback) { > >cfg->ctrl_ops->irq_callback is checked twice, I guess one of them >should've been ctrl_ops->hardirq_callback? Yes，Thank you for catching it。 > >Patch looks good otherwise. > >Thanks >Heiko > > >
Am Montag, 6. Januar 2025, 09:35:26 CET schrieb Andy Yan: > > Hi Heiko, > > At 2025-01-02 19:51:58, "Heiko Stübner" <heiko@sntech.de> wrote: > >Hi Andy, > > > >Am Mittwoch, 25. Dezember 2024, 11:37:29 CET schrieb Andy Yan: > >> From: Andy Yan <andy.yan@rock-chips.com> > >> > >> There are some control bits for IO and interrupts status scattered > >> across different GRF on differt SOC. > >> Add platform callback for this IO setting and interrupts status > >> handling. > >> > >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> > >> > >> --- > >> > >> .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 81 ++++++++++++------- > >> 1 file changed, 54 insertions(+), 27 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > >> index c36fc130b734..b21e868e7c16 100644 > >> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > >> @@ -62,6 +62,12 @@ struct rockchip_hdmi_qp { > >> int port_id; > >> }; > >> > >> +struct rockchip_hdmi_qp_ctrl_ops { > >> + void (*io_init)(struct rockchip_hdmi_qp *hdmi); > >> + irqreturn_t (*irq_callback)(int irq, void *dev_id); > >> + irqreturn_t (*hardirq_callback)(int irq, void *dev_id); > >> +}; > >> + > >> static struct rockchip_hdmi_qp *to_rockchip_hdmi_qp(struct drm_encoder *encoder) > >> { > >> struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder); > >> @@ -226,9 +232,47 @@ static irqreturn_t dw_hdmi_qp_rk3588_irq(int irq, void *dev_id) > >> return IRQ_HANDLED; > >> } > >> > >> +static void dw_hdmi_qp_rk3588_io_init(struct rockchip_hdmi_qp *hdmi) > >> +{ > >> + u32 val; > >> + > >> + val = HIWORD_UPDATE(RK3588_SCLIN_MASK, RK3588_SCLIN_MASK) | > >> + HIWORD_UPDATE(RK3588_SDAIN_MASK, RK3588_SDAIN_MASK) | > >> + HIWORD_UPDATE(RK3588_MODE_MASK, RK3588_MODE_MASK) | > >> + HIWORD_UPDATE(RK3588_I2S_SEL_MASK, RK3588_I2S_SEL_MASK); > >> + > > > >nit: below val = ... and regmap_write don't have a blank line between them > >which makes sense to show that they belong together. So the blank above > >can probably also go away. > > Okay, it will be removed in V2. just realized that I commented on the "wrong" version :-) https://lore.kernel.org/r/20241231094425.253398-1-andyshrk@163.com seems to be the most recent one. Heiko
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c index c36fc130b734..b21e868e7c16 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c @@ -62,6 +62,12 @@ struct rockchip_hdmi_qp { int port_id; }; +struct rockchip_hdmi_qp_ctrl_ops { + void (*io_init)(struct rockchip_hdmi_qp *hdmi); + irqreturn_t (*irq_callback)(int irq, void *dev_id); + irqreturn_t (*hardirq_callback)(int irq, void *dev_id); +}; + static struct rockchip_hdmi_qp *to_rockchip_hdmi_qp(struct drm_encoder *encoder) { struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder); @@ -226,9 +232,47 @@ static irqreturn_t dw_hdmi_qp_rk3588_irq(int irq, void *dev_id) return IRQ_HANDLED; } +static void dw_hdmi_qp_rk3588_io_init(struct rockchip_hdmi_qp *hdmi) +{ + u32 val; + + val = HIWORD_UPDATE(RK3588_SCLIN_MASK, RK3588_SCLIN_MASK) | + HIWORD_UPDATE(RK3588_SDAIN_MASK, RK3588_SDAIN_MASK) | + HIWORD_UPDATE(RK3588_MODE_MASK, RK3588_MODE_MASK) | + HIWORD_UPDATE(RK3588_I2S_SEL_MASK, RK3588_I2S_SEL_MASK); + + regmap_write(hdmi->vo_regmap, + hdmi->port_id ? RK3588_GRF_VO1_CON6 : RK3588_GRF_VO1_CON3, + val); + + val = HIWORD_UPDATE(RK3588_SET_HPD_PATH_MASK, RK3588_SET_HPD_PATH_MASK); + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON7, val); + + if (hdmi->port_id) + val = HIWORD_UPDATE(RK3588_HDMI1_GRANT_SEL, + RK3588_HDMI1_GRANT_SEL); + else + val = HIWORD_UPDATE(RK3588_HDMI0_GRANT_SEL, + RK3588_HDMI0_GRANT_SEL); + regmap_write(hdmi->vo_regmap, RK3588_GRF_VO1_CON9, val); + + if (hdmi->port_id) + val = HIWORD_UPDATE(RK3588_HDMI1_HPD_INT_MSK, RK3588_HDMI1_HPD_INT_MSK); + else + val = HIWORD_UPDATE(RK3588_HDMI0_HPD_INT_MSK, RK3588_HDMI0_HPD_INT_MSK); + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val); +} + +static const struct rockchip_hdmi_qp_ctrl_ops rk3588_hdmi_ctrl_ops = { + .io_init = dw_hdmi_qp_rk3588_io_init, + .irq_callback = dw_hdmi_qp_rk3588_irq, + .hardirq_callback = dw_hdmi_qp_rk3588_hardirq, +}; + struct rockchip_hdmi_qp_cfg { unsigned int num_ports; unsigned int port_ids[MAX_HDMI_PORT_NUM]; + const struct rockchip_hdmi_qp_ctrl_ops *ctrl_ops; const struct dw_hdmi_qp_phy_ops *phy_ops; }; @@ -238,6 +282,7 @@ static const struct rockchip_hdmi_qp_cfg rk3588_hdmi_cfg = { 0xfde80000, 0xfdea0000, }, + .ctrl_ops = &rk3588_hdmi_ctrl_ops, .phy_ops = &rk3588_hdmi_phy_ops, }; @@ -282,6 +327,12 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master, if (!cfg) return -ENODEV; + if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init || + !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->irq_callback) { + dev_err(dev, "Missing platform ctrl ops\n"); + return -ENODEV; + } + hdmi->dev = &pdev->dev; hdmi->port_id = -ENODEV; @@ -357,31 +408,7 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master, return ret; } - val = HIWORD_UPDATE(RK3588_SCLIN_MASK, RK3588_SCLIN_MASK) | - HIWORD_UPDATE(RK3588_SDAIN_MASK, RK3588_SDAIN_MASK) | - HIWORD_UPDATE(RK3588_MODE_MASK, RK3588_MODE_MASK) | - HIWORD_UPDATE(RK3588_I2S_SEL_MASK, RK3588_I2S_SEL_MASK); - regmap_write(hdmi->vo_regmap, - hdmi->port_id ? RK3588_GRF_VO1_CON6 : RK3588_GRF_VO1_CON3, - val); - - val = HIWORD_UPDATE(RK3588_SET_HPD_PATH_MASK, - RK3588_SET_HPD_PATH_MASK); - regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON7, val); - - if (hdmi->port_id) - val = HIWORD_UPDATE(RK3588_HDMI1_GRANT_SEL, - RK3588_HDMI1_GRANT_SEL); - else - val = HIWORD_UPDATE(RK3588_HDMI0_GRANT_SEL, - RK3588_HDMI0_GRANT_SEL); - regmap_write(hdmi->vo_regmap, RK3588_GRF_VO1_CON9, val); - - if (hdmi->port_id) - val = HIWORD_UPDATE(RK3588_HDMI1_HPD_INT_MSK, RK3588_HDMI1_HPD_INT_MSK); - else - val = HIWORD_UPDATE(RK3588_HDMI0_HPD_INT_MSK, RK3588_HDMI0_HPD_INT_MSK); - regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val); + cfg->ctrl_ops->io_init(hdmi); INIT_DELAYED_WORK(&hdmi->hpd_work, dw_hdmi_qp_rk3588_hpd_work); @@ -394,8 +421,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master, return irq; ret = devm_request_threaded_irq(hdmi->dev, irq, - dw_hdmi_qp_rk3588_hardirq, - dw_hdmi_qp_rk3588_irq, + cfg->ctrl_ops->hardirq_callback, + cfg->ctrl_ops->irq_callback, IRQF_SHARED, "dw-hdmi-qp-hpd", hdmi); if (ret)