1. Patchew
  2. linux
  3. [PATCH 0/3] Add HDMI support for rk3576
  4. View patch

[PATCH 1/3] drm/rockchip: dw_hdmi_qp: Add platform ctrl callback

Andy Yan posted 3 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH 1/3] drm/rockchip: dw_hdmi_qp: Add platform ctrl callback
Posted by Andy Yan 1 year, 1 month ago 
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);
+
+	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)
-- 
2.34.1
Re: [PATCH 1/3] drm/rockchip: dw_hdmi_qp: Add platform ctrl callback
Posted by Heiko Stübner 1 year, 1 month ago 
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
Re:Re: [PATCH 1/3] drm/rockchip: dw_hdmi_qp: Add platform ctrl callback
Posted by Andy Yan 1 year, 1 month ago 
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
>
>
>
Re: [PATCH 1/3] drm/rockchip: dw_hdmi_qp: Add platform ctrl callback
Posted by Heiko Stübner 1 year, 1 month ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.