[PATCH v6 00/14] Add eDP support for RK3588

[PATCH v6 00/14] Add eDP support for RK3588

[Damon Ding]

Picked from:
https://patchwork.kernel.org/project/linux-rockchip/list/?series=923593

These patchs have been tested with a 1536x2048p60 eDP panel on
RK3588S EVB1 board, and HDMI 1080P/4K display also has been verified
on RK3588 EVB1 board. Furthermore, the eDP display has been rechecked
on RK3399 sapphire excavator board.

Patch 1~4   are preparations for the RK3588 eDP support on both Analogix
            side and Rockchip side.
Patch 5~8  are to support to get panel from the DP AUX bus.
Patch 9~11 are the RK3588 Analogix DP driver support.
Patch 12    is to add the power sequencing delays for panel model
            LP079QX1-SP0V.
Patch 13    is the addition of RK3588 eDP0 node.
Patch 14    is to enable the eDP0 display on RK3588S EVB1 board.

Damon Ding (14):
  drm/rockchip: analogix_dp: Replace DRM_...() functions with drm_...()
  drm/rockchip: analogix_dp: Use formalized struct definition for grf
    field
  drm/rockchip: analogix_dp: Expand device data to support multiple edp
    display
  drm/bridge: analogix_dp: Add support for phy configuration.
  dt-bindings: display: rockchip: analogix-dp: Add support to get panel
    from the DP AUX bus
  drm/bridge: analogix_dp: support to get &analogix_dp_device.plat_data
    and &analogix_dp_device.aux
  drm/bridge: analogix_dp: Add support to get panel from the DP AUX bus
  drm/rockchip: analogix_dp: Add support to get panel from the DP AUX
    bus
  dt-bindings: display: rockchip: analogix-dp: Add support for RK3588
  drm/bridge: analogix_dp: Add support for RK3588
  drm/rockchip: analogix_dp: Add support for RK3588
  drm/edp-panel: Add LG Display panel model LP079QX1-SP0V
  arm64: dts: rockchip: Add eDP0 node for RK3588
  arm64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board

 .../rockchip/rockchip,analogix-dp.yaml        |  25 +-
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  28 +++
 .../boot/dts/rockchip/rk3588s-evb1-v10.dts    |  54 ++++
 .../drm/bridge/analogix/analogix_dp_core.c    |  76 +++---
 .../drm/bridge/analogix/analogix_dp_core.h    |   1 +
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  52 ++++
 drivers/gpu/drm/panel/panel-edp.c             |   8 +
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 238 +++++++++++++-----
 include/drm/bridge/analogix_dp.h              |   8 +-
 9 files changed, 401 insertions(+), 89 deletions(-)

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 01/14] drm/rockchip: analogix_dp: Replace DRM_...() functions with drm_...()

[Damon Ding]

According to the comments in include/drm/drm_print.h, the DRM_...()
functions are deprecated in favor of drm_...() or dev_...() functions.

Use drm_err()/drm_dbg_core()/drm_dbg_kms() instead of
DRM_DEV_ERROR()/DRM_ERROR()/DRM_DEV_DEBUG()/DRM_DEBUG_KMS().

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v6:
- Use drm_...() uniformly rather than mixing drm_...() and dev_..()
- Pass 'dp' in drm_...() rather than 'dp->drm_dev'
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 29 ++++++++++---------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 0844175c37c5..dd33d7540e4b 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -100,13 +100,13 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data)
 
 	ret = clk_prepare_enable(dp->pclk);
 	if (ret < 0) {
-		DRM_DEV_ERROR(dp->dev, "failed to enable pclk %d\n", ret);
+		drm_err(dp, "failed to enable pclk %d\n", ret);
 		return ret;
 	}
 
 	ret = rockchip_dp_pre_init(dp);
 	if (ret < 0) {
-		DRM_DEV_ERROR(dp->dev, "failed to dp pre init %d\n", ret);
+		drm_err(dp, "failed to dp pre init %d\n", ret);
 		clk_disable_unprepare(dp->pclk);
 		return ret;
 	}
@@ -126,12 +126,13 @@ static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
 static int rockchip_dp_get_modes(struct analogix_dp_plat_data *plat_data,
 				 struct drm_connector *connector)
 {
+	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
 	struct drm_display_info *di = &connector->display_info;
 	/* VOP couldn't output YUV video format for eDP rightly */
 	u32 mask = DRM_COLOR_FORMAT_YCBCR444 | DRM_COLOR_FORMAT_YCBCR422;
 
 	if ((di->color_formats & mask)) {
-		DRM_DEBUG_KMS("Swapping display color format from YUV to RGB\n");
+		drm_dbg_kms(dp, "Swapping display color format from YUV to RGB\n");
 		di->color_formats &= ~mask;
 		di->color_formats |= DRM_COLOR_FORMAT_RGB444;
 		di->bpc = 8;
@@ -201,17 +202,17 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 	else
 		val = dp->data->lcdsel_big;
 
-	DRM_DEV_DEBUG(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
+	drm_dbg_core(dp, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
 
 	ret = clk_prepare_enable(dp->grfclk);
 	if (ret < 0) {
-		DRM_DEV_ERROR(dp->dev, "failed to enable grfclk %d\n", ret);
+		drm_err(dp, "failed to enable grfclk %d\n", ret);
 		return;
 	}
 
 	ret = regmap_write(dp->grf, dp->data->lcdsel_grf_reg, val);
 	if (ret != 0)
-		DRM_DEV_ERROR(dp->dev, "Could not write to GRF: %d\n", ret);
+		drm_err(dp, "Could not write to GRF: %d\n", ret);
 
 	clk_disable_unprepare(dp->grfclk);
 }
@@ -236,7 +237,7 @@ static void rockchip_dp_drm_encoder_disable(struct drm_encoder *encoder,
 
 	ret = rockchip_drm_wait_vact_end(crtc, PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
 	if (ret)
-		DRM_DEV_ERROR(dp->dev, "line flag irq timed out\n");
+		drm_err(dp, "line flag irq timed out\n");
 }
 
 static int
@@ -277,7 +278,7 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 
 	dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
 	if (IS_ERR(dp->grf)) {
-		DRM_DEV_ERROR(dev, "failed to get rockchip,grf property\n");
+		drm_err(dp, "failed to get rockchip,grf property\n");
 		return PTR_ERR(dp->grf);
 	}
 
@@ -287,19 +288,19 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 	} else if (PTR_ERR(dp->grfclk) == -EPROBE_DEFER) {
 		return -EPROBE_DEFER;
 	} else if (IS_ERR(dp->grfclk)) {
-		DRM_DEV_ERROR(dev, "failed to get grf clock\n");
+		drm_err(dp, "failed to get grf clock\n");
 		return PTR_ERR(dp->grfclk);
 	}
 
 	dp->pclk = devm_clk_get(dev, "pclk");
 	if (IS_ERR(dp->pclk)) {
-		DRM_DEV_ERROR(dev, "failed to get pclk property\n");
+		drm_err(dp, "failed to get pclk property\n");
 		return PTR_ERR(dp->pclk);
 	}
 
 	dp->rst = devm_reset_control_get(dev, "dp");
 	if (IS_ERR(dp->rst)) {
-		DRM_DEV_ERROR(dev, "failed to get dp reset control\n");
+		drm_err(dp, "failed to get dp reset control\n");
 		return PTR_ERR(dp->rst);
 	}
 
@@ -315,12 +316,12 @@ static int rockchip_dp_drm_create_encoder(struct rockchip_dp_device *dp)
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
-	DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
+	drm_dbg_kms(dp, "possible_crtcs = 0x%x\n", encoder->possible_crtcs);
 
 	ret = drm_simple_encoder_init(drm_dev, encoder,
 				      DRM_MODE_ENCODER_TMDS);
 	if (ret) {
-		DRM_ERROR("failed to initialize encoder with drm\n");
+		drm_err(dp, "failed to initialize encoder with drm\n");
 		return ret;
 	}
 
@@ -340,7 +341,7 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
 
 	ret = rockchip_dp_drm_create_encoder(dp);
 	if (ret) {
-		DRM_ERROR("failed to create drm encoder\n");
+		drm_err(dp, "failed to create drm encoder\n");
 		return ret;
 	}
 
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 01/14] drm/rockchip: analogix_dp: Replace DRM_...() functions with drm_...()

[Jani Nikula]

On Thu, 23 Jan 2025, Damon Ding <damon.ding@rock-chips.com> wrote:
> According to the comments in include/drm/drm_print.h, the DRM_...()
> functions are deprecated in favor of drm_...() or dev_...() functions.
>
> Use drm_err()/drm_dbg_core()/drm_dbg_kms() instead of
> DRM_DEV_ERROR()/DRM_ERROR()/DRM_DEV_DEBUG()/DRM_DEBUG_KMS().
>
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>
> ---
>
> Changes in v6:
> - Use drm_...() uniformly rather than mixing drm_...() and dev_..()
> - Pass 'dp' in drm_...() rather than 'dp->drm_dev'
> ---
>  .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 29 ++++++++++---------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 0844175c37c5..dd33d7540e4b 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -100,13 +100,13 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data)
>  
>  	ret = clk_prepare_enable(dp->pclk);
>  	if (ret < 0) {
> -		DRM_DEV_ERROR(dp->dev, "failed to enable pclk %d\n", ret);
> +		drm_err(dp, "failed to enable pclk %d\n", ret);

Please don't do this.

You're supposed to pass struct drm_device to drm_err() and friends. Not
some random struct pointer that just happens to have a ->dev member.

The drm_* macros may change at any time to actually expect the correct
type.

BR,
Jani.


>  		return ret;
>  	}
>  
>  	ret = rockchip_dp_pre_init(dp);
>  	if (ret < 0) {
> -		DRM_DEV_ERROR(dp->dev, "failed to dp pre init %d\n", ret);
> +		drm_err(dp, "failed to dp pre init %d\n", ret);
>  		clk_disable_unprepare(dp->pclk);
>  		return ret;
>  	}
> @@ -126,12 +126,13 @@ static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
>  static int rockchip_dp_get_modes(struct analogix_dp_plat_data *plat_data,
>  				 struct drm_connector *connector)
>  {
> +	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
>  	struct drm_display_info *di = &connector->display_info;
>  	/* VOP couldn't output YUV video format for eDP rightly */
>  	u32 mask = DRM_COLOR_FORMAT_YCBCR444 | DRM_COLOR_FORMAT_YCBCR422;
>  
>  	if ((di->color_formats & mask)) {
> -		DRM_DEBUG_KMS("Swapping display color format from YUV to RGB\n");
> +		drm_dbg_kms(dp, "Swapping display color format from YUV to RGB\n");
>  		di->color_formats &= ~mask;
>  		di->color_formats |= DRM_COLOR_FORMAT_RGB444;
>  		di->bpc = 8;
> @@ -201,17 +202,17 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
>  	else
>  		val = dp->data->lcdsel_big;
>  
> -	DRM_DEV_DEBUG(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
> +	drm_dbg_core(dp, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
>  
>  	ret = clk_prepare_enable(dp->grfclk);
>  	if (ret < 0) {
> -		DRM_DEV_ERROR(dp->dev, "failed to enable grfclk %d\n", ret);
> +		drm_err(dp, "failed to enable grfclk %d\n", ret);
>  		return;
>  	}
>  
>  	ret = regmap_write(dp->grf, dp->data->lcdsel_grf_reg, val);
>  	if (ret != 0)
> -		DRM_DEV_ERROR(dp->dev, "Could not write to GRF: %d\n", ret);
> +		drm_err(dp, "Could not write to GRF: %d\n", ret);
>  
>  	clk_disable_unprepare(dp->grfclk);
>  }
> @@ -236,7 +237,7 @@ static void rockchip_dp_drm_encoder_disable(struct drm_encoder *encoder,
>  
>  	ret = rockchip_drm_wait_vact_end(crtc, PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
>  	if (ret)
> -		DRM_DEV_ERROR(dp->dev, "line flag irq timed out\n");
> +		drm_err(dp, "line flag irq timed out\n");
>  }
>  
>  static int
> @@ -277,7 +278,7 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
>  
>  	dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
>  	if (IS_ERR(dp->grf)) {
> -		DRM_DEV_ERROR(dev, "failed to get rockchip,grf property\n");
> +		drm_err(dp, "failed to get rockchip,grf property\n");
>  		return PTR_ERR(dp->grf);
>  	}
>  
> @@ -287,19 +288,19 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
>  	} else if (PTR_ERR(dp->grfclk) == -EPROBE_DEFER) {
>  		return -EPROBE_DEFER;
>  	} else if (IS_ERR(dp->grfclk)) {
> -		DRM_DEV_ERROR(dev, "failed to get grf clock\n");
> +		drm_err(dp, "failed to get grf clock\n");
>  		return PTR_ERR(dp->grfclk);
>  	}
>  
>  	dp->pclk = devm_clk_get(dev, "pclk");
>  	if (IS_ERR(dp->pclk)) {
> -		DRM_DEV_ERROR(dev, "failed to get pclk property\n");
> +		drm_err(dp, "failed to get pclk property\n");
>  		return PTR_ERR(dp->pclk);
>  	}
>  
>  	dp->rst = devm_reset_control_get(dev, "dp");
>  	if (IS_ERR(dp->rst)) {
> -		DRM_DEV_ERROR(dev, "failed to get dp reset control\n");
> +		drm_err(dp, "failed to get dp reset control\n");
>  		return PTR_ERR(dp->rst);
>  	}
>  
> @@ -315,12 +316,12 @@ static int rockchip_dp_drm_create_encoder(struct rockchip_dp_device *dp)
>  
>  	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
>  							     dev->of_node);
> -	DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
> +	drm_dbg_kms(dp, "possible_crtcs = 0x%x\n", encoder->possible_crtcs);
>  
>  	ret = drm_simple_encoder_init(drm_dev, encoder,
>  				      DRM_MODE_ENCODER_TMDS);
>  	if (ret) {
> -		DRM_ERROR("failed to initialize encoder with drm\n");
> +		drm_err(dp, "failed to initialize encoder with drm\n");
>  		return ret;
>  	}
>  
> @@ -340,7 +341,7 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
>  
>  	ret = rockchip_dp_drm_create_encoder(dp);
>  	if (ret) {
> -		DRM_ERROR("failed to create drm encoder\n");
> +		drm_err(dp, "failed to create drm encoder\n");
>  		return ret;
>  	}

-- 
Jani Nikula, Intel

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 01/14] drm/rockchip: analogix_dp: Replace DRM_...() functions with drm_...()

[Jani Nikula]

On Thu, 23 Jan 2025, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 23 Jan 2025, Damon Ding <damon.ding@rock-chips.com> wrote:
>> According to the comments in include/drm/drm_print.h, the DRM_...()
>> functions are deprecated in favor of drm_...() or dev_...() functions.
>>
>> Use drm_err()/drm_dbg_core()/drm_dbg_kms() instead of
>> DRM_DEV_ERROR()/DRM_ERROR()/DRM_DEV_DEBUG()/DRM_DEBUG_KMS().
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ---
>>
>> Changes in v6:
>> - Use drm_...() uniformly rather than mixing drm_...() and dev_..()
>> - Pass 'dp' in drm_...() rather than 'dp->drm_dev'
>> ---
>>  .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 29 ++++++++++---------
>>  1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> index 0844175c37c5..dd33d7540e4b 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -100,13 +100,13 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data)
>>  
>>  	ret = clk_prepare_enable(dp->pclk);
>>  	if (ret < 0) {
>> -		DRM_DEV_ERROR(dp->dev, "failed to enable pclk %d\n", ret);
>> +		drm_err(dp, "failed to enable pclk %d\n", ret);
>
> Please don't do this.
>
> You're supposed to pass struct drm_device to drm_err() and friends. Not
> some random struct pointer that just happens to have a ->dev member.
>
> The drm_* macros may change at any time to actually expect the correct
> type.

And with [1] I'm hoping "at any time" becomes "real soon now".

BR,
Jani.


[1] https://lore.kernel.org/all/cover.1737644530.git.jani.nikula@intel.com


-- 
Jani Nikula, Intel

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 02/14] drm/rockchip: analogix_dp: Use formalized struct definition for grf field

[Damon Ding]

The formalized struct definition will makes grf field operations more
concise and easier to extend.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v2:
- Initialize struct rockchip_dp_chip_data rk3399_edp/rk3288_dp in order
  of its members

Changes in v6:
- Pass 'dp' in drm_...() rather than 'dp->drm_dev'
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 77 +++++++++++--------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index dd33d7540e4b..648ea608cdf3 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -32,26 +32,29 @@
 
 #include "rockchip_drm_drv.h"
 
-#define RK3288_GRF_SOC_CON6		0x25c
-#define RK3288_EDP_LCDC_SEL		BIT(5)
-#define RK3399_GRF_SOC_CON20		0x6250
-#define RK3399_EDP_LCDC_SEL		BIT(5)
-
-#define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
-
 #define PSR_WAIT_LINE_FLAG_TIMEOUT_MS	100
 
+#define GRF_REG_FIELD(_reg, _lsb, _msb) {	\
+				.reg = _reg,	\
+				.lsb = _lsb,	\
+				.msb = _msb,	\
+				.valid = true,	\
+				}
+
+struct rockchip_grf_reg_field {
+	u32 reg;
+	u32 lsb;
+	u32 msb;
+	bool valid;
+};
+
 /**
  * struct rockchip_dp_chip_data - splite the grf setting of kind of chips
- * @lcdsel_grf_reg: grf register offset of lcdc select
- * @lcdsel_big: reg value of selecting vop big for eDP
- * @lcdsel_lit: reg value of selecting vop little for eDP
+ * @lcdc_sel: grf register field of lcdc_sel
  * @chip_type: specific chip type
  */
 struct rockchip_dp_chip_data {
-	u32	lcdsel_grf_reg;
-	u32	lcdsel_big;
-	u32	lcdsel_lit;
+	const struct rockchip_grf_reg_field lcdc_sel;
 	u32	chip_type;
 };
 
@@ -84,6 +87,26 @@ static struct rockchip_dp_device *pdata_encoder_to_dp(struct analogix_dp_plat_da
 	return container_of(plat_data, struct rockchip_dp_device, plat_data);
 }
 
+static int rockchip_grf_write(struct regmap *grf, u32 reg, u32 mask, u32 val)
+{
+	return regmap_write(grf, reg, (mask << 16) | (val & mask));
+}
+
+static int rockchip_grf_field_write(struct regmap *grf,
+				    const struct rockchip_grf_reg_field *field,
+				    u32 val)
+{
+	u32 mask;
+
+	if (!field->valid)
+		return 0;
+
+	mask = GENMASK(field->msb, field->lsb);
+	val <<= field->lsb;
+
+	return rockchip_grf_write(grf, field->reg, mask, val);
+}
+
 static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
 {
 	reset_control_assert(dp->rst);
@@ -182,7 +205,6 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
 	int ret;
-	u32 val;
 
 	crtc = rockchip_dp_drm_get_new_crtc(encoder, state);
 	if (!crtc)
@@ -193,24 +215,19 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 	if (old_crtc_state && old_crtc_state->self_refresh_active)
 		return;
 
-	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
-	if (ret < 0)
-		return;
-
-	if (ret)
-		val = dp->data->lcdsel_lit;
-	else
-		val = dp->data->lcdsel_big;
-
-	drm_dbg_core(dp, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
-
 	ret = clk_prepare_enable(dp->grfclk);
 	if (ret < 0) {
 		drm_err(dp, "failed to enable grfclk %d\n", ret);
 		return;
 	}
 
-	ret = regmap_write(dp->grf, dp->data->lcdsel_grf_reg, val);
+	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
+	if (ret < 0)
+		return;
+
+	drm_dbg_core(dp, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
+
+	ret = rockchip_grf_field_write(dp->grf, &dp->data->lcdc_sel, ret);
 	if (ret != 0)
 		drm_err(dp, "Could not write to GRF: %d\n", ret);
 
@@ -449,16 +466,12 @@ static DEFINE_RUNTIME_DEV_PM_OPS(rockchip_dp_pm_ops, rockchip_dp_suspend,
 		rockchip_dp_resume, NULL);
 
 static const struct rockchip_dp_chip_data rk3399_edp = {
-	.lcdsel_grf_reg = RK3399_GRF_SOC_CON20,
-	.lcdsel_big = HIWORD_UPDATE(0, RK3399_EDP_LCDC_SEL),
-	.lcdsel_lit = HIWORD_UPDATE(RK3399_EDP_LCDC_SEL, RK3399_EDP_LCDC_SEL),
+	.lcdc_sel = GRF_REG_FIELD(0x6250, 5, 5),
 	.chip_type = RK3399_EDP,
 };
 
 static const struct rockchip_dp_chip_data rk3288_dp = {
-	.lcdsel_grf_reg = RK3288_GRF_SOC_CON6,
-	.lcdsel_big = HIWORD_UPDATE(0, RK3288_EDP_LCDC_SEL),
-	.lcdsel_lit = HIWORD_UPDATE(RK3288_EDP_LCDC_SEL, RK3288_EDP_LCDC_SEL),
+	.lcdc_sel = GRF_REG_FIELD(0x025c, 5, 5),
 	.chip_type = RK3288_DP,
 };
 
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 03/14] drm/rockchip: analogix_dp: Expand device data to support multiple edp display

[Damon Ding]

There are two main modifications: one is expanding struct
rockchip_dp_chip_data to an array, and the other is adding
&rockchip_dp_chip_data.reg to separate different edp devices.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v6:
- Add the description of &rockchip_dp_chip_data.reg
- Use drm_...() uniformly rather than mixing drm_...() and dev_..()
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 42 +++++++++++++++----
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 648ea608cdf3..a63c2c867059 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -52,10 +52,12 @@ struct rockchip_grf_reg_field {
  * struct rockchip_dp_chip_data - splite the grf setting of kind of chips
  * @lcdc_sel: grf register field of lcdc_sel
  * @chip_type: specific chip type
+ * @reg: register base address
  */
 struct rockchip_dp_chip_data {
 	const struct rockchip_grf_reg_field lcdc_sel;
 	u32	chip_type;
+	u32	reg;
 };
 
 struct rockchip_dp_device {
@@ -397,6 +399,8 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 	const struct rockchip_dp_chip_data *dp_data;
 	struct drm_panel *panel = NULL;
 	struct rockchip_dp_device *dp;
+	struct resource *res;
+	int i;
 	int ret;
 
 	dp_data = of_device_get_match_data(dev);
@@ -411,9 +415,25 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 	if (!dp)
 		return -ENOMEM;
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	i = 0;
+	while (dp_data[i].reg) {
+		if (dp_data[i].reg == res->start) {
+			dp->data = &dp_data[i];
+			break;
+		}
+
+		i++;
+	}
+
+	if (!dp->data) {
+		drm_err(dp, "no chip-data for %s node\n", dev->of_node->name);
+		return -ENODEV;
+	}
+
 	dp->dev = dev;
 	dp->adp = ERR_PTR(-ENODEV);
-	dp->data = dp_data;
 	dp->plat_data.panel = panel;
 	dp->plat_data.dev_type = dp->data->chip_type;
 	dp->plat_data.power_on = rockchip_dp_poweron;
@@ -465,14 +485,22 @@ static int rockchip_dp_resume(struct device *dev)
 static DEFINE_RUNTIME_DEV_PM_OPS(rockchip_dp_pm_ops, rockchip_dp_suspend,
 		rockchip_dp_resume, NULL);
 
-static const struct rockchip_dp_chip_data rk3399_edp = {
-	.lcdc_sel = GRF_REG_FIELD(0x6250, 5, 5),
-	.chip_type = RK3399_EDP,
+static const struct rockchip_dp_chip_data rk3399_edp[] = {
+	{
+		.lcdc_sel = GRF_REG_FIELD(0x6250, 5, 5),
+		.chip_type = RK3399_EDP,
+		.reg = 0xff970000,
+	},
+	{ /* sentinel */ }
 };
 
-static const struct rockchip_dp_chip_data rk3288_dp = {
-	.lcdc_sel = GRF_REG_FIELD(0x025c, 5, 5),
-	.chip_type = RK3288_DP,
+static const struct rockchip_dp_chip_data rk3288_dp[] = {
+	{
+		.lcdc_sel = GRF_REG_FIELD(0x025c, 5, 5),
+		.chip_type = RK3288_DP,
+		.reg = 0xff970000,
+	},
+	{ /* sentinel */ }
 };
 
 static const struct of_device_id rockchip_dp_dt_ids[] = {
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 04/14] drm/bridge: analogix_dp: Add support for phy configuration.

[Damon Ding]

Add support to configurate link rate, lane count, voltage swing and
pre-emphasis with phy_configure(). It is helpful in application scenarios
where analogix controller is mixed with the phy of other vendors.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

---

Changes in v2:
- remove needless assignments for phy_configure()
- remove unnecessary changes for phy_power_on()/phy_power_off()

Changes in v4:
- remove unnecessary &phy_configure_opts_dp.lanes assignments in
  analogix_dp_set_link_bandwidth()
- remove needless &phy_configure_opts_dp.lanes and
  &phy_configure_opts_dp.link_rate assignments in
  analogix_dp_set_lane_link_training()

Changes in v5:
- include <drm/drm_print.h> for dev_err()
- use drm_err() instead of dev_err()

Changes in v6:
- Pass 'dp' in drm_...() rather than 'dp->drm_dev'
---
 .../drm/bridge/analogix/analogix_dp_core.c    |  1 +
 .../drm/bridge/analogix/analogix_dp_core.h    |  1 +
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 52 +++++++++++++++++++
 3 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index bfa88409a7ff..b05f5b9f5258 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1692,6 +1692,7 @@ int analogix_dp_resume(struct analogix_dp_device *dp)
 	if (dp->plat_data->power_on)
 		dp->plat_data->power_on(dp->plat_data);
 
+	phy_set_mode(dp->phy, PHY_MODE_DP);
 	phy_power_on(dp->phy);
 
 	analogix_dp_init_dp(dp);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 774d11574b09..4d36907aa371 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -11,6 +11,7 @@
 
 #include <drm/display/drm_dp_helper.h>
 #include <drm/drm_crtc.h>
+#include <drm/drm_print.h>
 
 #define DP_TIMEOUT_LOOP_COUNT 100
 #define MAX_CR_LOOP 5
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 3afc73c858c4..041733131b35 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -11,6 +11,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/phy/phy.h>
 
 #include <drm/bridge/analogix_dp.h>
 
@@ -513,10 +514,24 @@ void analogix_dp_enable_sw_function(struct analogix_dp_device *dp)
 void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype)
 {
 	u32 reg;
+	int ret;
 
 	reg = bwtype;
 	if ((bwtype == DP_LINK_BW_2_7) || (bwtype == DP_LINK_BW_1_62))
 		writel(reg, dp->reg_base + ANALOGIX_DP_LINK_BW_SET);
+
+	if (dp->phy) {
+		union phy_configure_opts phy_cfg = {0};
+
+		phy_cfg.dp.link_rate =
+			drm_dp_bw_code_to_link_rate(dp->link_train.link_rate) / 100;
+		phy_cfg.dp.set_rate = true;
+		ret = phy_configure(dp->phy, &phy_cfg);
+		if (ret && ret != -EOPNOTSUPP) {
+			drm_err(dp, "%s: phy_configure() failed: %d\n", __func__, ret);
+			return;
+		}
+	}
 }
 
 void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype)
@@ -530,9 +545,22 @@ void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype)
 void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count)
 {
 	u32 reg;
+	int ret;
 
 	reg = count;
 	writel(reg, dp->reg_base + ANALOGIX_DP_LANE_COUNT_SET);
+
+	if (dp->phy) {
+		union phy_configure_opts phy_cfg = {0};
+
+		phy_cfg.dp.lanes = dp->link_train.lane_count;
+		phy_cfg.dp.set_lanes = true;
+		ret = phy_configure(dp->phy, &phy_cfg);
+		if (ret && ret != -EOPNOTSUPP) {
+			drm_err(dp, "%s: phy_configure() failed: %d\n", __func__, ret);
+			return;
+		}
+	}
 }
 
 void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
@@ -546,10 +574,34 @@ void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
 void analogix_dp_set_lane_link_training(struct analogix_dp_device *dp)
 {
 	u8 lane;
+	int ret;
 
 	for (lane = 0; lane < dp->link_train.lane_count; lane++)
 		writel(dp->link_train.training_lane[lane],
 		       dp->reg_base + ANALOGIX_DP_LN0_LINK_TRAINING_CTL + 4 * lane);
+
+	if (dp->phy) {
+		union phy_configure_opts phy_cfg = {0};
+
+		for (lane = 0; lane < dp->link_train.lane_count; lane++) {
+			u8 training_lane = dp->link_train.training_lane[lane];
+			u8 vs, pe;
+
+			vs = (training_lane & DP_TRAIN_VOLTAGE_SWING_MASK) >>
+			     DP_TRAIN_VOLTAGE_SWING_SHIFT;
+			pe = (training_lane & DP_TRAIN_PRE_EMPHASIS_MASK) >>
+			     DP_TRAIN_PRE_EMPHASIS_SHIFT;
+			phy_cfg.dp.voltage[lane] = vs;
+			phy_cfg.dp.pre[lane] = pe;
+		}
+
+		phy_cfg.dp.set_voltages = true;
+		ret = phy_configure(dp->phy, &phy_cfg);
+		if (ret && ret != -EOPNOTSUPP) {
+			drm_err(dp, "%s: phy_configure() failed: %d\n", __func__, ret);
+			return;
+		}
+	}
 }
 
 u32 analogix_dp_get_lane_link_training(struct analogix_dp_device *dp, u8 lane)
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 05/14] dt-bindings: display: rockchip: analogix-dp: Add support to get panel from the DP AUX bus

[Damon Ding]

According to Documentation/devicetree/bindings/display/dp-aux-bus.yaml,
it is a good way to get panel through the DP AUX bus.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Move the dt-bindings commit before related driver commits

Changes in v5:
- Remove the unexpected change logs in commit message
---
 .../bindings/display/rockchip/rockchip,analogix-dp.yaml        | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index 60dedf9b2be7..eaf4e67e232e 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -41,6 +41,9 @@ properties:
     description:
       This SoC makes use of GRF regs.
 
+  aux-bus:
+    $ref: /schemas/display/dp-aux-bus.yaml#
+
 required:
   - compatible
   - clocks
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 06/14] drm/bridge: analogix_dp: support to get &analogix_dp_device.plat_data and &analogix_dp_device.aux

[Damon Ding]

Add two new functions: one to find &analogix_dp_device.plat_data via
&drm_dp_aux, and the other to get &analogix_dp_device.aux. Both of them
serve for the function of getting panel from DP AUX bus, which is why
they are included in a single commit.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 14 ++++++++++++++
 include/drm/bridge/analogix_dp.h                   |  4 ++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index b05f5b9f5258..8251adfce2f9 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1794,6 +1794,20 @@ int analogix_dp_stop_crc(struct drm_connector *connector)
 }
 EXPORT_SYMBOL_GPL(analogix_dp_stop_crc);
 
+struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux)
+{
+	struct analogix_dp_device *dp = to_dp(aux);
+
+	return dp->plat_data;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_aux_to_plat_data);
+
+struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp)
+{
+	return &dp->aux;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_get_aux);
+
 MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
 MODULE_DESCRIPTION("Analogix DP Core Driver");
 MODULE_LICENSE("GPL v2");
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 6002c5666031..1b119b547988 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -7,6 +7,7 @@
 #ifndef _ANALOGIX_DP_H_
 #define _ANALOGIX_DP_H_
 
+#include <drm/display/drm_dp_aux_bus.h>
 #include <drm/drm_crtc.h>
 
 struct analogix_dp_device;
@@ -48,4 +49,7 @@ void analogix_dp_unbind(struct analogix_dp_device *dp);
 int analogix_dp_start_crc(struct drm_connector *connector);
 int analogix_dp_stop_crc(struct drm_connector *connector);
 
+struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux);
+struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp);
+
 #endif /* _ANALOGIX_DP_H_ */
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 06/14] drm/bridge: analogix_dp: support to get &analogix_dp_device.plat_data and &analogix_dp_device.aux

[Dmitry Baryshkov]

On Thu, Jan 23, 2025 at 06:07:39PM +0800, Damon Ding wrote:
> Add two new functions: one to find &analogix_dp_device.plat_data via
> &drm_dp_aux, and the other to get &analogix_dp_device.aux. Both of them
> serve for the function of getting panel from DP AUX bus, which is why
> they are included in a single commit.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 14 ++++++++++++++
>  include/drm/bridge/analogix_dp.h                   |  4 ++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index b05f5b9f5258..8251adfce2f9 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1794,6 +1794,20 @@ int analogix_dp_stop_crc(struct drm_connector *connector)
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_stop_crc);
>  
> +struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux)
> +{
> +	struct analogix_dp_device *dp = to_dp(aux);
> +
> +	return dp->plat_data;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_aux_to_plat_data);
> +
> +struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp)
> +{
> +	return &dp->aux;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_get_aux);
> +
>  MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
>  MODULE_DESCRIPTION("Analogix DP Core Driver");
>  MODULE_LICENSE("GPL v2");
> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
> index 6002c5666031..1b119b547988 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -7,6 +7,7 @@
>  #ifndef _ANALOGIX_DP_H_
>  #define _ANALOGIX_DP_H_
>  
> +#include <drm/display/drm_dp_aux_bus.h>

Nit: it should be enough to just forward-declare struct drm_dp_aux
instead.

>  #include <drm/drm_crtc.h>
>  
>  struct analogix_dp_device;
> @@ -48,4 +49,7 @@ void analogix_dp_unbind(struct analogix_dp_device *dp);
>  int analogix_dp_start_crc(struct drm_connector *connector);
>  int analogix_dp_stop_crc(struct drm_connector *connector);
>  
> +struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux);
> +struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp);
> +
>  #endif /* _ANALOGIX_DP_H_ */
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 06/14] drm/bridge: analogix_dp: support to get &analogix_dp_device.plat_data and &analogix_dp_device.aux

[Damon Ding]

Hi Dmitry,

On 2025/1/23 19:19, Dmitry Baryshkov wrote:
> On Thu, Jan 23, 2025 at 06:07:39PM +0800, Damon Ding wrote:
>> Add two new functions: one to find &analogix_dp_device.plat_data via
>> &drm_dp_aux, and the other to get &analogix_dp_device.aux. Both of them
>> serve for the function of getting panel from DP AUX bus, which is why
>> they are included in a single commit.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 14 ++++++++++++++
>>   include/drm/bridge/analogix_dp.h                   |  4 ++++
>>   2 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index b05f5b9f5258..8251adfce2f9 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1794,6 +1794,20 @@ int analogix_dp_stop_crc(struct drm_connector *connector)
>>   }
>>   EXPORT_SYMBOL_GPL(analogix_dp_stop_crc);
>>   
>> +struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux)
>> +{
>> +	struct analogix_dp_device *dp = to_dp(aux);
>> +
>> +	return dp->plat_data;
>> +}
>> +EXPORT_SYMBOL_GPL(analogix_dp_aux_to_plat_data);
>> +
>> +struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp)
>> +{
>> +	return &dp->aux;
>> +}
>> +EXPORT_SYMBOL_GPL(analogix_dp_get_aux);
>> +
>>   MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
>>   MODULE_DESCRIPTION("Analogix DP Core Driver");
>>   MODULE_LICENSE("GPL v2");
>> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
>> index 6002c5666031..1b119b547988 100644
>> --- a/include/drm/bridge/analogix_dp.h
>> +++ b/include/drm/bridge/analogix_dp.h
>> @@ -7,6 +7,7 @@
>>   #ifndef _ANALOGIX_DP_H_
>>   #define _ANALOGIX_DP_H_
>>   
>> +#include <drm/display/drm_dp_aux_bus.h>
> 
> Nit: it should be enough to just forward-declare struct drm_dp_aux
> instead.
> 

I will move the header file to the driver of Rockchip Analogix DP and 
just use the forward-declare here in the next version.

>>   #include <drm/drm_crtc.h>
>>   
>>   struct analogix_dp_device;
>> @@ -48,4 +49,7 @@ void analogix_dp_unbind(struct analogix_dp_device *dp);
>>   int analogix_dp_start_crc(struct drm_connector *connector);
>>   int analogix_dp_stop_crc(struct drm_connector *connector);
>>   
>> +struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux);
>> +struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp);
>> +
>>   #endif /* _ANALOGIX_DP_H_ */
>> -- 
>> 2.34.1
>>
> 

Best regards,
Damon


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 07/14] drm/bridge: analogix_dp: Add support to get panel from the DP AUX bus

[Damon Ding]

The main modification is moving the DP AUX initialization from function
analogix_dp_bind() to analogix_dp_probe(). In order to get the EDID of
eDP panel during probing, it is also needed to advance PM operaions to
ensure that eDP controller and phy are prepared for AUX transmission.

In addtion, add a new function analogix_dp_remove() to ensure symmetry
for PM operations.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Use done_probing() to call drm_of_find_panel_or_bridge() and
  component_add() when getting panel from the DP AUX bus

Changes in v5:
- Advance PM operations to make eDP AUX work well

Changes in v6:
- Use devm_pm_runtime_enable() instead of devm_add_action_or_reset()
- Add a new function analogix_dp_remove() to ensure symmetry for PM
  operations
---
 .../drm/bridge/analogix/analogix_dp_core.c    | 57 ++++++++++---------
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  4 ++
 include/drm/bridge/analogix_dp.h              |  1 +
 3 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 8251adfce2f9..30da8a14361e 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1658,14 +1658,42 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
 	}
 	disable_irq(dp->irq);
 
+	dp->aux.name = "DP-AUX";
+	dp->aux.transfer = analogix_dpaux_transfer;
+	dp->aux.dev = dp->dev;
+	drm_dp_aux_init(&dp->aux);
+
+	if (IS_ENABLED(CONFIG_PM)) {
+		pm_runtime_use_autosuspend(dp->dev);
+		pm_runtime_set_autosuspend_delay(dp->dev, 100);
+		ret = devm_pm_runtime_enable(dp->dev);
+		if (ret)
+			goto err_disable_pm_runtime;
+	} else {
+		ret = analogix_dp_resume(dp);
+		if (ret)
+			goto err_disable_clk;
+	}
+
 	return dp;
 
+err_disable_pm_runtime:
+	pm_runtime_dont_use_autosuspend(dp->dev);
 err_disable_clk:
 	clk_disable_unprepare(dp->clock);
 	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_probe);
 
+void analogix_dp_remove(struct analogix_dp_device *dp)
+{
+	if (IS_ENABLED(CONFIG_PM))
+		pm_runtime_dont_use_autosuspend(dp->dev);
+	else
+		analogix_dp_suspend(dp);
+}
+EXPORT_SYMBOL_GPL(analogix_dp_remove);
+
 int analogix_dp_suspend(struct analogix_dp_device *dp)
 {
 	phy_power_off(dp->phy);
@@ -1708,25 +1736,12 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 	dp->drm_dev = drm_dev;
 	dp->encoder = dp->plat_data->encoder;
 
-	if (IS_ENABLED(CONFIG_PM)) {
-		pm_runtime_use_autosuspend(dp->dev);
-		pm_runtime_set_autosuspend_delay(dp->dev, 100);
-		pm_runtime_enable(dp->dev);
-	} else {
-		ret = analogix_dp_resume(dp);
-		if (ret)
-			return ret;
-	}
-
-	dp->aux.name = "DP-AUX";
-	dp->aux.transfer = analogix_dpaux_transfer;
-	dp->aux.dev = dp->dev;
 	dp->aux.drm_dev = drm_dev;
 
 	ret = drm_dp_aux_register(&dp->aux);
 	if (ret) {
 		DRM_ERROR("failed to register AUX (%d)\n", ret);
-		goto err_disable_pm_runtime;
+		return ret;
 	}
 
 	ret = analogix_dp_create_bridge(drm_dev, dp);
@@ -1739,13 +1754,6 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 
 err_unregister_aux:
 	drm_dp_aux_unregister(&dp->aux);
-err_disable_pm_runtime:
-	if (IS_ENABLED(CONFIG_PM)) {
-		pm_runtime_dont_use_autosuspend(dp->dev);
-		pm_runtime_disable(dp->dev);
-	} else {
-		analogix_dp_suspend(dp);
-	}
 
 	return ret;
 }
@@ -1762,13 +1770,6 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
 	}
 
 	drm_dp_aux_unregister(&dp->aux);
-
-	if (IS_ENABLED(CONFIG_PM)) {
-		pm_runtime_dont_use_autosuspend(dp->dev);
-		pm_runtime_disable(dp->dev);
-	} else {
-		analogix_dp_suspend(dp);
-	}
 }
 EXPORT_SYMBOL_GPL(analogix_dp_unbind);
 
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index a63c2c867059..13f32aeea7ca 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -459,6 +459,10 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 
 static void rockchip_dp_remove(struct platform_device *pdev)
 {
+	struct rockchip_dp_device *dp = dev_get_drvdata(&pdev->dev);
+
+	analogix_dp_remove(dp->adp);
+
 	component_del(&pdev->dev, &rockchip_dp_component_ops);
 }
 
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 1b119b547988..d9b721fd7201 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -43,6 +43,7 @@ int analogix_dp_suspend(struct analogix_dp_device *dp);
 
 struct analogix_dp_device *
 analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data);
+void analogix_dp_remove(struct analogix_dp_device *dp);
 int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev);
 void analogix_dp_unbind(struct analogix_dp_device *dp);
 
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 07/14] drm/bridge: analogix_dp: Add support to get panel from the DP AUX bus

[Dmitry Baryshkov]

On Thu, Jan 23, 2025 at 06:07:40PM +0800, Damon Ding wrote:
> The main modification is moving the DP AUX initialization from function
> analogix_dp_bind() to analogix_dp_probe(). In order to get the EDID of
> eDP panel during probing, it is also needed to advance PM operaions to
> ensure that eDP controller and phy are prepared for AUX transmission.

This doesn't sound right. Per the documentation of
drm_dp_aux::transfer(), the device should power itself up if transfer()
is called when it is powered off. The caller must only ensure that the
panel is on.

Doug, what's your opinion?

> 
> In addtion, add a new function analogix_dp_remove() to ensure symmetry
> for PM operations.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> ---
> 
> Changes in v4:
> - Use done_probing() to call drm_of_find_panel_or_bridge() and
>   component_add() when getting panel from the DP AUX bus
> 
> Changes in v5:
> - Advance PM operations to make eDP AUX work well
> 
> Changes in v6:
> - Use devm_pm_runtime_enable() instead of devm_add_action_or_reset()
> - Add a new function analogix_dp_remove() to ensure symmetry for PM
>   operations
> ---
>  .../drm/bridge/analogix/analogix_dp_core.c    | 57 ++++++++++---------
>  .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  4 ++
>  include/drm/bridge/analogix_dp.h              |  1 +
>  3 files changed, 34 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 8251adfce2f9..30da8a14361e 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1658,14 +1658,42 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
>  	}
>  	disable_irq(dp->irq);
>  
> +	dp->aux.name = "DP-AUX";
> +	dp->aux.transfer = analogix_dpaux_transfer;
> +	dp->aux.dev = dp->dev;
> +	drm_dp_aux_init(&dp->aux);
> +
> +	if (IS_ENABLED(CONFIG_PM)) {
> +		pm_runtime_use_autosuspend(dp->dev);
> +		pm_runtime_set_autosuspend_delay(dp->dev, 100);
> +		ret = devm_pm_runtime_enable(dp->dev);
> +		if (ret)
> +			goto err_disable_pm_runtime;
> +	} else {
> +		ret = analogix_dp_resume(dp);
> +		if (ret)
> +			goto err_disable_clk;
> +	}
> +
>  	return dp;
>  
> +err_disable_pm_runtime:
> +	pm_runtime_dont_use_autosuspend(dp->dev);
>  err_disable_clk:
>  	clk_disable_unprepare(dp->clock);
>  	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_probe);
>  
> +void analogix_dp_remove(struct analogix_dp_device *dp)
> +{
> +	if (IS_ENABLED(CONFIG_PM))
> +		pm_runtime_dont_use_autosuspend(dp->dev);
> +	else
> +		analogix_dp_suspend(dp);
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_remove);
> +
>  int analogix_dp_suspend(struct analogix_dp_device *dp)
>  {
>  	phy_power_off(dp->phy);
> @@ -1708,25 +1736,12 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>  	dp->drm_dev = drm_dev;
>  	dp->encoder = dp->plat_data->encoder;
>  
> -	if (IS_ENABLED(CONFIG_PM)) {
> -		pm_runtime_use_autosuspend(dp->dev);
> -		pm_runtime_set_autosuspend_delay(dp->dev, 100);
> -		pm_runtime_enable(dp->dev);
> -	} else {
> -		ret = analogix_dp_resume(dp);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	dp->aux.name = "DP-AUX";
> -	dp->aux.transfer = analogix_dpaux_transfer;
> -	dp->aux.dev = dp->dev;
>  	dp->aux.drm_dev = drm_dev;
>  
>  	ret = drm_dp_aux_register(&dp->aux);
>  	if (ret) {
>  		DRM_ERROR("failed to register AUX (%d)\n", ret);
> -		goto err_disable_pm_runtime;
> +		return ret;
>  	}
>  
>  	ret = analogix_dp_create_bridge(drm_dev, dp);
> @@ -1739,13 +1754,6 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>  
>  err_unregister_aux:
>  	drm_dp_aux_unregister(&dp->aux);
> -err_disable_pm_runtime:
> -	if (IS_ENABLED(CONFIG_PM)) {
> -		pm_runtime_dont_use_autosuspend(dp->dev);
> -		pm_runtime_disable(dp->dev);
> -	} else {
> -		analogix_dp_suspend(dp);
> -	}
>  
>  	return ret;
>  }
> @@ -1762,13 +1770,6 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
>  	}
>  
>  	drm_dp_aux_unregister(&dp->aux);
> -
> -	if (IS_ENABLED(CONFIG_PM)) {
> -		pm_runtime_dont_use_autosuspend(dp->dev);
> -		pm_runtime_disable(dp->dev);
> -	} else {
> -		analogix_dp_suspend(dp);
> -	}
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_unbind);
>  
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index a63c2c867059..13f32aeea7ca 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -459,6 +459,10 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  
>  static void rockchip_dp_remove(struct platform_device *pdev)
>  {
> +	struct rockchip_dp_device *dp = dev_get_drvdata(&pdev->dev);
> +
> +	analogix_dp_remove(dp->adp);
> +
>  	component_del(&pdev->dev, &rockchip_dp_component_ops);
>  }
>  
> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
> index 1b119b547988..d9b721fd7201 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -43,6 +43,7 @@ int analogix_dp_suspend(struct analogix_dp_device *dp);
>  
>  struct analogix_dp_device *
>  analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data);
> +void analogix_dp_remove(struct analogix_dp_device *dp);
>  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev);
>  void analogix_dp_unbind(struct analogix_dp_device *dp);
>  
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 07/14] drm/bridge: analogix_dp: Add support to get panel from the DP AUX bus

[Doug Anderson]

Hi,

On Thu, Jan 23, 2025 at 3:25 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Thu, Jan 23, 2025 at 06:07:40PM +0800, Damon Ding wrote:
> > The main modification is moving the DP AUX initialization from function
> > analogix_dp_bind() to analogix_dp_probe(). In order to get the EDID of
> > eDP panel during probing, it is also needed to advance PM operaions to

nit: s/operaions/operations


> > ensure that eDP controller and phy are prepared for AUX transmission.
>
> This doesn't sound right. Per the documentation of
> drm_dp_aux::transfer(), the device should power itself up if transfer()
> is called when it is powered off. The caller must only ensure that the
> panel is on.
>
> Doug, what's your opinion?

I think maybe the CL description is a bit confusing, but looking at
the patch I think that the general idea is correct. drm_dp_aux_init()
is expected to be called in probe() and not in bind(). ...and in order
for it to work then pm_runtime needs to be enabled at probe and not at
bind. So both of those two things that this patch does are (in my
opinion) correct.


> > In addtion, add a new function analogix_dp_remove() to ensure symmetry
> > for PM operations.
> >
> > Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> >
> > ---
> >
> > Changes in v4:
> > - Use done_probing() to call drm_of_find_panel_or_bridge() and
> >   component_add() when getting panel from the DP AUX bus
> >
> > Changes in v5:
> > - Advance PM operations to make eDP AUX work well
> >
> > Changes in v6:
> > - Use devm_pm_runtime_enable() instead of devm_add_action_or_reset()
> > - Add a new function analogix_dp_remove() to ensure symmetry for PM
> >   operations
> > ---
> >  .../drm/bridge/analogix/analogix_dp_core.c    | 57 ++++++++++---------
> >  .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  4 ++
> >  include/drm/bridge/analogix_dp.h              |  1 +
> >  3 files changed, 34 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > index 8251adfce2f9..30da8a14361e 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -1658,14 +1658,42 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
> >       }
> >       disable_irq(dp->irq);

not related to your patch, but probably needs to be a prerequisite of
your patch: instead of calling disable_irq() here, you should OR in
"IRQF_NO_AUTOEN" to the "irq_flags" of devm_request_threaded_irq().
That not only closes a potential race condition but also makes all the
error handling much more robust.


> > +     dp->aux.name = "DP-AUX";
> > +     dp->aux.transfer = analogix_dpaux_transfer;
> > +     dp->aux.dev = dp->dev;
> > +     drm_dp_aux_init(&dp->aux);

FWIW: I would highly encourage you to (in a separate patch) add
wait_hpd_asserted() support here. It is deprecated to not implement
wait_hpd_asserted(). See the definition of "wait_hpd_asserted" in
"struct drm_dp_aux" if you're going to support eDP panels.


> > +     if (IS_ENABLED(CONFIG_PM)) {

Do we really truly need this? Is there anyone actually using this
driver without "CONFIG_PM", or can we just assume CONFIG_PM. For the
most part drivers I've interacted with just assume CONFIG_PM and
they're a lot simpler because of it. If there's truly a need then we
can keep this complexity, but I'd rather wait until there is a user.
Maybe you could add this as a dependency in the Kconfig if needed.


> > +             pm_runtime_use_autosuspend(dp->dev);
> > +             pm_runtime_set_autosuspend_delay(dp->dev, 100);
> > +             ret = devm_pm_runtime_enable(dp->dev);
> > +             if (ret)
> > +                     goto err_disable_pm_runtime;
> > +     } else {
> > +             ret = analogix_dp_resume(dp);
> > +             if (ret)
> > +                     goto err_disable_clk;

IMO: if analogix_dp_resume() succeeds, use devm_add_action_or_reset()
to have a function call analogix_dp_suspend(). Then you can keep using
"devm" for everything and totally get rid of the need for the
analogix_dp_remove() function.


> > +     }
> > +
> >       return dp;
> >
> > +err_disable_pm_runtime:
> > +     pm_runtime_dont_use_autosuspend(dp->dev);

You don't need to call pm_runtime_dont_use_autosuspend(). If you
enabled pm_runtime with devm_pm_runtime_enable() then it's documented
to handle calling pm_runtime_dont_use_autosuspend() for you. See the
kernel doc comment for devm_pm_runtime_enable(). So you can get rid of
this.


> >  err_disable_clk:
> >       clk_disable_unprepare(dp->clock);
> >       return ERR_PTR(ret);

Huh? Why would you call "err_disable_clk" here? The only thing that
enables the clock is analogix_dp_resume(), right? There's something
fishy here and it predates your patch. I suspect there were problems
in commit f37952339cc2 ("drm/bridge: analogix_dp: handle clock via
runtime PM"). You should fix that in a separate patch before yours.


> > +void analogix_dp_remove(struct analogix_dp_device *dp)
> > +{
> > +     if (IS_ENABLED(CONFIG_PM))
> > +             pm_runtime_dont_use_autosuspend(dp->dev);
> > +     else
> > +             analogix_dp_suspend(dp);
> > +}
> > +EXPORT_SYMBOL_GPL(analogix_dp_remove);

See above. Proper use of "devm" should mean you don't need a remove() function.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 07/14] drm/bridge: analogix_dp: Add support to get panel from the DP AUX bus

[Damon Ding]

Hi Doug,

On 2025/1/24 11:13, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jan 23, 2025 at 3:25 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> On Thu, Jan 23, 2025 at 06:07:40PM +0800, Damon Ding wrote:
>>> The main modification is moving the DP AUX initialization from function
>>> analogix_dp_bind() to analogix_dp_probe(). In order to get the EDID of
>>> eDP panel during probing, it is also needed to advance PM operaions to
> 
> nit: s/operaions/operations
> 

I will fix it in the next version.

> 
>>> ensure that eDP controller and phy are prepared for AUX transmission.
>>
>> This doesn't sound right. Per the documentation of
>> drm_dp_aux::transfer(), the device should power itself up if transfer()
>> is called when it is powered off. The caller must only ensure that the
>> panel is on.
>>
>> Doug, what's your opinion?
> 
> I think maybe the CL description is a bit confusing, but looking at
> the patch I think that the general idea is correct. drm_dp_aux_init()
> is expected to be called in probe() and not in bind(). ...and in order
> for it to work then pm_runtime needs to be enabled at probe and not at
> bind. So both of those two things that this patch does are (in my
> opinion) correct.
> 
> 
>>> In addtion, add a new function analogix_dp_remove() to ensure symmetry
>>> for PM operations.
>>>
>>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>>
>>> ---
>>>
>>> Changes in v4:
>>> - Use done_probing() to call drm_of_find_panel_or_bridge() and
>>>    component_add() when getting panel from the DP AUX bus
>>>
>>> Changes in v5:
>>> - Advance PM operations to make eDP AUX work well
>>>
>>> Changes in v6:
>>> - Use devm_pm_runtime_enable() instead of devm_add_action_or_reset()
>>> - Add a new function analogix_dp_remove() to ensure symmetry for PM
>>>    operations
>>> ---
>>>   .../drm/bridge/analogix/analogix_dp_core.c    | 57 ++++++++++---------
>>>   .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  4 ++
>>>   include/drm/bridge/analogix_dp.h              |  1 +
>>>   3 files changed, 34 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index 8251adfce2f9..30da8a14361e 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> @@ -1658,14 +1658,42 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
>>>        }
>>>        disable_irq(dp->irq);
> 
> not related to your patch, but probably needs to be a prerequisite of
> your patch: instead of calling disable_irq() here, you should OR in
> "IRQF_NO_AUTOEN" to the "irq_flags" of devm_request_threaded_irq().
> That not only closes a potential race condition but also makes all the
> error handling much more robust.
> 

Yes, it is nice to set the IRQF_NO_AUTOEN flag rather than calling the 
disable_irq().

> 
>>> +     dp->aux.name = "DP-AUX";
>>> +     dp->aux.transfer = analogix_dpaux_transfer;
>>> +     dp->aux.dev = dp->dev;
>>> +     drm_dp_aux_init(&dp->aux);
> 
> FWIW: I would highly encourage you to (in a separate patch) add
> wait_hpd_asserted() support here. It is deprecated to not implement
> wait_hpd_asserted(). See the definition of "wait_hpd_asserted" in
> "struct drm_dp_aux" if you're going to support eDP panels.
> 

Indeed, the &drm_dp_aux.wait_hpd_asserted() help confirm the HPD state 
before doing AUX transfers. I will add it in the next version.

> 
>>> +     if (IS_ENABLED(CONFIG_PM)) {
> 
> Do we really truly need this? Is there anyone actually using this
> driver without "CONFIG_PM", or can we just assume CONFIG_PM. For the
> most part drivers I've interacted with just assume CONFIG_PM and
> they're a lot simpler because of it. If there's truly a need then we
> can keep this complexity, but I'd rather wait until there is a user.
> Maybe you could add this as a dependency in the Kconfig if needed.
> 

I would also like to remove the CONFIG_PM related check, which would 
make the code more concise. And I believe that it would be a good idea 
to remove it and wait until there is a user who actually needs it.

> 
>>> +             pm_runtime_use_autosuspend(dp->dev);
>>> +             pm_runtime_set_autosuspend_delay(dp->dev, 100);
>>> +             ret = devm_pm_runtime_enable(dp->dev);
>>> +             if (ret)
>>> +                     goto err_disable_pm_runtime;
>>> +     } else {
>>> +             ret = analogix_dp_resume(dp);
>>> +             if (ret)
>>> +                     goto err_disable_clk;
> 
> IMO: if analogix_dp_resume() succeeds, use devm_add_action_or_reset()
> to have a function call analogix_dp_suspend(). Then you can keep using
> "devm" for everything and totally get rid of the need for the
> analogix_dp_remove() function.
> 

It may be better to drop the 'complex' check about CONFIG_PM and just 
keep using 'devm' as mentioned above.

> 
>>> +     }
>>> +
>>>        return dp;
>>>
>>> +err_disable_pm_runtime:
>>> +     pm_runtime_dont_use_autosuspend(dp->dev);
> 
> You don't need to call pm_runtime_dont_use_autosuspend(). If you
> enabled pm_runtime with devm_pm_runtime_enable() then it's documented
> to handle calling pm_runtime_dont_use_autosuspend() for you. See the
> kernel doc comment for devm_pm_runtime_enable(). So you can get rid of
> this.
> 

Yeah, I find the comment in the definition of 
pm_runtime_use_autosuspend() and I will remove it in the next version.

> 
>>>   err_disable_clk:
>>>        clk_disable_unprepare(dp->clock);
>>>        return ERR_PTR(ret);
> 
> Huh? Why would you call "err_disable_clk" here? The only thing that
> enables the clock is analogix_dp_resume(), right? There's something
> fishy here and it predates your patch. I suspect there were problems
> in commit f37952339cc2 ("drm/bridge: analogix_dp: handle clock via
> runtime PM"). You should fix that in a separate patch before yours.
> 

Yes, I believe there is a small bug in the commit mentioned above and I 
will add a separate patch to fix it.

> 
>>> +void analogix_dp_remove(struct analogix_dp_device *dp)
>>> +{
>>> +     if (IS_ENABLED(CONFIG_PM))
>>> +             pm_runtime_dont_use_autosuspend(dp->dev);
>>> +     else
>>> +             analogix_dp_suspend(dp);
>>> +}
>>> +EXPORT_SYMBOL_GPL(analogix_dp_remove);
> 
> See above. Proper use of "devm" should mean you don't need a remove() function.
> 

Yeah, the analogix_dp_remove() is not needed based on the above discussion.

> 

Best regards
Damon


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 08/14] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

[Damon Ding]

Move drm_of_find_panel_or_bridge() a little later and combine it with
component_add() into a new function rockchip_dp_link_panel(). The function
will serve as done_probing() callback of devm_of_dp_aux_populate_bus(),
aiding to support for obtaining the eDP panel via the DP AUX bus.

If failed to get the panel from the DP AUX bus, it will then try the other
way to get panel information through the platform bus.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Use done_probing() to call drm_of_find_panel_or_bridge() and
  component_add() when getting panel from the DP AUX bus

Changes in v5:
- Use the functions exported by the Analogix side to get the pointers of
  struct analogix_dp_plat_data and struct drm_dp_aux.
- Use dev_err() instead of drm_err() in rockchip_dp_poweron().

Changes in v6:
- Keep drm_err() in rockchip_dp_poweron()
- Pass 'dp' in drm_...() rather than 'dp->drm_dev'
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 37 ++++++++++++++-----
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 13f32aeea7ca..004b1b68d1cf 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -393,11 +393,27 @@ static const struct component_ops rockchip_dp_component_ops = {
 	.unbind = rockchip_dp_unbind,
 };
 
+static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
+{
+	struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
+	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
+	int ret;
+
+	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
+	if (ret)
+		return ret;
+
+	ret = component_add(dp->dev, &rockchip_dp_component_ops);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
 static int rockchip_dp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	const struct rockchip_dp_chip_data *dp_data;
-	struct drm_panel *panel = NULL;
 	struct rockchip_dp_device *dp;
 	struct resource *res;
 	int i;
@@ -407,10 +423,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 	if (!dp_data)
 		return -ENODEV;
 
-	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
-	if (ret < 0 && ret != -ENODEV)
-		return ret;
-
 	dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
 	if (!dp)
 		return -ENOMEM;
@@ -434,7 +446,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 
 	dp->dev = dev;
 	dp->adp = ERR_PTR(-ENODEV);
-	dp->plat_data.panel = panel;
 	dp->plat_data.dev_type = dp->data->chip_type;
 	dp->plat_data.power_on = rockchip_dp_poweron;
 	dp->plat_data.power_off = rockchip_dp_powerdown;
@@ -450,9 +461,17 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 	if (IS_ERR(dp->adp))
 		return PTR_ERR(dp->adp);
 
-	ret = component_add(dev, &rockchip_dp_component_ops);
-	if (ret)
-		return ret;
+	ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
+	if (ret) {
+		if (ret != -ENODEV) {
+			drm_err(dp, "failed to populate aux bus : %d\n", ret);
+			return ret;
+		}
+
+		ret = rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 08/14] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

[Dmitry Baryshkov]

On Thu, Jan 23, 2025 at 06:07:41PM +0800, Damon Ding wrote:
> Move drm_of_find_panel_or_bridge() a little later and combine it with
> component_add() into a new function rockchip_dp_link_panel(). The function
> will serve as done_probing() callback of devm_of_dp_aux_populate_bus(),
> aiding to support for obtaining the eDP panel via the DP AUX bus.
> 
> If failed to get the panel from the DP AUX bus, it will then try the other
> way to get panel information through the platform bus.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> ---
> 
> Changes in v4:
> - Use done_probing() to call drm_of_find_panel_or_bridge() and
>   component_add() when getting panel from the DP AUX bus
> 
> Changes in v5:
> - Use the functions exported by the Analogix side to get the pointers of
>   struct analogix_dp_plat_data and struct drm_dp_aux.
> - Use dev_err() instead of drm_err() in rockchip_dp_poweron().
> 
> Changes in v6:
> - Keep drm_err() in rockchip_dp_poweron()
> - Pass 'dp' in drm_...() rather than 'dp->drm_dev'
> ---
>  .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 37 ++++++++++++++-----
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 13f32aeea7ca..004b1b68d1cf 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -393,11 +393,27 @@ static const struct component_ops rockchip_dp_component_ops = {
>  	.unbind = rockchip_dp_unbind,
>  };
>  
> +static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
> +{
> +	struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
> +	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
> +	int ret;
> +
> +	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
> +	if (ret)
> +		return ret;
> +
> +	ret = component_add(dp->dev, &rockchip_dp_component_ops);
> +	if (ret)
> +		return ret;
> +
> +	return ret;
> +}
> +
>  static int rockchip_dp_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	const struct rockchip_dp_chip_data *dp_data;
> -	struct drm_panel *panel = NULL;
>  	struct rockchip_dp_device *dp;
>  	struct resource *res;
>  	int i;
> @@ -407,10 +423,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  	if (!dp_data)
>  		return -ENODEV;
>  
> -	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
> -	if (ret < 0 && ret != -ENODEV)
> -		return ret;
> -
>  	dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
>  	if (!dp)
>  		return -ENOMEM;
> @@ -434,7 +446,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  
>  	dp->dev = dev;
>  	dp->adp = ERR_PTR(-ENODEV);
> -	dp->plat_data.panel = panel;
>  	dp->plat_data.dev_type = dp->data->chip_type;
>  	dp->plat_data.power_on = rockchip_dp_poweron;
>  	dp->plat_data.power_off = rockchip_dp_powerdown;
> @@ -450,9 +461,17 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  	if (IS_ERR(dp->adp))
>  		return PTR_ERR(dp->adp);
>  
> -	ret = component_add(dev, &rockchip_dp_component_ops);
> -	if (ret)
> -		return ret;
> +	ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
> +	if (ret) {
> +		if (ret != -ENODEV) {
> +			drm_err(dp, "failed to populate aux bus : %d\n", ret);
> +			return ret;

return dev_err_probe().

With that fixed:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> +		}
> +
> +		ret = rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
> +		if (ret)
> +			return ret;
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 08/14] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

[Heiko Stübner]

Hi Damon,

Am Donnerstag, 23. Januar 2025, 11:07:41 MEZ schrieb Damon Ding:
> Move drm_of_find_panel_or_bridge() a little later and combine it with
> component_add() into a new function rockchip_dp_link_panel(). The function
> will serve as done_probing() callback of devm_of_dp_aux_populate_bus(),
> aiding to support for obtaining the eDP panel via the DP AUX bus.
> 
> If failed to get the panel from the DP AUX bus, it will then try the other
> way to get panel information through the platform bus.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> ---

> @@ -450,9 +461,17 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  	if (IS_ERR(dp->adp))
>  		return PTR_ERR(dp->adp);
>  
> -	ret = component_add(dev, &rockchip_dp_component_ops);
> -	if (ret)
> -		return ret;
> +	ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);

This causes an undefined-reference error, so you probably need something like:

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index f9cbbb40b36f..fa946a809858 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -8,6 +8,7 @@ config DRM_ROCKCHIP
        select DRM_PANEL
        select VIDEOMODE_HELPERS
        select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
+       select DRM_DISPLAY_DP_AUX_BUS if ROCKCHIP_ANALOGIX_DP
        select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
        select DRM_DW_HDMI_QP if ROCKCHIP_DW_HDMI_QP
        select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI




-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 08/14] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

[Damon Ding]

Hi Heiko,

On 2025/1/31 4:33, Heiko Stübner wrote:
> Hi Damon,
> 
> Am Donnerstag, 23. Januar 2025, 11:07:41 MEZ schrieb Damon Ding:
>> Move drm_of_find_panel_or_bridge() a little later and combine it with
>> component_add() into a new function rockchip_dp_link_panel(). The function
>> will serve as done_probing() callback of devm_of_dp_aux_populate_bus(),
>> aiding to support for obtaining the eDP panel via the DP AUX bus.
>>
>> If failed to get the panel from the DP AUX bus, it will then try the other
>> way to get panel information through the platform bus.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ---
> 
>> @@ -450,9 +461,17 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>>   	if (IS_ERR(dp->adp))
>>   		return PTR_ERR(dp->adp);
>>   
>> -	ret = component_add(dev, &rockchip_dp_component_ops);
>> -	if (ret)
>> -		return ret;
>> +	ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
> 
> This causes an undefined-reference error, so you probably need something like:
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index f9cbbb40b36f..fa946a809858 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -8,6 +8,7 @@ config DRM_ROCKCHIP
>          select DRM_PANEL
>          select VIDEOMODE_HELPERS
>          select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
> +       select DRM_DISPLAY_DP_AUX_BUS if ROCKCHIP_ANALOGIX_DP
>          select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>          select DRM_DW_HDMI_QP if ROCKCHIP_DW_HDMI_QP
>          select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
> 
> 
> 
> 
> 

Yeah, I will add a new separate commit to do it in the next version.

Best regards
Damon


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 08/14] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

[Damon Ding]

On 2025/2/22 19:25, Damon Ding wrote:
> Hi Heiko,
> 
> On 2025/1/31 4:33, Heiko Stübner wrote:
>> Hi Damon,
>>
>> Am Donnerstag, 23. Januar 2025, 11:07:41 MEZ schrieb Damon Ding:
>>> Move drm_of_find_panel_or_bridge() a little later and combine it with
>>> component_add() into a new function rockchip_dp_link_panel(). The 
>>> function
>>> will serve as done_probing() callback of devm_of_dp_aux_populate_bus(),
>>> aiding to support for obtaining the eDP panel via the DP AUX bus.
>>>
>>> If failed to get the panel from the DP AUX bus, it will then try the 
>>> other
>>> way to get panel information through the platform bus.
>>>
>>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>>
>>> ---
>>
>>> @@ -450,9 +461,17 @@ static int rockchip_dp_probe(struct 
>>> platform_device *pdev)
>>>       if (IS_ERR(dp->adp))
>>>           return PTR_ERR(dp->adp);
>>> -    ret = component_add(dev, &rockchip_dp_component_ops);
>>> -    if (ret)
>>> -        return ret;
>>> +    ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), 
>>> rockchip_dp_link_panel);
>>
>> This causes an undefined-reference error, so you probably need 
>> something like:
>>
>> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/ 
>> rockchip/Kconfig
>> index f9cbbb40b36f..fa946a809858 100644
>> --- a/drivers/gpu/drm/rockchip/Kconfig
>> +++ b/drivers/gpu/drm/rockchip/Kconfig
>> @@ -8,6 +8,7 @@ config DRM_ROCKCHIP
>>          select DRM_PANEL
>>          select VIDEOMODE_HELPERS
>>          select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>> +       select DRM_DISPLAY_DP_AUX_BUS if ROCKCHIP_ANALOGIX_DP
>>          select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>>          select DRM_DW_HDMI_QP if ROCKCHIP_DW_HDMI_QP
>>          select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
>>
>>
>>
>>
>>
> 
> Yeah, I will add a new separate commit to do it in the next version.
>

Oh...I believe it would be better to do it directly in this patch. ;-)

> Best regards
> Damon
> 
> 
> 



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 08/14] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

[Lucas Stach]

Hi Damon,

Am Donnerstag, dem 23.01.2025 um 18:07 +0800 schrieb Damon Ding:
> Move drm_of_find_panel_or_bridge() a little later and combine it with
> component_add() into a new function rockchip_dp_link_panel(). The function
> will serve as done_probing() callback of devm_of_dp_aux_populate_bus(),
> aiding to support for obtaining the eDP panel via the DP AUX bus.
> 
> If failed to get the panel from the DP AUX bus, it will then try the other
> way to get panel information through the platform bus.
> 
The changes in this patch effectively revert 86caee745e45
("drm/rockchip: analogix_dp: allow to work without panel"). Please
correct this in the next revision of this patchset.

Regards,
Lucas

> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> ---
> 
> Changes in v4:
> - Use done_probing() to call drm_of_find_panel_or_bridge() and
>   component_add() when getting panel from the DP AUX bus
> 
> Changes in v5:
> - Use the functions exported by the Analogix side to get the pointers of
>   struct analogix_dp_plat_data and struct drm_dp_aux.
> - Use dev_err() instead of drm_err() in rockchip_dp_poweron().
> 
> Changes in v6:
> - Keep drm_err() in rockchip_dp_poweron()
> - Pass 'dp' in drm_...() rather than 'dp->drm_dev'
> ---
>  .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 37 ++++++++++++++-----
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 13f32aeea7ca..004b1b68d1cf 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -393,11 +393,27 @@ static const struct component_ops rockchip_dp_component_ops = {
>  	.unbind = rockchip_dp_unbind,
>  };
>  
> +static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
> +{
> +	struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
> +	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
> +	int ret;
> +
> +	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
> +	if (ret)
> +		return ret;
> +
> +	ret = component_add(dp->dev, &rockchip_dp_component_ops);
> +	if (ret)
> +		return ret;
> +
> +	return ret;
> +}
> +
>  static int rockchip_dp_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	const struct rockchip_dp_chip_data *dp_data;
> -	struct drm_panel *panel = NULL;
>  	struct rockchip_dp_device *dp;
>  	struct resource *res;
>  	int i;
> @@ -407,10 +423,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  	if (!dp_data)
>  		return -ENODEV;
>  
> -	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
> -	if (ret < 0 && ret != -ENODEV)
> -		return ret;
> -
>  	dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
>  	if (!dp)
>  		return -ENOMEM;
> @@ -434,7 +446,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  
>  	dp->dev = dev;
>  	dp->adp = ERR_PTR(-ENODEV);
> -	dp->plat_data.panel = panel;
>  	dp->plat_data.dev_type = dp->data->chip_type;
>  	dp->plat_data.power_on = rockchip_dp_poweron;
>  	dp->plat_data.power_off = rockchip_dp_powerdown;
> @@ -450,9 +461,17 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  	if (IS_ERR(dp->adp))
>  		return PTR_ERR(dp->adp);
>  
> -	ret = component_add(dev, &rockchip_dp_component_ops);
> -	if (ret)
> -		return ret;
> +	ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
> +	if (ret) {
> +		if (ret != -ENODEV) {
> +			drm_err(dp, "failed to populate aux bus : %d\n", ret);
> +			return ret;
> +		}
> +
> +		ret = rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
> +		if (ret)
> +			return ret;
> +	}
>  
>  	return 0;
>  }


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 08/14] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

[Damon Ding]

Hi Lucas,

On 2025/2/8 3:42, Lucas Stach wrote:
> Hi Damon,
> 
> Am Donnerstag, dem 23.01.2025 um 18:07 +0800 schrieb Damon Ding:
>> Move drm_of_find_panel_or_bridge() a little later and combine it with
>> component_add() into a new function rockchip_dp_link_panel(). The function
>> will serve as done_probing() callback of devm_of_dp_aux_populate_bus(),
>> aiding to support for obtaining the eDP panel via the DP AUX bus.
>>
>> If failed to get the panel from the DP AUX bus, it will then try the other
>> way to get panel information through the platform bus.
>>
> The changes in this patch effectively revert 86caee745e45
> ("drm/rockchip: analogix_dp: allow to work without panel"). Please
> correct this in the next revision of this patchset.
> 
> Regards,
> Lucas
> 

Oh, I see. I will fix it in the next version.

>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ---
>>
>> Changes in v4:
>> - Use done_probing() to call drm_of_find_panel_or_bridge() and
>>    component_add() when getting panel from the DP AUX bus
>>
>> Changes in v5:
>> - Use the functions exported by the Analogix side to get the pointers of
>>    struct analogix_dp_plat_data and struct drm_dp_aux.
>> - Use dev_err() instead of drm_err() in rockchip_dp_poweron().
>>
>> Changes in v6:
>> - Keep drm_err() in rockchip_dp_poweron()
>> - Pass 'dp' in drm_...() rather than 'dp->drm_dev'
>> ---
>>   .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 37 ++++++++++++++-----
>>   1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> index 13f32aeea7ca..004b1b68d1cf 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -393,11 +393,27 @@ static const struct component_ops rockchip_dp_component_ops = {
>>   	.unbind = rockchip_dp_unbind,
>>   };
>>   
>> +static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
>> +{
>> +	struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
>> +	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
>> +	int ret;
>> +
>> +	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = component_add(dp->dev, &rockchip_dp_component_ops);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return ret;
>> +}
>> +
>>   static int rockchip_dp_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	const struct rockchip_dp_chip_data *dp_data;
>> -	struct drm_panel *panel = NULL;
>>   	struct rockchip_dp_device *dp;
>>   	struct resource *res;
>>   	int i;
>> @@ -407,10 +423,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>>   	if (!dp_data)
>>   		return -ENODEV;
>>   
>> -	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
>> -	if (ret < 0 && ret != -ENODEV)
>> -		return ret;
>> -
>>   	dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
>>   	if (!dp)
>>   		return -ENOMEM;
>> @@ -434,7 +446,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>>   
>>   	dp->dev = dev;
>>   	dp->adp = ERR_PTR(-ENODEV);
>> -	dp->plat_data.panel = panel;
>>   	dp->plat_data.dev_type = dp->data->chip_type;
>>   	dp->plat_data.power_on = rockchip_dp_poweron;
>>   	dp->plat_data.power_off = rockchip_dp_powerdown;
>> @@ -450,9 +461,17 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>>   	if (IS_ERR(dp->adp))
>>   		return PTR_ERR(dp->adp);
>>   
>> -	ret = component_add(dev, &rockchip_dp_component_ops);
>> -	if (ret)
>> -		return ret;
>> +	ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
>> +	if (ret) {
>> +		if (ret != -ENODEV) {
>> +			drm_err(dp, "failed to populate aux bus : %d\n", ret);
>> +			return ret;
>> +		}
>> +
>> +		ret = rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
>> +		if (ret)
>> +			return ret;
>> +	}
>>   
>>   	return 0;
>>   }
> 
> 
> 

Best regards
Damon


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 09/14] dt-bindings: display: rockchip: analogix-dp: Add support for RK3588

[Damon Ding]

Compared with RK3288/RK3399, the HBR2 link rate support is the main
improvement of RK3588 eDP TX controller, and there are also two
independent eDP display interfaces on RK3588 Soc.

The newly added 'apb' reset is to ensure the APB bus of eDP controller
works well on the RK3588 SoC.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v2:
- Add the main defferences of the RK3588 eDP and the previous versions
  in commit message

Changes in v3:
- Expand the property clock-names, resets and reset-names

Changes in v4:
- Remove 'spdif' clock which added in v3
- Add the comment of newly added 'apb' reset in commit message

Changes in v5:
- Put the differences between RK3288/RK3399 and RK3588 into 'allOf'

Changes in v6:
- Keep the widest constraints and add only RK3588 related constraints
  into 'allOf'
---
 .../rockchip/rockchip,analogix-dp.yaml        | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index eaf4e67e232e..baee021314e7 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -15,6 +15,7 @@ properties:
     enum:
       - rockchip,rk3288-dp
       - rockchip,rk3399-edp
+      - rockchip,rk3588-edp
 
   clocks:
     minItems: 2
@@ -31,9 +32,12 @@ properties:
     maxItems: 1
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   reset-names:
+    minItems: 1
+    maxItems: 2
     const: dp
 
   rockchip,grf:
@@ -55,6 +59,22 @@ required:
 allOf:
   - $ref: /schemas/display/bridge/analogix,dp.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - rockchip,rk3588-edp
+    then:
+      properties:
+        resets:
+          minItems: 2
+          maxItems: 2
+        reset-names:
+          items:
+            - const: dp
+            - const: apb
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 09/14] dt-bindings: display: rockchip: analogix-dp: Add support for RK3588

[Rob Herring (Arm)]

On Thu, 23 Jan 2025 18:07:42 +0800, Damon Ding wrote:
> Compared with RK3288/RK3399, the HBR2 link rate support is the main
> improvement of RK3588 eDP TX controller, and there are also two
> independent eDP display interfaces on RK3588 Soc.
> 
> The newly added 'apb' reset is to ensure the APB bus of eDP controller
> works well on the RK3588 SoC.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> ---
> 
> Changes in v2:
> - Add the main defferences of the RK3588 eDP and the previous versions
>   in commit message
> 
> Changes in v3:
> - Expand the property clock-names, resets and reset-names
> 
> Changes in v4:
> - Remove 'spdif' clock which added in v3
> - Add the comment of newly added 'apb' reset in commit message
> 
> Changes in v5:
> - Put the differences between RK3288/RK3399 and RK3588 into 'allOf'
> 
> Changes in v6:
> - Keep the widest constraints and add only RK3588 related constraints
>   into 'allOf'
> ---
>  .../rockchip/rockchip,analogix-dp.yaml        | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml: properties:reset-names: 'const' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
	hint: Scalar and array keywords cannot be mixed
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml: properties:reset-names: 'const' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
	hint: Scalar and array keywords cannot be mixed
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250123100747.1841357-10-damon.ding@rock-chips.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 09/14] dt-bindings: display: rockchip: analogix-dp: Add support for RK3588

[Damon Ding]

Hi Rob,

On 2025/1/23 19:22, Rob Herring (Arm) wrote:
> 
> On Thu, 23 Jan 2025 18:07:42 +0800, Damon Ding wrote:
>> Compared with RK3288/RK3399, the HBR2 link rate support is the main
>> improvement of RK3588 eDP TX controller, and there are also two
>> independent eDP display interfaces on RK3588 Soc.
>>
>> The newly added 'apb' reset is to ensure the APB bus of eDP controller
>> works well on the RK3588 SoC.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ---
>>
>> Changes in v2:
>> - Add the main defferences of the RK3588 eDP and the previous versions
>>    in commit message
>>
>> Changes in v3:
>> - Expand the property clock-names, resets and reset-names
>>
>> Changes in v4:
>> - Remove 'spdif' clock which added in v3
>> - Add the comment of newly added 'apb' reset in commit message
>>
>> Changes in v5:
>> - Put the differences between RK3288/RK3399 and RK3588 into 'allOf'
>>
>> Changes in v6:
>> - Keep the widest constraints and add only RK3588 related constraints
>>    into 'allOf'
>> ---
>>   .../rockchip/rockchip,analogix-dp.yaml        | 22 ++++++++++++++++++-
>>   1 file changed, 21 insertions(+), 1 deletion(-)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml: properties:reset-names: 'const' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
> 	hint: Scalar and array keywords cannot be mixed
> 	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml: properties:reset-names: 'const' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
> 	hint: Scalar and array keywords cannot be mixed
> 	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250123100747.1841357-10-damon.ding@rock-chips.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 
> 
> 

I have found these errors using dt_binding_check and will fix them in 
the next version.

Best regards
Damon

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 10/14] drm/bridge: analogix_dp: Add support for RK3588

[Damon Ding]

Expand enum analogix_dp_devtype with RK3588_EDP, and add max_link_rate
and max_lane_count configs for it.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

---

Changes in v5:
- Add the RK3588_EDP related modification in analogix_dp.h
- Move this commit above related commit on the Rockchip side
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 ++++
 include/drm/bridge/analogix_dp.h                   | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 30da8a14361e..5d095d085fcb 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1513,6 +1513,10 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 		video_info->max_link_rate = 0x0A;
 		video_info->max_lane_count = 0x04;
 		break;
+	case RK3588_EDP:
+		video_info->max_link_rate = 0x14;
+		video_info->max_lane_count = 0x04;
+		break;
 	case EXYNOS_DP:
 		/*
 		 * NOTE: those property parseing code is used for
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index d9b721fd7201..eb48ee2bb08d 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -16,11 +16,12 @@ enum analogix_dp_devtype {
 	EXYNOS_DP,
 	RK3288_DP,
 	RK3399_EDP,
+	RK3588_EDP,
 };
 
 static inline bool is_rockchip(enum analogix_dp_devtype type)
 {
-	return type == RK3288_DP || type == RK3399_EDP;
+	return type == RK3288_DP || type == RK3399_EDP || type == RK3588_EDP;
 }
 
 struct analogix_dp_plat_data {
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 11/14] drm/rockchip: analogix_dp: Add support for RK3588

[Damon Ding]

RK3588 integrates the Analogix eDP 1.3 TX controller IP and the HDMI/eDP
TX Combo PHY based on a Samsung IP block. There are also two independent
eDP display interface with different address on RK3588 Soc.

The patch currently adds only the basic support, specifically RGB output
up to 4K@60Hz, without the tests for audio, PSR and other eDP 1.3 specific
features.

In additon, the above Analogix IP has always been utilized as eDP on
Rockchip platform, despite its capability to also support the DP v1.2.
Therefore, the newly added logs will contain the term 'edp' rather than
'dp'. And the newly added 'apb' reset control is to ensure the APB bus
of eDP controller works well on the RK3588 SoC.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v2:
- Add support for the other eDP output edp1

Changes in v3:
- Fix the unexpected use of alias
- Add more details in commit message

Changes in v4:
- Add the 'apb' reset control

Changes in v5:
- Use drm_...()/dev_...() instead of DRM_...()
- Clean &rockchip_dp_chip_data.reg related comments in commit message
- Move the modifications in anlogix_dp.h to the Analogix side in order
  to avoid the warning:

  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1506:10: warning:
  enumeration value 'RK3588_EDP' not handled in switch [-Wswitch]
          switch (dp->plat_data->dev_type) {

Changes in v6:
- Pass 'dp' in drm_...() rather than 'dp->drm_dev'
- Use drm_...() uniformly rather than mixing drm_...() and dev_..()
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 61 ++++++++++++++++++-
 1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 004b1b68d1cf..a86d655b021e 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -51,11 +51,13 @@ struct rockchip_grf_reg_field {
 /**
  * struct rockchip_dp_chip_data - splite the grf setting of kind of chips
  * @lcdc_sel: grf register field of lcdc_sel
+ * @edp_mode: grf register field of edp_mode
  * @chip_type: specific chip type
  * @reg: register base address
  */
 struct rockchip_dp_chip_data {
 	const struct rockchip_grf_reg_field lcdc_sel;
+	const struct rockchip_grf_reg_field edp_mode;
 	u32	chip_type;
 	u32	reg;
 };
@@ -70,6 +72,7 @@ struct rockchip_dp_device {
 	struct clk               *grfclk;
 	struct regmap            *grf;
 	struct reset_control     *rst;
+	struct reset_control     *apbrst;
 
 	const struct rockchip_dp_chip_data *data;
 
@@ -115,6 +118,10 @@ static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
 	usleep_range(10, 20);
 	reset_control_deassert(dp->rst);
 
+	reset_control_assert(dp->apbrst);
+	usleep_range(10, 20);
+	reset_control_deassert(dp->apbrst);
+
 	return 0;
 }
 
@@ -136,12 +143,21 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data)
 		return ret;
 	}
 
+	ret = rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 1);
+	if (ret != 0)
+		drm_err(dp, "failed to set edp mode %d\n", ret);
+
 	return ret;
 }
 
 static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
 {
 	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
+	int ret;
+
+	ret = rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 0);
+	if (ret != 0)
+		drm_err(dp, "failed to set edp mode %d\n", ret);
 
 	clk_disable_unprepare(dp->pclk);
 
@@ -206,6 +222,10 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 	struct rockchip_dp_device *dp = encoder_to_dp(encoder);
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
+	struct of_endpoint endpoint;
+	struct device_node *remote_port, *remote_port_parent;
+	char name[32];
+	u32 port_id;
 	int ret;
 
 	crtc = rockchip_dp_drm_get_new_crtc(encoder, state);
@@ -223,13 +243,27 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 		return;
 	}
 
-	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
+	ret = drm_of_encoder_active_endpoint(dp->dev->of_node, encoder, &endpoint);
 	if (ret < 0)
 		return;
 
-	drm_dbg_core(dp, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
+	remote_port_parent = of_graph_get_remote_port_parent(endpoint.local_node);
+	if (remote_port_parent) {
+		if (of_get_child_by_name(remote_port_parent, "ports")) {
+			remote_port = of_graph_get_remote_port(endpoint.local_node);
+			of_property_read_u32(remote_port, "reg", &port_id);
+			of_node_put(remote_port);
+			sprintf(name, "%s vp%d", remote_port_parent->full_name, port_id);
+		} else {
+			sprintf(name, "%s %s",
+				remote_port_parent->full_name, endpoint.id ? "vopl" : "vopb");
+		}
+		of_node_put(remote_port_parent);
+
+		drm_dbg_core(dp, "%s output to edp\n", name);
+	}
 
-	ret = rockchip_grf_field_write(dp->grf, &dp->data->lcdc_sel, ret);
+	ret = rockchip_grf_field_write(dp->grf, &dp->data->lcdc_sel, endpoint.id);
 	if (ret != 0)
 		drm_err(dp, "Could not write to GRF: %d\n", ret);
 
@@ -323,6 +357,12 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 		return PTR_ERR(dp->rst);
 	}
 
+	dp->apbrst = devm_reset_control_get_optional(dev, "apb");
+	if (IS_ERR(dp->apbrst)) {
+		drm_err(dp, "failed to get apb reset control\n");
+		return PTR_ERR(dp->apbrst);
+	}
+
 	return 0;
 }
 
@@ -526,9 +566,24 @@ static const struct rockchip_dp_chip_data rk3288_dp[] = {
 	{ /* sentinel */ }
 };
 
+static const struct rockchip_dp_chip_data rk3588_edp[] = {
+	{
+		.edp_mode = GRF_REG_FIELD(0x0000, 0, 0),
+		.chip_type = RK3588_EDP,
+		.reg = 0xfdec0000,
+	},
+	{
+		.edp_mode = GRF_REG_FIELD(0x0004, 0, 0),
+		.chip_type = RK3588_EDP,
+		.reg = 0xfded0000,
+	},
+	{ /* sentinel */ }
+};
+
 static const struct of_device_id rockchip_dp_dt_ids[] = {
 	{.compatible = "rockchip,rk3288-dp", .data = &rk3288_dp },
 	{.compatible = "rockchip,rk3399-edp", .data = &rk3399_edp },
+	{.compatible = "rockchip,rk3588-edp", .data = &rk3588_edp },
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_dp_dt_ids);
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 11/14] drm/rockchip: analogix_dp: Add support for RK3588

[Dmitry Baryshkov]

On Thu, Jan 23, 2025 at 06:07:44PM +0800, Damon Ding wrote:
> RK3588 integrates the Analogix eDP 1.3 TX controller IP and the HDMI/eDP
> TX Combo PHY based on a Samsung IP block. There are also two independent
> eDP display interface with different address on RK3588 Soc.
> 
> The patch currently adds only the basic support, specifically RGB output
> up to 4K@60Hz, without the tests for audio, PSR and other eDP 1.3 specific
> features.
> 
> In additon, the above Analogix IP has always been utilized as eDP on
> Rockchip platform, despite its capability to also support the DP v1.2.
> Therefore, the newly added logs will contain the term 'edp' rather than
> 'dp'. And the newly added 'apb' reset control is to ensure the APB bus
> of eDP controller works well on the RK3588 SoC.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> ---
> 

Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 12/14] drm/edp-panel: Add LG Display panel model LP079QX1-SP0V

[Damon Ding]

The raw edid for LP079QX1-SP0V panel model is:

00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00
04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26
0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01
01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10
84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c
50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc
00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index f8511fe5fb0d..77e3fd3ed160 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = {
 	.enable = 50,
 };
 
+static const struct panel_delay delay_50_500_e200 = {
+	.hpd_absent = 50,
+	.unprepare = 500,
+	.enable = 200,
+};
+
 #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
 { \
 	.ident = { \
@@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = {
 	EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"),
 	EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"),
 
+	EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"),
+
 	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d51, &delay_200_500_e200, "Unknown"),
 	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5b, &delay_200_500_e200, "MB116AN01"),
 	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5c, &delay_200_500_e200, "MB116AN01-2"),
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 12/14] drm/edp-panel: Add LG Display panel model LP079QX1-SP0V

[Dmitry Baryshkov]

On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote:
> The raw edid for LP079QX1-SP0V panel model is:
> 
> 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00
> 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26
> 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01
> 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10
> 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c
> 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc
> 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> ---
>  drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas
to CC manually.

Or, better, split irrelevant patches to separate series.

> 
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index f8511fe5fb0d..77e3fd3ed160 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = {
>  	.enable = 50,
>  };
>  
> +static const struct panel_delay delay_50_500_e200 = {
> +	.hpd_absent = 50,
> +	.unprepare = 500,
> +	.enable = 200,
> +};
> +
>  #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
>  { \
>  	.ident = { \
> @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = {
>  	EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"),
>  	EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"),
>  
> +	EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"),
> +
>  	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d51, &delay_200_500_e200, "Unknown"),
>  	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5b, &delay_200_500_e200, "MB116AN01"),
>  	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5c, &delay_200_500_e200, "MB116AN01-2"),
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 12/14] drm/edp-panel: Add LG Display panel model LP079QX1-SP0V

[Doug Anderson]

Hi,

On Thu, Jan 23, 2025 at 3:31 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote:
> > The raw edid for LP079QX1-SP0V panel model is:
> >
> > 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00
> > 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26
> > 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01
> > 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10
> > 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c
> > 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc
> > 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f
> >
> > Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> > ---
> >  drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
>
> Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas
> to CC manually.
>
> Or, better, split irrelevant patches to separate series.
>
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > index f8511fe5fb0d..77e3fd3ed160 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = {
> >       .enable = 50,
> >  };
> >
> > +static const struct panel_delay delay_50_500_e200 = {
> > +     .hpd_absent = 50,
> > +     .unprepare = 500,
> > +     .enable = 200,
> > +};

Wow, hpd_absent is 50ms. That's excellent!

I was curious if this was really correct since it's the lowest I've
seen, but I searched the internet and found a datasheet that confirms
it. Crazy. Although my datasheet has that whole section grayed out and
says "Measurement not available". How worried should be about that? I
guess if "hpd_absent" of 50 eventually doesn't work it can always be
increased.

Looking at that datasheet (assuming you can find the same one), I
wonder how you handle the T4 120ms requirement. It looks like that's
the time from tcon reset (which is nearly power on) until you're
allowed to put valid data on the panel. I _think_ you could meet the
requirement via setting `powered_on_to_enable` to 320 though, right?
...or maybe 335 to handle the maximum value of "tcon_reset" (no idea
how you control this signal). If I remember correctly (it's been a
while), things will already be clocking but outputting black when the
panel's "enable" function is called. After the function is called then
we'll turn on the backlight (and still outputting black) and
eventually we'll put valid data. So as long as you consider the
continued output of black as "valid data" then the timing diagram is
satisfied.

BTW: don't you also need a 200 ms "disable"?


> >  #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
> >  { \
> >       .ident = { \
> > @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = {
> >       EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"),
> >       EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"),
> >
> > +     EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"),

I don't love that the ID is 0x0000. That makes me nervous that the
panel vendor isn't setting the ID properly. ...but at least the string
matches in the EDID so hopefully that will be enough to uniquely
identify things.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 12/14] drm/edp-panel: Add LG Display panel model LP079QX1-SP0V

[Damon Ding]

Hi Doug,

On 2025/1/24 8:30, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jan 23, 2025 at 3:31 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote:
>>> The raw edid for LP079QX1-SP0V panel model is:
>>>
>>> 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00
>>> 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26
>>> 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01
>>> 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10
>>> 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c
>>> 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc
>>> 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f
>>>
>>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>> ---
>>>   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>
>> Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas
>> to CC manually.
>>
>> Or, better, split irrelevant patches to separate series.
>>
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>>> index f8511fe5fb0d..77e3fd3ed160 100644
>>> --- a/drivers/gpu/drm/panel/panel-edp.c
>>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>>> @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = {
>>>        .enable = 50,
>>>   };
>>>
>>> +static const struct panel_delay delay_50_500_e200 = {
>>> +     .hpd_absent = 50,
>>> +     .unprepare = 500,
>>> +     .enable = 200,
>>> +};
> 
> Wow, hpd_absent is 50ms. That's excellent!
> 
> I was curious if this was really correct since it's the lowest I've
> seen, but I searched the internet and found a datasheet that confirms
> it. Crazy. Although my datasheet has that whole section grayed out and
> says "Measurement not available". How worried should be about that? I
> guess if "hpd_absent" of 50 eventually doesn't work it can always be
> increased.
> 

The datasheet I have should be the same as yours, and the T3 is "Min. 
2ms | Typ. 30ms | Max. 50ms" with the gray "Measurement not available".

> Looking at that datasheet (assuming you can find the same one), I
> wonder how you handle the T4 120ms requirement. It looks like that's
> the time from tcon reset (which is nearly power on) until you're
> allowed to put valid data on the panel. I _think_ you could meet the
> requirement via setting `powered_on_to_enable` to 320 though, right?
> ...or maybe 335 to handle the maximum value of "tcon_reset" (no idea
> how you control this signal). If I remember correctly (it's been a
> while), things will already be clocking but outputting black when the
> panel's "enable" function is called. After the function is called then
> we'll turn on the backlight (and still outputting black) and
> eventually we'll put valid data. So as long as you consider the
> continued output of black as "valid data" then the timing diagram is
> satisfied.
> 

Yes, it is indeed better to set the "powered_on_to_enable" to 335ms, as 
this will help prevent the display from showing invalid black screens. I 
will add it in the next version.

> BTW: don't you also need a 200 ms "disable"?
> 

According to the datasheet, it is a good idea to set "disable" to 200ms.

> 
>>>   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
>>>   { \
>>>        .ident = { \
>>> @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = {
>>>        EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"),
>>>        EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"),
>>>
>>> +     EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"),
> 
> I don't love that the ID is 0x0000. That makes me nervous that the
> panel vendor isn't setting the ID properly. ...but at least the string
> matches in the EDID so hopefully that will be enough to uniquely
> identify things.
> 
> 

The ID "0x0000" makes me nervous too, but the EDID obtained from this 
panel indeed show it, which is quite surprising. May I still set it to 
"0x0000" as it really is?

Best regards,
Damon


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 12/14] drm/edp-panel: Add LG Display panel model LP079QX1-SP0V

[Doug Anderson]

Hi,

On Fri, Jan 24, 2025 at 8:18 PM Damon Ding <damon.ding@rock-chips.com> wrote:
>
> >>>   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
> >>>   { \
> >>>        .ident = { \
> >>> @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = {
> >>>        EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"),
> >>>        EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"),
> >>>
> >>> +     EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"),
> >
> > I don't love that the ID is 0x0000. That makes me nervous that the
> > panel vendor isn't setting the ID properly. ...but at least the string
> > matches in the EDID so hopefully that will be enough to uniquely
> > identify things.
> >
> >
>
> The ID "0x0000" makes me nervous too, but the EDID obtained from this
> panel indeed show it, which is quite surprising. May I still set it to
> "0x0000" as it really is?

Yeah, keep it as 0x0000. Since the panel name is in the EDID then
commit bf201127c1b8 ("drm/panel-edp: Match edp_panels with panel
identity") should provide some safety.

-Doug

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 12/14] drm/edp-panel: Add LG Display panel model LP079QX1-SP0V

[Damon Ding]

Hi Dmitry,

On 2025/1/23 19:31, Dmitry Baryshkov wrote:
> On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote:
>> The raw edid for LP079QX1-SP0V panel model is:
>>
>> 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00
>> 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26
>> 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01
>> 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10
>> 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c
>> 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc
>> 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>> ---
>>   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
> 
> Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas
> to CC manually.
> 
> Or, better, split irrelevant patches to separate series.
> 

I will split the panel related patch into a separate series in the next 
version.

>>
>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>> index f8511fe5fb0d..77e3fd3ed160 100644
>> --- a/drivers/gpu/drm/panel/panel-edp.c
>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>> @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = {
>>   	.enable = 50,
>>   };
>>   
>> +static const struct panel_delay delay_50_500_e200 = {
>> +	.hpd_absent = 50,
>> +	.unprepare = 500,
>> +	.enable = 200,
>> +};
>> +
>>   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
>>   { \
>>   	.ident = { \
>> @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = {
>>   	EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"),
>>   	EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"),
>>   
>> +	EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"),
>> +
>>   	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d51, &delay_200_500_e200, "Unknown"),
>>   	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5b, &delay_200_500_e200, "MB116AN01"),
>>   	EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5c, &delay_200_500_e200, "MB116AN01-2"),
>> -- 
>> 2.34.1
>>
> 

Best regards,
Damon


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 13/14] arm64: dts: rockchip: Add eDP0 node for RK3588

[Damon Ding]

Add support for the eDP0 output on RK3588 SoC.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v3:
- Remove currently unsupported property '#sound-dai-cells'

Changes in v4:
- Remove currently unsupported clock 'spdif'
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 7e125897b0cd..7ab460c28c51 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1411,6 +1411,34 @@ hdmi0_out: port@1 {
 		};
 	};
 
+	edp0: edp@fdec0000 {
+		compatible = "rockchip,rk3588-edp";
+		reg = <0x0 0xfdec0000 0x0 0x1000>;
+		clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>;
+		clock-names = "dp", "pclk";
+		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH 0>;
+		phys = <&hdptxphy0>;
+		phy-names = "dp";
+		power-domains = <&power RK3588_PD_VO1>;
+		resets = <&cru SRST_EDP0_24M>, <&cru SRST_P_EDP0>;
+		reset-names = "dp", "apb";
+		rockchip,grf = <&vo1_grf>;
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			edp0_in: port@0 {
+				reg = <0>;
+			};
+
+			edp0_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu_m0: qos@fdf35000 {
 		compatible = "rockchip,rk3588-qos", "syscon";
 		reg = <0x0 0xfdf35000 0x0 0x20>;
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v6 14/14] arm64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board

[Damon Ding]

Add the necessary DT changes to enable eDP0 on RK3588S EVB1 board:
- Set pinctrl of pwm12 for backlight
- Enable edp0/hdptxphy0/vp2
- Assign the parent of DCLK_VOP2_SRC to PLL_V0PLL
- Add aux-bus/panel nodes

For RK3588, the PLL_V0PLL is specifically designed for the VOP2. This
means the clock rate of PLL_V0PLL can be adjusted according to the dclk
rate of relevant VP. It is typically assigned as the dclk source of a
specific VP when the clock of relevant display mode is unusual, such as
the eDP panel 'lg,lp079qx1-sp0v' paired with RK3588S EVB1, which has a
clock rate of 202.02MHz.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v2:
- Remove brightness-levels and default-brightness-level properties in
  backlight node.
- Add the detail DT changes to commit message.

Changes in v3:
- Use aux-bus instead of platform bus for edp-panel.

Changes in v4:
- Add comments related to the use of panel compatible "lg,lp079qx1-sp0v"
  in the commit message.

Changes in v5:
- Use "edp-panel" instead of "lg,lp079qx1-sp0v"
- Remove unnecessary comments in commit message
- Assign the parent of DCLK_VOP2_SRC to PLL_V0PLL

Changes in v6:
- Add PLL_V0PLL related descriptions in commit message
---
 .../boot/dts/rockchip/rk3588s-evb1-v10.dts    | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
index bc4077575beb..a8c151b41e21 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
@@ -9,6 +9,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/soc/rockchip,vop2.h>
 #include <dt-bindings/usb/pd.h>
 #include "rk3588s.dtsi"
 
@@ -238,6 +239,41 @@ &combphy2_psu {
 	status = "okay";
 };
 
+&edp0 {
+	force-hpd;
+	status = "okay";
+
+	aux-bus {
+		panel {
+			compatible = "edp-panel";
+			backlight = <&backlight>;
+			power-supply = <&vcc3v3_lcd_edp>;
+
+			port {
+				panel_in_edp: endpoint {
+					remote-endpoint = <&edp_out_panel>;
+				};
+			};
+		};
+	};
+};
+
+&edp0_in {
+	edp0_in_vp2: endpoint {
+		remote-endpoint = <&vp2_out_edp0>;
+	};
+};
+
+&edp0_out {
+	edp_out_panel: endpoint {
+		remote-endpoint = <&panel_in_edp>;
+	};
+};
+
+&hdptxphy0 {
+	status = "okay";
+};
+
 &i2c3 {
 	status = "okay";
 
@@ -399,6 +435,7 @@ usbc0_int: usbc0-int {
 };
 
 &pwm12 {
+	pinctrl-0 = <&pwm12m1_pins>;
 	status = "okay";
 };
 
@@ -1168,3 +1205,20 @@ usbdp_phy0_dp_altmode_mux: endpoint@1 {
 		};
 	};
 };
+
+&vop_mmu {
+	status = "okay";
+};
+
+&vop {
+	assigned-clocks = <&cru DCLK_VOP2_SRC>;
+	assigned-clock-parents = <&cru PLL_V0PLL>;
+	status = "okay";
+};
+
+&vp2 {
+	vp2_out_edp0: endpoint@ROCKCHIP_VOP2_EP_EDP0 {
+		reg = <ROCKCHIP_VOP2_EP_EDP0>;
+		remote-endpoint = <&edp0_in_vp2>;
+	};
+};
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 14/14] arm64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board

[Dmitry Baryshkov]

On Thu, Jan 23, 2025 at 06:07:47PM +0800, Damon Ding wrote:
> Add the necessary DT changes to enable eDP0 on RK3588S EVB1 board:
> - Set pinctrl of pwm12 for backlight
> - Enable edp0/hdptxphy0/vp2
> - Assign the parent of DCLK_VOP2_SRC to PLL_V0PLL
> - Add aux-bus/panel nodes
> 
> For RK3588, the PLL_V0PLL is specifically designed for the VOP2. This
> means the clock rate of PLL_V0PLL can be adjusted according to the dclk
> rate of relevant VP. It is typically assigned as the dclk source of a
> specific VP when the clock of relevant display mode is unusual, such as
> the eDP panel 'lg,lp079qx1-sp0v' paired with RK3588S EVB1, which has a
> clock rate of 202.02MHz.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> ---
> 
> Changes in v2:
> - Remove brightness-levels and default-brightness-level properties in
>   backlight node.
> - Add the detail DT changes to commit message.
> 
> Changes in v3:
> - Use aux-bus instead of platform bus for edp-panel.
> 
> Changes in v4:
> - Add comments related to the use of panel compatible "lg,lp079qx1-sp0v"
>   in the commit message.
> 
> Changes in v5:
> - Use "edp-panel" instead of "lg,lp079qx1-sp0v"
> - Remove unnecessary comments in commit message
> - Assign the parent of DCLK_VOP2_SRC to PLL_V0PLL
> 
> Changes in v6:
> - Add PLL_V0PLL related descriptions in commit message
> ---
>  .../boot/dts/rockchip/rk3588s-evb1-v10.dts    | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> index bc4077575beb..a8c151b41e21 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> @@ -9,6 +9,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/soc/rockchip,vop2.h>
>  #include <dt-bindings/usb/pd.h>
>  #include "rk3588s.dtsi"
>  
> @@ -238,6 +239,41 @@ &combphy2_psu {
>  	status = "okay";
>  };
>  
> +&edp0 {
> +	force-hpd;

Why? Please mention the reason in the commit message.

> +	status = "okay";
> +
> +	aux-bus {
> +		panel {
> +			compatible = "edp-panel";
> +			backlight = <&backlight>;
> +			power-supply = <&vcc3v3_lcd_edp>;
> +
> +			port {
> +				panel_in_edp: endpoint {
> +					remote-endpoint = <&edp_out_panel>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&edp0_in {
> +	edp0_in_vp2: endpoint {
> +		remote-endpoint = <&vp2_out_edp0>;
> +	};
> +};
> +
> +&edp0_out {
> +	edp_out_panel: endpoint {
> +		remote-endpoint = <&panel_in_edp>;
> +	};
> +};
> +
> +&hdptxphy0 {
> +	status = "okay";
> +};
> +
>  &i2c3 {
>  	status = "okay";
>  
> @@ -399,6 +435,7 @@ usbc0_int: usbc0-int {
>  };
>  
>  &pwm12 {
> +	pinctrl-0 = <&pwm12m1_pins>;
>  	status = "okay";
>  };
>  
> @@ -1168,3 +1205,20 @@ usbdp_phy0_dp_altmode_mux: endpoint@1 {
>  		};
>  	};
>  };
> +
> +&vop_mmu {
> +	status = "okay";
> +};
> +
> +&vop {
> +	assigned-clocks = <&cru DCLK_VOP2_SRC>;
> +	assigned-clock-parents = <&cru PLL_V0PLL>;
> +	status = "okay";
> +};
> +
> +&vp2 {
> +	vp2_out_edp0: endpoint@ROCKCHIP_VOP2_EP_EDP0 {
> +		reg = <ROCKCHIP_VOP2_EP_EDP0>;
> +		remote-endpoint = <&edp0_in_vp2>;
> +	};
> +};
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 14/14] arm64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board

[Damon Ding]

Hi Dmitry,

On 2025/1/23 19:32, Dmitry Baryshkov wrote:
> On Thu, Jan 23, 2025 at 06:07:47PM +0800, Damon Ding wrote:
>> Add the necessary DT changes to enable eDP0 on RK3588S EVB1 board:
>> - Set pinctrl of pwm12 for backlight
>> - Enable edp0/hdptxphy0/vp2
>> - Assign the parent of DCLK_VOP2_SRC to PLL_V0PLL
>> - Add aux-bus/panel nodes
>>
>> For RK3588, the PLL_V0PLL is specifically designed for the VOP2. This
>> means the clock rate of PLL_V0PLL can be adjusted according to the dclk
>> rate of relevant VP. It is typically assigned as the dclk source of a
>> specific VP when the clock of relevant display mode is unusual, such as
>> the eDP panel 'lg,lp079qx1-sp0v' paired with RK3588S EVB1, which has a
>> clock rate of 202.02MHz.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ---
>>
>> Changes in v2:
>> - Remove brightness-levels and default-brightness-level properties in
>>    backlight node.
>> - Add the detail DT changes to commit message.
>>
>> Changes in v3:
>> - Use aux-bus instead of platform bus for edp-panel.
>>
>> Changes in v4:
>> - Add comments related to the use of panel compatible "lg,lp079qx1-sp0v"
>>    in the commit message.
>>
>> Changes in v5:
>> - Use "edp-panel" instead of "lg,lp079qx1-sp0v"
>> - Remove unnecessary comments in commit message
>> - Assign the parent of DCLK_VOP2_SRC to PLL_V0PLL
>>
>> Changes in v6:
>> - Add PLL_V0PLL related descriptions in commit message
>> ---
>>   .../boot/dts/rockchip/rk3588s-evb1-v10.dts    | 54 +++++++++++++++++++
>>   1 file changed, 54 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
>> index bc4077575beb..a8c151b41e21 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
>> @@ -9,6 +9,7 @@
>>   #include <dt-bindings/gpio/gpio.h>
>>   #include <dt-bindings/input/input.h>
>>   #include <dt-bindings/pinctrl/rockchip.h>
>> +#include <dt-bindings/soc/rockchip,vop2.h>
>>   #include <dt-bindings/usb/pd.h>
>>   #include "rk3588s.dtsi"
>>   
>> @@ -238,6 +239,41 @@ &combphy2_psu {
>>   	status = "okay";
>>   };
>>   
>> +&edp0 {
>> +	force-hpd;
> 
> Why? Please mention the reason in the commit message.
> 

The 'force-hpd' is set for edp0 because the HPD pin on the panel side is 
not connected to the eDP HPD pin on the SoC side according to the 
RK3588S EVB1 hardware design.

And I will add this description to the commit message in the next version.

>> +	status = "okay";
>> +
>> +	aux-bus {
>> +		panel {
>> +			compatible = "edp-panel";
>> +			backlight = <&backlight>;
>> +			power-supply = <&vcc3v3_lcd_edp>;
>> +
>> +			port {
>> +				panel_in_edp: endpoint {
>> +					remote-endpoint = <&edp_out_panel>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +&edp0_in {
>> +	edp0_in_vp2: endpoint {
>> +		remote-endpoint = <&vp2_out_edp0>;
>> +	};
>> +};
>> +
>> +&edp0_out {
>> +	edp_out_panel: endpoint {
>> +		remote-endpoint = <&panel_in_edp>;
>> +	};
>> +};
>> +
>> +&hdptxphy0 {
>> +	status = "okay";
>> +};
>> +
>>   &i2c3 {
>>   	status = "okay";
>>   
>> @@ -399,6 +435,7 @@ usbc0_int: usbc0-int {
>>   };
>>   
>>   &pwm12 {
>> +	pinctrl-0 = <&pwm12m1_pins>;
>>   	status = "okay";
>>   };
>>   
>> @@ -1168,3 +1205,20 @@ usbdp_phy0_dp_altmode_mux: endpoint@1 {
>>   		};
>>   	};
>>   };
>> +
>> +&vop_mmu {
>> +	status = "okay";
>> +};
>> +
>> +&vop {
>> +	assigned-clocks = <&cru DCLK_VOP2_SRC>;
>> +	assigned-clock-parents = <&cru PLL_V0PLL>;
>> +	status = "okay";
>> +};
>> +
>> +&vp2 {
>> +	vp2_out_edp0: endpoint@ROCKCHIP_VOP2_EP_EDP0 {
>> +		reg = <ROCKCHIP_VOP2_EP_EDP0>;
>> +		remote-endpoint = <&edp0_in_vp2>;
>> +	};
>> +};
>> -- 
>> 2.34.1
>>
> 

Best regards,
Damon


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 00/14] Add eDP support for RK3588

[Piotr Oniszczuk]

> Wiadomość napisana przez Damon Ding <damon.ding@rock-chips.com> w dniu 23 sty 2025, o godz. 11:07:
> 
> Picked from:
> https://patchwork.kernel.org/project/linux-rockchip/list/?series=923593
> 
> These patchs have been tested with a 1536x2048p60 eDP panel on
> RK3588S EVB1 board, and HDMI 1080P/4K display also has been verified
> on RK3588 EVB1 board. Furthermore, the eDP display has been rechecked
> on RK3399 sapphire excavator board.
> 
> Patch 1~4   are preparations for the RK3588 eDP support on both Analogix
>            side and Rockchip side.
> Patch 5~8  are to support to get panel from the DP AUX bus.
> Patch 9~11 are the RK3588 Analogix DP driver support.
> Patch 12    is to add the power sequencing delays for panel model
>            LP079QX1-SP0V.
> Patch 13    is the addition of RK3588 eDP0 node.
> Patch 14    is to enable the eDP0 display on RK3588S EVB1 board.
> 
> Damon Ding (14):
>  drm/rockchip: analogix_dp: Replace DRM_...() functions with drm_...()
>  drm/rockchip: analogix_dp: Use formalized struct definition for grf
>    field
>  drm/rockchip: analogix_dp: Expand device data to support multiple edp
>    display
>  drm/bridge: analogix_dp: Add support for phy configuration.
>  dt-bindings: display: rockchip: analogix-dp: Add support to get panel
>    from the DP AUX bus
>  drm/bridge: analogix_dp: support to get &analogix_dp_device.plat_data
>    and &analogix_dp_device.aux
>  drm/bridge: analogix_dp: Add support to get panel from the DP AUX bus
>  drm/rockchip: analogix_dp: Add support to get panel from the DP AUX
>    bus
>  dt-bindings: display: rockchip: analogix-dp: Add support for RK3588
>  drm/bridge: analogix_dp: Add support for RK3588
>  drm/rockchip: analogix_dp: Add support for RK3588
>  drm/edp-panel: Add LG Display panel model LP079QX1-SP0V
>  arm64: dts: rockchip: Add eDP0 node for RK3588
>  arm64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board
> 
> .../rockchip/rockchip,analogix-dp.yaml        |  25 +-
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  28 +++
> .../boot/dts/rockchip/rk3588s-evb1-v10.dts    |  54 ++++
> .../drm/bridge/analogix/analogix_dp_core.c    |  76 +++---
> .../drm/bridge/analogix/analogix_dp_core.h    |   1 +
> .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  52 ++++
> drivers/gpu/drm/panel/panel-edp.c             |   8 +
> .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 238 +++++++++++++-----
> include/drm/bridge/analogix_dp.h              |   8 +-
> 9 files changed, 401 insertions(+), 89 deletions(-)
> 
> -- 
> 2.34.1
> 

Damon,

I’m playing with hdmi0 port enablement on radxa rock5 itx board with 6.14 mainline.

rock5 itx has 2 hdmi ports: hdmi0 is wired to rk3588 typeC1/eDP1 eDP with ra620 eDP->HDMI converter and hdmi1 from hdmi/edp tx1
(see page3 & page29 at https://dl.radxa.com/rock5/5itx/v1110/radxa_rock_5itx_v1110_schematic.pdf)

I’m on 6.14-rc2 with applied: 
[1] Cristicc hdmi code (https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-bridge)
[2] eDP support for RK3588 (https://patchwork.kernel.org/project/linux-rockchip/list/?series=927765)
[3] Add eDP mode support for Rockchip Samsung HDPTX PHY (https://patchwork.kernel.org/project/linux-rockchip/cover/20250205105157.580060-1-damon.ding@rock-chips.com/)

Is edp1 supported by yours [2] series?

If yes - may you pls hint me about required dts additions in https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts?h=v6.14-rc2 ?

(for reference: rock5 itx bsp dts goes like this: https://github.com/radxa/kernel/blob/linux-6.1-stan-rkr1/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts)

in [2] i see dts adds for EVB1 - but i’m not sure will simple extrapolation of evb1 edp0 dts additions to edp1 in rock5 be enough….

thx in advance!


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 00/14] Add eDP support for RK3588

[Diederik de Haas]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1966 bytes --]

On Thu Feb 13, 2025 at 3:54 PM CET, Piotr Oniszczuk wrote:
>
>
>> Wiadomość napisana przez Damon Ding <damon.ding@rock-chips.com> w dniu 23 sty 2025, o godz. 11:07:
>> 
>> Picked from:
>> https://patchwork.kernel.org/project/linux-rockchip/list/?series=923593
>> 
>> These patchs have been tested with a 1536x2048p60 eDP panel on
>> RK3588S EVB1 board, and HDMI 1080P/4K display also has been verified
>> on RK3588 EVB1 board. Furthermore, the eDP display has been rechecked
>> on RK3399 sapphire excavator board.
>> ...
>> 9 files changed, 401 insertions(+), 89 deletions(-)
>> 
>> -- 
>> 2.34.1
>> 
>
> Damon,
>
> I’m playing with hdmi0 port enablement on radxa rock5 itx board with 6.14 mainline.
>
> rock5 itx has 2 hdmi ports: hdmi0 is wired to rk3588 typeC1/eDP1 eDP with ra620 eDP->HDMI converter and hdmi1 from hdmi/edp tx1
> (see page3 & page29 at https://dl.radxa.com/rock5/5itx/v1110/radxa_rock_5itx_v1110_schematic.pdf)
>
> I’m on 6.14-rc2 with applied: 
> [1] Cristicc hdmi code (https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-bridge)
> [2] eDP support for RK3588 (https://patchwork.kernel.org/project/linux-rockchip/list/?series=927765)
> [3] Add eDP mode support for Rockchip Samsung HDPTX PHY (https://patchwork.kernel.org/project/linux-rockchip/cover/20250205105157.580060-1-damon.ding@rock-chips.com/)
>
> Is edp1 supported by yours [2] series?
>
> If yes - may you pls hint me about required dts additions in https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts?h=v6.14-rc2 ?

I don't know if it's relevant, but while HDMI0 got enabled for quite a
few devices in 6.13, it did NOT get enabled for Rock 5 ITX.
I made a local patch (which does the same thing as was done for Rock 5B)
but I have no idea if it actually works (I don't have the board).

Maybe you need that for eDP to work?

Cheers,
  Diederik

[-- Attachment #1.1.2: 0001-arm64-dts-rockchip-Enable-HDMI0-on-ROCK-5-ITX.patch --]
[-- Type: text/x-patch, Size: 2248 bytes --]

From 5156cfb53ac20887cf3ceb8598b3f35ee74161e9 Mon Sep 17 00:00:00 2001
From: Diederik de Haas <didi.debian@cknow.org>
Date: Sat, 2 Nov 2024 00:10:20 +0100
Subject: [PATCH] arm64: dts: rockchip: Enable HDMI0 on ROCK 5 ITX

Add the necessary DT changes to enable HDMI0 on Radxa ROCK 5 ITX.

This is the same patch as for the ROCK 5B, but then adjusted for the
ROCK 5 ITX's context, but the new statements are exactly the same.

Signed-off-by: Diederik de Haas <didi.debian@cknow.org>
Link: https://lore.kernel.org/all/20241019-rk3588-hdmi0-dt-v2-0-466cd80e8ff9@collabora.com/
---
 .../boot/dts/rockchip/rk3588-rock-5-itx.dts   | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
index 6d68f70284e4..a1b89aba977d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
@@ -11,6 +11,7 @@
 #include <dt-bindings/leds/common.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/pwm/pwm.h>
+#include <dt-bindings/soc/rockchip,vop2.h>
 #include "dt-bindings/usb/pd.h"
 #include "rk3588.dtsi"
 
@@ -72,6 +73,17 @@ hdd-led2 {
 		};
 	};
 
+	hdmi0-con {
+		compatible = "hdmi-connector";
+		type = "a";
+
+		port {
+			hdmi0_con_in: endpoint {
+				remote-endpoint = <&hdmi0_out_con>;
+			};
+		};
+	};
+
 	/* Unnamed gated oscillator: 100MHz,3.3V,3225 */
 	pcie30_port0_refclk: pcie30_port1_refclk: pcie-oscillator {
 		compatible = "gated-fixed-clock";
@@ -261,6 +273,26 @@ &gpu {
 	status = "okay";
 };
 
+&hdmi0 {
+	status = "okay";
+};
+
+&hdmi0_in {
+	hdmi0_in_vp0: endpoint {
+		remote-endpoint = <&vp0_out_hdmi0>;
+	};
+};
+
+&hdmi0_out {
+	hdmi0_out_con: endpoint {
+		remote-endpoint = <&hdmi0_con_in>;
+	};
+};
+
+&hdptxphy_hdmi0 {
+	status = "okay";
+};
+
 &i2c0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c0m2_xfer>;
@@ -1209,3 +1241,18 @@ &usbdp_phy1 {
 	rockchip,dp-lane-mux = <2 3>;
 	status = "okay";
 };
+
+&vop_mmu {
+	status = "okay";
+};
+
+&vop {
+	status = "okay";
+};
+
+&vp0 {
+	vp0_out_hdmi0: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
+		reg = <ROCKCHIP_VOP2_EP_HDMI0>;
+		remote-endpoint = <&hdmi0_in_vp0>;
+	};
+};
-- 
2.47.2


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 00/14] Add eDP support for RK3588

[Piotr Oniszczuk]

> Wiadomość napisana przez Diederik de Haas <didi.debian@cknow.org> w dniu 13 lut 2025, o godz. 17:56:
> 
> On Thu Feb 13, 2025 at 3:54 PM CET, Piotr Oniszczuk wrote:
>> 
>> 
> 
> I don't know if it's relevant, but while HDMI0 got enabled for quite a
> few devices in 6.13, it did NOT get enabled for Rock 5 ITX.
> I made a local patch (which does the same thing as was done for Rock 5B)
> but I have no idea if it actually works (I don't have the board).
> 
> Maybe you need that for eDP to work?
> 
> Cheers,
>  Diederik
> <0001-arm64-dts-rockchip-Enable-HDMI0-on-ROCK-5-ITX.patch>

Diederik,
Thx for looking on this.

Im afraid patch will not make hdmi0 working as rock5 itx board has hdmi0 designed to use 3588 eDP1 + eDP->HDMI converter (in-house developed rk620 chip).

I suspect we need: 
-enable eDP1 support (hope Damon code can do this easily)
-map eDP to port0 in vop2 (port1 should be mapped to eDP/HDMI tx1)
-set typeC1/eDP1 mux to eDP1
Then maybe we should have hdmi0 working?

Im curious a bit about vop port mappings: maybe we can go with generic approach like:

eDP/HDMI tx0 -> port0   
eDP/HDMI tx1 -> port1   
typeC0/eDP0 -> port2 (+ exception for for rock5 itx: typeC1/eDP1 -> port2)



    

 
-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 00/14] Add eDP support for RK3588

[Sebastian Reichel]

[-- Attachment #1.1: Type: text/plain, Size: 2343 bytes --]

Hi,

On Thu, Feb 13, 2025 at 05:56:55PM +0100, Diederik de Haas wrote:
> On Thu Feb 13, 2025 at 3:54 PM CET, Piotr Oniszczuk wrote:
> >
> >
> >> Wiadomość napisana przez Damon Ding <damon.ding@rock-chips.com> w dniu 23 sty 2025, o godz. 11:07:
> >> 
> >> Picked from:
> >> https://patchwork.kernel.org/project/linux-rockchip/list/?series=923593
> >> 
> >> These patchs have been tested with a 1536x2048p60 eDP panel on
> >> RK3588S EVB1 board, and HDMI 1080P/4K display also has been verified
> >> on RK3588 EVB1 board. Furthermore, the eDP display has been rechecked
> >> on RK3399 sapphire excavator board.
> >> ...
> >> 9 files changed, 401 insertions(+), 89 deletions(-)
> >> 
> >> -- 
> >> 2.34.1
> >> 
> >
> > Damon,
> >
> > I’m playing with hdmi0 port enablement on radxa rock5 itx board with 6.14 mainline.
> >
> > rock5 itx has 2 hdmi ports: hdmi0 is wired to rk3588 typeC1/eDP1 eDP with ra620 eDP->HDMI converter and hdmi1 from hdmi/edp tx1
> > (see page3 & page29 at https://dl.radxa.com/rock5/5itx/v1110/radxa_rock_5itx_v1110_schematic.pdf)
> >
> > I’m on 6.14-rc2 with applied: 
> > [1] Cristicc hdmi code (https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-bridge)
> > [2] eDP support for RK3588 (https://patchwork.kernel.org/project/linux-rockchip/list/?series=927765)
> > [3] Add eDP mode support for Rockchip Samsung HDPTX PHY (https://patchwork.kernel.org/project/linux-rockchip/cover/20250205105157.580060-1-damon.ding@rock-chips.com/)
> >
> > Is edp1 supported by yours [2] series?
> >
> > If yes - may you pls hint me about required dts additions in https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts?h=v6.14-rc2 ?
> 
> I don't know if it's relevant, but while HDMI0 got enabled for quite a
> few devices in 6.13, it did NOT get enabled for Rock 5 ITX.
> I made a local patch (which does the same thing as was done for Rock 5B)
> but I have no idea if it actually works (I don't have the board).

I don't have the board either, but the schematics suggests that your
patch is not correct. On the Rock 5 ITX the RK3588's first HDMI/eDP
port should be enabled in eDP mode to be used with an eDP panel via
connector J11. This series is needed for that.

Greetings,

-- Sebastian

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 00/14] Add eDP support for RK3588

[Diederik de Haas]

[-- Attachment #1.1: Type: text/plain, Size: 1665 bytes --]

Hi,

On Thu Feb 13, 2025 at 6:33 PM CET, Sebastian Reichel wrote:
> On Thu, Feb 13, 2025 at 05:56:55PM +0100, Diederik de Haas wrote:
>> On Thu Feb 13, 2025 at 3:54 PM CET, Piotr Oniszczuk wrote:
>> > I’m playing with hdmi0 port enablement on radxa rock5 itx board with 6.14 mainline.
>> >
>> > rock5 itx has 2 hdmi ports: hdmi0 is wired to rk3588 typeC1/eDP1 eDP with ra620 eDP->HDMI converter and hdmi1 from hdmi/edp tx1
>> > (see page3 & page29 at https://dl.radxa.com/rock5/5itx/v1110/radxa_rock_5itx_v1110_schematic.pdf)
>> >
>> > I’m on 6.14-rc2 with applied: 
>> > [1] Cristicc hdmi code (https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-bridge)
>> > [2] eDP support for RK3588 (https://patchwork.kernel.org/project/linux-rockchip/list/?series=927765)
>> > [3] Add eDP mode support for Rockchip Samsung HDPTX PHY (https://patchwork.kernel.org/project/linux-rockchip/cover/20250205105157.580060-1-damon.ding@rock-chips.com/)
>> >
>> 
>> I don't know if it's relevant, but while HDMI0 got enabled for quite a
>> few devices in 6.13, it did NOT get enabled for Rock 5 ITX.
>> I made a local patch (which does the same thing as was done for Rock 5B)
>> but I have no idea if it actually works (I don't have the board).
>
> I don't have the board either, but the schematics suggests that your
> patch is not correct. On the Rock 5 ITX the RK3588's first HDMI/eDP
> port should be enabled in eDP mode to be used with an eDP panel via
> connector J11. This series is needed for that.

Ah, then there was more to it then simply being overlooked/forgotten.
Thanks for the clarification :-)

Cheers,
  Diederik

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 00/14] Add eDP support for RK3588

[Sebastian Reichel]

[-- Attachment #1.1: Type: text/plain, Size: 4127 bytes --]

Hello Piotr,

On Thu, Feb 13, 2025 at 03:54:00PM +0100, Piotr Oniszczuk wrote:
> > Wiadomość napisana przez Damon Ding <damon.ding@rock-chips.com> w dniu 23 sty 2025, o godz. 11:07:
> > 
> > Picked from:
> > https://patchwork.kernel.org/project/linux-rockchip/list/?series=923593
> > 
> > These patchs have been tested with a 1536x2048p60 eDP panel on
> > RK3588S EVB1 board, and HDMI 1080P/4K display also has been verified
> > on RK3588 EVB1 board. Furthermore, the eDP display has been rechecked
> > on RK3399 sapphire excavator board.
> > 
> > Patch 1~4   are preparations for the RK3588 eDP support on both Analogix
> >            side and Rockchip side.
> > Patch 5~8  are to support to get panel from the DP AUX bus.
> > Patch 9~11 are the RK3588 Analogix DP driver support.
> > Patch 12    is to add the power sequencing delays for panel model
> >            LP079QX1-SP0V.
> > Patch 13    is the addition of RK3588 eDP0 node.
> > Patch 14    is to enable the eDP0 display on RK3588S EVB1 board.
> > 
> > Damon Ding (14):
> >  drm/rockchip: analogix_dp: Replace DRM_...() functions with drm_...()
> >  drm/rockchip: analogix_dp: Use formalized struct definition for grf
> >    field
> >  drm/rockchip: analogix_dp: Expand device data to support multiple edp
> >    display
> >  drm/bridge: analogix_dp: Add support for phy configuration.
> >  dt-bindings: display: rockchip: analogix-dp: Add support to get panel
> >    from the DP AUX bus
> >  drm/bridge: analogix_dp: support to get &analogix_dp_device.plat_data
> >    and &analogix_dp_device.aux
> >  drm/bridge: analogix_dp: Add support to get panel from the DP AUX bus
> >  drm/rockchip: analogix_dp: Add support to get panel from the DP AUX
> >    bus
> >  dt-bindings: display: rockchip: analogix-dp: Add support for RK3588
> >  drm/bridge: analogix_dp: Add support for RK3588
> >  drm/rockchip: analogix_dp: Add support for RK3588
> >  drm/edp-panel: Add LG Display panel model LP079QX1-SP0V
> >  arm64: dts: rockchip: Add eDP0 node for RK3588
> >  arm64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board
> > 
> > .../rockchip/rockchip,analogix-dp.yaml        |  25 +-
> > arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  28 +++
> > .../boot/dts/rockchip/rk3588s-evb1-v10.dts    |  54 ++++
> > .../drm/bridge/analogix/analogix_dp_core.c    |  76 +++---
> > .../drm/bridge/analogix/analogix_dp_core.h    |   1 +
> > .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  52 ++++
> > drivers/gpu/drm/panel/panel-edp.c             |   8 +
> > .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 238 +++++++++++++-----
> > include/drm/bridge/analogix_dp.h              |   8 +-
> > 9 files changed, 401 insertions(+), 89 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 
> 
> Damon,
> 
> I’m playing with hdmi0 port enablement on radxa rock5 itx board with 6.14 mainline.
> 
> rock5 itx has 2 hdmi ports: hdmi0 is wired to rk3588 typeC1/eDP1
> eDP with ra620 eDP->HDMI converter and hdmi1 from hdmi/edp tx1
> (see page3 & page29 at
> https://dl.radxa.com/rock5/5itx/v1110/radxa_rock_5itx_v1110_schematic.pdf)

The RK3588 has two different DP controllers. The one handled in this
series should get the eDP port running, which is routed to the
"eDP Panel" in the block diagram on page 3 of the Rock 5 ITX schematics.
So this series adds support for using "HDMI/eDP TX0" and "HDMI/eDP TX1"
in DP mode.

The port routed to HDMI0 via RA620 is the other DP controller, which
also exists twice: "TYPEC0/DP0" and "TYPEC1/DP1". This DP controller
is not yet supported upstream and there is no pending patchset. As far
as I know Rockchip plans to work on preparing upstream support for that
soon.

Note, that the two DisplayPort controllers are completely different.
The HDMI/eDP controller is a design from Analogix and the TypeC/DP
controller is a design from Synopsys.

P.S.: Heiko merged support for HDMI1 (RK3588 SoC level) recently. So you
should be able to get that running by some DT additions to the Rock 5
ITX board DT with the latest linux-next code :)

Greetings,

-- Sebastian

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 00/14] Add eDP support for RK3588

[Piotr Oniszczuk]

> Wiadomość napisana przez Sebastian Reichel <sebastian.reichel@collabora.com> w dniu 13 lut 2025, o godz. 18:26:
> 
> Hello Piotr,
> 
> 
> The RK3588 has two different DP controllers. The one handled in this
> series should get the eDP port running, which is routed to the
> "eDP Panel" in the block diagram on page 3 of the Rock 5 ITX schematics.
> So this series adds support for using "HDMI/eDP TX0" and "HDMI/eDP TX1"
> in DP mode.
> 
> The port routed to HDMI0 via RA620 is the other DP controller, which
> also exists twice: "TYPEC0/DP0" and "TYPEC1/DP1". This DP controller
> is not yet supported upstream and there is no pending patchset. As far
> as I know Rockchip plans to work on preparing upstream support for that
> soon.
> 

Oh - this is very valuable info and explains a lot.
Thx. 

> Note, that the two DisplayPort controllers are completely different.
> The HDMI/eDP controller is a design from Analogix and the TypeC/DP
> controller is a design from Synopsys.
> 
> P.S.: Heiko merged support for HDMI1 (RK3588 SoC level) recently. So you
> should be able to get that running by some DT additions to the Rock 5
> ITX board DT with the latest linux-next code :)
> 

And it works perfectly on all my 3588 bards (including audio and my cec addition)
But only on boards using both hdmi tx0 and tx1 to hdmi0/hdmi1 ports.

on rock5 itx:  

With both hdmi tx0 and tx1 enabled in dts it looks like kernel detects hdmi tx0 as connected and tries read edid. 
As nothing is connected to J11 (eDP con) - i’m getting i2c ddc timeouts 
[   54.188880] dwhdmiqp-rockchip fde80000.hdmi: i2c read timed out
[   54.292207] dwhdmiqp-rockchip fde80000.hdmi: i2c read timed out
[   54.395554] dwhdmiqp-rockchip fde80000.hdmi: i2c read timed out


disabling hdmi0 makes kernel talking to hdmi1 but with issues: 
[    0.540446] dwhdmiqp-rockchip fdea0000.hdmi: registered DesignWare HDMI QP I2C bus driver
[    0.541230] rockchip-drm display-subsystem: bound fdea0000.hdmi (ops dw_hdmi_qp_rockchip_ops)
[    0.542285] [drm] Initialized rockchip 1.0.0 for display-subsystem on minor 0
[    0.542933] rockchip-drm display-subsystem: [drm] Cannot find any crtc or sizes
[    0.543605] rockchip-drm display-subsystem: [drm] Cannot find any crtc or sizes

and here i’m a bit puzzled as on other 3588 boards (i.e. rock5b) - when i connected monitor to hdmi1 - all works perfectly
(kernel, OS and app nicely skipping hdmi0 and talking to actually connected hdmi1 port. all works) 

on rock5 itx this not works.

my observations so far are:
-on rock5 itx hdmi0 is falsely detected as connected - even when eDP connector (J11) has nothing connected
-on rock5 itx disabling hdmi0 in dts breaks hdmi1 
-on other 3588 (e.g. rock 5b) disabling hdmi0 in dts NOT breaks hdmi1  

strange….

   

> Greetings,
> 
> -- Sebastian
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v6 00/14] Add eDP support for RK3588

[Damon Ding]

Hi Piotr,

On 2025/2/14 20:00, Piotr Oniszczuk wrote:
> 
> 
>> Wiadomość napisana przez Sebastian Reichel <sebastian.reichel@collabora.com> w dniu 13 lut 2025, o godz. 18:26:
>>
>> Hello Piotr,
>>
>>
>> The RK3588 has two different DP controllers. The one handled in this
>> series should get the eDP port running, which is routed to the
>> "eDP Panel" in the block diagram on page 3 of the Rock 5 ITX schematics.
>> So this series adds support for using "HDMI/eDP TX0" and "HDMI/eDP TX1"
>> in DP mode.
>>
>> The port routed to HDMI0 via RA620 is the other DP controller, which
>> also exists twice: "TYPEC0/DP0" and "TYPEC1/DP1". This DP controller
>> is not yet supported upstream and there is no pending patchset. As far
>> as I know Rockchip plans to work on preparing upstream support for that
>> soon.

Yes, Andy has recently updated the DP controller related patchset:
https://patchwork.kernel.org/project/linux-rockchip/list/?series=936784

>>
> 
> Oh - this is very valuable info and explains a lot.
> Thx.
> 
>> Note, that the two DisplayPort controllers are completely different.
>> The HDMI/eDP controller is a design from Analogix and the TypeC/DP
>> controller is a design from Synopsys.
>>
>> P.S.: Heiko merged support for HDMI1 (RK3588 SoC level) recently. So you
>> should be able to get that running by some DT additions to the Rock 5
>> ITX board DT with the latest linux-next code :)
>>
> 
> And it works perfectly on all my 3588 bards (including audio and my cec addition)
> But only on boards using both hdmi tx0 and tx1 to hdmi0/hdmi1 ports.
> 
> on rock5 itx:

According to the schematic of ROCK 5 ITX, I find the display routes are:
- DP0 -> TYPEC0 Connector
- DP1 -> RA620 -> HDMI Connector (Andy's patch series will help it work)
- HDMI/eDP TX0 -> eDP Panel (This patch series will help it work)
- HDMI/eDP TX1 -> HDMI Connector

So the HDMI/eDP TX0 should be enabled as eDP in DTS. Maybe it's just a 
test to enable both HDMI0 and HDMI1 in the ROCK 5 ITX board?

> With both hdmi tx0 and tx1 enabled in dts it looks like kernel detects hdmi tx0 as connected and tries read edid.
> As nothing is connected to J11 (eDP con) - i’m getting i2c ddc timeouts
> [   54.188880] dwhdmiqp-rockchip fde80000.hdmi: i2c read timed out
> [   54.292207] dwhdmiqp-rockchip fde80000.hdmi: i2c read timed out
> [   54.395554] dwhdmiqp-rockchip fde80000.hdmi: i2c read timed out
> 

I think the reason may be that HDMI0 is enabled, and HPD pin of HDMI0 is 
also configured, but the HPD pin is in an unstable state in the 
hardware, which is mistakenly detected as connected.

> 
> disabling hdmi0 makes kernel talking to hdmi1 but with issues:
> [    0.540446] dwhdmiqp-rockchip fdea0000.hdmi: registered DesignWare HDMI QP I2C bus driver
> [    0.541230] rockchip-drm display-subsystem: bound fdea0000.hdmi (ops dw_hdmi_qp_rockchip_ops)
> [    0.542285] [drm] Initialized rockchip 1.0.0 for display-subsystem on minor 0
> [    0.542933] rockchip-drm display-subsystem: [drm] Cannot find any crtc or sizes
> [    0.543605] rockchip-drm display-subsystem: [drm] Cannot find any crtc or sizes
> 
> and here i’m a bit puzzled as on other 3588 boards (i.e. rock5b) - when i connected monitor to hdmi1 - all works perfectly
> (kernel, OS and app nicely skipping hdmi0 and talking to actually connected hdmi1 port. all works)
> 
> on rock5 itx this not works.
> 
> my observations so far are:
> -on rock5 itx hdmi0 is falsely detected as connected - even when eDP connector (J11) has nothing connected
> -on rock5 itx disabling hdmi0 in dts breaks hdmi1
> -on other 3588 (e.g. rock 5b) disabling hdmi0 in dts NOT breaks hdmi1
> 
> strange….
> 
> 

Best regards
Damon


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy