linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon
@ 2021-05-05  8:16 Sankeerth Billakanti
  2021-05-05  8:16 ` [PATCH v1 3/3] drm/msm/disp/dpu1: add support for edp encoder Sankeerth Billakanti
  2021-05-05 10:01 ` [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon Dmitry Baryshkov
  0 siblings, 2 replies; 9+ messages in thread
From: Sankeerth Billakanti @ 2021-05-05  8:16 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm, freedreno, linux-kernel
  Cc: Sankeerth Billakanti, robdclark, seanpaul, swboyd, kalyan_t,
	abhinavk, dianders, khsieh, mkrishn

These patches add support for the next generation eDP driver on SnapDragon
with dpu support. The existing eDP driver cannot support the new eDP
hardware. So, to maintain backward compatibility, the older eDP driver is
moved to v200 folder and the new generation eDP driver is added in
the v510 folder.

These are baseline changes with which we can enable display. The new eDP
controller can also support additional features such as backlight control,
PSR etc. which will be enabled in subsequent patch series.

Summary of changes:
DPU driver interface to the new eDP v510 display driver.
New generation eDP controller and phy driver implementation.
A common interface to choose enable the required eDP driver.

Sankeerth Billakanti (3):
  drm/msm/edp: support multiple generations of edp hardware
  drm/msm/edp: add support for next gen edp
  drm/msm/disp/dpu1: add support for edp encoder

 drivers/gpu/drm/msm/Makefile                      |   19 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c       |    7 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c           |   33 +
 drivers/gpu/drm/msm/edp/edp.c                     |  198 ---
 drivers/gpu/drm/msm/edp/edp.h                     |   78 -
 drivers/gpu/drm/msm/edp/edp.xml.h                 |  380 -----
 drivers/gpu/drm/msm/edp/edp_aux.c                 |  264 ----
 drivers/gpu/drm/msm/edp/edp_bridge.c              |  111 --
 drivers/gpu/drm/msm/edp/edp_common.c              |   38 +
 drivers/gpu/drm/msm/edp/edp_common.h              |   47 +
 drivers/gpu/drm/msm/edp/edp_connector.c           |  132 --
 drivers/gpu/drm/msm/edp/edp_ctrl.c                | 1375 ------------------
 drivers/gpu/drm/msm/edp/edp_phy.c                 |   98 --
 drivers/gpu/drm/msm/edp/v200/edp.xml.h            |  380 +++++
 drivers/gpu/drm/msm/edp/v200/edp_v200.c           |  210 +++
 drivers/gpu/drm/msm/edp/v200/edp_v200.h           |   70 +
 drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c       |  264 ++++
 drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c    |  111 ++
 drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c |  132 ++
 drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c      | 1375 ++++++++++++++++++
 drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c       |   98 ++
 drivers/gpu/drm/msm/edp/v510/edp_v510.c           |  220 +++
 drivers/gpu/drm/msm/edp/v510/edp_v510.h           |  151 ++
 drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c       |  268 ++++
 drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c    |  111 ++
 drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c |  117 ++
 drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c      | 1583 +++++++++++++++++++++
 drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c       |  641 +++++++++
 drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h       |  339 +++++
 29 files changed, 6207 insertions(+), 2643 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/edp/edp.c
 delete mode 100644 drivers/gpu/drm/msm/edp/edp.h
 delete mode 100644 drivers/gpu/drm/msm/edp/edp.xml.h
 delete mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c
 delete mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c
 create mode 100644 drivers/gpu/drm/msm/edp/edp_common.c
 create mode 100644 drivers/gpu/drm/msm/edp/edp_common.h
 delete mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c
 delete mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c
 delete mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c
 create mode 100644 drivers/gpu/drm/msm/edp/v200/edp.xml.h
 create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.c
 create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.h
 create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c
 create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c
 create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c
 create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c
 create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c
 create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.c
 create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.h
 create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c
 create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c
 create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c
 create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c
 create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c
 create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h

-- 
The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 3/3] drm/msm/disp/dpu1: add support for edp encoder
  2021-05-05  8:16 [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon Sankeerth Billakanti
@ 2021-05-05  8:16 ` Sankeerth Billakanti
  2021-05-05 10:01 ` [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon Dmitry Baryshkov
  1 sibling, 0 replies; 9+ messages in thread
From: Sankeerth Billakanti @ 2021-05-05  8:16 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm, freedreno, linux-kernel
  Cc: Sankeerth Billakanti, robdclark, seanpaul, swboyd, kalyan_t,
	abhinavk, dianders, khsieh, mkrishn

This change will enable dpu encoder support for the native
eDP interface on next generation snapdragon platforms.

Signed-off-by: Sankeerth Billakanti <sbillaka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  7 +++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 33 +++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8d94205..9f5185bb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2075,7 +2075,12 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
 		intf_type = INTF_DSI;
 		break;
 	case DRM_MODE_ENCODER_TMDS:
-		intf_type = INTF_DP;
+		if (disp_info->capabilities & MSM_DISPLAY_CAP_CMD_MODE) {
+			intf_type = INTF_EDP;
+			/* PSR CMD mode not supported */
+			disp_info->capabilities = MSM_DISPLAY_CAP_VID_MODE;
+		} else
+			intf_type = INTF_DP;
 		break;
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 88e9cc3..ecd7dc8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -499,6 +499,33 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
 	return rc;
 }
 
+static int _dpu_kms_initialize_edp(struct drm_device *dev,
+				    struct msm_drm_private *priv,
+				    struct dpu_kms *dpu_kms)
+{
+	struct drm_encoder *encoder = NULL;
+	int rc = 0;
+
+	if (!priv->edp)
+		return rc;
+
+	encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+	if (IS_ERR(encoder)) {
+		DPU_ERROR("encoder init failed for eDP display\n");
+		return PTR_ERR(encoder);
+	}
+
+	rc = msm_edp_modeset_init(priv->edp, dev, encoder);
+	if (rc) {
+		DPU_ERROR("modeset_init failed for eDP, rc = %d\n", rc);
+		drm_encoder_cleanup(encoder);
+		return rc;
+	}
+
+	priv->encoders[priv->num_encoders++] = encoder;
+	return rc;
+}
+
 static int _dpu_kms_initialize_displayport(struct drm_device *dev,
 					    struct msm_drm_private *priv,
 					    struct dpu_kms *dpu_kms)
@@ -546,6 +573,12 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
 		return rc;
 	}
 
+	rc = _dpu_kms_initialize_edp(dev, priv, dpu_kms);
+	if (rc) {
+		DPU_ERROR("initialize_eDP failed, rc = %d\n", rc);
+		return rc;
+	}
+
 	rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
 	if (rc) {
 		DPU_ERROR("initialize_DP failed, rc = %d\n", rc);
-- 
The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon
  2021-05-05  8:16 [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon Sankeerth Billakanti
  2021-05-05  8:16 ` [PATCH v1 3/3] drm/msm/disp/dpu1: add support for edp encoder Sankeerth Billakanti
@ 2021-05-05 10:01 ` Dmitry Baryshkov
  2021-05-06  6:47   ` sbillaka
  1 sibling, 1 reply; 9+ messages in thread
From: Dmitry Baryshkov @ 2021-05-05 10:01 UTC (permalink / raw)
  To: Sankeerth Billakanti
  Cc: open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list:DRM DRIVER FOR MSM ADRENO GPU, freedreno, open list,
	Rob Clark, Sean Paul, Stephen Boyd, Kalyan Thota, Abhinav Kumar,
	Douglas Anderson, khsieh, Krishna Manikandan

Hi,

On Wed, 5 May 2021 at 11:17, Sankeerth Billakanti
<sbillaka@codeaurora.org> wrote:
>
> These patches add support for the next generation eDP driver on SnapDragon
> with dpu support. The existing eDP driver cannot support the new eDP
> hardware. So, to maintain backward compatibility, the older eDP driver is
> moved to v200 folder and the new generation eDP driver is added in
> the v510 folder.

What exactly does this version correspond to?
I assume that v510 corresponds to sdmshrike/sc8180x. Is it right?
Is it really so specific, or just v2/v5 would be enough? Not to
mention that this is the MDP/ version, while other blocks tend to use
block-specific versions/ids.

Also, how much does it differ from the current DP core supported via
drivers/gpu/drm/msm/dp ?

First two patches did not make it to the linux-msm, so I can not
comment on each of the lines.
However just my few cents (other reviewers might disagree though):

- I see little benefit in renaming the folders just for the sake of
renaming. You can put your code in drivers/gpu/drm/msm/edp-v510, if
you really insist on that. Note that for all other (even incompatible)
hardware types we still use single level of folders.

- Also I see that significant parts of code (e.g. AUX, bridge,
connector, maybe more) are just c&p of old edp code pieces. Please
share the code instead of duplicating it.

- Please consider updating register definitions in xml form and then
providing both changed xml files (to mesa project (?)) and generated
headers into the kernel.

- Please consider using clk_bulk_* functions instead of using
dss_module_power. I'm going to send a patchset reworking current users
to use the generic clk_bulk_* function family.

- In generic, this eDP clock handling seems to match closely DP clocks
handling (with all the name comparison, etc). Consider moving this to
a generic piece of code

- PHY seems to be a version of QMP PHY. Please use it, like it was
done for the DP itself. There is support for combined USB+DP PHYs
(both v3 and v4), so it should be possible to extend that for eDP.


> These are baseline changes with which we can enable display. The new eDP
> controller can also support additional features such as backlight control,
> PSR etc. which will be enabled in subsequent patch series.
>
> Summary of changes:
> DPU driver interface to the new eDP v510 display driver.
> New generation eDP controller and phy driver implementation.
> A common interface to choose enable the required eDP driver.
>
> Sankeerth Billakanti (3):
>   drm/msm/edp: support multiple generations of edp hardware
>   drm/msm/edp: add support for next gen edp
>   drm/msm/disp/dpu1: add support for edp encoder
>
>  drivers/gpu/drm/msm/Makefile                      |   19 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c       |    7 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c           |   33 +
>  drivers/gpu/drm/msm/edp/edp.c                     |  198 ---
>  drivers/gpu/drm/msm/edp/edp.h                     |   78 -
>  drivers/gpu/drm/msm/edp/edp.xml.h                 |  380 -----
>  drivers/gpu/drm/msm/edp/edp_aux.c                 |  264 ----
>  drivers/gpu/drm/msm/edp/edp_bridge.c              |  111 --
>  drivers/gpu/drm/msm/edp/edp_common.c              |   38 +
>  drivers/gpu/drm/msm/edp/edp_common.h              |   47 +
>  drivers/gpu/drm/msm/edp/edp_connector.c           |  132 --
>  drivers/gpu/drm/msm/edp/edp_ctrl.c                | 1375 ------------------
>  drivers/gpu/drm/msm/edp/edp_phy.c                 |   98 --
>  drivers/gpu/drm/msm/edp/v200/edp.xml.h            |  380 +++++
>  drivers/gpu/drm/msm/edp/v200/edp_v200.c           |  210 +++
>  drivers/gpu/drm/msm/edp/v200/edp_v200.h           |   70 +
>  drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c       |  264 ++++
>  drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c    |  111 ++
>  drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c |  132 ++
>  drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c      | 1375 ++++++++++++++++++
>  drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c       |   98 ++
>  drivers/gpu/drm/msm/edp/v510/edp_v510.c           |  220 +++
>  drivers/gpu/drm/msm/edp/v510/edp_v510.h           |  151 ++
>  drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c       |  268 ++++
>  drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c    |  111 ++
>  drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c |  117 ++
>  drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c      | 1583 +++++++++++++++++++++
>  drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c       |  641 +++++++++
>  drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h       |  339 +++++
>  29 files changed, 6207 insertions(+), 2643 deletions(-)
>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.c
>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.h
>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.xml.h
>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c
>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c
>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.c
>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.h
>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c
>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c
>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp.xml.h
>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.h
>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.h
>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c
>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h
>
> --
> The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
>


--
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon
  2021-05-05 10:01 ` [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon Dmitry Baryshkov
@ 2021-05-06  6:47   ` sbillaka
  2021-05-06 12:14     ` Dmitry Baryshkov
  2021-05-06 15:02     ` Rob Clark
  0 siblings, 2 replies; 9+ messages in thread
From: sbillaka @ 2021-05-06  6:47 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list:DRM DRIVER FOR MSM ADRENO GPU, freedreno, open list,
	Rob Clark, Sean Paul, Stephen Boyd, Kalyan Thota, Abhinav Kumar,
	Douglas Anderson, khsieh, Krishna Manikandan

On 2021-05-05 15:31, Dmitry Baryshkov wrote:
> Hi,
> 
> On Wed, 5 May 2021 at 11:17, Sankeerth Billakanti
> <sbillaka@codeaurora.org> wrote:
>> 
>> These patches add support for the next generation eDP driver on 
>> SnapDragon
>> with dpu support. The existing eDP driver cannot support the new eDP
>> hardware. So, to maintain backward compatibility, the older eDP driver 
>> is
>> moved to v200 folder and the new generation eDP driver is added in
>> the v510 folder.
> 
> What exactly does this version correspond to?
> I assume that v510 corresponds to sdmshrike/sc8180x. Is it right?
[Sankeerth] This is for sc7280.

> Is it really so specific, or just v2/v5 would be enough? Not to
> mention that this is the MDP/ version, while other blocks tend to use
> block-specific versions/ids.
[Sankeerth] I can rename it as edp-v1 and edp-v2. Edp v1 is very old 
chip and there is considerable HW delta between v1 and v2. So, we want 
to separate the driver. We followed similar model for DPU driver where, 
MDP4, MDP5 and DPU have separate folders. EDP v1 belongs to MDP4 
generation.

> 
> Also, how much does it differ from the current DP core supported via
> drivers/gpu/drm/msm/dp ?
[Sankeerth] eDP is a native controller like DP but does not have audio, 
content protection and interoperability requirement. Upstream already 
supports eDP as a new interface driver found here: 
drivers/gpu/drm/msm/edp.
I wanted to add the new controller driver as part of that folder.

> 
> First two patches did not make it to the linux-msm, so I can not
> comment on each of the lines.
[Sankeerth] I am also not sure why they did not make it to patchwork. I 
will repost them.

> However just my few cents (other reviewers might disagree though):
> 
> - I see little benefit in renaming the folders just for the sake of
> renaming. You can put your code in drivers/gpu/drm/msm/edp-v510, if
> you really insist on that. Note that for all other (even incompatible)
> hardware types we still use single level of folders.
> 
> - Also I see that significant parts of code (e.g. AUX, bridge,
> connector, maybe more) are just c&p of old edp code pieces. Please
> share the code instead of duplicating it.
[Sankeerth] It is a baseline driver. As we add more features, it will 
considerably deviate a lot. The effort seems to be very high to maintain 
the common portion of code as I expect a lot of deviation.
> 
> - Please consider updating register definitions in xml form and then
> providing both changed xml files (to mesa project (?)) and generated
> headers into the kernel.
[Sankeerth] I followed what was done in the DP driver at 
/drivers/gpu/drm/msm/dp. I need to explore the xml approach to generate 
the register definitions.
> 
> - Please consider using clk_bulk_* functions instead of using
> dss_module_power. I'm going to send a patchset reworking current users
> to use the generic clk_bulk_* function family.
[Sankeerth] I will explore and rebase after your patch is available.
> 
> - In generic, this eDP clock handling seems to match closely DP clocks
> handling (with all the name comparison, etc). Consider moving this to
> a generic piece of code
> 
> - PHY seems to be a version of QMP PHY. Please use it, like it was
> done for the DP itself. There is support for combined USB+DP PHYs
> (both v3 and v4), so it should be possible to extend that for eDP.
[Sankeerth] The DP phy is a combophy which supports both usb and dp phy 
concurrently, unlike eDP phy which is specific to only the eDP 
controller in sc7280. So, I implemented the edp phy sequences in the 
same folder.
> 
> 
>> These are baseline changes with which we can enable display. The new 
>> eDP
>> controller can also support additional features such as backlight 
>> control,
>> PSR etc. which will be enabled in subsequent patch series.
>> 
>> Summary of changes:
>> DPU driver interface to the new eDP v510 display driver.
>> New generation eDP controller and phy driver implementation.
>> A common interface to choose enable the required eDP driver.
>> 
>> Sankeerth Billakanti (3):
>>   drm/msm/edp: support multiple generations of edp hardware
>>   drm/msm/edp: add support for next gen edp
>>   drm/msm/disp/dpu1: add support for edp encoder
>> 
>>  drivers/gpu/drm/msm/Makefile                      |   19 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c       |    7 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c           |   33 +
>>  drivers/gpu/drm/msm/edp/edp.c                     |  198 ---
>>  drivers/gpu/drm/msm/edp/edp.h                     |   78 -
>>  drivers/gpu/drm/msm/edp/edp.xml.h                 |  380 -----
>>  drivers/gpu/drm/msm/edp/edp_aux.c                 |  264 ----
>>  drivers/gpu/drm/msm/edp/edp_bridge.c              |  111 --
>>  drivers/gpu/drm/msm/edp/edp_common.c              |   38 +
>>  drivers/gpu/drm/msm/edp/edp_common.h              |   47 +
>>  drivers/gpu/drm/msm/edp/edp_connector.c           |  132 --
>>  drivers/gpu/drm/msm/edp/edp_ctrl.c                | 1375 
>> ------------------
>>  drivers/gpu/drm/msm/edp/edp_phy.c                 |   98 --
>>  drivers/gpu/drm/msm/edp/v200/edp.xml.h            |  380 +++++
>>  drivers/gpu/drm/msm/edp/v200/edp_v200.c           |  210 +++
>>  drivers/gpu/drm/msm/edp/v200/edp_v200.h           |   70 +
>>  drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c       |  264 ++++
>>  drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c    |  111 ++
>>  drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c |  132 ++
>>  drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c      | 1375 
>> ++++++++++++++++++
>>  drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c       |   98 ++
>>  drivers/gpu/drm/msm/edp/v510/edp_v510.c           |  220 +++
>>  drivers/gpu/drm/msm/edp/v510/edp_v510.h           |  151 ++
>>  drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c       |  268 ++++
>>  drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c    |  111 ++
>>  drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c |  117 ++
>>  drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c      | 1583 
>> +++++++++++++++++++++
>>  drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c       |  641 +++++++++
>>  drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h       |  339 +++++
>>  29 files changed, 6207 insertions(+), 2643 deletions(-)
>>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.c
>>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.h
>>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.xml.h
>>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c
>>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.h
>>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c
>>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c
>>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp.xml.h
>>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.h
>>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.h
>>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c
>>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h
>> 
>> --
>> The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora 
>> Forum, a Linux Foundation Collaborative Project
>> 
> 
> 
> --
> With best wishes
> Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon
  2021-05-06  6:47   ` sbillaka
@ 2021-05-06 12:14     ` Dmitry Baryshkov
  2021-05-06 15:02     ` Rob Clark
  1 sibling, 0 replies; 9+ messages in thread
From: Dmitry Baryshkov @ 2021-05-06 12:14 UTC (permalink / raw)
  To: Sankeerth Billakanti
  Cc: open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list:DRM DRIVER FOR MSM ADRENO GPU, freedreno, open list,
	Rob Clark, Sean Paul, Stephen Boyd, Kalyan Thota, Abhinav Kumar,
	Douglas Anderson, khsieh, Krishna Manikandan

On Thu, 6 May 2021 at 09:47, <sbillaka@codeaurora.org> wrote:
>
> On 2021-05-05 15:31, Dmitry Baryshkov wrote:
> > Hi,
> >
> > On Wed, 5 May 2021 at 11:17, Sankeerth Billakanti
> > <sbillaka@codeaurora.org> wrote:
> >>
> >> These patches add support for the next generation eDP driver on
> >> SnapDragon
> >> with dpu support. The existing eDP driver cannot support the new eDP
> >> hardware. So, to maintain backward compatibility, the older eDP driver
> >> is
> >> moved to v200 folder and the new generation eDP driver is added in
> >> the v510 folder.
> >
> > What exactly does this version correspond to?
> > I assume that v510 corresponds to sdmshrike/sc8180x. Is it right?
> [Sankeerth] This is for sc7280.
>
> > Is it really so specific, or just v2/v5 would be enough? Not to
> > mention that this is the MDP/ version, while other blocks tend to use
> > block-specific versions/ids.
> [Sankeerth] I can rename it as edp-v1 and edp-v2. Edp v1 is very old
> chip and there is considerable HW delta between v1 and v2. So, we want
> to separate the driver. We followed similar model for DPU driver where,
> MDP4, MDP5 and DPU have separate folders. EDP v1 belongs to MDP4
> generation.

I'd say, just leave old eDP as is, add eDPv2 in a separate folder.

> > Also, how much does it differ from the current DP core supported via
> > drivers/gpu/drm/msm/dp ?
> [Sankeerth] eDP is a native controller like DP but does not have audio,
> content protection and interoperability requirement. Upstream already
> supports eDP as a new interface driver found here:
> drivers/gpu/drm/msm/edp.
> I wanted to add the new controller driver as part of that folder.

This does not answer if there is significant difference from the
supported DP core.
Does it differ significantly or not?

> > First two patches did not make it to the linux-msm, so I can not
> > comment on each of the lines.
> [Sankeerth] I am also not sure why they did not make it to patchwork. I
> will repost them.

Because your patches are huge and thus they do not pass size limits
for linux-arm-msm ML.
Reposting didn't help of course. In future, if you plan to do mass
renaming, please pass -M -C to git-format-patch (or git send-email if
you use it directly).

> > However just my few cents (other reviewers might disagree though):
> >
> > - I see little benefit in renaming the folders just for the sake of
> > renaming. You can put your code in drivers/gpu/drm/msm/edp-v510, if
> > you really insist on that. Note that for all other (even incompatible)
> > hardware types we still use single level of folders.
> >
> > - Also I see that significant parts of code (e.g. AUX, bridge,
> > connector, maybe more) are just c&p of old edp code pieces. Please
> > share the code instead of duplicating it.
> [Sankeerth] It is a baseline driver. As we add more features, it will
> considerably deviate a lot. The effort seems to be very high to maintain
> the common portion of code as I expect a lot of deviation.

Can we find support for this eDP core in the display drivers pack somewhere?

> >
> > - Please consider updating register definitions in xml form and then
> > providing both changed xml files (to mesa project (?)) and generated
> > headers into the kernel.
> [Sankeerth] I followed what was done in the DP driver at
> /drivers/gpu/drm/msm/dp. I need to explore the xml approach to generate
> the register definitions.

We'd need to convert them to xml too.

> >
> > - Please consider using clk_bulk_* functions instead of using
> > dss_module_power. I'm going to send a patchset reworking current users
> > to use the generic clk_bulk_* function family.
> [Sankeerth] I will explore and rebase after your patch is available.
> >
> > - In generic, this eDP clock handling seems to match closely DP clocks
> > handling (with all the name comparison, etc). Consider moving this to
> > a generic piece of code
> >
> > - PHY seems to be a version of QMP PHY. Please use it, like it was
> > done for the DP itself. There is support for combined USB+DP PHYs
> > (both v3 and v4), so it should be possible to extend that for eDP.
> [Sankeerth] The DP phy is a combophy which supports both usb and dp phy
> concurrently, unlike eDP phy which is specific to only the eDP
> controller in sc7280. So, I implemented the edp phy sequences in the
> same folder.

Please use the tthe qmp driver for qmp phys. Judging from the register
definitions, your hardware is definitely the QMP v4. Initial support
for DP on this generation has been merged during this merge window.
Feel free to correct/extend it.

> >> These are baseline changes with which we can enable display. The new
> >> eDP
> >> controller can also support additional features such as backlight
> >> control,
> >> PSR etc. which will be enabled in subsequent patch series.
> >>
> >> Summary of changes:
> >> DPU driver interface to the new eDP v510 display driver.
> >> New generation eDP controller and phy driver implementation.
> >> A common interface to choose enable the required eDP driver.
> >>
> >> Sankeerth Billakanti (3):
> >>   drm/msm/edp: support multiple generations of edp hardware
> >>   drm/msm/edp: add support for next gen edp
> >>   drm/msm/disp/dpu1: add support for edp encoder
> >>
> >>  drivers/gpu/drm/msm/Makefile                      |   19 +-
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c       |    7 +-
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c           |   33 +
> >>  drivers/gpu/drm/msm/edp/edp.c                     |  198 ---
> >>  drivers/gpu/drm/msm/edp/edp.h                     |   78 -
> >>  drivers/gpu/drm/msm/edp/edp.xml.h                 |  380 -----
> >>  drivers/gpu/drm/msm/edp/edp_aux.c                 |  264 ----
> >>  drivers/gpu/drm/msm/edp/edp_bridge.c              |  111 --
> >>  drivers/gpu/drm/msm/edp/edp_common.c              |   38 +
> >>  drivers/gpu/drm/msm/edp/edp_common.h              |   47 +
> >>  drivers/gpu/drm/msm/edp/edp_connector.c           |  132 --
> >>  drivers/gpu/drm/msm/edp/edp_ctrl.c                | 1375
> >> ------------------
> >>  drivers/gpu/drm/msm/edp/edp_phy.c                 |   98 --
> >>  drivers/gpu/drm/msm/edp/v200/edp.xml.h            |  380 +++++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200.c           |  210 +++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200.h           |   70 +
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c       |  264 ++++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c    |  111 ++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c |  132 ++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c      | 1375
> >> ++++++++++++++++++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c       |   98 ++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510.c           |  220 +++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510.h           |  151 ++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c       |  268 ++++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c    |  111 ++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c |  117 ++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c      | 1583
> >> +++++++++++++++++++++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c       |  641 +++++++++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h       |  339 +++++
> >>  29 files changed, 6207 insertions(+), 2643 deletions(-)
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.c
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.h
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.xml.h
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.h
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp.xml.h
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.h
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.h
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h
> >>
> >> --
> >> The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora
> >> Forum, a Linux Foundation Collaborative Project
> >>
> >
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon
  2021-05-06  6:47   ` sbillaka
  2021-05-06 12:14     ` Dmitry Baryshkov
@ 2021-05-06 15:02     ` Rob Clark
  2021-05-10 12:16       ` sbillaka
  1 sibling, 1 reply; 9+ messages in thread
From: Rob Clark @ 2021-05-06 15:02 UTC (permalink / raw)
  To: sbillaka
  Cc: Dmitry Baryshkov, open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list:DRM DRIVER FOR MSM ADRENO GPU, freedreno, open list,
	Sean Paul, Stephen Boyd, Kalyan Thota, Abhinav Kumar,
	Douglas Anderson, Kuogee Hsieh, Krishna Manikandan

On Wed, May 5, 2021 at 11:47 PM <sbillaka@codeaurora.org> wrote:
>
> On 2021-05-05 15:31, Dmitry Baryshkov wrote:
> > Hi,
> >
> > On Wed, 5 May 2021 at 11:17, Sankeerth Billakanti
> > <sbillaka@codeaurora.org> wrote:
> >>
> >> These patches add support for the next generation eDP driver on
> >> SnapDragon
> >> with dpu support. The existing eDP driver cannot support the new eDP
> >> hardware. So, to maintain backward compatibility, the older eDP driver
> >> is
> >> moved to v200 folder and the new generation eDP driver is added in
> >> the v510 folder.
> >
> > What exactly does this version correspond to?
> > I assume that v510 corresponds to sdmshrike/sc8180x. Is it right?
> [Sankeerth] This is for sc7280.
>
> > Is it really so specific, or just v2/v5 would be enough? Not to
> > mention that this is the MDP/ version, while other blocks tend to use
> > block-specific versions/ids.
> [Sankeerth] I can rename it as edp-v1 and edp-v2. Edp v1 is very old
> chip and there is considerable HW delta between v1 and v2. So, we want
> to separate the driver. We followed similar model for DPU driver where,
> MDP4, MDP5 and DPU have separate folders. EDP v1 belongs to MDP4
> generation.

Bjorn brought up the idea of just dropping the existing drm/msm/edp..
since the efforts to upstream the platform it worked on (8084?)
fizzled out, I don't think there is any device which uses it.

But it does sound like edp is a subset of the the newer dp driver, so
seems sort of like the better approach would be to add edp support to
dp.  I believe Bjorn has something based on this approach which is
working for sc8280 (although not sure if it is in shape to post
patches yet)

BR,
-R

> >
> > Also, how much does it differ from the current DP core supported via
> > drivers/gpu/drm/msm/dp ?
> [Sankeerth] eDP is a native controller like DP but does not have audio,
> content protection and interoperability requirement. Upstream already
> supports eDP as a new interface driver found here:
> drivers/gpu/drm/msm/edp.
> I wanted to add the new controller driver as part of that folder.
>
> >
> > First two patches did not make it to the linux-msm, so I can not
> > comment on each of the lines.
> [Sankeerth] I am also not sure why they did not make it to patchwork. I
> will repost them.
>
> > However just my few cents (other reviewers might disagree though):
> >
> > - I see little benefit in renaming the folders just for the sake of
> > renaming. You can put your code in drivers/gpu/drm/msm/edp-v510, if
> > you really insist on that. Note that for all other (even incompatible)
> > hardware types we still use single level of folders.
> >
> > - Also I see that significant parts of code (e.g. AUX, bridge,
> > connector, maybe more) are just c&p of old edp code pieces. Please
> > share the code instead of duplicating it.
> [Sankeerth] It is a baseline driver. As we add more features, it will
> considerably deviate a lot. The effort seems to be very high to maintain
> the common portion of code as I expect a lot of deviation.
> >
> > - Please consider updating register definitions in xml form and then
> > providing both changed xml files (to mesa project (?)) and generated
> > headers into the kernel.
> [Sankeerth] I followed what was done in the DP driver at
> /drivers/gpu/drm/msm/dp. I need to explore the xml approach to generate
> the register definitions.
> >
> > - Please consider using clk_bulk_* functions instead of using
> > dss_module_power. I'm going to send a patchset reworking current users
> > to use the generic clk_bulk_* function family.
> [Sankeerth] I will explore and rebase after your patch is available.
> >
> > - In generic, this eDP clock handling seems to match closely DP clocks
> > handling (with all the name comparison, etc). Consider moving this to
> > a generic piece of code
> >
> > - PHY seems to be a version of QMP PHY. Please use it, like it was
> > done for the DP itself. There is support for combined USB+DP PHYs
> > (both v3 and v4), so it should be possible to extend that for eDP.
> [Sankeerth] The DP phy is a combophy which supports both usb and dp phy
> concurrently, unlike eDP phy which is specific to only the eDP
> controller in sc7280. So, I implemented the edp phy sequences in the
> same folder.
> >
> >
> >> These are baseline changes with which we can enable display. The new
> >> eDP
> >> controller can also support additional features such as backlight
> >> control,
> >> PSR etc. which will be enabled in subsequent patch series.
> >>
> >> Summary of changes:
> >> DPU driver interface to the new eDP v510 display driver.
> >> New generation eDP controller and phy driver implementation.
> >> A common interface to choose enable the required eDP driver.
> >>
> >> Sankeerth Billakanti (3):
> >>   drm/msm/edp: support multiple generations of edp hardware
> >>   drm/msm/edp: add support for next gen edp
> >>   drm/msm/disp/dpu1: add support for edp encoder
> >>
> >>  drivers/gpu/drm/msm/Makefile                      |   19 +-
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c       |    7 +-
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c           |   33 +
> >>  drivers/gpu/drm/msm/edp/edp.c                     |  198 ---
> >>  drivers/gpu/drm/msm/edp/edp.h                     |   78 -
> >>  drivers/gpu/drm/msm/edp/edp.xml.h                 |  380 -----
> >>  drivers/gpu/drm/msm/edp/edp_aux.c                 |  264 ----
> >>  drivers/gpu/drm/msm/edp/edp_bridge.c              |  111 --
> >>  drivers/gpu/drm/msm/edp/edp_common.c              |   38 +
> >>  drivers/gpu/drm/msm/edp/edp_common.h              |   47 +
> >>  drivers/gpu/drm/msm/edp/edp_connector.c           |  132 --
> >>  drivers/gpu/drm/msm/edp/edp_ctrl.c                | 1375
> >> ------------------
> >>  drivers/gpu/drm/msm/edp/edp_phy.c                 |   98 --
> >>  drivers/gpu/drm/msm/edp/v200/edp.xml.h            |  380 +++++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200.c           |  210 +++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200.h           |   70 +
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c       |  264 ++++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c    |  111 ++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c |  132 ++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c      | 1375
> >> ++++++++++++++++++
> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c       |   98 ++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510.c           |  220 +++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510.h           |  151 ++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c       |  268 ++++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c    |  111 ++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c |  117 ++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c      | 1583
> >> +++++++++++++++++++++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c       |  641 +++++++++
> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h       |  339 +++++
> >>  29 files changed, 6207 insertions(+), 2643 deletions(-)
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.c
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.h
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.xml.h
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.h
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c
> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp.xml.h
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.h
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.h
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c
> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h
> >>
> >> --
> >> The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora
> >> Forum, a Linux Foundation Collaborative Project
> >>
> >
> >
> > --
> > With best wishes
> > Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon
  2021-05-06 15:02     ` Rob Clark
@ 2021-05-10 12:16       ` sbillaka
  2021-05-11  4:55         ` Bjorn Andersson
  0 siblings, 1 reply; 9+ messages in thread
From: sbillaka @ 2021-05-10 12:16 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov
  Cc: open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list:DRM DRIVER FOR MSM ADRENO GPU, freedreno, open list,
	Sean Paul, Stephen Boyd, Kalyan Thota, Abhinav Kumar,
	Douglas Anderson, Kuogee Hsieh, Krishna Manikandan

On 2021-05-06 20:32, Rob Clark wrote:
> On Wed, May 5, 2021 at 11:47 PM <sbillaka@codeaurora.org> wrote:
>> 
>> On 2021-05-05 15:31, Dmitry Baryshkov wrote:
>> > Hi,
>> >
>> > On Wed, 5 May 2021 at 11:17, Sankeerth Billakanti
>> > <sbillaka@codeaurora.org> wrote:
>> >>
>> >> These patches add support for the next generation eDP driver on
>> >> SnapDragon
>> >> with dpu support. The existing eDP driver cannot support the new eDP
>> >> hardware. So, to maintain backward compatibility, the older eDP driver
>> >> is
>> >> moved to v200 folder and the new generation eDP driver is added in
>> >> the v510 folder.
>> >
>> > What exactly does this version correspond to?
>> > I assume that v510 corresponds to sdmshrike/sc8180x. Is it right?
>> [Sankeerth] This is for sc7280.
>> 
>> > Is it really so specific, or just v2/v5 would be enough? Not to
>> > mention that this is the MDP/ version, while other blocks tend to use
>> > block-specific versions/ids.
>> [Sankeerth] I can rename it as edp-v1 and edp-v2. Edp v1 is very old
>> chip and there is considerable HW delta between v1 and v2. So, we want
>> to separate the driver. We followed similar model for DPU driver 
>> where,
>> MDP4, MDP5 and DPU have separate folders. EDP v1 belongs to MDP4
>> generation.
> 
> Bjorn brought up the idea of just dropping the existing drm/msm/edp..
> since the efforts to upstream the platform it worked on (8084?)
> fizzled out, I don't think there is any device which uses it.
> 
> But it does sound like edp is a subset of the the newer dp driver, so
> seems sort of like the better approach would be to add edp support to
> dp.  I believe Bjorn has something based on this approach which is
> working for sc8280 (although not sure if it is in shape to post
> patches yet)
> 
> BR,
> -R
Hi Rob,
I will explore to integrate native eDP driver as part of DP driver. Will 
follow up with new patchsets.

Hi Dmitry,
I will move the eDP phy to qmp drivers folder in the new patchsets so 
that it can reuse the dp core driver.

Sankeerth

> 
>> >
>> > Also, how much does it differ from the current DP core supported via
>> > drivers/gpu/drm/msm/dp ?
>> [Sankeerth] eDP is a native controller like DP but does not have 
>> audio,
>> content protection and interoperability requirement. Upstream already
>> supports eDP as a new interface driver found here:
>> drivers/gpu/drm/msm/edp.
>> I wanted to add the new controller driver as part of that folder.
>> 
>> >
>> > First two patches did not make it to the linux-msm, so I can not
>> > comment on each of the lines.
>> [Sankeerth] I am also not sure why they did not make it to patchwork. 
>> I
>> will repost them.
>> 
>> > However just my few cents (other reviewers might disagree though):
>> >
>> > - I see little benefit in renaming the folders just for the sake of
>> > renaming. You can put your code in drivers/gpu/drm/msm/edp-v510, if
>> > you really insist on that. Note that for all other (even incompatible)
>> > hardware types we still use single level of folders.
>> >
>> > - Also I see that significant parts of code (e.g. AUX, bridge,
>> > connector, maybe more) are just c&p of old edp code pieces. Please
>> > share the code instead of duplicating it.
>> [Sankeerth] It is a baseline driver. As we add more features, it will
>> considerably deviate a lot. The effort seems to be very high to 
>> maintain
>> the common portion of code as I expect a lot of deviation.
>> >
>> > - Please consider updating register definitions in xml form and then
>> > providing both changed xml files (to mesa project (?)) and generated
>> > headers into the kernel.
>> [Sankeerth] I followed what was done in the DP driver at
>> /drivers/gpu/drm/msm/dp. I need to explore the xml approach to 
>> generate
>> the register definitions.
>> >
>> > - Please consider using clk_bulk_* functions instead of using
>> > dss_module_power. I'm going to send a patchset reworking current users
>> > to use the generic clk_bulk_* function family.
>> [Sankeerth] I will explore and rebase after your patch is available.
>> >
>> > - In generic, this eDP clock handling seems to match closely DP clocks
>> > handling (with all the name comparison, etc). Consider moving this to
>> > a generic piece of code
>> >
>> > - PHY seems to be a version of QMP PHY. Please use it, like it was
>> > done for the DP itself. There is support for combined USB+DP PHYs
>> > (both v3 and v4), so it should be possible to extend that for eDP.
>> [Sankeerth] The DP phy is a combophy which supports both usb and dp 
>> phy
>> concurrently, unlike eDP phy which is specific to only the eDP
>> controller in sc7280. So, I implemented the edp phy sequences in the
>> same folder.
>> >
>> >
>> >> These are baseline changes with which we can enable display. The new
>> >> eDP
>> >> controller can also support additional features such as backlight
>> >> control,
>> >> PSR etc. which will be enabled in subsequent patch series.
>> >>
>> >> Summary of changes:
>> >> DPU driver interface to the new eDP v510 display driver.
>> >> New generation eDP controller and phy driver implementation.
>> >> A common interface to choose enable the required eDP driver.
>> >>
>> >> Sankeerth Billakanti (3):
>> >>   drm/msm/edp: support multiple generations of edp hardware
>> >>   drm/msm/edp: add support for next gen edp
>> >>   drm/msm/disp/dpu1: add support for edp encoder
>> >>
>> >>  drivers/gpu/drm/msm/Makefile                      |   19 +-
>> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c       |    7 +-
>> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c           |   33 +
>> >>  drivers/gpu/drm/msm/edp/edp.c                     |  198 ---
>> >>  drivers/gpu/drm/msm/edp/edp.h                     |   78 -
>> >>  drivers/gpu/drm/msm/edp/edp.xml.h                 |  380 -----
>> >>  drivers/gpu/drm/msm/edp/edp_aux.c                 |  264 ----
>> >>  drivers/gpu/drm/msm/edp/edp_bridge.c              |  111 --
>> >>  drivers/gpu/drm/msm/edp/edp_common.c              |   38 +
>> >>  drivers/gpu/drm/msm/edp/edp_common.h              |   47 +
>> >>  drivers/gpu/drm/msm/edp/edp_connector.c           |  132 --
>> >>  drivers/gpu/drm/msm/edp/edp_ctrl.c                | 1375
>> >> ------------------
>> >>  drivers/gpu/drm/msm/edp/edp_phy.c                 |   98 --
>> >>  drivers/gpu/drm/msm/edp/v200/edp.xml.h            |  380 +++++
>> >>  drivers/gpu/drm/msm/edp/v200/edp_v200.c           |  210 +++
>> >>  drivers/gpu/drm/msm/edp/v200/edp_v200.h           |   70 +
>> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c       |  264 ++++
>> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c    |  111 ++
>> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c |  132 ++
>> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c      | 1375
>> >> ++++++++++++++++++
>> >>  drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c       |   98 ++
>> >>  drivers/gpu/drm/msm/edp/v510/edp_v510.c           |  220 +++
>> >>  drivers/gpu/drm/msm/edp/v510/edp_v510.h           |  151 ++
>> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c       |  268 ++++
>> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c    |  111 ++
>> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c |  117 ++
>> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c      | 1583
>> >> +++++++++++++++++++++
>> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c       |  641 +++++++++
>> >>  drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h       |  339 +++++
>> >>  29 files changed, 6207 insertions(+), 2643 deletions(-)
>> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.c
>> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.h
>> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp.xml.h
>> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c
>> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/edp_common.h
>> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c
>> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c
>> >>  delete mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp.xml.h
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.h
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.h
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c
>> >>  create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h
>> >>
>> >> --
>> >> The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora
>> >> Forum, a Linux Foundation Collaborative Project
>> >>
>> >
>> >
>> > --
>> > With best wishes
>> > Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon
  2021-05-10 12:16       ` sbillaka
@ 2021-05-11  4:55         ` Bjorn Andersson
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Andersson @ 2021-05-11  4:55 UTC (permalink / raw)
  To: sbillaka
  Cc: Rob Clark, Dmitry Baryshkov,
	open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list:DRM DRIVER FOR MSM ADRENO GPU, freedreno, open list,
	Sean Paul, Stephen Boyd, Kalyan Thota, Abhinav Kumar,
	Douglas Anderson, Kuogee Hsieh, Krishna Manikandan

On Mon 10 May 07:16 CDT 2021, sbillaka@codeaurora.org wrote:

> On 2021-05-06 20:32, Rob Clark wrote:
> > On Wed, May 5, 2021 at 11:47 PM <sbillaka@codeaurora.org> wrote:
> > > 
> > > On 2021-05-05 15:31, Dmitry Baryshkov wrote:
> > > > Hi,
> > > >
> > > > On Wed, 5 May 2021 at 11:17, Sankeerth Billakanti
> > > > <sbillaka@codeaurora.org> wrote:
> > > >>
> > > >> These patches add support for the next generation eDP driver on
> > > >> SnapDragon
> > > >> with dpu support. The existing eDP driver cannot support the new eDP
> > > >> hardware. So, to maintain backward compatibility, the older eDP driver
> > > >> is
> > > >> moved to v200 folder and the new generation eDP driver is added in
> > > >> the v510 folder.
> > > >
> > > > What exactly does this version correspond to?
> > > > I assume that v510 corresponds to sdmshrike/sc8180x. Is it right?
> > > [Sankeerth] This is for sc7280.
> > > 
> > > > Is it really so specific, or just v2/v5 would be enough? Not to
> > > > mention that this is the MDP/ version, while other blocks tend to use
> > > > block-specific versions/ids.
> > > [Sankeerth] I can rename it as edp-v1 and edp-v2. Edp v1 is very old
> > > chip and there is considerable HW delta between v1 and v2. So, we want
> > > to separate the driver. We followed similar model for DPU driver
> > > where,
> > > MDP4, MDP5 and DPU have separate folders. EDP v1 belongs to MDP4
> > > generation.
> > 
> > Bjorn brought up the idea of just dropping the existing drm/msm/edp..
> > since the efforts to upstream the platform it worked on (8084?)
> > fizzled out, I don't think there is any device which uses it.
> > 
> > But it does sound like edp is a subset of the the newer dp driver, so
> > seems sort of like the better approach would be to add edp support to
> > dp.  I believe Bjorn has something based on this approach which is
> > working for sc8280 (although not sure if it is in shape to post
> > patches yet)
> > 
> > BR,
> > -R
> Hi Rob,
> I will explore to integrate native eDP driver as part of DP driver. Will
> follow up with new patchsets.
> 
> Hi Dmitry,
> I will move the eDP phy to qmp drivers folder in the new patchsets so that
> it can reuse the dp core driver.
> 

Hi Sankeerth,

I've been working on eDP support for sc8180x recently, which afaict is
identical to sc7280 in this regard. I finally got the patches cleaned up
and posted here:
https://lore.kernel.org/linux-arm-msm/20210511042043.592802-1-bjorn.andersson@linaro.org/T/#t
https://lore.kernel.org/linux-arm-msm/20210511041930.592483-1-bjorn.andersson@linaro.org/T/#t

My initial patches added widebus support, rather than disabling it. But
those patches needs a little bit more polishing - and I finally figured
was able to disable the feature. So I will get back to this.

There's currently a few seconds delay on plug detection, so this needs
to be investigated further and I haven't looked at backlight handling
yet.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 3/3] drm/msm/disp/dpu1: add support for edp encoder
  2021-05-06  6:46 [PATCH v1 0/3] Add support for next gen eDP driver on sc7280 Sankeerth Billakanti
@ 2021-05-06  6:46 ` Sankeerth Billakanti
  0 siblings, 0 replies; 9+ messages in thread
From: Sankeerth Billakanti @ 2021-05-06  6:46 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm, freedreno, linux-kernel
  Cc: Sankeerth Billakanti, robdclark, seanpaul, swboyd, kalyan_t,
	abhinavk, dianders, khsieh, mkrishn

This change will enable dpu encoder support for the native
eDP interface on next generation snapdragon platforms.

Signed-off-by: Sankeerth Billakanti <sbillaka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  7 +++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 33 +++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8d94205..9f5185bb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2075,7 +2075,12 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
 		intf_type = INTF_DSI;
 		break;
 	case DRM_MODE_ENCODER_TMDS:
-		intf_type = INTF_DP;
+		if (disp_info->capabilities & MSM_DISPLAY_CAP_CMD_MODE) {
+			intf_type = INTF_EDP;
+			/* PSR CMD mode not supported */
+			disp_info->capabilities = MSM_DISPLAY_CAP_VID_MODE;
+		} else
+			intf_type = INTF_DP;
 		break;
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 88e9cc3..ecd7dc8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -499,6 +499,33 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
 	return rc;
 }
 
+static int _dpu_kms_initialize_edp(struct drm_device *dev,
+				    struct msm_drm_private *priv,
+				    struct dpu_kms *dpu_kms)
+{
+	struct drm_encoder *encoder = NULL;
+	int rc = 0;
+
+	if (!priv->edp)
+		return rc;
+
+	encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+	if (IS_ERR(encoder)) {
+		DPU_ERROR("encoder init failed for eDP display\n");
+		return PTR_ERR(encoder);
+	}
+
+	rc = msm_edp_modeset_init(priv->edp, dev, encoder);
+	if (rc) {
+		DPU_ERROR("modeset_init failed for eDP, rc = %d\n", rc);
+		drm_encoder_cleanup(encoder);
+		return rc;
+	}
+
+	priv->encoders[priv->num_encoders++] = encoder;
+	return rc;
+}
+
 static int _dpu_kms_initialize_displayport(struct drm_device *dev,
 					    struct msm_drm_private *priv,
 					    struct dpu_kms *dpu_kms)
@@ -546,6 +573,12 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
 		return rc;
 	}
 
+	rc = _dpu_kms_initialize_edp(dev, priv, dpu_kms);
+	if (rc) {
+		DPU_ERROR("initialize_eDP failed, rc = %d\n", rc);
+		return rc;
+	}
+
 	rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
 	if (rc) {
 		DPU_ERROR("initialize_DP failed, rc = %d\n", rc);
-- 
The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-05-11  4:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-05  8:16 [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon Sankeerth Billakanti
2021-05-05  8:16 ` [PATCH v1 3/3] drm/msm/disp/dpu1: add support for edp encoder Sankeerth Billakanti
2021-05-05 10:01 ` [PATCH v1 0/3] Add support for next gen eDP driver on SnapDragon Dmitry Baryshkov
2021-05-06  6:47   ` sbillaka
2021-05-06 12:14     ` Dmitry Baryshkov
2021-05-06 15:02     ` Rob Clark
2021-05-10 12:16       ` sbillaka
2021-05-11  4:55         ` Bjorn Andersson
2021-05-06  6:46 [PATCH v1 0/3] Add support for next gen eDP driver on sc7280 Sankeerth Billakanti
2021-05-06  6:46 ` [PATCH v1 3/3] drm/msm/disp/dpu1: add support for edp encoder Sankeerth Billakanti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).