Re: [PATCH v2 2/7] soc: qcom: smem: Add a feature code getter
On Wed, Apr 17, 2024 at 10:02:54PM GMT, Konrad Dybcio wrote:
[..]
> diff --git a/include/linux/soc/qcom/socinfo.h
> b/include/linux/soc/qcom/socinfo.h
> index 10e0a4c287f4..52439f48428f 100644
> --- a/include/linux/soc/qcom/socinfo.h
> +++ b/include/linux/soc/qcom/socinfo.h
> @@ -3,6 +3,8 @@
> #ifndef __QCOM_SOCINFO_H__
> #define __QCOM_SOCINFO_H__
>
> +#include
> +
> /*
> * SMEM item id, used to acquire handles to respective
> * SMEM region.
> @@ -82,4 +84,28 @@ struct socinfo {
> __le32 boot_core;
> };
>
> +/* Internal feature codes */
> +enum qcom_socinfo_feature_code {
> + /* External feature codes */
> + SOCINFO_FC_UNKNOWN = 0x0,
> + SOCINFO_FC_AA,
> + SOCINFO_FC_AB,
> + SOCINFO_FC_AC,
> + SOCINFO_FC_AD,
> + SOCINFO_FC_AE,
> + SOCINFO_FC_AF,
> + SOCINFO_FC_AG,
> + SOCINFO_FC_AH,
> +};
> +
> +/* Internal feature codes */
> +/* Valid values: 0 <= n <= 0xf */
> +#define SOCINFO_FC_Yn(n) (0xf1 + n)
Please wrap that 'n' in some () here and below...
> +#define SOCINFO_FC_INT_MAX SOCINFO_FC_Yn(0x10)
"MAX" sounds inclusive, but the value is exclusive.
Regards,
Bjorn
> +
> +/* Product codes */
> +#define SOCINFO_PC_UNKNOWN 0
> +#define SOCINFO_PCn(n) (n + 1)
> +#define SOCINFO_PC_RESERVE (BIT(31) - 1)
> +
> #endif
>
> --
> 2.44.0
>
Re: (subset) [PATCH 0/4] Various fixes for the lm3630a backlight driver
On Tue, 20 Feb 2024 00:11:18 +0100, Luca Weiss wrote: > On the MSM8974 Nexus 5 and OnePlus One phones (latter doesn't have > display upstream) the display backlight was turning off whenever you > would write a brightness to sysfs since a recent commit to the driver > (kernel v6.5). > > backlight: lm3630a: Turn off both led strings when display is blank > > [...] Applied, thanks! [4/4] ARM: dts: qcom: msm8974-hammerhead: Hook up backlight commit: e23dfb4ee30a120a947abb94a718ccc1eb5f87ff Best regards, -- Bjorn Andersson
Re: (subset) [PATCH 0/8] MSM8976 MDSS/GPU/WCNSS support
On Sun, 21 Jan 2024 20:40:58 +0100, Adam Skladowski wrote: > This patch series provide support for display subsystem, gpu > and also adds wireless connectivity subsystem support. > > Adam Skladowski (8): > arm64: dts: qcom: msm8976: Add IOMMU nodes > dt-bindings: dsi-controller-main: Document missing msm8976 compatible > dt-bindings: msm: qcom,mdss: Include ommited fam-b compatible > arm64: dts: qcom: msm8976: Add MDSS nodes > dt-bindings: drm/msm/gpu: Document AON clock for A506/A510 > arm64: dts: qcom: msm8976: Add Adreno GPU > arm64: dts: qcom: msm8976: Declare and wire SDC pins > arm64: dts: qcom: msm8976: Add WCNSS node > > [...] Applied, thanks! [1/8] arm64: dts: qcom: msm8976: Add IOMMU nodes commit: 418c2ffd7df9bfc25c21172bd881b78d7569fb4d Best regards, -- Bjorn Andersson
Re: [PATCH] drm/msm/adreno: Check for zap node availability
On Fri, May 17, 2024 at 12:50:19PM -0700, Rob Clark wrote:
> From: Rob Clark
>
> This should allow disabling the zap node via an overlay, for slbounce.
>
> Suggested-by: Nikita Travkin
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index d9ea15994ae9..a00241e3373b 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -46,7 +46,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
> char *fwname,
> }
>
> np = of_get_child_by_name(dev->of_node, "zap-shader");
> - if (!np) {
> + if (!np || !of_device_is_available(np)) {
if (!of_device_is_available(np)) {
would cover both cases and be slightly cleaner imho...
But this looks reasonable either way.
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> zap_available = false;
> return -ENODEV;
> }
> --
> 2.45.1
>
Re: (subset) [PATCH v2 0/3] DisplayPort support for SM6350/SM7225
On Fri, 29 Mar 2024 08:45:53 +0100, Luca Weiss wrote: > Add the required changes to support DisplayPort (normally(?) available > via the USB-C connector) on the SM6350/SM7225 SoC. > > This has been tested on a Fairphone 4 smartphone with additional changes > not included in this series (mostly just wiring up TCPM and the SBU > mux). > > [...] Applied, thanks! [3/3] arm64: dts: qcom: sm6350: Add DisplayPort controller commit: 62f87a3cac4e70fa916914a359d3f045a5ad8b9b Best regards, -- Bjorn Andersson
Re: [PATCH] drm/msm: Drop msm_read/writel
On Thu, Apr 11, 2024 at 04:31:41AM +0300, Dmitry Baryshkov wrote:
> On Wed, Apr 10, 2024 at 11:52:52PM +0200, Konrad Dybcio wrote:
[..]
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > index e4275d3ad581..5a5dc3faa971 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > @@ -12,10 +12,10 @@
> >
> > #include "dsi.h"
> >
> > -#define dsi_phy_read(offset) msm_readl((offset))
> > -#define dsi_phy_write(offset, data) msm_writel((data), (offset))
> > -#define dsi_phy_write_udelay(offset, data, delay_us) { msm_writel((data),
> > (offset)); udelay(delay_us); }
> > -#define dsi_phy_write_ndelay(offset, data, delay_ns) { msm_writel((data),
> > (offset)); ndelay(delay_ns); }
> > +#define dsi_phy_read(offset) readl((offset))
> > +#define dsi_phy_write(offset, data) writel((data), (offset))
> > +#define dsi_phy_write_udelay(offset, data, delay_us) { writel((data),
> > (offset)); udelay(delay_us); }
> > +#define dsi_phy_write_ndelay(offset, data, delay_ns) { writel((data),
> > (offset)); ndelay(delay_ns); }
>
> What about also inlining these wrappers?
>
But that should be done in a separate commit, no?
PS. Too much scrolling to find your comments, please trim your replies.
Thanks,
Bjorn
Re: [PATCH v2 6/6] drm/msm/dp: Use function arguments for audio operations
On Mon, Apr 08, 2024 at 02:31:45PM -0700, Abhinav Kumar wrote: > On 3/28/2024 7:40 AM, Bjorn Andersson wrote: > > From: Bjorn Andersson > > > > The dp_audio read and write operations uses members in struct dp_catalog > > for passing arguments and return values. This adds unnecessary > > complexity to the implementation, as it turns out after detangling the > > logic that no state is actually held in these variables. > > > > Clean this up by using function arguments and return values for passing > > the data. > > > > Reviewed-by: Dmitry Baryshkov > > Signed-off-by: Bjorn Andersson > > --- > > drivers/gpu/drm/msm/dp/dp_audio.c | 20 +-- > > drivers/gpu/drm/msm/dp/dp_catalog.c | 39 > > + > > drivers/gpu/drm/msm/dp/dp_catalog.h | 18 + > > 3 files changed, 28 insertions(+), 49 deletions(-) > > > > One quick question, was DP audio re-tested after this patch? No, sorry for not being explicit about that. I don't have any target with working DP audio... Regards, Bjorn
Re: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
On Fri, Apr 05, 2024 at 10:41:30AM +0200, Konrad Dybcio wrote:
> Introduce getters for SoC product and feature codes and export them.
>
Thought I commented on this already, but I don't see my reply...
Can you please elaborate on what this stuff is, such that we have a
track record in the history of this driver as well, for those of us that
don't know what "feature/product codes" contain or are good for (or have
forgotten next week).
Regards,
Bjorn
> Signed-off-by: Konrad Dybcio
> ---
> drivers/soc/qcom/smem.c | 66
> +++
> include/linux/soc/qcom/smem.h | 2 ++
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index 7191fa0c087f..e89b4d26877a 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
> @@ -795,6 +795,72 @@ int qcom_smem_get_soc_id(u32 *id)
> }
> EXPORT_SYMBOL_GPL(qcom_smem_get_soc_id);
>
> +/**
> + * qcom_smem_get_feature_code() - return the feature code
> + * @id: On success, we return the feature code here.
> + *
> + * Look up the feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_feature_code(u32 *code)
> +{
> + struct socinfo *info;
> + u32 raw_code;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> +
> + /* This only makes sense for socinfo >= 16 */
> + if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> + return -EINVAL;
> +
> + raw_code = __le32_to_cpu(info->feature_code);
> +
> + /* Ensure the value makes sense */
> + if (raw_code >= SOCINFO_FC_INT_RESERVE)
> + raw_code = SOCINFO_FC_UNKNOWN;
> +
> + *code = raw_code;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_feature_code);
> +
> +/**
> + * qcom_smem_get_product_code() - return the product code
> + * @id: On success, we return the product code here.
> + *
> + * Look up feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_product_code(u32 *code)
> +{
> + struct socinfo *info;
> + u32 raw_code;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> +
> + /* This only makes sense for socinfo >= 16 */
> + if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> + return -EINVAL;
> +
> + raw_code = __le32_to_cpu(info->pcode);
> +
> + /* Ensure the value makes sense */
> + if (raw_code >= SOCINFO_FC_INT_RESERVE)
> + raw_code = SOCINFO_FC_UNKNOWN;
> +
> + *code = raw_code;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_product_code);
> +
> static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
> {
> struct smem_header *header;
> diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
> index a36a3b9d4929..aef8c9fc6c08 100644
> --- a/include/linux/soc/qcom/smem.h
> +++ b/include/linux/soc/qcom/smem.h
> @@ -13,5 +13,7 @@ int qcom_smem_get_free_space(unsigned host);
> phys_addr_t qcom_smem_virt_to_phys(void *p);
>
> int qcom_smem_get_soc_id(u32 *id);
> +int qcom_smem_get_feature_code(u32 *code);
> +int qcom_smem_get_product_code(u32 *code);
>
> #endif
>
> --
> 2.40.1
>
Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD
On Tue, Apr 09, 2024 at 01:07:57AM +0300, Dmitry Baryshkov wrote: > On Tue, 9 Apr 2024 at 00:23, Abhinav Kumar wrote: > > On 4/8/2024 2:12 PM, Dmitry Baryshkov wrote: > > > On Mon, 8 Apr 2024 at 22:43, Abhinav Kumar > > > wrote: > > >> On 4/7/2024 11:48 AM, Bjorn Andersson wrote: > > >>> On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote: > > >>>> From: Kuogee Hsieh > > >>> [..] > > >>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > >>>> b/drivers/gpu/drm/msm/dp/dp_display.c [..] > > >> > > >> I will need sometime to address that use-case as I need to see if we can > > >> handle that better and then drop the the DISCONNECT_PENDING state to > > >> address this fully. But it needs more testing. > > >> > > >> But, we will need this patch anyway because without this we will not be > > >> able to fix even the most regular and commonly seen case of basic > > >> connect/disconnect receiving complementary events. > > > > > > Hmm, no. We need to drop the HPD state machine, not to patch it. Once > > > the driver has proper detect() callback, there will be no > > > complementary events. That is a proper way to fix the code, not these > > > kinds of band-aids patches. > > > > > > > I had discussed this part too :) > > > > I totally agree we should fix .detect()'s behavior to just match cable > > connect/disconnect and not link_ready status. > > > > But that alone would not have fixed this issue. If HPD thread does not > > get scheduled and plug_handle() was not executed, .detect() would have > > still returned old status as we will update the cable status only in > > plug_handle() / unplug_handle() to have a common API between internal > > and external hpd execution. > > I think there should be just hpd_notify, which if the HPD is up, > attempts to read the DPCD. No need for separate plug/unplug_handle. > The detect() can be as simple as !drm_dp_is_branch() || sink_count != 0. > What is detect() supposed to return in the event that we have external HPD handler? The link state? While the external HPD bridge would return the HPD state? If a driver only drives the link inbetween atomic_enable() and atomic_disable() will the "connected state" then ever be reported as "connected"? (I'm sure I'm still missing pieces of this puzzle). Regards, Bjorn
Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD
On Mon, Apr 08, 2024 at 12:43:34PM -0700, Abhinav Kumar wrote:
>
>
> On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
> > On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
> > > From: Kuogee Hsieh
> > [..]
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> > > b/drivers/gpu/drm/msm/dp/dp_display.c
> > > index d80f89581760..bfb6dfff27e8 100644
> > > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > > @@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
> > > return;
> > > if (!dp_display->link_ready && status ==
> > > connector_status_connected)
> > > - dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> > > + dp_hpd_plug_handle(dp, 0);
> >
> > If I read the code correctly, and we get an external connect event
> > inbetween a previous disconnect and the related disable call, this
> > should result in a PLUG_INT being injected into the queue still.
> >
> > Will that not cause the same problem?
> >
> > Regards,
> > Bjorn
> >
>
> Yes, your observation is correct and I had asked the same question to kuogee
> before taking over this change and posting.
>
> We will have to handle that case separately. I don't have a good solution
> yet for it without requiring further rework or we drop the below snippet.
>
> if (state == ST_DISCONNECT_PENDING) {
> /* wait until ST_DISCONNECTED */
> dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
> mutex_unlock(>event_mutex);
> return 0;
> }
>
> I will need sometime to address that use-case as I need to see if we can
> handle that better and then drop the the DISCONNECT_PENDING state to address
> this fully. But it needs more testing.
>
> But, we will need this patch anyway because without this we will not be able
> to fix even the most regular and commonly seen case of basic
> connect/disconnect receiving complementary events.
>
I did some more testing on this patch. Connecting and disconnecting the
cable while in fbcon works reliably, except for:
- edid/modes are not read before we bring up the link so I always end up
with 640x480
- if I run modetest -s : the link is brought up with the new
resolution and I get my test image on the screen.
But as we're shutting down the link for the resolution chance I end up
in dp_bridge_hpd_notify() with link_ready && state = disconnected.
This triggers an unplug which hangs on the event_mutex, such that as
soon as I get the test image, the state machine enters
DISCONNECT_PENDING. This is immediately followed by another
!link_ready && status = connected, which attempts to perform the plug
operation, but as we're in DISCONNECT_PENDING this is posted on the
event queue. From there I get a log entry from my PLUG_INT, every
100ms stating that we're still in DISCONNECT_PENDING. If I exit
modetest the screen goes black, and no new mode can be selected,
because we're in DISCONNECT_PENDING. The only way out is to disconnect
the cable to complete the DISCONNECT_PENDING.
Regards,
Bjorn
>
> > > else if (dp_display->link_ready && status ==
> > > connector_status_disconnected)
> > > - dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
> > > + dp_hpd_unplug_handle(dp, 0);
> > > }
> > > --
> > > 2.43.2
> > >
Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD
On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote: > From: Kuogee Hsieh [..] > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index d80f89581760..bfb6dfff27e8 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge, > return; > > if (!dp_display->link_ready && status == connector_status_connected) > - dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0); > + dp_hpd_plug_handle(dp, 0); If I read the code correctly, and we get an external connect event inbetween a previous disconnect and the related disable call, this should result in a PLUG_INT being injected into the queue still. Will that not cause the same problem? Regards, Bjorn > else if (dp_display->link_ready && status == > connector_status_disconnected) > - dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0); > + dp_hpd_unplug_handle(dp, 0); > } > -- > 2.43.2 >
[PATCH v2] drm/msm/dp: Remove now unused connector_type from desc
Now that the connector_type is dynamically determined, the
connector_type of the struct msm_dp_desc is unused. Clean it up.
Remaining duplicate entries are squashed.
Signed-off-by: Bjorn Andersson
---
This cleans up after, and hence depends on,
https://lore.kernel.org/all/20240324-x1e80100-display-refactor-connector-v4-1-e0ebaea66...@linaro.org/
---
Changes in v2:
- Squashed now duplicate entries
- Link to v1:
https://lore.kernel.org/r/20240328-dp-connector-type-cleanup-v1-1-9bf84c5a6...@quicinc.com
---
drivers/gpu/drm/msm/dp/dp_display.c | 48 +
1 file changed, 17 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 521cba76d2a0..12c01625c551 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -119,55 +119,41 @@ struct dp_display_private {
struct msm_dp_desc {
phys_addr_t io_start;
unsigned int id;
- unsigned int connector_type;
bool wide_bus_supported;
};
static const struct msm_dp_desc sc7180_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0 },
{}
};
static const struct msm_dp_desc sc7280_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
{}
};
static const struct msm_dp_desc sc8180x_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
- { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0 },
+ { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1 },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2 },
{}
};
static const struct msm_dp_desc sc8280xp_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x2209, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x22098000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x2209a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x220a, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- {}
-};
-
-static const struct msm_dp_desc sc8280xp_edp_descs[] = {
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x2209a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x220a, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- {}
-};
-
-static const struct msm_dp_desc sm8350_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2,
.wide_bus_supported = true },
+ { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3,
.wide_bus_supported = true },
+ { .io_start = 0x2209, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x22098000, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
+ { .io_start
Re: (subset) [PATCH v3 0/4] arm64: dts: fix several display-related schema warnings
On Tue, 02 Apr 2024 05:57:14 +0300, Dmitry Baryshkov wrote: > Fix several warnings produced by the display nodes. > > Please excuse me for the spam for sending v3 soon after v2. > > Applied, thanks! [2/4] arm64: dts: qcom: sc8180x: drop legacy property #stream-id-cells commit: 7fb5680b589d5eae64ada1d917b6ff2dab82f5ae [3/4] arm64: dts: qcom: sc8180x: Drop flags for mdss irqs commit: 580701ec27f61e0996dd5fcd23b091b6bf6933e3 [4/4] arm64: dts: qcom: sc8180x: add dp_p1 register blocks to DP nodes commit: 1106ea2266d11ebd97c3493a0c36a45272bfb67a Best regards, -- Bjorn Andersson
Re: [PATCH] drm/msm/dp: Remove now unused connector_type from desc
On Fri, Mar 29, 2024 at 07:23:07AM +0200, Dmitry Baryshkov wrote:
> On Fri, 29 Mar 2024 at 06:02, Bjorn Andersson
> wrote:
> > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> > b/drivers/gpu/drm/msm/dp/dp_display.c
[..]
> > static const struct msm_dp_desc sc8280xp_edp_descs[] = {
>
> This can now be merged with sc8280xp_dp_descs
>
You're right, only saw the first level of cleanup. Will repsin this.
Thanks,
Bjorn
[PATCH] drm/msm/dp: Remove now unused connector_type from desc
Now that the connector_type is dynamically determined, the
connector_type of the struct msm_dp_desc is unused. Clean it up.
Signed-off-by: Bjorn Andersson
---
This cleans up after, and hence depends on,
https://lore.kernel.org/all/20240324-x1e80100-display-refactor-connector-v4-1-e0ebaea66...@linaro.org/
---
drivers/gpu/drm/msm/dp/dp_display.c | 41 ++---
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 521cba76d2a0..a18fb8b32c16 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -119,55 +119,54 @@ struct dp_display_private {
struct msm_dp_desc {
phys_addr_t io_start;
unsigned int id;
- unsigned int connector_type;
bool wide_bus_supported;
};
static const struct msm_dp_desc sc7180_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0 },
{}
};
static const struct msm_dp_desc sc7280_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
{}
};
static const struct msm_dp_desc sc8180x_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
- { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0 },
+ { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1 },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2 },
{}
};
static const struct msm_dp_desc sc8280xp_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x2209, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x22098000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x2209a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x220a, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2,
.wide_bus_supported = true },
+ { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3,
.wide_bus_supported = true },
+ { .io_start = 0x2209, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x22098000, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
+ { .io_start = 0x2209a000, .id = MSM_DP_CONTROLLER_2,
.wide_bus_supported = true },
+ { .io_start = 0x220a, .id = MSM_DP_CONTROLLER_3,
.wide_bus_supported = true },
{}
};
static const struct msm_dp_desc sc8280xp_edp_descs[] = {
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x2209a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x220a, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2,
.wide_bus_supported = true },
+ { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3,
.wide_bus_supported = true },
+ { .io_start = 0x2209a000, .id
Re: [PATCH v1] drm/msm/dp: use dp_hpd_plug_handle() and dp_hpd_unplug_handle() directly
On Thu, Mar 28, 2024 at 02:21:14PM -0700, Abhinav Kumar wrote:
>
>
> On 3/28/2024 1:58 PM, Stephen Boyd wrote:
> > Quoting Abhinav Kumar (2024-03-28 13:24:34)
> > > + Johan and Bjorn for FYI
> > >
> > > On 3/28/2024 1:04 PM, Kuogee Hsieh wrote:
> > > > For internal HPD case, hpd_event_thread is created to handle HPD
> > > > interrupts generated by HPD block of DP controller. It converts
> > > > HPD interrupts into events and executed them under hpd_event_thread
> > > > context. For external HPD case, HPD events is delivered by way of
> > > > dp_bridge_hpd_notify() under thread context. Since they are executed
> > > > under thread context already, there is no reason to hand over those
> > > > events to hpd_event_thread. Hence dp_hpd_plug_handle() and
> > > > dp_hpd_unplug_hanlde() are called directly at dp_bridge_hpd_notify().
> > > >
> > > > Signed-off-by: Kuogee Hsieh
> > > > ---
> > > >drivers/gpu/drm/msm/dp/dp_display.c | 5 +++--
> > > >1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > >
> > > Fixes: 542b37efc20e ("drm/msm/dp: Implement hpd_notify()")
> >
> > Is this a bug fix or an optimization? The commit text doesn't tell me.
> >
>
> I would say both.
>
> optimization as it avoids the need to go through the hpd_event thread
> processing.
>
> bug fix because once you go through the hpd event thread processing it
> exposes and often breaks the already fragile hpd handling state machine
> which can be avoided in this case.
>
It removes the main users of the thread, but there's still code paths
which will post events on the thread.
I think I like the direction this is taking, but does it really fix the
whole problem, or just patch one case?
PS. Please read go/upstream and switch to b4, to avoid some practical
issues with the way you posted this patch.
Thanks,
Bjorn
> > >
> > > Looks right to me,
> > >
> > > Reviewed-by: Abhinav Kumar
[PATCH v2 3/6] drm/msm/dp: Remove unused defines and members
From: Bjorn Andersson
Throughout the Qualcomm Displayport driver a number of defines and
struct members has become unused, but lingers in the code. Remove these.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_audio.c | 5 -
drivers/gpu/drm/msm/dp/dp_catalog.c | 1 -
drivers/gpu/drm/msm/dp/dp_catalog.h | 17 -
drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 -
drivers/gpu/drm/msm/dp/dp_display.c | 5 -
drivers/gpu/drm/msm/dp/dp_display.h | 3 ---
drivers/gpu/drm/msm/dp/dp_drm.c | 2 --
drivers/gpu/drm/msm/dp/dp_link.c| 4
drivers/gpu/drm/msm/dp/dp_link.h| 1 -
drivers/gpu/drm/msm/dp/dp_panel.h | 2 --
10 files changed, 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 7634e4b74208..7fd0c1793ba3 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -22,9 +22,7 @@ struct dp_audio_private {
struct platform_device *pdev;
struct drm_device *drm_dev;
struct dp_catalog *catalog;
- struct dp_panel *panel;
- bool engine_on;
u32 channels;
struct dp_audio dp_audio;
@@ -356,8 +354,6 @@ static void dp_audio_enable(struct dp_audio_private *audio,
bool enable)
catalog->audio_data = enable;
dp_catalog_audio_enable(catalog);
-
- audio->engine_on = enable;
}
static struct dp_audio_private *dp_audio_get_data(struct platform_device *pdev)
@@ -571,7 +567,6 @@ struct dp_audio *dp_audio_get(struct platform_device *pdev,
}
audio->pdev = pdev;
- audio->panel = panel;
audio->catalog = catalog;
dp_audio = >dp_audio;
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 8c72d532d96b..55114a6aba7e 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -81,7 +81,6 @@ struct dp_catalog_private {
struct dss_io_data io;
u32 (*audio_map)[DP_AUDIO_SDP_HEADER_MAX];
struct dp_catalog dp_catalog;
- u8 aux_lut_cfg_index[PHY_AUX_CFG_MAX];
};
void dp_catalog_snapshot(struct dp_catalog *dp_catalog, struct msm_disp_state
*disp_state)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index b85ad6bdb2e7..2c2dbeee7634 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -28,26 +28,9 @@
#define DP_INTR_FRAME_END BIT(6)
#define DP_INTR_CRC_UPDATEDBIT(9)
-#define DP_AUX_CFG_MAX_VALUE_CNT 3
-
#define DP_HW_VERSION_1_0 0x1000
#define DP_HW_VERSION_1_2 0x1002
-/* PHY AUX config registers */
-enum dp_phy_aux_config_type {
- PHY_AUX_CFG0,
- PHY_AUX_CFG1,
- PHY_AUX_CFG2,
- PHY_AUX_CFG3,
- PHY_AUX_CFG4,
- PHY_AUX_CFG5,
- PHY_AUX_CFG6,
- PHY_AUX_CFG7,
- PHY_AUX_CFG8,
- PHY_AUX_CFG9,
- PHY_AUX_CFG_MAX,
-};
-
enum dp_catalog_audio_sdp_type {
DP_AUDIO_SDP_STREAM,
DP_AUDIO_SDP_TIMESTAMP,
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index fa014cee7e21..ffcbd9a25748 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -12,7 +12,6 @@
#include "dp_catalog.h"
struct dp_ctrl {
- atomic_t aborted;
bool wide_bus_en;
};
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index ba658c1637d1..6f6ff13844cf 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -74,7 +74,6 @@ struct dp_event {
};
struct dp_display_private {
- char *name;
int irq;
unsigned int id;
@@ -82,11 +81,9 @@ struct dp_display_private {
/* state variables */
bool core_initialized;
bool phy_initialized;
- bool hpd_irq_on;
bool audio_supported;
struct drm_device *drm_dev;
- struct dentry *root;
struct dp_catalog *catalog;
struct drm_dp_aux *aux;
@@ -800,7 +797,6 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
drm_mode_copy(>panel->dp_mode.drm_mode, >drm_mode);
dp->panel->dp_mode.bpp = mode->bpp;
- dp->panel->dp_mode.capabilities = mode->capabilities;
dp->panel->dp_mode.out_fmt_is_yuv_420 = mode->out_fmt_is_yuv_420;
dp_panel_init_panel_info(dp->panel);
return 0;
@@ -1260,7 +1256,6 @@ static int dp_display_probe(struct platform_device *pdev)
return -EINVAL;
dp->dp_display.pdev = pdev;
- dp->name = "drm_dp";
dp->id = desc->id;
dp->dp_display.connector_type = desc->connector_type;
dp->wide_bus_supported = desc->wide_bus_supported;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp
[PATCH v2 2/6] drm/msm/dp: Removed fixed nvid "support"
From: Bjorn Andersson
The "desc" member of struct dp_panel is zero-initialized during
allocation and never assigned, resulting in dp_ctrl_use_fixed_nvid()
never returning true. This returned boolean value is passed around but
never acted upon.
Perform constant propagation and remove the traces of "fixed nvid".
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 2 +-
drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +-
drivers/gpu/drm/msm/dp/dp_ctrl.c| 17 +
drivers/gpu/drm/msm/dp/dp_panel.h | 1 -
4 files changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 3e7c84cdef47..8c72d532d96b 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -469,7 +469,7 @@ void dp_catalog_setup_peripheral_flush(struct dp_catalog
*dp_catalog)
void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog,
u32 rate, u32 stream_rate_khz,
- bool fixed_nvid, bool is_ycbcr_420)
+ bool is_ycbcr_420)
{
u32 pixel_m, pixel_n;
u32 mvid, nvid, pixel_div = 0, dispcc_input_rate;
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 75ec290127c7..b85ad6bdb2e7 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -99,7 +99,7 @@ void dp_catalog_ctrl_psr_mainlink_enable(struct dp_catalog
*dp_catalog, bool ena
void dp_catalog_setup_peripheral_flush(struct dp_catalog *dp_catalog);
void dp_catalog_ctrl_config_misc(struct dp_catalog *dp_catalog, u32 cc, u32
tb);
void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog, u32 rate,
- u32 stream_rate_khz, bool fixed_nvid, bool
is_ycbcr_420);
+ u32 stream_rate_khz, bool is_ycbcr_420);
int dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog *dp_catalog, u32
pattern);
u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog);
void dp_catalog_ctrl_reset(struct dp_catalog *dp_catalog);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index c4dda1faef67..e65a460fb52d 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1566,21 +1566,6 @@ void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl)
phy, phy->init_count, phy->power_count);
}
-static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl)
-{
- const u8 *dpcd = ctrl->panel->dpcd;
-
- /*
-* For better interop experience, used a fixed NVID=0x8000
-* whenever connected to a VGA dongle downstream.
-*/
- if (drm_dp_is_branch(dpcd))
- return (drm_dp_has_quirk(>panel->desc,
-DP_DPCD_QUIRK_CONSTANT_N));
-
- return false;
-}
-
static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl)
{
struct phy *phy = ctrl->phy;
@@ -2022,7 +2007,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool
force_link_train)
dp_catalog_ctrl_config_msa(ctrl->catalog,
ctrl->link->link_params.rate,
- pixel_rate_orig, dp_ctrl_use_fixed_nvid(ctrl),
+ pixel_rate_orig,
ctrl->panel->dp_mode.out_fmt_is_yuv_420);
dp_ctrl_setup_tr_unit(ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index e843f5062d1f..9afd99e00b0c 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -40,7 +40,6 @@ struct dp_panel {
u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
struct dp_link_info link_info;
- struct drm_dp_desc desc;
struct edid *edid;
struct drm_connector *connector;
struct dp_display_mode dp_mode;
--
2.43.0
[PATCH v2 6/6] drm/msm/dp: Use function arguments for audio operations
From: Bjorn Andersson
The dp_audio read and write operations uses members in struct dp_catalog
for passing arguments and return values. This adds unnecessary
complexity to the implementation, as it turns out after detangling the
logic that no state is actually held in these variables.
Clean this up by using function arguments and return values for passing
the data.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_audio.c | 20 +--
drivers/gpu/drm/msm/dp/dp_catalog.c | 39 +
drivers/gpu/drm/msm/dp/dp_catalog.h | 18 +
3 files changed, 28 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 7fd0c1793ba3..a599fc5d63c5 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -32,11 +32,7 @@ static u32 dp_audio_get_header(struct dp_catalog *catalog,
enum dp_catalog_audio_sdp_type sdp,
enum dp_catalog_audio_header_type header)
{
- catalog->sdp_type = sdp;
- catalog->sdp_header = header;
- dp_catalog_audio_get_header(catalog);
-
- return catalog->audio_data;
+ return dp_catalog_audio_get_header(catalog, sdp, header);
}
static void dp_audio_set_header(struct dp_catalog *catalog,
@@ -44,10 +40,7 @@ static void dp_audio_set_header(struct dp_catalog *catalog,
enum dp_catalog_audio_sdp_type sdp,
enum dp_catalog_audio_header_type header)
{
- catalog->sdp_type = sdp;
- catalog->sdp_header = header;
- catalog->audio_data = data;
- dp_catalog_audio_set_header(catalog);
+ dp_catalog_audio_set_header(catalog, sdp, header, data);
}
static void dp_audio_stream_sdp(struct dp_audio_private *audio)
@@ -317,8 +310,7 @@ static void dp_audio_setup_acr(struct dp_audio_private
*audio)
break;
}
- catalog->audio_data = select;
- dp_catalog_audio_config_acr(catalog);
+ dp_catalog_audio_config_acr(catalog, select);
}
static void dp_audio_safe_to_exit_level(struct dp_audio_private *audio)
@@ -344,16 +336,14 @@ static void dp_audio_safe_to_exit_level(struct
dp_audio_private *audio)
break;
}
- catalog->audio_data = safe_to_exit_level;
- dp_catalog_audio_sfe_level(catalog);
+ dp_catalog_audio_sfe_level(catalog, safe_to_exit_level);
}
static void dp_audio_enable(struct dp_audio_private *audio, bool enable)
{
struct dp_catalog *catalog = audio->catalog;
- catalog->audio_data = enable;
- dp_catalog_audio_enable(catalog);
+ dp_catalog_audio_enable(catalog, enable);
}
static struct dp_audio_private *dp_audio_get_data(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 00ad3ebaa5a1..970d62e1610c 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -1159,34 +1159,28 @@ struct dp_catalog *dp_catalog_get(struct device *dev)
return >dp_catalog;
}
-void dp_catalog_audio_get_header(struct dp_catalog *dp_catalog)
+u32 dp_catalog_audio_get_header(struct dp_catalog *dp_catalog,
+ enum dp_catalog_audio_sdp_type sdp,
+ enum dp_catalog_audio_header_type header)
{
struct dp_catalog_private *catalog;
u32 (*sdp_map)[DP_AUDIO_SDP_HEADER_MAX];
- enum dp_catalog_audio_sdp_type sdp;
- enum dp_catalog_audio_header_type header;
-
- if (!dp_catalog)
- return;
catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
sdp_map = catalog->audio_map;
- sdp = dp_catalog->sdp_type;
- header = dp_catalog->sdp_header;
- dp_catalog->audio_data = dp_read_link(catalog,
- sdp_map[sdp][header]);
+ return dp_read_link(catalog, sdp_map[sdp][header]);
}
-void dp_catalog_audio_set_header(struct dp_catalog *dp_catalog)
+void dp_catalog_audio_set_header(struct dp_catalog *dp_catalog,
+enum dp_catalog_audio_sdp_type sdp,
+enum dp_catalog_audio_header_type header,
+u32 data)
{
struct dp_catalog_private *catalog;
u32 (*sdp_map)[DP_AUDIO_SDP_HEADER_MAX];
- enum dp_catalog_audio_sdp_type sdp;
- enum dp_catalog_audio_header_type header;
- u32 data;
if (!dp_catalog)
return;
@@ -1195,17 +1189,14 @@ void dp_catalog_audio_set_header(struct dp_catalog
*dp_catalog)
struct dp_catalog_private, dp_catalog);
sdp_map = catalog->audio_map;
- sdp = dp_catalog->sdp_type;
- header = dp_catalog->sdp_header;
- data= dp_catalog->audio_d
[PATCH v2 4/6] drm/msm/dp: Use function arguments for aux writes
From: Bjorn Andersson
The dp_aux write operations takes the data to be operated on through a
member of struct dp_catalog, rather than as an argument to the function.
No state is maintained other than across the calling of the functions,
so replace this member with a function argument.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_aux.c | 9 +++--
drivers/gpu/drm/msm/dp/dp_catalog.c | 8
drivers/gpu/drm/msm/dp/dp_catalog.h | 5 ++---
3 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
index adbd5a367395..2c8bcc60692a 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -87,8 +87,7 @@ static ssize_t dp_aux_write(struct dp_aux_private *aux,
/* index = 0, write */
if (i == 0)
reg |= DP_AUX_DATA_INDEX_WRITE;
- aux->catalog->aux_data = reg;
- dp_catalog_aux_write_data(aux->catalog);
+ dp_catalog_aux_write_data(aux->catalog, reg);
}
dp_catalog_aux_clear_trans(aux->catalog, false);
@@ -106,8 +105,7 @@ static ssize_t dp_aux_write(struct dp_aux_private *aux,
}
reg |= DP_AUX_TRANS_CTRL_GO;
- aux->catalog->aux_data = reg;
- dp_catalog_aux_write_trans(aux->catalog);
+ dp_catalog_aux_write_trans(aux->catalog, reg);
return len;
}
@@ -145,8 +143,7 @@ static ssize_t dp_aux_cmd_fifo_rx(struct dp_aux_private
*aux,
data = DP_AUX_DATA_INDEX_WRITE; /* INDEX_WRITE */
data |= DP_AUX_DATA_READ; /* read */
- aux->catalog->aux_data = data;
- dp_catalog_aux_write_data(aux->catalog);
+ dp_catalog_aux_write_data(aux->catalog, data);
dp = msg->buffer;
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 55114a6aba7e..295bd4cb72cc 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -169,21 +169,21 @@ u32 dp_catalog_aux_read_data(struct dp_catalog
*dp_catalog)
return dp_read_aux(catalog, REG_DP_AUX_DATA);
}
-int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog)
+int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog, u32 data)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
- dp_write_aux(catalog, REG_DP_AUX_DATA, dp_catalog->aux_data);
+ dp_write_aux(catalog, REG_DP_AUX_DATA, data);
return 0;
}
-int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog)
+int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog, u32 data)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
- dp_write_aux(catalog, REG_DP_AUX_TRANS_CTRL, dp_catalog->aux_data);
+ dp_write_aux(catalog, REG_DP_AUX_TRANS_CTRL, data);
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 2c2dbeee7634..290ef8180c12 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -48,7 +48,6 @@ enum dp_catalog_audio_header_type {
};
struct dp_catalog {
- u32 aux_data;
u32 total;
u32 sync_start;
u32 width_blanking;
@@ -64,8 +63,8 @@ void dp_catalog_snapshot(struct dp_catalog *dp_catalog,
struct msm_disp_state *d
/* AUX APIs */
u32 dp_catalog_aux_read_data(struct dp_catalog *dp_catalog);
-int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog);
-int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog);
+int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog, u32 data);
+int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog, u32 data);
int dp_catalog_aux_clear_trans(struct dp_catalog *dp_catalog, bool read);
int dp_catalog_aux_clear_hw_interrupts(struct dp_catalog *dp_catalog);
void dp_catalog_aux_reset(struct dp_catalog *dp_catalog);
--
2.43.0
[PATCH v2 5/6] drm/msm/dp: Use function arguments for timing configuration
From: Bjorn Andersson
dp_catalog_panel_timing_cfg() takes 4 arguments, which are passed from
the calling function through members of struct dp_catalog.
No state is maintained other than across this call, so switch to
function arguments to clean up the code.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 14 ++
drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++-
drivers/gpu/drm/msm/dp/dp_panel.c | 14 +-
3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 295bd4cb72cc..00ad3ebaa5a1 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -880,19 +880,17 @@ u32 dp_catalog_ctrl_read_phy_pattern(struct dp_catalog
*dp_catalog)
}
/* panel related catalog functions */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, u32 total,
+ u32 sync_start, u32 width_blanking, u32
dp_active)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
u32 reg;
- dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
- dp_catalog->total);
- dp_write_link(catalog, REG_DP_START_HOR_VER_FROM_SYNC,
- dp_catalog->sync_start);
- dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
- dp_catalog->width_blanking);
- dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER, dp_catalog->dp_active);
+ dp_write_link(catalog, REG_DP_TOTAL_HOR_VER, total);
+ dp_write_link(catalog, REG_DP_START_HOR_VER_FROM_SYNC, sync_start);
+ dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
width_blanking);
+ dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER, dp_active);
reg = dp_read_p0(catalog, MMSS_DP_INTF_CONFIG);
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 290ef8180c12..a82ab4856b50 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -48,10 +48,6 @@ enum dp_catalog_audio_header_type {
};
struct dp_catalog {
- u32 total;
- u32 sync_start;
- u32 width_blanking;
- u32 dp_active;
enum dp_catalog_audio_sdp_type sdp_type;
enum dp_catalog_audio_header_type sdp_header;
u32 audio_data;
@@ -106,7 +102,8 @@ void dp_catalog_ctrl_send_phy_pattern(struct dp_catalog
*dp_catalog,
u32 dp_catalog_ctrl_read_phy_pattern(struct dp_catalog *dp_catalog);
/* DP Panel APIs */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog);
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, u32 total,
+ u32 sync_start, u32 width_blanking, u32
dp_active);
void dp_catalog_panel_enable_vsc_sdp(struct dp_catalog *dp_catalog, struct
dp_sdp *vsc_sdp);
void dp_catalog_panel_disable_vsc_sdp(struct dp_catalog *dp_catalog);
void dp_catalog_dump_regs(struct dp_catalog *dp_catalog);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 8e7069453952..07db8f37cd06 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -353,6 +353,10 @@ int dp_panel_timing_cfg(struct dp_panel *dp_panel)
struct dp_catalog *catalog;
struct dp_panel_private *panel;
struct drm_display_mode *drm_mode;
+ u32 width_blanking;
+ u32 sync_start;
+ u32 dp_active;
+ u32 total;
panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
catalog = panel->catalog;
@@ -376,13 +380,13 @@ int dp_panel_timing_cfg(struct dp_panel *dp_panel)
data <<= 16;
data |= total_hor;
- catalog->total = data;
+ total = data;
data = (drm_mode->vtotal - drm_mode->vsync_start);
data <<= 16;
data |= (drm_mode->htotal - drm_mode->hsync_start);
- catalog->sync_start = data;
+ sync_start = data;
data = drm_mode->vsync_end - drm_mode->vsync_start;
data <<= 16;
@@ -390,15 +394,15 @@ int dp_panel_timing_cfg(struct dp_panel *dp_panel)
data |= drm_mode->hsync_end - drm_mode->hsync_start;
data |= (panel->dp_panel.dp_mode.h_active_low << 15);
- catalog->width_blanking = data;
+ width_blanking = data;
data = drm_mode->vdisplay;
data <<= 16;
data |= drm_mode->hdisplay;
- catalog->dp_active = data;
+ dp_active = data;
- dp_catalog_panel_timing_cfg(catalog);
+ dp_catalog_panel_timing_cfg(catalog, total, sync_start, width_blanking,
dp_active);
if (dp_panel->dp_mode.out_fmt_is_yuv_420)
dp_panel_setup_vsc_sdp_yuv_420(dp_panel);
--
2.43.0
[PATCH v2 1/6] drm/msm/dp: Drop unused dp_debug struct
From: Bjorn Andersson
The members of struct dp_debug are no longer used, so the only purpose
of this struct is as a type of the return value of dp_debug_get(), to
signal success/error.
Drop the struct in favor of signalling the result of initialization
using an int, then merge dp_debug_get() with dp_debug_init() to avoid
the unnecessar boilerplate code.
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_debug.c | 59 +++--
drivers/gpu/drm/msm/dp/dp_debug.h | 38 +++-
drivers/gpu/drm/msm/dp/dp_display.c | 10 ++-
3 files changed, 31 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
b/drivers/gpu/drm/msm/dp/dp_debug.c
index eca5a02f9003..b8611f6d2296 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -21,8 +21,6 @@ struct dp_debug_private {
struct dp_link *link;
struct dp_panel *panel;
struct drm_connector *connector;
-
- struct dp_debug dp_debug;
};
static int dp_debug_show(struct seq_file *seq, void *p)
@@ -199,10 +197,24 @@ static const struct file_operations test_active_fops = {
.write = dp_test_active_write
};
-static void dp_debug_init(struct dp_debug *dp_debug, struct dentry *root, bool
is_edp)
+int dp_debug_init(struct device *dev, struct dp_panel *panel,
+ struct dp_link *link,
+ struct drm_connector *connector,
+ struct dentry *root, bool is_edp)
{
- struct dp_debug_private *debug = container_of(dp_debug,
- struct dp_debug_private, dp_debug);
+ struct dp_debug_private *debug;
+
+ if (!dev || !panel || !link) {
+ DRM_ERROR("invalid input\n");
+ return -EINVAL;
+ }
+
+ debug = devm_kzalloc(dev, sizeof(*debug), GFP_KERNEL);
+ if (!debug)
+ return -ENOMEM;
+
+ debug->link = link;
+ debug->panel = panel;
debugfs_create_file("dp_debug", 0444, root,
debug, _debug_fops);
@@ -220,41 +232,6 @@ static void dp_debug_init(struct dp_debug *dp_debug,
struct dentry *root, bool i
root,
debug, _test_type_fops);
}
-}
-struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
- struct dp_link *link,
- struct drm_connector *connector,
- struct dentry *root, bool is_edp)
-{
- struct dp_debug_private *debug;
- struct dp_debug *dp_debug;
- int rc;
-
- if (!dev || !panel || !link) {
- DRM_ERROR("invalid input\n");
- rc = -EINVAL;
- goto error;
- }
-
- debug = devm_kzalloc(dev, sizeof(*debug), GFP_KERNEL);
- if (!debug) {
- rc = -ENOMEM;
- goto error;
- }
-
- debug->dp_debug.debug_en = false;
- debug->link = link;
- debug->panel = panel;
-
- dp_debug = >dp_debug;
- dp_debug->vdisplay = 0;
- dp_debug->hdisplay = 0;
- dp_debug->vrefresh = 0;
-
- dp_debug_init(dp_debug, root, is_edp);
-
- return dp_debug;
- error:
- return ERR_PTR(rc);
+ return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.h
b/drivers/gpu/drm/msm/dp/dp_debug.h
index 9b3b2e702f65..7e1aa892fc09 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.h
+++ b/drivers/gpu/drm/msm/dp/dp_debug.h
@@ -9,22 +9,6 @@
#include "dp_panel.h"
#include "dp_link.h"
-/**
- * struct dp_debug
- * @debug_en: specifies whether debug mode enabled
- * @vdisplay: used to filter out vdisplay value
- * @hdisplay: used to filter out hdisplay value
- * @vrefresh: used to filter out vrefresh value
- * @tpg_state: specifies whether tpg feature is enabled
- */
-struct dp_debug {
- bool debug_en;
- int aspect_ratio;
- int vdisplay;
- int hdisplay;
- int vrefresh;
-};
-
#if defined(CONFIG_DEBUG_FS)
/**
@@ -41,22 +25,22 @@ struct dp_debug {
* This function sets up the debug module and provides a way
* for debugfs input to be communicated with existing modules
*/
-struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
- struct dp_link *link,
- struct drm_connector *connector,
- struct dentry *root,
- bool is_edp);
+int dp_debug_init(struct device *dev, struct dp_panel *panel,
+ struct dp_link *link,
+ struct drm_connector *connector,
+ struct dentry *root,
+ bool is_edp);
#else
static inline
-struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
- struct dp_link *link,
- struct drm_connector *connector,
- struct dentry *root,
- bool is_edp)
+int dp_debug_init(struc
[PATCH v2 0/6] drm/msm/dp: Spring cleaning
Spring is in the air, clean out some dust that has accumulated in the Qualcomm DP driver. Signed-off-by: Bjorn Andersson --- Changes in v2: - Merge dp_debug_get() and dp_debug_init() - Link to v1: https://lore.kernel.org/r/20240326-msm-dp-cleanup-v1-0-e775556ec...@quicinc.com --- Bjorn Andersson (6): drm/msm/dp: Drop unused dp_debug struct drm/msm/dp: Removed fixed nvid "support" drm/msm/dp: Remove unused defines and members drm/msm/dp: Use function arguments for aux writes drm/msm/dp: Use function arguments for timing configuration drm/msm/dp: Use function arguments for audio operations drivers/gpu/drm/msm/dp/dp_audio.c | 25 +++ drivers/gpu/drm/msm/dp/dp_aux.c | 9 ++ drivers/gpu/drm/msm/dp/dp_catalog.c | 64 ++--- drivers/gpu/drm/msm/dp/dp_catalog.h | 49 +--- drivers/gpu/drm/msm/dp/dp_ctrl.c| 17 +- drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 - drivers/gpu/drm/msm/dp/dp_debug.c | 59 +++--- drivers/gpu/drm/msm/dp/dp_debug.h | 38 +++--- drivers/gpu/drm/msm/dp/dp_display.c | 15 ++--- drivers/gpu/drm/msm/dp/dp_display.h | 3 -- drivers/gpu/drm/msm/dp/dp_drm.c | 2 -- drivers/gpu/drm/msm/dp/dp_link.c| 4 --- drivers/gpu/drm/msm/dp/dp_link.h| 1 - drivers/gpu/drm/msm/dp/dp_panel.c | 14 +--- drivers/gpu/drm/msm/dp/dp_panel.h | 3 -- 15 files changed, 88 insertions(+), 216 deletions(-) --- base-commit: 084c8e315db34b59d38d06e684b1a0dd07d30287 change-id: 20240323-msm-dp-cleanup-f8ba32f62fb9 Best regards, -- Bjorn Andersson
Re: [PATCH v4 2/2] drm/msm/dp: Add support for the X1E80100
On Sun, Mar 24, 2024 at 08:56:52PM +0200, Abel Vesa wrote:
> Add the X1E80100 DP descs and compatible. This platform will be using
> a single compatible for both eDP and DP mode. The actual mode will
> be set based on the presence of the panel node in DT.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Abel Vesa
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 9169a739cc54..521cba76d2a0 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -171,6 +171,14 @@ static const struct msm_dp_desc sm8650_dp_descs[] = {
> {}
> };
>
> +static const struct msm_dp_desc x1e80100_dp_descs[] = {
> + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0,
> .wide_bus_supported = true },
> + { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1,
> .wide_bus_supported = true },
> + { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2,
> .wide_bus_supported = true },
> + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3,
> .wide_bus_supported = true },
> + {}
> +};
> +
> static const struct of_device_id dp_dt_match[] = {
> { .compatible = "qcom,sc7180-dp", .data = _dp_descs },
> { .compatible = "qcom,sc7280-dp", .data = _dp_descs },
> @@ -182,6 +190,7 @@ static const struct of_device_id dp_dt_match[] = {
> { .compatible = "qcom,sdm845-dp", .data = _dp_descs },
> { .compatible = "qcom,sm8350-dp", .data = _dp_descs },
> { .compatible = "qcom,sm8650-dp", .data = _dp_descs },
> + { .compatible = "qcom,x1e80100-dp", .data = _dp_descs },
> {}
> };
>
>
> --
> 2.34.1
>
Re: [PATCH v4 1/2] drm/msm/dp: Add support for determining the eDP/DP mode from DT
On Sun, Mar 24, 2024 at 08:56:51PM +0200, Abel Vesa wrote:
> Instead of relying on different compatibles for eDP and DP, lookup
> the panel node in devicetree to figure out the connector type and
> then pass on that information to the PHY. External DP doesn't have
> a panel described in DT, therefore, assume it's eDP if panel node
> is present.
>
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> Signed-off-by: Abel Vesa
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 29 -
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index c4cb82af5c2f..9169a739cc54 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -726,6 +726,14 @@ static int dp_init_sub_modules(struct dp_display_private
> *dp)
> if (IS_ERR(phy))
> return PTR_ERR(phy);
>
> + rc = phy_set_mode_ext(phy, PHY_MODE_DP,
> + dp->dp_display.is_edp ? PHY_SUBMODE_EDP :
> PHY_SUBMODE_DP);
> + if (rc) {
> + DRM_ERROR("failed to set phy submode, rc = %d\n", rc);
> + dp->catalog = NULL;
> + goto error;
> + }
> +
> dp->catalog = dp_catalog_get(dev);
> if (IS_ERR(dp->catalog)) {
> rc = PTR_ERR(dp->catalog);
> @@ -1241,6 +1249,25 @@ static int dp_auxbus_done_probe(struct drm_dp_aux *aux)
> return dp_display_probe_tail(aux->dev);
> }
>
> +static int dp_display_get_connector_type(struct platform_device *pdev,
> + const struct msm_dp_desc *desc)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct device_node *aux_bus = of_get_child_by_name(node, "aux-bus");
> + struct device_node *panel = of_get_child_by_name(aux_bus, "panel");
> + int connector_type;
> +
> + if (panel)
> + connector_type = DRM_MODE_CONNECTOR_eDP;
> + else
> + connector_type = DRM_MODE_SUBCONNECTOR_DisplayPort;
> +
> + of_node_put(panel);
> + of_node_put(aux_bus);
> +
> + return connector_type;
> +}
> +
> static int dp_display_probe(struct platform_device *pdev)
> {
> int rc = 0;
> @@ -1263,7 +1290,7 @@ static int dp_display_probe(struct platform_device
> *pdev)
> dp->dp_display.pdev = pdev;
> dp->name = "drm_dp";
> dp->id = desc->id;
> - dp->dp_display.connector_type = desc->connector_type;
> + dp->dp_display.connector_type = dp_display_get_connector_type(pdev,
> desc);
> dp->wide_bus_supported = desc->wide_bus_supported;
> dp->dp_display.is_edp =
> (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
>
> --
> 2.34.1
>
[PATCH 2/6] drm/msm/dp: Removed fixed nvid "support"
From: Bjorn Andersson
The "desc" member of struct dp_panel is zero-initialized during
allocation and never assigned, resulting in dp_ctrl_use_fixed_nvid()
never returning true. This returned boolean value is passed around but
never acted upon.
Perform constant propagation and remove the traces of "fixed nvid".
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 2 +-
drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +-
drivers/gpu/drm/msm/dp/dp_ctrl.c| 17 +
drivers/gpu/drm/msm/dp/dp_panel.h | 1 -
4 files changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 3e7c84cdef47..8c72d532d96b 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -469,7 +469,7 @@ void dp_catalog_setup_peripheral_flush(struct dp_catalog
*dp_catalog)
void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog,
u32 rate, u32 stream_rate_khz,
- bool fixed_nvid, bool is_ycbcr_420)
+ bool is_ycbcr_420)
{
u32 pixel_m, pixel_n;
u32 mvid, nvid, pixel_div = 0, dispcc_input_rate;
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 75ec290127c7..b85ad6bdb2e7 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -99,7 +99,7 @@ void dp_catalog_ctrl_psr_mainlink_enable(struct dp_catalog
*dp_catalog, bool ena
void dp_catalog_setup_peripheral_flush(struct dp_catalog *dp_catalog);
void dp_catalog_ctrl_config_misc(struct dp_catalog *dp_catalog, u32 cc, u32
tb);
void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog, u32 rate,
- u32 stream_rate_khz, bool fixed_nvid, bool
is_ycbcr_420);
+ u32 stream_rate_khz, bool is_ycbcr_420);
int dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog *dp_catalog, u32
pattern);
u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog);
void dp_catalog_ctrl_reset(struct dp_catalog *dp_catalog);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index c4dda1faef67..e65a460fb52d 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1566,21 +1566,6 @@ void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl)
phy, phy->init_count, phy->power_count);
}
-static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl)
-{
- const u8 *dpcd = ctrl->panel->dpcd;
-
- /*
-* For better interop experience, used a fixed NVID=0x8000
-* whenever connected to a VGA dongle downstream.
-*/
- if (drm_dp_is_branch(dpcd))
- return (drm_dp_has_quirk(>panel->desc,
-DP_DPCD_QUIRK_CONSTANT_N));
-
- return false;
-}
-
static int dp_ctrl_reinitialize_mainlink(struct dp_ctrl_private *ctrl)
{
struct phy *phy = ctrl->phy;
@@ -2022,7 +2007,7 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool
force_link_train)
dp_catalog_ctrl_config_msa(ctrl->catalog,
ctrl->link->link_params.rate,
- pixel_rate_orig, dp_ctrl_use_fixed_nvid(ctrl),
+ pixel_rate_orig,
ctrl->panel->dp_mode.out_fmt_is_yuv_420);
dp_ctrl_setup_tr_unit(ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index e843f5062d1f..9afd99e00b0c 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -40,7 +40,6 @@ struct dp_panel {
u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
struct dp_link_info link_info;
- struct drm_dp_desc desc;
struct edid *edid;
struct drm_connector *connector;
struct dp_display_mode dp_mode;
--
2.43.0
[PATCH 6/6] drm/msm/dp: Use function arguments for audio operations
From: Bjorn Andersson
The dp_audio read and write operations uses members in struct dp_catalog
for passing arguments and return values. This adds unnecessary
complexity to the implementation, as it turns out after detangling the
logic that no state is actually held in these variables.
Clean this up by using function arguments and return values for passing
the data.
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_audio.c | 20 +--
drivers/gpu/drm/msm/dp/dp_catalog.c | 39 +
drivers/gpu/drm/msm/dp/dp_catalog.h | 18 +
3 files changed, 28 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 7fd0c1793ba3..a599fc5d63c5 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -32,11 +32,7 @@ static u32 dp_audio_get_header(struct dp_catalog *catalog,
enum dp_catalog_audio_sdp_type sdp,
enum dp_catalog_audio_header_type header)
{
- catalog->sdp_type = sdp;
- catalog->sdp_header = header;
- dp_catalog_audio_get_header(catalog);
-
- return catalog->audio_data;
+ return dp_catalog_audio_get_header(catalog, sdp, header);
}
static void dp_audio_set_header(struct dp_catalog *catalog,
@@ -44,10 +40,7 @@ static void dp_audio_set_header(struct dp_catalog *catalog,
enum dp_catalog_audio_sdp_type sdp,
enum dp_catalog_audio_header_type header)
{
- catalog->sdp_type = sdp;
- catalog->sdp_header = header;
- catalog->audio_data = data;
- dp_catalog_audio_set_header(catalog);
+ dp_catalog_audio_set_header(catalog, sdp, header, data);
}
static void dp_audio_stream_sdp(struct dp_audio_private *audio)
@@ -317,8 +310,7 @@ static void dp_audio_setup_acr(struct dp_audio_private
*audio)
break;
}
- catalog->audio_data = select;
- dp_catalog_audio_config_acr(catalog);
+ dp_catalog_audio_config_acr(catalog, select);
}
static void dp_audio_safe_to_exit_level(struct dp_audio_private *audio)
@@ -344,16 +336,14 @@ static void dp_audio_safe_to_exit_level(struct
dp_audio_private *audio)
break;
}
- catalog->audio_data = safe_to_exit_level;
- dp_catalog_audio_sfe_level(catalog);
+ dp_catalog_audio_sfe_level(catalog, safe_to_exit_level);
}
static void dp_audio_enable(struct dp_audio_private *audio, bool enable)
{
struct dp_catalog *catalog = audio->catalog;
- catalog->audio_data = enable;
- dp_catalog_audio_enable(catalog);
+ dp_catalog_audio_enable(catalog, enable);
}
static struct dp_audio_private *dp_audio_get_data(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 00ad3ebaa5a1..970d62e1610c 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -1159,34 +1159,28 @@ struct dp_catalog *dp_catalog_get(struct device *dev)
return >dp_catalog;
}
-void dp_catalog_audio_get_header(struct dp_catalog *dp_catalog)
+u32 dp_catalog_audio_get_header(struct dp_catalog *dp_catalog,
+ enum dp_catalog_audio_sdp_type sdp,
+ enum dp_catalog_audio_header_type header)
{
struct dp_catalog_private *catalog;
u32 (*sdp_map)[DP_AUDIO_SDP_HEADER_MAX];
- enum dp_catalog_audio_sdp_type sdp;
- enum dp_catalog_audio_header_type header;
-
- if (!dp_catalog)
- return;
catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
sdp_map = catalog->audio_map;
- sdp = dp_catalog->sdp_type;
- header = dp_catalog->sdp_header;
- dp_catalog->audio_data = dp_read_link(catalog,
- sdp_map[sdp][header]);
+ return dp_read_link(catalog, sdp_map[sdp][header]);
}
-void dp_catalog_audio_set_header(struct dp_catalog *dp_catalog)
+void dp_catalog_audio_set_header(struct dp_catalog *dp_catalog,
+enum dp_catalog_audio_sdp_type sdp,
+enum dp_catalog_audio_header_type header,
+u32 data)
{
struct dp_catalog_private *catalog;
u32 (*sdp_map)[DP_AUDIO_SDP_HEADER_MAX];
- enum dp_catalog_audio_sdp_type sdp;
- enum dp_catalog_audio_header_type header;
- u32 data;
if (!dp_catalog)
return;
@@ -1195,17 +1189,14 @@ void dp_catalog_audio_set_header(struct dp_catalog
*dp_catalog)
struct dp_catalog_private, dp_catalog);
sdp_map = catalog->audio_map;
- sdp = dp_catalog->sdp_type;
- header = dp_catalog->sdp_header;
- data= dp_catalog->audio_data;
dp_wr
[PATCH 1/6] drm/msm/dp: Drop unused dp_debug struct
From: Bjorn Andersson
The members of struct dp_debug are no longer used, so the only purpose
of this struct is as a type of the return value of dp_debug_get(), to
signal success/error.
Drop the struct in favor of signalling the result of initialization
using an int.
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_debug.c | 38 ++---
drivers/gpu/drm/msm/dp/dp_debug.h | 38 +++--
drivers/gpu/drm/msm/dp/dp_display.c | 10 ++
3 files changed, 23 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
b/drivers/gpu/drm/msm/dp/dp_debug.c
index eca5a02f9003..a631cbe0e599 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -21,8 +21,6 @@ struct dp_debug_private {
struct dp_link *link;
struct dp_panel *panel;
struct drm_connector *connector;
-
- struct dp_debug dp_debug;
};
static int dp_debug_show(struct seq_file *seq, void *p)
@@ -199,11 +197,8 @@ static const struct file_operations test_active_fops = {
.write = dp_test_active_write
};
-static void dp_debug_init(struct dp_debug *dp_debug, struct dentry *root, bool
is_edp)
+static void dp_debug_init(struct dp_debug_private *debug, struct dentry *root,
bool is_edp)
{
- struct dp_debug_private *debug = container_of(dp_debug,
- struct dp_debug_private, dp_debug);
-
debugfs_create_file("dp_debug", 0444, root,
debug, _debug_fops);
@@ -222,39 +217,26 @@ static void dp_debug_init(struct dp_debug *dp_debug,
struct dentry *root, bool i
}
}
-struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
- struct dp_link *link,
- struct drm_connector *connector,
- struct dentry *root, bool is_edp)
+int dp_debug_get(struct device *dev, struct dp_panel *panel,
+struct dp_link *link,
+struct drm_connector *connector,
+struct dentry *root, bool is_edp)
{
struct dp_debug_private *debug;
- struct dp_debug *dp_debug;
- int rc;
if (!dev || !panel || !link) {
DRM_ERROR("invalid input\n");
- rc = -EINVAL;
- goto error;
+ return -EINVAL;
}
debug = devm_kzalloc(dev, sizeof(*debug), GFP_KERNEL);
- if (!debug) {
- rc = -ENOMEM;
- goto error;
- }
+ if (!debug)
+ return -ENOMEM;
- debug->dp_debug.debug_en = false;
debug->link = link;
debug->panel = panel;
- dp_debug = >dp_debug;
- dp_debug->vdisplay = 0;
- dp_debug->hdisplay = 0;
- dp_debug->vrefresh = 0;
-
- dp_debug_init(dp_debug, root, is_edp);
+ dp_debug_init(debug, root, is_edp);
- return dp_debug;
- error:
- return ERR_PTR(rc);
+ return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.h
b/drivers/gpu/drm/msm/dp/dp_debug.h
index 9b3b2e702f65..c57200751c9f 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.h
+++ b/drivers/gpu/drm/msm/dp/dp_debug.h
@@ -9,22 +9,6 @@
#include "dp_panel.h"
#include "dp_link.h"
-/**
- * struct dp_debug
- * @debug_en: specifies whether debug mode enabled
- * @vdisplay: used to filter out vdisplay value
- * @hdisplay: used to filter out hdisplay value
- * @vrefresh: used to filter out vrefresh value
- * @tpg_state: specifies whether tpg feature is enabled
- */
-struct dp_debug {
- bool debug_en;
- int aspect_ratio;
- int vdisplay;
- int hdisplay;
- int vrefresh;
-};
-
#if defined(CONFIG_DEBUG_FS)
/**
@@ -41,22 +25,22 @@ struct dp_debug {
* This function sets up the debug module and provides a way
* for debugfs input to be communicated with existing modules
*/
-struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
- struct dp_link *link,
- struct drm_connector *connector,
- struct dentry *root,
- bool is_edp);
+int dp_debug_get(struct device *dev, struct dp_panel *panel,
+struct dp_link *link,
+struct drm_connector *connector,
+struct dentry *root,
+bool is_edp);
#else
static inline
-struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
- struct dp_link *link,
- struct drm_connector *connector,
- struct dentry *root,
- bool is_edp)
+int dp_debug_get(struct device *dev, struct dp_panel *panel,
+struct dp_link *link,
+struct drm_connector *connector,
+struct dentry *root,
+bool is_edp)
{
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
#endif /* defined(CONFIG_DEBUG_FS) */
diff --git a/drivers/gpu/d
[PATCH 3/6] drm/msm/dp: Remove unused defines and members
From: Bjorn Andersson
Throughout the Qualcomm Displayport driver a number of defines and
struct members has become unused, but lingers in the code. Remove these.
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_audio.c | 5 -
drivers/gpu/drm/msm/dp/dp_catalog.c | 1 -
drivers/gpu/drm/msm/dp/dp_catalog.h | 17 -
drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 -
drivers/gpu/drm/msm/dp/dp_display.c | 5 -
drivers/gpu/drm/msm/dp/dp_display.h | 3 ---
drivers/gpu/drm/msm/dp/dp_drm.c | 2 --
drivers/gpu/drm/msm/dp/dp_link.c| 4
drivers/gpu/drm/msm/dp/dp_link.h| 1 -
drivers/gpu/drm/msm/dp/dp_panel.h | 2 --
10 files changed, 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 7634e4b74208..7fd0c1793ba3 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -22,9 +22,7 @@ struct dp_audio_private {
struct platform_device *pdev;
struct drm_device *drm_dev;
struct dp_catalog *catalog;
- struct dp_panel *panel;
- bool engine_on;
u32 channels;
struct dp_audio dp_audio;
@@ -356,8 +354,6 @@ static void dp_audio_enable(struct dp_audio_private *audio,
bool enable)
catalog->audio_data = enable;
dp_catalog_audio_enable(catalog);
-
- audio->engine_on = enable;
}
static struct dp_audio_private *dp_audio_get_data(struct platform_device *pdev)
@@ -571,7 +567,6 @@ struct dp_audio *dp_audio_get(struct platform_device *pdev,
}
audio->pdev = pdev;
- audio->panel = panel;
audio->catalog = catalog;
dp_audio = >dp_audio;
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 8c72d532d96b..55114a6aba7e 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -81,7 +81,6 @@ struct dp_catalog_private {
struct dss_io_data io;
u32 (*audio_map)[DP_AUDIO_SDP_HEADER_MAX];
struct dp_catalog dp_catalog;
- u8 aux_lut_cfg_index[PHY_AUX_CFG_MAX];
};
void dp_catalog_snapshot(struct dp_catalog *dp_catalog, struct msm_disp_state
*disp_state)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index b85ad6bdb2e7..2c2dbeee7634 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -28,26 +28,9 @@
#define DP_INTR_FRAME_END BIT(6)
#define DP_INTR_CRC_UPDATEDBIT(9)
-#define DP_AUX_CFG_MAX_VALUE_CNT 3
-
#define DP_HW_VERSION_1_0 0x1000
#define DP_HW_VERSION_1_2 0x1002
-/* PHY AUX config registers */
-enum dp_phy_aux_config_type {
- PHY_AUX_CFG0,
- PHY_AUX_CFG1,
- PHY_AUX_CFG2,
- PHY_AUX_CFG3,
- PHY_AUX_CFG4,
- PHY_AUX_CFG5,
- PHY_AUX_CFG6,
- PHY_AUX_CFG7,
- PHY_AUX_CFG8,
- PHY_AUX_CFG9,
- PHY_AUX_CFG_MAX,
-};
-
enum dp_catalog_audio_sdp_type {
DP_AUDIO_SDP_STREAM,
DP_AUDIO_SDP_TIMESTAMP,
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index fa014cee7e21..ffcbd9a25748 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -12,7 +12,6 @@
#include "dp_catalog.h"
struct dp_ctrl {
- atomic_t aborted;
bool wide_bus_en;
};
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index a9187be95166..f7692c20bb04 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -74,7 +74,6 @@ struct dp_event {
};
struct dp_display_private {
- char *name;
int irq;
unsigned int id;
@@ -82,11 +81,9 @@ struct dp_display_private {
/* state variables */
bool core_initialized;
bool phy_initialized;
- bool hpd_irq_on;
bool audio_supported;
struct drm_device *drm_dev;
- struct dentry *root;
struct dp_catalog *catalog;
struct drm_dp_aux *aux;
@@ -800,7 +797,6 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
drm_mode_copy(>panel->dp_mode.drm_mode, >drm_mode);
dp->panel->dp_mode.bpp = mode->bpp;
- dp->panel->dp_mode.capabilities = mode->capabilities;
dp->panel->dp_mode.out_fmt_is_yuv_420 = mode->out_fmt_is_yuv_420;
dp_panel_init_panel_info(dp->panel);
return 0;
@@ -1260,7 +1256,6 @@ static int dp_display_probe(struct platform_device *pdev)
return -EINVAL;
dp->dp_display.pdev = pdev;
- dp->name = "drm_dp";
dp->id = desc->id;
dp->dp_display.connector_type = desc->connector_type;
dp->wide_bus_supported = desc->wide_bus_supported;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 234
[PATCH 4/6] drm/msm/dp: Use function arguments for aux writes
From: Bjorn Andersson
The dp_aux write operations takes the data to be operated on through a
member of struct dp_catalog, rather than as an argument to the function.
No state is maintained other than across the calling of the functions,
so replace this member with a function argument.
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_aux.c | 9 +++--
drivers/gpu/drm/msm/dp/dp_catalog.c | 8
drivers/gpu/drm/msm/dp/dp_catalog.h | 5 ++---
3 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
index adbd5a367395..2c8bcc60692a 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -87,8 +87,7 @@ static ssize_t dp_aux_write(struct dp_aux_private *aux,
/* index = 0, write */
if (i == 0)
reg |= DP_AUX_DATA_INDEX_WRITE;
- aux->catalog->aux_data = reg;
- dp_catalog_aux_write_data(aux->catalog);
+ dp_catalog_aux_write_data(aux->catalog, reg);
}
dp_catalog_aux_clear_trans(aux->catalog, false);
@@ -106,8 +105,7 @@ static ssize_t dp_aux_write(struct dp_aux_private *aux,
}
reg |= DP_AUX_TRANS_CTRL_GO;
- aux->catalog->aux_data = reg;
- dp_catalog_aux_write_trans(aux->catalog);
+ dp_catalog_aux_write_trans(aux->catalog, reg);
return len;
}
@@ -145,8 +143,7 @@ static ssize_t dp_aux_cmd_fifo_rx(struct dp_aux_private
*aux,
data = DP_AUX_DATA_INDEX_WRITE; /* INDEX_WRITE */
data |= DP_AUX_DATA_READ; /* read */
- aux->catalog->aux_data = data;
- dp_catalog_aux_write_data(aux->catalog);
+ dp_catalog_aux_write_data(aux->catalog, data);
dp = msg->buffer;
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 55114a6aba7e..295bd4cb72cc 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -169,21 +169,21 @@ u32 dp_catalog_aux_read_data(struct dp_catalog
*dp_catalog)
return dp_read_aux(catalog, REG_DP_AUX_DATA);
}
-int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog)
+int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog, u32 data)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
- dp_write_aux(catalog, REG_DP_AUX_DATA, dp_catalog->aux_data);
+ dp_write_aux(catalog, REG_DP_AUX_DATA, data);
return 0;
}
-int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog)
+int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog, u32 data)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
- dp_write_aux(catalog, REG_DP_AUX_TRANS_CTRL, dp_catalog->aux_data);
+ dp_write_aux(catalog, REG_DP_AUX_TRANS_CTRL, data);
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 2c2dbeee7634..290ef8180c12 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -48,7 +48,6 @@ enum dp_catalog_audio_header_type {
};
struct dp_catalog {
- u32 aux_data;
u32 total;
u32 sync_start;
u32 width_blanking;
@@ -64,8 +63,8 @@ void dp_catalog_snapshot(struct dp_catalog *dp_catalog,
struct msm_disp_state *d
/* AUX APIs */
u32 dp_catalog_aux_read_data(struct dp_catalog *dp_catalog);
-int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog);
-int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog);
+int dp_catalog_aux_write_data(struct dp_catalog *dp_catalog, u32 data);
+int dp_catalog_aux_write_trans(struct dp_catalog *dp_catalog, u32 data);
int dp_catalog_aux_clear_trans(struct dp_catalog *dp_catalog, bool read);
int dp_catalog_aux_clear_hw_interrupts(struct dp_catalog *dp_catalog);
void dp_catalog_aux_reset(struct dp_catalog *dp_catalog);
--
2.43.0
[PATCH 5/6] drm/msm/dp: Use function arguments for timing configuration
From: Bjorn Andersson
dp_catalog_panel_timing_cfg() takes 4 arguments, which are passed from
the calling function through members of struct dp_catalog.
No state is maintained other than across this call, so switch to
function arguments to clean up the code.
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 14 ++
drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++-
drivers/gpu/drm/msm/dp/dp_panel.c | 14 +-
3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 295bd4cb72cc..00ad3ebaa5a1 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -880,19 +880,17 @@ u32 dp_catalog_ctrl_read_phy_pattern(struct dp_catalog
*dp_catalog)
}
/* panel related catalog functions */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, u32 total,
+ u32 sync_start, u32 width_blanking, u32
dp_active)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
u32 reg;
- dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
- dp_catalog->total);
- dp_write_link(catalog, REG_DP_START_HOR_VER_FROM_SYNC,
- dp_catalog->sync_start);
- dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
- dp_catalog->width_blanking);
- dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER, dp_catalog->dp_active);
+ dp_write_link(catalog, REG_DP_TOTAL_HOR_VER, total);
+ dp_write_link(catalog, REG_DP_START_HOR_VER_FROM_SYNC, sync_start);
+ dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
width_blanking);
+ dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER, dp_active);
reg = dp_read_p0(catalog, MMSS_DP_INTF_CONFIG);
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 290ef8180c12..a82ab4856b50 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -48,10 +48,6 @@ enum dp_catalog_audio_header_type {
};
struct dp_catalog {
- u32 total;
- u32 sync_start;
- u32 width_blanking;
- u32 dp_active;
enum dp_catalog_audio_sdp_type sdp_type;
enum dp_catalog_audio_header_type sdp_header;
u32 audio_data;
@@ -106,7 +102,8 @@ void dp_catalog_ctrl_send_phy_pattern(struct dp_catalog
*dp_catalog,
u32 dp_catalog_ctrl_read_phy_pattern(struct dp_catalog *dp_catalog);
/* DP Panel APIs */
-int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog);
+int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog, u32 total,
+ u32 sync_start, u32 width_blanking, u32
dp_active);
void dp_catalog_panel_enable_vsc_sdp(struct dp_catalog *dp_catalog, struct
dp_sdp *vsc_sdp);
void dp_catalog_panel_disable_vsc_sdp(struct dp_catalog *dp_catalog);
void dp_catalog_dump_regs(struct dp_catalog *dp_catalog);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 8e7069453952..07db8f37cd06 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -353,6 +353,10 @@ int dp_panel_timing_cfg(struct dp_panel *dp_panel)
struct dp_catalog *catalog;
struct dp_panel_private *panel;
struct drm_display_mode *drm_mode;
+ u32 width_blanking;
+ u32 sync_start;
+ u32 dp_active;
+ u32 total;
panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
catalog = panel->catalog;
@@ -376,13 +380,13 @@ int dp_panel_timing_cfg(struct dp_panel *dp_panel)
data <<= 16;
data |= total_hor;
- catalog->total = data;
+ total = data;
data = (drm_mode->vtotal - drm_mode->vsync_start);
data <<= 16;
data |= (drm_mode->htotal - drm_mode->hsync_start);
- catalog->sync_start = data;
+ sync_start = data;
data = drm_mode->vsync_end - drm_mode->vsync_start;
data <<= 16;
@@ -390,15 +394,15 @@ int dp_panel_timing_cfg(struct dp_panel *dp_panel)
data |= drm_mode->hsync_end - drm_mode->hsync_start;
data |= (panel->dp_panel.dp_mode.h_active_low << 15);
- catalog->width_blanking = data;
+ width_blanking = data;
data = drm_mode->vdisplay;
data <<= 16;
data |= drm_mode->hdisplay;
- catalog->dp_active = data;
+ dp_active = data;
- dp_catalog_panel_timing_cfg(catalog);
+ dp_catalog_panel_timing_cfg(catalog, total, sync_start, width_blanking,
dp_active);
if (dp_panel->dp_mode.out_fmt_is_yuv_420)
dp_panel_setup_vsc_sdp_yuv_420(dp_panel);
--
2.43.0
[PATCH 0/6] drm/msm/dp: Spring cleaning
Spring is in the air, clean out some dust that has accumulated in the Qualcomm DP driver. Signed-off-by: Bjorn Andersson --- Bjorn Andersson (6): drm/msm/dp: Drop unused dp_debug struct drm/msm/dp: Removed fixed nvid "support" drm/msm/dp: Remove unused defines and members drm/msm/dp: Use function arguments for aux writes drm/msm/dp: Use function arguments for timing configuration drm/msm/dp: Use function arguments for audio operations drivers/gpu/drm/msm/dp/dp_audio.c | 25 +++ drivers/gpu/drm/msm/dp/dp_aux.c | 9 ++ drivers/gpu/drm/msm/dp/dp_catalog.c | 64 ++--- drivers/gpu/drm/msm/dp/dp_catalog.h | 49 +--- drivers/gpu/drm/msm/dp/dp_ctrl.c| 17 +- drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 - drivers/gpu/drm/msm/dp/dp_debug.c | 38 ++ drivers/gpu/drm/msm/dp/dp_debug.h | 38 +++--- drivers/gpu/drm/msm/dp/dp_display.c | 15 ++--- drivers/gpu/drm/msm/dp/dp_display.h | 3 -- drivers/gpu/drm/msm/dp/dp_drm.c | 2 -- drivers/gpu/drm/msm/dp/dp_link.c| 4 --- drivers/gpu/drm/msm/dp/dp_link.h| 1 - drivers/gpu/drm/msm/dp/dp_panel.c | 14 +--- drivers/gpu/drm/msm/dp/dp_panel.h | 3 -- 15 files changed, 80 insertions(+), 203 deletions(-) --- base-commit: 084c8e315db34b59d38d06e684b1a0dd07d30287 change-id: 20240323-msm-dp-cleanup-f8ba32f62fb9 Best regards, -- Bjorn Andersson
Re: [PATCH] accel/qaic: Add Sahara implementation for firmware loading
On Thu, Mar 21, 2024 at 09:49:17PM -0600, Jeffrey Hugo wrote: > The AIC100 secondary bootloader uses the Sahara protocol for two > purposes - loading the runtime firmware images from the host, and > offloading crashdumps to the host. The crashdump functionality is only > invoked when the AIC100 device encounters a crash and dumps are enabled. > Also the collection of the dump is optional - the host can reject > collecting the dump. > > The Sahara protocol contains many features and modes including firmware > upload, crashdump download, and client commands. For simplicity, > implement the parts of the protocol needed for loading firmware to the > device. > > Fundamentally, the Sahara protocol is an embedded file transfer > protocol. Both sides negotiate a connection through a simple exchange of > hello messages. After handshaking through a hello message, the device > either sends a message requesting images, or a message advertising the > memory dump available for the host. For image transfer, the remote device > issues a read data request that provides an image (by ID), an offset, and > a length. The host has an internal mapping of image IDs to filenames. The > host is expected to access the image and transfer the requested chunk to > the device. The device can issue additional read requests, or signal that > it has consumed enough data from this image with an end of image message. > The host confirms the end of image, and the device can proceed with > another image by starting over with the hello exchange again. > > Some images may be optional, and only provided as part of a provisioning > flow. The host is not aware of this information, and thus should report > an error to the device when an image is not available. The device will > evaluate if the image is required or not, and take the appropriate > action. > > Signed-off-by: Jeffrey Hugo > Reviewed-by: Carl Vanderlip > Reviewed-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Bjorn Andersson Regards, Bjorn
Re: [PATCH v3 1/2] drm/msm/dp: Add support for determining the eDP/DP mode from DT
On Fri, Mar 22, 2024 at 03:22:22PM +0200, Abel Vesa wrote:
> Instead of relying on different compatibles for eDP and DP, lookup
> the panel node in devicetree to figure out the connector type and
> then pass on that information to the PHY. External DP is not described
> in DT, therefore, assume it's eDP if panel node is present.
>
> Signed-off-by: Abel Vesa
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 43
> +
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index c4cb82af5c2f..c9763f77c832 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -726,6 +726,14 @@ static int dp_init_sub_modules(struct dp_display_private
> *dp)
> if (IS_ERR(phy))
> return PTR_ERR(phy);
>
> + rc = phy_set_mode_ext(phy, PHY_MODE_DP,
> + dp->dp_display.is_edp ? PHY_SUBMODE_EDP :
> PHY_SUBMODE_DP);
> + if (rc) {
> + DRM_ERROR("failed to set phy submode, rc = %d\n", rc);
> + dp->catalog = NULL;
> + goto error;
> + }
> +
> dp->catalog = dp_catalog_get(dev);
> if (IS_ERR(dp->catalog)) {
> rc = PTR_ERR(dp->catalog);
> @@ -734,9 +742,7 @@ static int dp_init_sub_modules(struct dp_display_private
> *dp)
> goto error;
> }
>
> - dp->aux = dp_aux_get(dev, dp->catalog,
> - phy,
> - dp->dp_display.is_edp);
> + dp->aux = dp_aux_get(dev, dp->catalog, phy, dp->dp_display.is_edp);
> if (IS_ERR(dp->aux)) {
> rc = PTR_ERR(dp->aux);
> DRM_ERROR("failed to initialize aux, rc = %d\n", rc);
> @@ -1241,6 +1247,35 @@ static int dp_auxbus_done_probe(struct drm_dp_aux *aux)
> return dp_display_probe_tail(aux->dev);
> }
>
> +static int dp_display_get_connector_type(struct platform_device *pdev,
> + const struct msm_dp_desc *desc)
> +{
> + struct device *dev = >dev;
> + struct device_node *aux_bus;
> + struct device_node *panel;
> + int ret = DRM_MODE_CONNECTOR_DisplayPort;
> +
> + /* legacy platforms specify connector type in match data */
> + if (desc->connector_type == DRM_MODE_CONNECTOR_eDP ||
> + desc->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
> + return desc->connector_type;
> +
> + aux_bus = of_get_child_by_name(dev->of_node, "aux-bus");
> + if (!aux_bus)
> + goto out;
My compiler warns that if we take this code path, then you will
of_node_put() below.
> +
> + panel = of_get_child_by_name(aux_bus, "panel");
> + if (!panel)
> + goto out;
> +
> + ret = DRM_MODE_CONNECTOR_eDP;
My brain read this function as:
check something
if (error)
bailout!
check something
if (error)
bailout!
ret should be edp
I then have to scan the code again to figure out what ret is otherwise,
and convince myself that the error path is never an error, but a totally
normal case.
If you instead rely on the fact that both of_get_child_by_name() and
of_node_put() can be passed NULL, you can write this as:
static int fn(..) {
aux_bus = of_get_child_by_name(dev->of_node, "aux-bus");
panel = of_get_child_by_name(aux_bus, "panel");
if (panel)
connector_type = DRM_MODE_CONNECTOR_eDP;
else
connector_type = DRM_MODE_CONNECTOR_DisplayPort;
of_node_put(panel);
of_node_put(aux_bus);
return connector_type;
}
Much easier to read, and you don't even have to zero-initialize panel to
avoid that compiler warning.
Regards,
Bjorn
> +
> +out:
> + of_node_put(panel);
> + of_node_put(aux_bus);
> + return ret;
> +}
> +
> static int dp_display_probe(struct platform_device *pdev)
> {
> int rc = 0;
> @@ -1263,7 +1298,7 @@ static int dp_display_probe(struct platform_device
> *pdev)
> dp->dp_display.pdev = pdev;
> dp->name = "drm_dp";
> dp->id = desc->id;
> - dp->dp_display.connector_type = desc->connector_type;
> + dp->dp_display.connector_type = dp_display_get_connector_type(pdev,
> desc);
> dp->wide_bus_supported = desc->wide_bus_supported;
> dp->dp_display.is_edp =
> (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
>
> --
> 2.34.1
>
Re: (subset) [PATCH 0/3] drm/panel: Pixel 3a Panel
On Thu, 08 Feb 2024 19:16:41 -0500, Richard Acayan wrote: > This adds support for the AMS559NK06 panel with the S6E3FA7 display > controller and enables the display subsystem on the Pixel 3a. > > Richard Acayan (3): > dt-bindings: display: panel-simple-dsi: add s6e3fa7 ams559nk06 compat > drm/panel: add samsung s6e3fa7 panel driver > arm64: dts: qcom: sdm670-google-sargo: add panel > > [...] Applied, thanks! [3/3] arm64: dts: qcom: sdm670-google-sargo: add panel commit: 232490b925272d54dd91847a183bdbc2deec904b Best regards, -- Bjorn Andersson
Re: [PATCH v2 2/3] drm/msm/dp: Add support for setting the eDP mode from devicetree
On Thu, Feb 22, 2024 at 05:55:07PM +0200, Abel Vesa wrote:
> Instead of relying on different compatibles for eDP and DP, use
> the is-edp property from DT to figure out the connector type and
> then pass on that information to the PHY.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Abel Vesa
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/gpu/drm/msm/dp/dp_ctrl.c| 11 +++
> drivers/gpu/drm/msm/dp/dp_ctrl.h| 1 +
> drivers/gpu/drm/msm/dp/dp_display.c | 17 ++---
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 320f17fce9a6..bd81cc6bd5e3 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1533,6 +1533,17 @@ void dp_ctrl_set_psr(struct dp_ctrl *dp_ctrl, bool
> enter)
> }
> }
>
> +int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int submode)
> +{
> + struct dp_ctrl_private *ctrl;
> + struct phy *phy;
> +
> + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
> + phy = ctrl->phy;
> +
> + return phy_set_mode_ext(phy, PHY_MODE_DP, submode);
> +}
> +
> void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
> {
> struct dp_ctrl_private *ctrl;
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index fa014cee7e21..a10d1b19d172 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -32,6 +32,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct
> dp_link *link,
> struct phy *phy);
>
> void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable);
> +int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int mode);
> void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl);
> void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl);
> void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index e4433891becb..e01b41ad2e2a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1229,6 +1229,7 @@ static int dp_display_probe(struct platform_device
> *pdev)
> int rc = 0;
> struct dp_display_private *dp;
> const struct msm_dp_desc *desc;
> + bool is_edp;
>
> if (!pdev || !pdev->dev.of_node) {
> DRM_ERROR("pdev not found\n");
> @@ -1243,13 +1244,17 @@ static int dp_display_probe(struct platform_device
> *pdev)
> if (!desc)
> return -EINVAL;
>
> + is_edp = (desc->connector_type == DRM_MODE_CONNECTOR_eDP) ||
> + of_property_read_bool(pdev->dev.of_node, "is-edp");
> +
> dp->dp_display.pdev = pdev;
> dp->name = "drm_dp";
> dp->id = desc->id;
> - dp->dp_display.connector_type = desc->connector_type;
> + dp->dp_display.connector_type = is_edp ?
> + DRM_MODE_CONNECTOR_eDP :
> + DRM_MODE_CONNECTOR_DisplayPort;
> dp->wide_bus_en = desc->wide_bus_en;
> - dp->dp_display.is_edp =
> - (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
> + dp->dp_display.is_edp = is_edp;
>
> rc = dp_init_sub_modules(dp);
> if (rc) {
> @@ -1257,6 +1262,12 @@ static int dp_display_probe(struct platform_device
> *pdev)
> return -EPROBE_DEFER;
> }
>
> + rc = dp_ctrl_phy_set_mode(dp->ctrl, is_edp ? PHY_SUBMODE_EDP :
> PHY_SUBMODE_DP);
> + if (rc) {
> + DRM_ERROR("setting PHY submode failed\n");
> + goto err;
> + }
> +
> /* setup event q */
> mutex_init(>event_mutex);
> init_waitqueue_head(>event_q);
>
> --
> 2.34.1
>
Re: [PATCH 3/9] arm64: dts: qcom: sc7280: Enable MDP turbo mode
On Thu, Feb 22, 2024 at 11:46:26AM +0200, Dmitry Baryshkov wrote: > On Thu, 22 Feb 2024 at 11:28, Konrad Dybcio wrote: > > > > > > > > On 2/22/24 10:04, Dmitry Baryshkov wrote: > > > On Thu, 22 Feb 2024 at 10:56, Konrad Dybcio > > > wrote: > > >> > > >> > > >> > > >> On 2/22/24 00:41, Dmitry Baryshkov wrote: > > >>> On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson > > >>> wrote: > > >>>> > > >>>> The max frequency listed in the DPU opp-table is 506MHz, this is not > > >>>> sufficient to drive a 4k@60 display, resulting in constant underrun. > > >>>> > > >>>> Add the missing MDP_CLK turbo frequency of 608MHz to the opp-table to > > >>>> fix this. > > >>> > > >>> I think we might want to keep this disabled for ChromeOS devices. Doug? > > >> > > >> ChromeOS devices don't get a special SoC > > > > > > But they have the sc7280-chrome-common.dtsi, which might contain a > > > corresponding /delete-node/ . > > > > What does that change? The clock rates are bound to the > > SoC and the effective values are limited by link-frequencies > > or the panel driver. > > Preventing the DPU from overheating? Or spending too much power? > Perhaps I'm misunderstanding the implementation then, are we always running at the max opp? I thought the opp was selected based on the current need for performance? Regards, Bjorn > -- > With best wishes > Dmitry
Re: [PATCH 6/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching
On Thu, Feb 22, 2024 at 01:50:12AM +0200, Dmitry Baryshkov wrote:
> On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson
> wrote:
> >
> > With the ADSP remoteproc loaded pmic_glink can be introduced and wired
> > up to provide role and orientation switching signals.
> >
> > Signed-off-by: Bjorn Andersson
> > ---
> > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 48
> > +++-
> > 1 file changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > index ab498494caea..079bf43b14cc 100644
> > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > @@ -121,6 +121,41 @@ debug_vm_mem: debug-vm@d060 {
> > };
> > };
> >
> > + pmic-glink {
> > + compatible = "qcom,qcm6490-pmic-glink", "qcom,pmic-glink";
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + connector@0 {
> > + compatible = "usb-c-connector";
> > + reg = <0>;
> > + power-role = "dual";
> > + data-role = "dual";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > +
> > + pmic_glink_hs_in: endpoint {
> > + remote-endpoint =
> > <_1_dwc3_hs>;
> > + };
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > +
> > + pmic_glink_ss_in: endpoint {
> > + remote-endpoint =
> > <_1_dwc3_ss>;
>
> This should be connected to the QMP PHY rather than to the USB host.
>
Ahh, you're right, otherwise the orientation-switch below isn't of much
use.
> Also it might be better to squash this patch with the patch 8. Or at
> least to get redriver into the picture in this patch (and keep only
> display-related parts in that patch).
>
The idea was to only bring in the pmic-glink here and then do the
plumbing between all the components separately, but I guess the
orientation-switch in the redriver means that it should go here as
well...
I'll shuffle this into something that makes sense.
Thanks,
Bjorn
>
> > + };
> > + };
> > + };
> > + };
> > + };
> > +
> > vph_pwr: vph-pwr-regulator {
> > compatible = "regulator-fixed";
> > regulator-name = "vph_pwr";
> > @@ -476,7 +511,16 @@ _1 {
> > };
> >
> > _1_dwc3 {
> > - dr_mode = "peripheral";
> > + dr_mode = "otg";
> > + usb-role-switch;
> > +};
> > +
> > +_1_dwc3_hs {
> > + remote-endpoint = <_glink_hs_in>;
> > +};
> > +
> > +_1_dwc3_ss {
> > + remote-endpoint = <_glink_ss_in>;
> > };
> >
> > _1_hsphy {
> > @@ -491,6 +535,8 @@ _1_qmpphy {
> > vdda-phy-supply = <_l6b_1p2>;
> > vdda-pll-supply = <_l1b_0p912>;
> >
> > + orientation-switch;
> > +
> > status = "okay";
> > };
> >
> >
> > --
> > 2.25.1
> >
>
>
> --
> With best wishes
> Dmitry
Re: [PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280
On Thu, Feb 22, 2024 at 01:38:45AM +0200, Dmitry Baryshkov wrote:
> On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson
> wrote:
> >
> > When upstreamed the SC7280 DP controllers where described as one being
> > DP and one eDP, but they can infact both be DP or eDP.
> >
> > Extend the list of DP controllers to cover both instances, and rely on
> > the newly introduced mechanism for selecting which mode they should
> > operate in.
> >
> > Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree
> > will continue to select eDP.
> >
> > Signed-off-by: Bjorn Andersson
> > ---
> > drivers/gpu/drm/msm/dp/dp_display.c | 9 +++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> > b/drivers/gpu/drm/msm/dp/dp_display.c
> > index 7b8c695d521a..1792ba9f7259 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > @@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = {
> > };
> >
> > static const struct msm_dp_desc sc7280_dp_descs[] = {
> > - { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0,
> > .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
> > + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en =
> > true },
> > + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en =
> > true },
>
> I think we need to keep .connector_type here, don't we?
>
No, Abel removes the need for that in his patches - and while that logic
is slightly broken in the RFC I think it looks good.
Regards,
Bjorn
> > + {}
> > +};
> > +
> > +static const struct msm_dp_desc sc7280_edp_descs[] = {
> > { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1,
> > .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
> > {}
> > };
> > @@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
> > static const struct of_device_id dp_dt_match[] = {
> > { .compatible = "qcom,sc7180-dp", .data = _dp_descs },
> > { .compatible = "qcom,sc7280-dp", .data = _dp_descs },
> > - { .compatible = "qcom,sc7280-edp", .data = _dp_descs },
> > + { .compatible = "qcom,sc7280-edp", .data = _edp_descs },
> > { .compatible = "qcom,sc8180x-dp", .data = _dp_descs },
> > { .compatible = "qcom,sc8180x-edp", .data = _dp_descs },
> > { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs },
> >
> > --
> > 2.25.1
> >
>
>
> --
> With best wishes
> Dmitry
Re: [PATCH RFC 3/3] drm/msm/dp: Add support for the X1E80100
On Wed, Feb 21, 2024 at 12:50:33AM +0200, Abel Vesa wrote:
> Add the X1E80100 DP descs and compatible. This platform will be using
> a single compatible for both eDP and DP mode. The actual mode will
> be set in devicetree via is-edp flag.
>
> Signed-off-by: Abel Vesa
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 9e58285d4ec6..7b8c695d521a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -171,6 +171,14 @@ static const struct msm_dp_desc sm8650_dp_descs[] = {
> {}
> };
>
> +static const struct msm_dp_desc x1e80100_dp_descs[] = {
> + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en =
> true },
> + { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .wide_bus_en =
> true },
> + { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .wide_bus_en =
> true },
> + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .wide_bus_en =
> true },
> + {}
> +};
> +
> static const struct of_device_id dp_dt_match[] = {
> { .compatible = "qcom,sc7180-dp", .data = _dp_descs },
> { .compatible = "qcom,sc7280-dp", .data = _dp_descs },
> @@ -179,6 +187,7 @@ static const struct of_device_id dp_dt_match[] = {
> { .compatible = "qcom,sc8180x-edp", .data = _dp_descs },
> { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs },
> { .compatible = "qcom,sc8280xp-edp", .data = _edp_descs },
> + { .compatible = "qcom,x1e80100-dp", .data = _dp_descs },
This doesn't look like alphabetical order.
Regards,
Bjorn
> { .compatible = "qcom,sdm845-dp", .data = _dp_descs },
> { .compatible = "qcom,sm8350-dp", .data = _dp_descs },
> { .compatible = "qcom,sm8650-dp", .data = _dp_descs },
>
> --
> 2.34.1
>
Re: [PATCH RFC 1/3] dt-bindings: display: msm: dp-controller: document X1E80100 compatible
On Wed, Feb 21, 2024 at 12:50:31AM +0200, Abel Vesa wrote: > Add the X1E80100 to the list of compatibles and docoment the is-edp s/docoment/document/ > flag. This new flag will be used from now on to dictate the mode from s/from now on// Perhaps cleaner to spell out that the controllers are expected to operate in DP mode by default, and this flag can be used to select eDP mode. > devicetree, instead of having separate compatibles for eDP and DP. > Regards, Bjorn > Signed-off-by: Abel Vesa > --- > Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > index ae53cbfb2193..ed11852e403d 100644 > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > @@ -27,6 +27,7 @@ properties: >- qcom,sdm845-dp >- qcom,sm8350-dp >- qcom,sm8650-dp > + - qcom,x1e80100-dp >- items: >- enum: >- qcom,sm8150-dp > @@ -73,6 +74,11 @@ properties: >- description: phy 0 parent >- description: phy 1 parent > > + is-edp: > +$ref: /schemas/types.yaml#/definitions/flag > +description: > + Tells the controller to switch to eDP mode > + >phys: > maxItems: 1 > > > -- > 2.34.1 >
Re: [PATCH RFC 2/3] drm/msm/dp: Add support for setting the eDP mode from devicetree
On Wed, Feb 21, 2024 at 12:50:32AM +0200, Abel Vesa wrote:
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index fa014cee7e21..a10d1b19d172 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -32,6 +32,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct
> dp_link *link,
> struct phy *phy);
>
> void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable);
> +int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int mode);
> void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl);
> void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl);
> void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index e4433891becb..9e58285d4ec6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1229,6 +1229,7 @@ static int dp_display_probe(struct platform_device
> *pdev)
> int rc = 0;
> struct dp_display_private *dp;
> const struct msm_dp_desc *desc;
> + bool is_edp = false;
>
> if (!pdev || !pdev->dev.of_node) {
> DRM_ERROR("pdev not found\n");
> @@ -1243,13 +1244,19 @@ static int dp_display_probe(struct platform_device
> *pdev)
> if (!desc)
> return -EINVAL;
>
> + if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP ||
dp is just allocated, and will be 0. You need to check
desc->connector_type here.
Regards,
Bjorn
> + of_property_read_bool(pdev->dev.of_node, "is-edp"))
> + is_edp = true;
> +
> + dp->dp_display.is_edp = is_edp;
> dp->dp_display.pdev = pdev;
> dp->name = "drm_dp";
> dp->id = desc->id;
> - dp->dp_display.connector_type = desc->connector_type;
> dp->wide_bus_en = desc->wide_bus_en;
> - dp->dp_display.is_edp =
> - (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
> +
> + dp->dp_display.connector_type = is_edp ?
> + DRM_MODE_CONNECTOR_eDP :
> + DRM_MODE_CONNECTOR_DisplayPort;
>
> rc = dp_init_sub_modules(dp);
> if (rc) {
> @@ -1257,6 +1264,12 @@ static int dp_display_probe(struct platform_device
> *pdev)
> return -EPROBE_DEFER;
> }
>
> + rc = dp_ctrl_phy_set_mode(dp->ctrl, is_edp ? PHY_SUBMODE_EDP :
> PHY_SUBMODE_DP);
> + if (rc) {
> + DRM_ERROR("setting PHY submode failed\n");
> + goto err;
> + }
> +
> /* setup event q */
> mutex_init(>event_mutex);
> init_waitqueue_head(>event_q);
>
> --
> 2.34.1
>
Re: [PATCH 6/6] phy: qcom-qmp-combo: fix type-c switch registration
On Sat, Feb 17, 2024 at 04:02:28PM +0100, Johan Hovold wrote:
> Due to a long-standing issue in driver core, drivers may not probe defer
> after having registered child devices to avoid triggering a probe
> deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of
> -EPROBE_DEFER")).
>
> Move registration of the typec switch to after looking up clocks and
> other resources.
>
> Note that PHY creation can in theory also trigger a probe deferral when
> a 'phy' supply is used. This does not seem to affect the QMP PHY driver
> but the PHY subsystem should be reworked to address this (i.e. by
> separating initialisation and registration of the PHY).
>
> Fixes: 2851117f8f42 ("phy: qcom-qmp-combo: Introduce orientation switching")
> Cc: sta...@vger.kernel.org # 6.5
> Cc: Bjorn Andersson
> Signed-off-by: Johan Hovold
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index e19d6a084f10..17c4ad7553a5 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -3562,10 +3562,6 @@ static int qmp_combo_probe(struct platform_device
> *pdev)
> if (ret)
> return ret;
>
> - ret = qmp_combo_typec_switch_register(qmp);
> - if (ret)
> - return ret;
> -
> /* Check for legacy binding with child nodes. */
> usb_np = of_get_child_by_name(dev->of_node, "usb3-phy");
> if (usb_np) {
> @@ -3585,6 +3581,10 @@ static int qmp_combo_probe(struct platform_device
> *pdev)
> if (ret)
> goto err_node_put;
>
> + ret = qmp_combo_typec_switch_register(qmp);
> + if (ret)
> + goto err_node_put;
> +
> ret = drm_aux_bridge_register(dev);
> if (ret)
> goto err_node_put;
> --
> 2.43.0
>
Re: [PATCH 5/6] phy: qcom-qmp-combo: fix drm bridge registration
On Sat, Feb 17, 2024 at 04:02:27PM +0100, Johan Hovold wrote:
> Due to a long-standing issue in driver core, drivers may not probe defer
> after having registered child devices to avoid triggering a probe
> deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of
> -EPROBE_DEFER")).
>
> This could potentially also trigger a bug in the DRM bridge
> implementation which does not expect bridges to go away even if device
> links may avoid triggering this (when enabled).
>
> Move registration of the DRM aux bridge to after looking up clocks and
> other resources.
>
> Note that PHY creation can in theory also trigger a probe deferral when
> a 'phy' supply is used. This does not seem to affect the QMP PHY driver
> but the PHY subsystem should be reworked to address this (i.e. by
> separating initialisation and registration of the PHY).
>
> Fixes: 35921910bbd0 ("phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE")
> Fixes: 1904c3f578dc ("phy: qcom-qmp-combo: Introduce drm_bridge")
> Cc: sta...@vger.kernel.org # 6.5
> Cc: Bjorn Andersson
> Cc: Dmitry Baryshkov
> Signed-off-by: Johan Hovold
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 1ad10110dd25..e19d6a084f10 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -3566,10 +3566,6 @@ static int qmp_combo_probe(struct platform_device
> *pdev)
> if (ret)
> return ret;
>
> - ret = drm_aux_bridge_register(dev);
> - if (ret)
> - return ret;
> -
> /* Check for legacy binding with child nodes. */
> usb_np = of_get_child_by_name(dev->of_node, "usb3-phy");
> if (usb_np) {
> @@ -3589,6 +3585,10 @@ static int qmp_combo_probe(struct platform_device
> *pdev)
> if (ret)
> goto err_node_put;
>
> + ret = drm_aux_bridge_register(dev);
> + if (ret)
> + goto err_node_put;
> +
> pm_runtime_set_active(dev);
> ret = devm_pm_runtime_enable(dev);
> if (ret)
> --
> 2.43.0
>
Re: [PATCH 4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m
On Sat, Feb 17, 2024 at 04:02:26PM +0100, Johan Hovold wrote:
> From: Rob Clark
>
> We need to bail out before adding/removing devices if we are going to
> -EPROBE_DEFER. Otherwise boot can get stuck in a probe deferral loop due
> to a long-standing issue in driver core (see fbc35b45f9f6 ("Add
> documentation on meaning of -EPROBE_DEFER")).
>
> Deregistering the altmode child device can potentially also trigger bugs
> in the DRM bridge implementation, which does not expect bridges to go
> away.
>
> Suggested-by: Dmitry Baryshkov
> Signed-off-by: Rob Clark
> Link: https://lore.kernel.org/r/20231213210644.8702-1-robdcl...@gmail.com
> [ johan: rebase on 6.8-rc4, amend commit message and mention DRM ]
> Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK
> driver")
> Cc: sta...@vger.kernel.org # 6.3
> Cc: Bjorn Andersson
> Signed-off-by: Johan Hovold
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/soc/qcom/pmic_glink.c | 21 +++--
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
> index f4bfd24386f1..f913e9bd57ed 100644
> --- a/drivers/soc/qcom/pmic_glink.c
> +++ b/drivers/soc/qcom/pmic_glink.c
> @@ -265,10 +265,17 @@ static int pmic_glink_probe(struct platform_device
> *pdev)
>
> pg->client_mask = *match_data;
>
> + pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg);
> + if (IS_ERR(pg->pdr)) {
> + ret = dev_err_probe(>dev, PTR_ERR(pg->pdr),
> + "failed to initialize pdr\n");
> + return ret;
> + }
> +
> if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI)) {
> ret = pmic_glink_add_aux_device(pg, >ucsi_aux, "ucsi");
> if (ret)
> - return ret;
> + goto out_release_pdr_handle;
> }
> if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE)) {
> ret = pmic_glink_add_aux_device(pg, >altmode_aux,
> "altmode");
> @@ -281,17 +288,11 @@ static int pmic_glink_probe(struct platform_device
> *pdev)
> goto out_release_altmode_aux;
> }
>
> - pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg);
> - if (IS_ERR(pg->pdr)) {
> - ret = dev_err_probe(>dev, PTR_ERR(pg->pdr), "failed to
> initialize pdr\n");
> - goto out_release_aux_devices;
> - }
> -
> service = pdr_add_lookup(pg->pdr, "tms/servreg", "msm/adsp/charger_pd");
> if (IS_ERR(service)) {
> ret = dev_err_probe(>dev, PTR_ERR(service),
> "failed adding pdr lookup for
> charger_pd\n");
> - goto out_release_pdr_handle;
> + goto out_release_aux_devices;
> }
>
> mutex_lock(&__pmic_glink_lock);
> @@ -300,8 +301,6 @@ static int pmic_glink_probe(struct platform_device *pdev)
>
> return 0;
>
> -out_release_pdr_handle:
> - pdr_handle_release(pg->pdr);
> out_release_aux_devices:
> if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
> pmic_glink_del_aux_device(pg, >ps_aux);
> @@ -311,6 +310,8 @@ static int pmic_glink_probe(struct platform_device *pdev)
> out_release_ucsi_aux:
> if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
> pmic_glink_del_aux_device(pg, >ucsi_aux);
> +out_release_pdr_handle:
> + pdr_handle_release(pg->pdr);
>
> return ret;
> }
> --
> 2.43.0
>
Re: [PATCH 3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Sat, Feb 17, 2024 at 04:02:25PM +0100, Johan Hovold wrote:
> A recent DRM series purporting to simplify support for "transparent
> bridges" and handling of probe deferrals ironically exposed a
> use-after-free issue on pmic_glink_altmode probe deferral.
>
> This has manifested itself as the display subsystem occasionally failing
> to initialise and NULL-pointer dereferences during boot of machines like
> the Lenovo ThinkPad X13s.
>
> Specifically, the dp-hpd bridge is currently registered before all
> resources have been acquired which means that it can also be
> deregistered on probe deferrals.
>
> In the meantime there is a race window where the new aux bridge driver
> (or PHY driver previously) may have looked up the dp-hpd bridge and
> stored a (non-reference-counted) pointer to the bridge which is about to
> be deallocated.
>
> When the display controller is later initialised, this triggers a
> use-after-free when attaching the bridges:
>
> dp -> aux -> dp-hpd (freed)
>
> which may, for example, result in the freed bridge failing to attach:
>
> [drm:drm_bridge_attach [drm]] *ERROR* failed to attach bridge
> /soc@0/phy@88eb000 to encoder TMDS-31: -16
>
> or a NULL-pointer dereference:
>
> Unable to handle kernel NULL pointer dereference at virtual address
>
> ...
> Call trace:
> drm_bridge_attach+0x70/0x1a8 [drm]
> drm_aux_bridge_attach+0x24/0x38 [aux_bridge]
> drm_bridge_attach+0x80/0x1a8 [drm]
> dp_bridge_init+0xa8/0x15c [msm]
> msm_dp_modeset_init+0x28/0xc4 [msm]
>
> The DRM bridge implementation is clearly fragile and implicitly built on
> the assumption that bridges may never go away. In this case, the fix is
> to move the bridge registration in the pmic_glink_altmode driver to
> after all resources have been looked up.
>
> Incidentally, with the new dp-hpd bridge implementation, which registers
> child devices, this is also a requirement due to a long-standing issue
> in driver core that can otherwise lead to a probe deferral loop (see
> fbc35b45f9f6 ("Add documentation on meaning of -EPROBE_DEFER")).
>
> Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
> Fixes: 2bcca96abfbf ("soc: qcom: pmic-glink: switch to DRM_AUX_HPD_BRIDGE")
> Cc: sta...@vger.kernel.org # 6.3
> Cc: Bjorn Andersson
> Cc: Dmitry Baryshkov
> Signed-off-by: Johan Hovold
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/soc/qcom/pmic_glink_altmode.c | 16 +---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c
> b/drivers/soc/qcom/pmic_glink_altmode.c
> index 5fcd0fdd2faa..b3808fc24c69 100644
> --- a/drivers/soc/qcom/pmic_glink_altmode.c
> +++ b/drivers/soc/qcom/pmic_glink_altmode.c
> @@ -76,7 +76,7 @@ struct pmic_glink_altmode_port {
>
> struct work_struct work;
>
> - struct device *bridge;
> + struct auxiliary_device *bridge;
>
> enum typec_orientation orientation;
> u16 svid;
> @@ -230,7 +230,7 @@ static void pmic_glink_altmode_worker(struct work_struct
> *work)
> else
> pmic_glink_altmode_enable_usb(altmode, alt_port);
>
> - drm_aux_hpd_bridge_notify(alt_port->bridge,
> + drm_aux_hpd_bridge_notify(_port->bridge->dev,
> alt_port->hpd_state ?
> connector_status_connected :
> connector_status_disconnected);
> @@ -454,7 +454,7 @@ static int pmic_glink_altmode_probe(struct
> auxiliary_device *adev,
> alt_port->index = port;
> INIT_WORK(_port->work, pmic_glink_altmode_worker);
>
> - alt_port->bridge = drm_dp_hpd_bridge_register(dev,
> to_of_node(fwnode));
> + alt_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev,
> to_of_node(fwnode));
> if (IS_ERR(alt_port->bridge)) {
> fwnode_handle_put(fwnode);
> return PTR_ERR(alt_port->bridge);
> @@ -510,6 +510,16 @@ static int pmic_glink_altmode_probe(struct
> auxiliary_device *adev,
> }
> }
>
> + for (port = 0; port < ARRAY_SIZE(altmode->ports); port++) {
> + alt_port = >ports[port];
> + if (!alt_port->bridge)
> + continue;
> +
> + ret = devm_drm_dp_hpd_bridge_add(dev, alt_port->bridge);
> + if (ret)
> + return ret;
> + }
> +
> altmode->client = devm_pmic_glink_register_client(dev,
> altmode->owner_id,
>
> pmic_glink_altmode_callback,
> --
> 2.43.0
>
Re: [PATCH 2/6] drm/bridge: aux-hpd: separate allocation and registration
On Sat, Feb 17, 2024 at 04:02:24PM +0100, Johan Hovold wrote: > diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c > b/drivers/gpu/drm/bridge/aux-hpd-bridge.c [..] > +/** > + * devm_drm_dp_hpd_bridge_add - register a HDP DisplayPort bridge kernel-doc wants () after function names. > + * @dev: struct device to tie registration lifetime to > + * @adev: bridge auxiliary device to be registered > + * > + * Returns: zero on success or a negative errno and "Return:" without the 's'. This could however be done in a separate patch, as the file is already wrong in this regard. Reviewed-by: Bjorn Andersson Regards, Bjorn
Re: [PATCH 1/6] drm/bridge: aux-hpd: fix OF node leaks
On Sat, Feb 17, 2024 at 04:02:23PM +0100, Johan Hovold wrote:
> The two device node references taken during allocation need to be
> dropped when the auxiliary device is freed.
>
> Fixes: 6914968a0b52 ("drm/bridge: properly refcount DT nodes in aux bridge
> drivers")
> Cc: Dmitry Baryshkov
> Cc: Neil Armstrong
> Signed-off-by: Johan Hovold
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/gpu/drm/bridge/aux-hpd-bridge.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> index bb55f697a181..9e71daf95bde 100644
> --- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> @@ -25,6 +25,7 @@ static void drm_aux_hpd_bridge_release(struct device *dev)
> ida_free(_aux_hpd_bridge_ida, adev->id);
>
> of_node_put(adev->dev.platform_data);
> + of_node_put(adev->dev.of_node);
>
> kfree(adev);
> }
> @@ -74,6 +75,8 @@ struct device *drm_dp_hpd_bridge_register(struct device
> *parent,
>
> ret = auxiliary_device_init(adev);
> if (ret) {
> + of_node_put(adev->dev.platform_data);
> + of_node_put(adev->dev.of_node);
> ida_free(_aux_hpd_bridge_ida, adev->id);
> kfree(adev);
> return ERR_PTR(ret);
> --
> 2.43.0
>
[PATCH 4/9] arm64: dts: qcom: qcs6490-rb3gen2: Add DP output
The RB3Gen2 board comes with a mini DP connector, describe this, enable
MDSS, DP controller and the PHY that drives this.
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 23 +++
1 file changed, 23 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index ac4579119d3b..32313f47602a 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -430,6 +430,23 @@ {
;
};
+ {
+ status = "okay";
+};
+
+_edp {
+ status = "okay";
+};
+
+_edp_out {
+ data-lanes = <0 1 2 3>;
+ link-frequencies = /bits/ 64 <162000 27 54
81>;
+};
+
+_edp_phy {
+ status = "okay";
+};
+
_id_0 {
status = "okay";
};
@@ -470,3 +487,9 @@ _1_qmpphy {
{
memory-region = <_fw_mem>;
};
+
+/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
+
+_hot_plug_det {
+ bias-disable;
+};
--
2.25.1
[PATCH 9/9] arm64: defconfig: Enable sc7280 display and gpu clock controllers
Enable the SC7280 display and gpu clock controllers to enable display support on the QCS6490 RB3gen2. Signed-off-by: Bjorn Andersson --- arch/arm64/configs/defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index b8adb28185ad..193d504041dd 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -1283,6 +1283,7 @@ CONFIG_QCM_DISPCC_2290=m CONFIG_QCS_GCC_404=y CONFIG_QDU_GCC_1000=y CONFIG_SC_CAMCC_8280XP=m +CONFIG_SC_DISPCC_7280=m CONFIG_SC_DISPCC_8280XP=m CONFIG_SA_GCC_8775P=y CONFIG_SA_GPUCC_8775P=m @@ -1290,6 +1291,7 @@ CONFIG_SC_GCC_7180=y CONFIG_SC_GCC_7280=y CONFIG_SC_GCC_8180X=y CONFIG_SC_GCC_8280XP=y +CONFIG_SC_GPUCC_7280=m CONFIG_SC_GPUCC_8280XP=m CONFIG_SC_LPASSCC_8280XP=m CONFIG_SDM_CAMCC_845=m -- 2.25.1
[PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280
When upstreamed the SC7280 DP controllers where described as one being
DP and one eDP, but they can infact both be DP or eDP.
Extend the list of DP controllers to cover both instances, and rely on
the newly introduced mechanism for selecting which mode they should
operate in.
Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree
will continue to select eDP.
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_display.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 7b8c695d521a..1792ba9f7259 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = {
};
static const struct msm_dp_desc sc7280_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en =
true },
+ { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en =
true },
+ {}
+};
+
+static const struct msm_dp_desc sc7280_edp_descs[] = {
{ .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
{}
};
@@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
static const struct of_device_id dp_dt_match[] = {
{ .compatible = "qcom,sc7180-dp", .data = _dp_descs },
{ .compatible = "qcom,sc7280-dp", .data = _dp_descs },
- { .compatible = "qcom,sc7280-edp", .data = _dp_descs },
+ { .compatible = "qcom,sc7280-edp", .data = _edp_descs },
{ .compatible = "qcom,sc8180x-dp", .data = _dp_descs },
{ .compatible = "qcom,sc8180x-edp", .data = _dp_descs },
{ .compatible = "qcom,sc8280xp-dp", .data = _dp_descs },
--
2.25.1
[PATCH 5/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable adsp and cdsp
Define firmware paths and enable the ADSP and CDSP remoteprocs.
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 32313f47602a..ab498494caea 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -451,6 +451,16 @@ _id_0 {
status = "okay";
};
+_adsp {
+ firmware-name = "qcom/qcs6490/rb3gen2/adsp.mbn";
+ status = "okay";
+};
+
+_cdsp {
+ firmware-name = "qcom/qcs6490/rb3gen2/cdsp.mbn";
+ status = "okay";
+};
+
{
gpio-reserved-ranges = <32 2>, /* ADSP */
<48 4>; /* NFC */
--
2.25.1
[PATCH 8/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB Type-C display
With MDSS, pmic_glink, and the redriver in place, wire up the various
components to enable USB Type-C display on the RB3gen2.
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 63 +++-
1 file changed, 61 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 171ed979d55f..4bf1c6351467 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -149,7 +149,15 @@ port@1 {
reg = <1>;
pmic_glink_ss_in: endpoint {
- remote-endpoint =
<_1_dwc3_ss>;
+ remote-endpoint =
<_usb_con_ss>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ pmic_glink_sbu_in: endpoint {
+ remote-endpoint =
<_usb_con_sbu>;
};
};
};
@@ -476,6 +484,36 @@ typec-mux@1c {
retimer-switch;
orientation-switch;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ redriver_usb_con_ss: endpoint {
+ remote-endpoint = <_glink_ss_in>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ redriver_phy_con_ss: endpoint {
+ remote-endpoint = <_dp_qmpphy_out>;
+ data-lanes = <0 1 2 3>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ redriver_usb_con_sbu: endpoint {
+ remote-endpoint = <_glink_sbu_in>;
+ };
+ };
+ };
};
};
@@ -483,6 +521,15 @@ {
status = "okay";
};
+_dp {
+ status = "okay";
+};
+
+_dp_out {
+ data-lanes = <0 1>;
+ remote-endpoint = <_dp_qmpphy_dp_in>;
+};
+
_edp {
status = "okay";
};
@@ -534,7 +581,7 @@ _1_dwc3_hs {
};
_1_dwc3_ss {
- remote-endpoint = <_glink_ss_in>;
+ remote-endpoint = <_dp_qmpphy_usb_ss_in>;
};
_1_hsphy {
@@ -554,6 +601,18 @@ _1_qmpphy {
status = "okay";
};
+_dp_qmpphy_out {
+ remote-endpoint = <_phy_con_ss>;
+};
+
+_dp_qmpphy_usb_ss_in {
+ remote-endpoint = <_1_dwc3_ss>;
+};
+
+_dp_qmpphy_dp_in {
+ remote-endpoint = <_dp_out>;
+};
+
{
memory-region = <_fw_mem>;
};
--
2.25.1
[PATCH 2/9] arm64: dts: qcom: sc7280: Make eDP/DP controller default DP
The newly introduced mechanism for selecting eDP mode allow us to make a
DisplayPort controller operate in eDP mode, but not the other way
around. The qcom,sc7280-edp compatible is obviously tied to eDP, so this
would not allow us to select DisplayPort-mode.
Switch the compatible of the mdss_edp instance and make it eDP for the
SC7280 qcard.
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 ++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
index f9b96bd2477e..e339b181a9ac 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
@@ -348,6 +348,8 @@ _va_macro {
/* NOTE: Not all Qcards have eDP connector stuffed */
_edp {
+ is-edp;
+
aux-bus {
edp_panel: panel {
compatible = "edp-panel";
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 581818676a4c..a19c278ebec9 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -4513,7 +4513,7 @@ mdss_dsi_phy: phy@ae94400 {
};
mdss_edp: edp@aea {
- compatible = "qcom,sc7280-edp";
+ compatible = "qcom,sc7280-dp";
pinctrl-names = "default";
pinctrl-0 = <_hot_plug_det>;
--
2.25.1
[PATCH 7/9] arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver
The RB3gen2 has a USB redriver on APPS_I2C, enable the bus and introduce
the redriver. The plumbing with other components is kept separate for
clarity.
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 079bf43b14cc..171ed979d55f 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -465,6 +465,20 @@ {
;
};
+ {
+ status = "okay";
+
+ typec-mux@1c {
+ compatible = "onnn,nb7vpq904m";
+ reg = <0x1c>;
+
+ vcc-supply = <_l18b_1p8>;
+
+ retimer-switch;
+ orientation-switch;
+ };
+};
+
{
status = "okay";
};
--
2.25.1
[PATCH 6/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching
With the ADSP remoteproc loaded pmic_glink can be introduced and wired
up to provide role and orientation switching signals.
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 48 +++-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index ab498494caea..079bf43b14cc 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -121,6 +121,41 @@ debug_vm_mem: debug-vm@d060 {
};
};
+ pmic-glink {
+ compatible = "qcom,qcm6490-pmic-glink", "qcom,pmic-glink";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ connector@0 {
+ compatible = "usb-c-connector";
+ reg = <0>;
+ power-role = "dual";
+ data-role = "dual";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ pmic_glink_hs_in: endpoint {
+ remote-endpoint =
<_1_dwc3_hs>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ pmic_glink_ss_in: endpoint {
+ remote-endpoint =
<_1_dwc3_ss>;
+ };
+ };
+ };
+ };
+ };
+
vph_pwr: vph-pwr-regulator {
compatible = "regulator-fixed";
regulator-name = "vph_pwr";
@@ -476,7 +511,16 @@ _1 {
};
_1_dwc3 {
- dr_mode = "peripheral";
+ dr_mode = "otg";
+ usb-role-switch;
+};
+
+_1_dwc3_hs {
+ remote-endpoint = <_glink_hs_in>;
+};
+
+_1_dwc3_ss {
+ remote-endpoint = <_glink_ss_in>;
};
_1_hsphy {
@@ -491,6 +535,8 @@ _1_qmpphy {
vdda-phy-supply = <_l6b_1p2>;
vdda-pll-supply = <_l1b_0p912>;
+ orientation-switch;
+
status = "okay";
};
--
2.25.1
[PATCH 3/9] arm64: dts: qcom: sc7280: Enable MDP turbo mode
The max frequency listed in the DPU opp-table is 506MHz, this is not
sufficient to drive a 4k@60 display, resulting in constant underrun.
Add the missing MDP_CLK turbo frequency of 608MHz to the opp-table to
fix this.
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index a19c278ebec9..a2a6717c6c87 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -4417,6 +4417,11 @@ opp-50667 {
opp-hz = /bits/ 64 <50667>;
required-opps =
<_opp_nom>;
};
+
+ opp-60800 {
+ opp-hz = /bits/ 64 <60800>;
+ required-opps =
<_opp_turbo>;
+ };
};
};
--
2.25.1
[PATCH 0/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable two displays
RB3Gen2 is capable of producing DisplayPort output on a dedicated mini-DP connector and USB Type-C. Utilize Abel's work for DP vs eDP selection to allow configuring both controllers in DP-mode, then enable the two output paths. Tested by driving fbcon to 4k@60 + 4k@30 concurrently. Depends on https://lore.kernel.org/linux-arm-msm/20240220-x1e80100-display-v4-0-971afd9de...@linaro.org/ Signed-off-by: Bjorn Andersson --- Bjorn Andersson (9): drm/msm/dp: Add DP support to combo instance in SC7280 arm64: dts: qcom: sc7280: Make eDP/DP controller default DP arm64: dts: qcom: sc7280: Enable MDP turbo mode arm64: dts: qcom: qcs6490-rb3gen2: Add DP output arm64: dts: qcom: qcs6490-rb3gen2: Enable adsp and cdsp arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver arm64: dts: qcom: qcs6490-rb3gen2: Enable USB Type-C display arm64: defconfig: Enable sc7280 display and gpu clock controllers arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 154 ++- arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 + arch/arm64/boot/dts/qcom/sc7280.dtsi | 7 +- arch/arm64/configs/defconfig | 2 + drivers/gpu/drm/msm/dp/dp_display.c | 9 +- 5 files changed, 170 insertions(+), 4 deletions(-) --- base-commit: aba508318eec7acad2373296279d6447fd35f83f change-id: 20240209-rb3gen2-dp-connector-bddfb892ff20 Best regards, -- Bjorn Andersson
Re: [PATCH] arm64: dts: qcom: sm6115: fix USB PHY configuration
On Tue, 20 Feb 2024 19:31:04 +0200, Dmitry Baryshkov wrote: > The patch adding Type-C support for sm6115 was misapplied. All the > orientation switch configuration ended up at the UFS PHY node instead of > the USB PHY node. Move the data bits to the correct place. > > Applied, thanks! [1/1] arm64: dts: qcom: sm6115: fix USB PHY configuration commit: f176168bcb95bd1fdd32f5a794e68b7a36ac8740 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v3 0/4] Add display support for Fairphone 4
On Fri, 16 Feb 2024 11:10:47 +0100, Luca Weiss wrote: > Introduce the bindings and panel driver for the LCD panel with the model > number 9A-3R063-1102B from DJN which is using the HX83112A driver IC. It > is used on the Fairphone 4 smartphone. > > Then we can add the panel to the device dts and also enable the GPU. > > > [...] Applied, thanks! [3/4] arm64: dts: qcom: sm6350: Remove "disabled" state of GMU commit: 2abe4a310cc742332038aed5f9f4a15e65a0bcc1 [4/4] arm64: dts: qcom: sm7225-fairphone-fp4: Enable display and GPU commit: 891af1aa1ea42514b9a7f42caaa1fa0c32f8e232 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v4 0/6] SDM670 display subsystem support
On Mon, 16 Oct 2023 22:18:07 -0400, Richard Acayan wrote: > Changes since v3 (2023100927.485054-8-mailingrad...@gmail.com): > - move status properties down (review tag retained) (6/6) > - accumulate review tag (3/6) > > Changes since v2 (20231003012119.857198-9-mailingrad...@gmail.com): > - rebase on series and reference generic sblk definitions (5/6) > - add interconnects properties in example (3/6) > - remove phy-names properties from dtsi (6/6) > - accumulate review tags (4/6, 6/6) > > [...] Applied, thanks! [6/6] arm64: dts: qcom: sdm670: add display subsystem commit: 5f8ba4f28ddb432c8a9720c337f9047e38fa7e36 Best regards, -- Bjorn Andersson
Re: [PATCH] drm/msm/dp: call dp_display_get_next_bridge() during probe
On Tue, Nov 07, 2023 at 02:43:33AM +0200, Dmitry Baryshkov wrote:
> The funcion dp_display_get_next_bridge() can return -EPROBE_DEFER if the
> next bridge is not (yet) available. However returning -EPROBE_DEFER from
> msm_dp_modeset_init() is not ideal. This leads to -EPROBE return from
> component_bind, which can easily result in -EPROBE_DEFR loops.
>
Nice!
> Signed-off-by: Dmitry Baryshkov
> ---
>
> Dependencies: https://patchwork.freedesktop.org/series/120375/
>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 36 +
> 1 file changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index d542db37763a..ddb3c84f39a2 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1197,15 +1197,27 @@ static const struct msm_dp_desc
> *dp_display_get_desc(struct platform_device *pde
> return NULL;
> }
>
> -static int dp_auxbus_done_probe(struct drm_dp_aux *aux)
> +static int dp_display_get_next_bridge(struct msm_dp *dp);
> +
> +static int dp_display_probe_tail(struct device *dev)
> {
> - int rc;
> + struct msm_dp *dp = dev_get_drvdata(dev);
> + int ret;
>
> - rc = component_add(aux->dev, _display_comp_ops);
> - if (rc)
> - DRM_ERROR("eDP component add failed, rc=%d\n", rc);
> + ret = dp_display_get_next_bridge(dp);
> + if (ret)
> + return ret;
>
> - return rc;
> + ret = component_add(dev, _display_comp_ops);
> + if (ret)
> + DRM_ERROR("component add failed, rc=%d\n", ret);
> +
> + return ret;
> +}
> +
> +static int dp_auxbus_done_probe(struct drm_dp_aux *aux)
> +{
> + return dp_display_probe_tail(aux->dev);
> }
>
> static int dp_display_probe(struct platform_device *pdev)
> @@ -1280,11 +1292,9 @@ static int dp_display_probe(struct platform_device
> *pdev)
> goto err;
> }
> } else {
> - rc = component_add(>dev, _display_comp_ops);
> - if (rc) {
> - DRM_ERROR("component add failed, rc=%d\n", rc);
> + rc = dp_display_probe_tail(>dev);
> + if (rc)
> goto err;
> - }
> }
>
> return rc;
> @@ -1415,7 +1425,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>* For DisplayPort interfaces external bridges are optional, so
>* silently ignore an error if one is not present (-ENODEV).
>*/
> - rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser);
> + rc = devm_dp_parser_find_next_bridge(>pdev->dev, dp_priv->parser);
This transition worried me, but after reading the code the current model
of mixing devices for devres scares me more. So, nice cleanup! But I
think we have a few more of these...
That said, >pdev->dev is dp_priv->parser->dev, the function no
longer relate to the "parser module", and we stash the return value of
devm_drm_of_get_bridge(dev, dev->of_node, 1, 0)
in parser->next_brigde, so that we 5 lines below this call can move it
into dp->next_bridge.
As such, I'd like to propose that we change
devm_dp_parser_find_next_bridge() to just take >pdev->dev and return
the next_bridge, in an ERR_PTR().
But that's follow-up-patch material.
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> if (!dp->is_edp && rc == -ENODEV)
> return 0;
>
> @@ -1435,10 +1445,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display,
> struct drm_device *dev,
>
> dp_priv = container_of(dp_display, struct dp_display_private,
> dp_display);
>
> - ret = dp_display_get_next_bridge(dp_display);
> - if (ret)
> - return ret;
> -
> ret = dp_bridge_init(dp_display, dev, encoder);
> if (ret) {
> DRM_DEV_ERROR(dev->dev,
> --
> 2.42.0
>
>
Re: (subset) [PATCH 0/3] Add GPU support for MSM8226 (Adreno A305B)
On Thu, 30 Nov 2023 21:35:17 +0100, Luca Weiss wrote: > Add the necessary bits to bring up the GPU on msm8226. > > Tested on apq8026-lg-lenok. > > Applied, thanks! [3/3] ARM: dts: qcom: msm8226: Add GPU commit: fc209f869310776c437daba478246df64d82c38b Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v3 00/12] RB1/QCM2290 features
On Wed, 29 Nov 2023 15:43:57 +0100, Konrad Dybcio wrote: > This series brings: > - interconnect plumbing > - display setup > > for QCM2290/QRB2210 and > > - CAN bus controller > - HDMI display > - wifi fw variant name > > [...] Applied, thanks! [04/12] dt-bindings: firmware: qcom,scm: Allow interconnect for everyone commit: 56fdc35ef067c8dffee22038dd3a84bb3fa6d2a4 Best regards, -- Bjorn Andersson
Re: [PATCH 3/3] arm64: dts: qcom: sm8650: Add DisplayPort device nodes
On Thu, Dec 07, 2023 at 05:37:19PM +0100, Neil Armstrong wrote:
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> b/arch/arm64/boot/dts/qcom/sm8650.dtsi
[..]
> +
> + mdss_dp0: displayport-controller@af54000 {
> + compatible = "qcom,sm8650-dp";
> + reg = <0 0xaf54000 0 0x200>,
> + <0 0xaf54200 0 0x200>,
> + <0 0xaf55000 0 0xc00>,
> + <0 0xaf56000 0 0x400>,
> + <0 0xaf57000 0 0x400>;
> +
> + interrupts-extended = < 12>;
> +
> + clocks = < DISP_CC_MDSS_AHB_CLK>,
> + < DISP_CC_MDSS_DPTX0_AUX_CLK>,
> + < DISP_CC_MDSS_DPTX0_LINK_CLK>,
> + <
> DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>,
> + <
> DISP_CC_MDSS_DPTX0_PIXEL0_CLK>;
> + clock-names = "core_iface",
> + "core_aux",
> + "ctrl_link",
> + "ctrl_link_iface",
> + "stream_pixel";
> +
> + assigned-clocks = <
> DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>,
> + <
> DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>;
> + assigned-clock-parents = <_dp_qmpphy
> QMP_USB43DP_DP_LINK_CLK>,
> + <_dp_qmpphy
> QMP_USB43DP_DP_VCO_DIV_CLK>;
> +
> + operating-points-v2 = <_opp_table>;
> +
> + power-domains = < RPMHPD_MX>;
Are you sure the DP TX block sits in MX? I'd expect this to be
RPMHPD_MMCX, and then the PHY partially in MX...
> +
> + phys = <_dp_qmpphy QMP_USB43DP_DP_PHY>;
> + phy-names = "dp";
> +
> + #sound-dai-cells = <0>;
> +
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + mdss_dp0_in: endpoint {
> + remote-endpoint =
> <_intf0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + mdss_dp0_out: endpoint {
> + };
> + };
> + };
> +
> + dp_opp_table: opp-table {
Is there any reason why we keep sorting 'o' after 'p' in these nodes?
Regards,
Bjorn
Re: [PATCH v2 0/4] Adreno 643 + fixes
On Mon, 20 Nov 2023 13:12:51 +0100, Konrad Dybcio wrote: > as it says on the can > > drm/msm patches for Rob > arm64 patches for linux-arm-msm > > for use with https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25408 > > [...] Applied, thanks! [1/4] arm64: dts: qcom: sc7280: Add ZAP shader support commit: 0ab1bef0b7c359e672cc2b8d51f0179cefa369fc [2/4] arm64: dts: qcom: sc7280: Fix up GPU SIDs commit: 94085049fdad7a36fe14dd55e72e712fe55d6bca [3/4] arm64: dts: qcom: sc7280: Mark Adreno SMMU as DMA coherent commit: 31edad478534186a2718be9206ce7b19f2735f6e [4/4] arm64: dts: qcom: sc7280: Add 0xac Adreno speed bin commit: 6a7f8c635dab30233df93b5566d4169ed956b71b Best regards, -- Bjorn Andersson
Re: [PATCH v3 2/2] drm/msm/dpu: Add mutex lock in control vblank irq
On Mon, Dec 04, 2023 at 11:22:24AM -0800, Abhinav Kumar wrote: > > > On 12/3/2023 7:31 PM, Bjorn Andersson wrote: > > On Fri, Dec 01, 2023 at 11:43:36AM -0800, Abhinav Kumar wrote: > > > > > > > > > On 12/1/2023 8:22 AM, Bjorn Andersson wrote: > > > > On Fri, Dec 01, 2023 at 10:34:50AM +0200, Dmitry Baryshkov wrote: > > > > > On Fri, 1 Dec 2023 at 05:47, Bjorn Andersson > > > > > wrote: > > > > > > On Thu, Nov 30, 2023 at 05:40:55PM -0800, Paloma Arellano wrote: > > > > [..] > > > > > > > @@ -2386,6 +2390,7 @@ struct drm_encoder *dpu_encoder_init(struct > > > > > > > drm_device *dev, > > > > > > > dpu_enc->enabled = false; > > > > > > > mutex_init(_enc->enc_lock); > > > > > > > mutex_init(_enc->rc_lock); > > > > > > > + mutex_init(_enc->vblank_ctl_lock); > > > > > > > > > > > > Is this somehow propagated to multiple different dpu_encoder_phys > > > > > > instances, or why do you need to initialize it here and pass the > > > > > > pointer > > > > > > through 2 different intermediate structures before assigning it to > > > > > > phys_enc->vblank_ctl_lock below? > > > > > > > > > > Yes, there can be two phys_enc instances for a single encoder, so this > > > > > part is fine. > > > > > > > > > > > > > Thanks for the clarification, Dmitry. Sounds like it make sense then. > > > > > > > > But, if I read the code correctly the two instances will have separate > > > > vblank_refcount copies, and the dpu_core_irq_*() interface does mutual > > > > exclusion within. So why do we need shared mutual exclusion between the > > > > two? (This is where a proper description of the problem in the commit > > > > message would have been very helpful) > > > > > > > > > > Are you suggesting we just have one vblank_ctl_lock per encoder and not > > > have > > > one vblank_ctl_lock per phys encoder? I cannot think of a display specific > > > reason for that other than just the SW layout. > > > > > > The reason its like this today is that control_vblank_irq is an encoder > > > phys > > > op because it does different things based on the type of encoder. > > > > > > Because its an encoder phys op, it has the vblank_ctl_lock at the phys > > > structure and not the encoder one. > > > > > > Its just more about how the phys op is defined that each phys op operates > > > on > > > its phys's structure. > > > > > > Generally, if we have one encoder with two physical encoders we anyways > > > bail > > > out early for the other encoder so this is mostly a no-op for the slave > > > phys > > > encoder. > > > > > > Please take a look at below return point. > > > > > > 715 /* Slave encoders don't report vblank */ > > > 716 if (!sde_encoder_phys_vid_is_master(phys_enc)) > > > 717 goto end; > > > 718 > > > > > > So technically its still providing protection for the same phys encoder > > > but > > > the catch is this control_vblank_irq can get called from different threads > > > hence we need exclusion. > > > > > > > The way I understand the code is that the atomic is used to refcount > > when to enable/disable the interrupt, and the new lock protects this > > refcount during concurrent updates. I have no concerns with this part. > > > > Correct. > > > > > What I'm seeing is that the refcount it per phys_enc, and as such there > > would be no reason to have a common mutex to protect the two independent > > refcounts. > > > > But I'm probably misunderstanding something here... > > > > There is no reason to have a common mutex to protect the two independent > refcounts. In fact, there is no need to even have two independent refcounts > because whenever we have one encoder with two physical encoders, we use only > the master physical encoder for vblanks like I pointed above. > > The only reason we have it like this is because today the vblank_refcount is > part of phys_enc so the mutex handle is also now a part of it. > > Do you think if we move both the mutex and the vblank_refcount to the > dpu_encoder from the dpu_encoder_phys and maintain the mutex at that level > it will be less confusing for you? > The two functions operate on dpu_encoder_phys objects, and as you say above the two instances doesn't need to be handled under shared mutual exclusion. Moving the serialization mechanism to dpu_encoder seems like it would create an entanglement, for the sake of making the lock common. If nothing else this would act as documentation to me that the two functions are intertwined somehow. I was rather hoping that we'd move the mutex_init() to dpu_encoder_phys_init() and avoid passing a reference around in unrelated parts of the code just to set up the sharing, if that's not necessary. Regards, Bjorn > I am fine with that. > > > Regards, > > Bjorn > >
Re: [PATCH v3 2/2] drm/msm/dpu: Add mutex lock in control vblank irq
On Fri, Dec 01, 2023 at 11:43:36AM -0800, Abhinav Kumar wrote: > > > On 12/1/2023 8:22 AM, Bjorn Andersson wrote: > > On Fri, Dec 01, 2023 at 10:34:50AM +0200, Dmitry Baryshkov wrote: > > > On Fri, 1 Dec 2023 at 05:47, Bjorn Andersson > > > wrote: > > > > On Thu, Nov 30, 2023 at 05:40:55PM -0800, Paloma Arellano wrote: > > [..] > > > > > @@ -2386,6 +2390,7 @@ struct drm_encoder *dpu_encoder_init(struct > > > > > drm_device *dev, > > > > >dpu_enc->enabled = false; > > > > >mutex_init(_enc->enc_lock); > > > > >mutex_init(_enc->rc_lock); > > > > > + mutex_init(_enc->vblank_ctl_lock); > > > > > > > > Is this somehow propagated to multiple different dpu_encoder_phys > > > > instances, or why do you need to initialize it here and pass the pointer > > > > through 2 different intermediate structures before assigning it to > > > > phys_enc->vblank_ctl_lock below? > > > > > > Yes, there can be two phys_enc instances for a single encoder, so this > > > part is fine. > > > > > > > Thanks for the clarification, Dmitry. Sounds like it make sense then. > > > > But, if I read the code correctly the two instances will have separate > > vblank_refcount copies, and the dpu_core_irq_*() interface does mutual > > exclusion within. So why do we need shared mutual exclusion between the > > two? (This is where a proper description of the problem in the commit > > message would have been very helpful) > > > > Are you suggesting we just have one vblank_ctl_lock per encoder and not have > one vblank_ctl_lock per phys encoder? I cannot think of a display specific > reason for that other than just the SW layout. > > The reason its like this today is that control_vblank_irq is an encoder phys > op because it does different things based on the type of encoder. > > Because its an encoder phys op, it has the vblank_ctl_lock at the phys > structure and not the encoder one. > > Its just more about how the phys op is defined that each phys op operates on > its phys's structure. > > Generally, if we have one encoder with two physical encoders we anyways bail > out early for the other encoder so this is mostly a no-op for the slave phys > encoder. > > Please take a look at below return point. > > 715 /* Slave encoders don't report vblank */ > 716 if (!sde_encoder_phys_vid_is_master(phys_enc)) > 717 goto end; > 718 > > So technically its still providing protection for the same phys encoder but > the catch is this control_vblank_irq can get called from different threads > hence we need exclusion. > The way I understand the code is that the atomic is used to refcount when to enable/disable the interrupt, and the new lock protects this refcount during concurrent updates. I have no concerns with this part. What I'm seeing is that the refcount it per phys_enc, and as such there would be no reason to have a common mutex to protect the two independent refcounts. But I'm probably misunderstanding something here... Regards, Bjorn
Re: (subset) [PATCH v3 00/12] RB1/QCM2290 features
On Wed, 29 Nov 2023 15:43:57 +0100, Konrad Dybcio wrote: > This series brings: > - interconnect plumbing > - display setup > > for QCM2290/QRB2210 and > > - CAN bus controller > - HDMI display > - wifi fw variant name > > [...] Applied, thanks! [06/12] arm64: dts: qcom: sc7180: Add the missing MDSS icc path commit: 8786398f8686d1a4267ab52f830b25f17e6d62fc [07/12] arm64: dts: qcom: sc7280: Add the missing MDSS icc path commit: c657056d99878c8a8ea84d5d4a9101bcb90b47f2 [08/12] arm64: dts: qcom: qcm2290: Add display nodes commit: a2b32096709dbf4af02675d98356a9d3ad86ff05 [09/12] arm64: dts: qcom: qcm2290: Hook up interconnects commit: 5b970ff0193d67da4a8d2d5fda50dd8ddb50a71e [10/12] arm64: dts: qcom: qrb2210-rb1: Set up HDMI commit: 616eda24edd48b8b56516886c51d211fbfd2679b [11/12] arm64: dts: qcom: qrb2210-rb1: Enable CAN bus controller commit: 252bc7ad359478dba8d77bce9502f2cc7bb547a3 [12/12] arm64: dts: qcom: qrb2210-rb1: add wifi variant property commit: b6a56a5a25d6273729b2b5139d58e3d390318ed2 Best regards, -- Bjorn Andersson
Re: [PATCH] drm/msm/dpu: Add missing safe_lut_tbl in sc8180x catalog
On Fri, Dec 01, 2023 at 09:06:45AM +0100, Johan Hovold wrote:
> On Thu, Nov 30, 2023 at 04:35:01PM -0800, Bjorn Andersson wrote:
> > Similar to SC8280XP, the misconfigured SAFE logic causes rather
> > significant delays in __arm_smmu_tlb_sync(), resulting in poor
> > performance for things such as USB.
> >
> > Introduce appropriate SAFE values for SC8180X to correct this.
> >
> > Fixes: f3af2d6ee9ab ("drm/msm/dpu: Add SC8180x to hw catalog")
>
> Missing CC stable tag?
>
I figured it doesn't matter in practice, there's still a few issues left
preventing people from just running a stable kernel on this platform.
But it would be the right thing to do...
Cc: sta...@vger.kernel.org
Regards,
Bjorn
> > Signed-off-by: Bjorn Andersson
>
> Johan
>
Re: [PATCH v3 2/2] drm/msm/dpu: Add mutex lock in control vblank irq
On Fri, Dec 01, 2023 at 10:34:50AM +0200, Dmitry Baryshkov wrote: > On Fri, 1 Dec 2023 at 05:47, Bjorn Andersson > wrote: > > On Thu, Nov 30, 2023 at 05:40:55PM -0800, Paloma Arellano wrote: [..] > > > @@ -2386,6 +2390,7 @@ struct drm_encoder *dpu_encoder_init(struct > > > drm_device *dev, > > > dpu_enc->enabled = false; > > > mutex_init(_enc->enc_lock); > > > mutex_init(_enc->rc_lock); > > > + mutex_init(_enc->vblank_ctl_lock); > > > > Is this somehow propagated to multiple different dpu_encoder_phys > > instances, or why do you need to initialize it here and pass the pointer > > through 2 different intermediate structures before assigning it to > > phys_enc->vblank_ctl_lock below? > > Yes, there can be two phys_enc instances for a single encoder, so this > part is fine. > Thanks for the clarification, Dmitry. Sounds like it make sense then. But, if I read the code correctly the two instances will have separate vblank_refcount copies, and the dpu_core_irq_*() interface does mutual exclusion within. So why do we need shared mutual exclusion between the two? (This is where a proper description of the problem in the commit message would have been very helpful) Regards, Bjorn
Re: [PATCH v3 2/2] drm/msm/dpu: Add mutex lock in control vblank irq
On Thu, Nov 30, 2023 at 05:40:55PM -0800, Paloma Arellano wrote:
> Add a missing mutex lock to control vblank irq. Thus prevent race
> conditions when registering/unregistering the irq callback.
>
I'm guessing that the mutex is needed because vblank_refcount, while
being an atomic_t, doesn't actually provide any protection during
concurrency?
I also tried to follow the calls backwards, but I'm uncertain how you
end up here concurrently.
When wrapped in proper mutual exclusion, can't vblank_refcount just be
turned into an "int"...given that you're not actually able to rely on
it's atomic behavior anyways...
So, please rewrite the commit message with a detailed description of how
the concurrency happens, and please review if vblank_refcount should be
an atomic at all...
> v2: Slightly changed wording of commit message
> v3: Mistakenly did not change wording in last version. It is done now.
>
> Signed-off-by: Paloma Arellano
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 6 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 ++
> 4 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 1cf7ff6caff4e..19ff7d1d5ccad 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -119,6 +119,8 @@ enum dpu_enc_rc_states {
> * Virtual encoder defers as much as possible to the physical encoders.
> * Virtual encoder registers itself with the DRM Framework as the encoder.
> * @base:drm_encoder base class for registration with DRM
> + * @vblank_ctl_lock: Vblank ctl mutex lock to protect physical encoder
> + * for IRQ purposes
I think this protects vblank_refcount, so state that instead of the
vague "for IRQ purposes".
> * @enc_spinlock:Virtual-Encoder-Wide Spin Lock for IRQ purposes
> * @enabled: True if the encoder is active, protected by enc_lock
> * @num_phys_encs: Actual number of physical encoders contained.
> @@ -166,6 +168,7 @@ enum dpu_enc_rc_states {
> */
> struct dpu_encoder_virt {
> struct drm_encoder base;
> + struct mutex vblank_ctl_lock;
> spinlock_t enc_spinlock;
>
> bool enabled;
> @@ -2255,6 +2258,7 @@ static int dpu_encoder_setup_display(struct
> dpu_encoder_virt *dpu_enc,
> phys_params.dpu_kms = dpu_kms;
> phys_params.parent = _enc->base;
> phys_params.enc_spinlock = _enc->enc_spinlock;
> + phys_params.vblank_ctl_lock = _enc->vblank_ctl_lock;
>
> WARN_ON(disp_info->num_of_h_tiles < 1);
>
> @@ -2386,6 +2390,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device
> *dev,
> dpu_enc->enabled = false;
> mutex_init(_enc->enc_lock);
> mutex_init(_enc->rc_lock);
> + mutex_init(_enc->vblank_ctl_lock);
Is this somehow propagated to multiple different dpu_encoder_phys
instances, or why do you need to initialize it here and pass the pointer
through 2 different intermediate structures before assigning it to
phys_enc->vblank_ctl_lock below?
>
> ret = dpu_encoder_setup_display(dpu_enc, dpu_kms, disp_info);
> if (ret)
> @@ -2495,6 +2500,7 @@ void dpu_encoder_phys_init(struct dpu_encoder_phys
> *phys_enc,
> phys_enc->dpu_kms = p->dpu_kms;
> phys_enc->split_role = p->split_role;
> phys_enc->enc_spinlock = p->enc_spinlock;
> + phys_enc->vblank_ctl_lock = p->vblank_ctl_lock;
Could you not just mutex_init() the one and only vblank_ctl_lock here?
> phys_enc->enable_state = DPU_ENC_DISABLED;
>
> atomic_set(_enc->vblank_refcount, 0);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index 6f04c3d56e77c..5691bf6b82ee6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -155,6 +155,8 @@ enum dpu_intr_idx {
> * @hw_wb: Hardware interface to the wb registers
> * @dpu_kms: Pointer to the dpu_kms top level
> * @cached_mode: DRM mode cached at mode_set time, acted on in enable
> + * @vblank_ctl_lock: Vblank ctl mutex lock to protect physical encoder
> + * for IRQ purposes
Same here.
> * @enabled: Whether the encoder has enabled and running a mode
> * @split_role: Role to play in a split-panel configuration
> * @intf_mode: Interface mode
> @@ -183,6 +185,7 @@ struct dpu_encoder_phys {
> struct dpu_hw_wb *hw_wb;
> struct dpu_kms *dpu_kms;
> struct drm_display_mode cached_mode;
> + struct mutex *vblank_ctl_lock;
> enum dpu_enc_split_role split_role;
> enum dpu_intf_mode intf_mode;
> spinlock_t
Re: [PATCH] drm/msm/dpu: Add missing safe_lut_tbl in sc8180x catalog
On Thu, Nov 30, 2023 at 04:35:01PM -0800, Bjorn Andersson wrote:
> Similar to SC8280XP, the misconfigured SAFE logic causes rather
> significant delays in __arm_smmu_tlb_sync(), resulting in poor
> performance for things such as USB.
>
> Introduce appropriate SAFE values for SC8180X to correct this.
>
> Fixes: f3af2d6ee9ab ("drm/msm/dpu: Add SC8180x to hw catalog")
Reported-by: Anton Bambura
> Signed-off-by: Bjorn Andersson
> ---
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> index e07f4c8c25b9..9ffc8804a6fc 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> @@ -367,6 +367,7 @@ static const struct dpu_perf_cfg sc8180x_perf_data = {
> .min_llcc_ib = 80,
> .min_dram_ib = 80,
> .danger_lut_tbl = {0xf, 0x, 0x0},
> + .safe_lut_tbl = {0xfff0, 0xf000, 0x},
> .qos_lut_tbl = {
> {.nentry = ARRAY_SIZE(sc7180_qos_linear),
> .entries = sc7180_qos_linear
>
> ---
> base-commit: 3cd3fe06ff81cfb3a969acb12a56796cff5af23d
> change-id: 20231130-sc8180x-dpu-safe-lut-ffd0df221d67
>
> Best regards,
> --
> Bjorn Andersson
>
[PATCH] drm/msm/dpu: Add missing safe_lut_tbl in sc8180x catalog
Similar to SC8280XP, the misconfigured SAFE logic causes rather
significant delays in __arm_smmu_tlb_sync(), resulting in poor
performance for things such as USB.
Introduce appropriate SAFE values for SC8180X to correct this.
Fixes: f3af2d6ee9ab ("drm/msm/dpu: Add SC8180x to hw catalog")
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
index e07f4c8c25b9..9ffc8804a6fc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
@@ -367,6 +367,7 @@ static const struct dpu_perf_cfg sc8180x_perf_data = {
.min_llcc_ib = 80,
.min_dram_ib = 80,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfff0, 0xf000, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear
---
base-commit: 3cd3fe06ff81cfb3a969acb12a56796cff5af23d
change-id: 20231130-sc8180x-dpu-safe-lut-ffd0df221d67
Best regards,
--
Bjorn Andersson
[PATCH] drm/msm/adreno: Fix A680 chip id
The only A680 referenced from DeviceTree is patch level 1, which since
commit '90b593ce1c9e ("drm/msm/adreno: Switch to chip-id for identifying
GPU")' isn't a known chip id.
Correct the chip id to allow the A680 to be recognized again.
Fixes: 90b593ce1c9e ("drm/msm/adreno: Switch to chip-id for identifying GPU")
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index f62ab5257e66..2ce7d7b1690d 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -464,7 +464,7 @@ static const struct adreno_info gpulist[] = {
{ 190, 1 },
),
}, {
- .chip_ids = ADRENO_CHIP_IDS(0x0608),
+ .chip_ids = ADRENO_CHIP_IDS(0x06080001),
.family = ADRENO_6XX_GEN2,
.revn = 680,
.fw = {
---
base-commit: 3cd3fe06ff81cfb3a969acb12a56796cff5af23d
change-id: 20231130-adreno-a680-1-639717a53b56
Best regards,
--
Bjorn Andersson
[PATCH] drm/msm/dpu: Add missing safe_lut_tbl in sc8280xp catalog
During USB transfers on the SC8280XP __arm_smmu_tlb_sync() is seen to
typically take 1-2ms to complete. As expected this results in poor
performance, something that has been mitigated by proposing running the
iommu in non-strict mode (boot with iommu.strict=0).
This turns out to be related to the SAFE logic, and programming the QOS
SAFE values in the DPU (per suggestion from Rob and Doug) reduces the
TLB sync time to below 10us, which means significant less time spent
with interrupts disabled and a significant boost in throughput.
Fixes: 4a352c2fc15a ("drm/msm/dpu: Introduce SC8280XP")
Cc: sta...@vger.kernel.org
Suggested-by: Doug Anderson
Suggested-by: Rob Clark
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
index 1ccd1edd693c..4c0528794e7a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
@@ -406,6 +406,7 @@ static const struct dpu_perf_cfg sc8280xp_perf_data = {
.min_llcc_ib = 0,
.min_dram_ib = 80,
.danger_lut_tbl = {0xf, 0x, 0x0},
+ .safe_lut_tbl = {0xfe00, 0xfe00, 0x},
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc8180x_qos_linear),
.entries = sc8180x_qos_linear
---
base-commit: c503e3eec382ac708ee7adf874add37b77c5d312
change-id: 20231030-sc8280xp-dpu-safe-lut-9769027b8452
Best regards,
--
Bjorn Andersson
Re: [PATCH v5 5/6] soc: qcom: pmic-glink: switch to DRM_AUX_HPD_BRIDGE
On Thu, Oct 26, 2023 at 01:28:06AM +0300, Dmitry Baryshkov wrote: > Use the freshly defined DRM_AUX_HPD_BRIDGE instead of open-coding the > same functionality for the DRM bridge chain termination. > Reviewed-by: Bjorn Andersson Acked-by: Bjorn Andersson Regards, Bjorn
Re: [PATCH v2 0/4] arm64: dts: qcom: qrb5165-rb5: enable DP support
On Thu, 17 Aug 2023 17:59:36 +0300, Dmitry Baryshkov wrote: > Implement DisplayPort support for the Qualcomm RB5 platform. > > Note: while testing this, I had link training issues with several > dongles with DP connectors. Other DisplayPort-USB-C dongles (with HDMI > or VGA connectors) work perfectly. > > Dependencies: [1] > Soft-dependencies: [2], [3] > > [...] Applied, thanks! [1/4] arm64: dts: qcom: sm8250: Add DisplayPort device node commit: 956aa24b16350a50d3a6beb9237bc35aa2f447d6 [2/4] arm64: dts: qcom: qrb5165-rb5: add onboard USB-C redriver commit: d342e1c993bd7589cad9d2da099c6a9c652ecb9f [3/4] arm64: dts: qcom: qrb5165-rb5: enable displayport controller commit: 96387ee7534dc449be35a9bb98b7668da2bed545 [4/4] arm64: dts: qcom: qrb5165-rb5: enable DP altmode commit: b3dea914127e9065df003002ed13a2ef40d19877 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH 0/5] arm64: dts: qcom: qrb5165-rb5: enable DP support
On Sun, 09 Jul 2023 07:19:21 +0300, Dmitry Baryshkov wrote: > Implement DisplayPort support for the Qualcomm RB5 platform. > > Note: while testing this, I had link training issues with several > dongles with DP connectors. Other DisplayPort-USB-C dongles (with HDMI > or VGA connectors) work perfectly. > > Dependencies: [1] > Soft-dependencies: [2], [3] > > [...] Applied, thanks! [2/5] arm64: dts: qcom: sm8250: Add DisplayPort device node commit: 956aa24b16350a50d3a6beb9237bc35aa2f447d6 [3/5] arm64: dts: qcom: qrb5165-rb5: add onboard USB-C redriver commit: d342e1c993bd7589cad9d2da099c6a9c652ecb9f [4/5] arm64: dts: qcom: qrb5165-rb5: enable displayport controller commit: 96387ee7534dc449be35a9bb98b7668da2bed545 [5/5] arm64: dts: qcom: qrb5165-rb5: enable DP altmode commit: b3dea914127e9065df003002ed13a2ef40d19877 Best regards, -- Bjorn Andersson
Re: [PATCH v2 1/4] arm64: dts: qcom: sm8250: Add DisplayPort device node
On Thu, Aug 17, 2023 at 05:59:37PM +0300, Dmitry Baryshkov wrote:
> Declare the displayport controller present on the Qualcomm SM8250 SoC.
>
> Signed-off-by: Dmitry Baryshkov
> ---
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 89
> 1 file changed, 89 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index eb00bbd3e1f3..8d705a1713fb 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -3638,6 +3638,8 @@ port@1 {
>
> port@2 {
> reg = <2>;
> +
> + usb_1_qmpphy_dp_in: endpoint {};
> };
> };
> };
> @@ -4405,6 +4407,14 @@ dpu_intf2_out: endpoint {
> remote-endpoint =
> <_dsi1_in>;
> };
> };
> +
> + port@2 {
> + reg = <2>;
> +
> + dpu_intf0_out: endpoint {
> + remote-endpoint =
> <_dp_in>;
> + };
> + };
> };
>
> mdp_opp_table: opp-table {
> @@ -4432,6 +4442,85 @@ opp-46000 {
> };
> };
>
> + mdss_dp: displayport-controller@ae9 {
displayport-controller does not seem to be a valid child node of the
sm8250 mdss. Please make sure that the binding is updated, if not
already done.
Thanks,
Bjorn
Re: (subset) [PATCH v4 00/17] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel
On Sun, 23 Jul 2023 18:08:38 +0200, Marijn Suijten wrote: > Bring up the SM6125 DPU now that all preliminary series (such as INTF > TE) have been merged (for me to test the hardware properly), and most > other conflicting work (barring ongoing catalog *improvements*) has made > its way in as well or is still being discussed. > > The second part of the series complements that by immediately utilizing > this hardware in DT, and even enabling the MDSS/DSI nodes complete with > a 6.0" 1080x2520 panel for Sony's Seine PDX201 (Xperia 10 II). > > [...] Applied, thanks! [02/17] arm64: dts: qcom: sm6125: Pad APPS IOMMU address to 8 characters commit: 310cdafc4a56827d1aeda7cc297939034adb8f99 [03/17] arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg commit: 3d06cee2249f4764f01a9f602ec1cc1bf4562ca6 [14/17] arm64: dts: qcom: sm6125: Switch fixed xo_board clock to RPM XO clock commit: cbe82d7d0b149aa9c0c000f7ffd2b18bfd248d35 [15/17] arm64: dts: qcom: sm6125: Add dispcc node commit: 491ec067c3e6d382de1583b7f5b1095ddea2 [16/17] arm64: dts: qcom: sm6125: Add display hardware nodes commit: 0865d23a02260a76963bd18d9ae603e77cdd0eba [17/17] arm64: dts: qcom: sm6125-seine: Configure MDSS, DSI and panel commit: 5078dfe3c5c7b8d2d6494c26de81a4f3d4a5a3d7 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v4 00/11] drm/panel and i2c-hid: Allow panels and touchscreens to power sequence together
On Thu, 27 Jul 2023 10:16:27 -0700, Douglas Anderson wrote:
> The big motivation for this patch series is mostly described in the patch
> ("drm/panel: Add a way for other devices to follow panel state"), but to
> quickly summarize here: for touchscreens that are connected to a panel we
> need the ability to power sequence the two device together. This is not a
> new need, but so far we've managed to get by through a combination of
> inefficiency, added costs, or perhaps just a little bit of brokenness.
> It's time to do better. This patch series allows us to do better.
>
> [...]
Applied, thanks!
[11/11] arm64: dts: qcom: sc7180: Link trogdor touchscreens to the panels
commit: 989aac9dea7fcfc33b5eedc4ae44abbf71460a4d
Best regards,
--
Bjorn Andersson
Re: [RFC PATCH v1 07/12] soc: qcom: pmic_glink_altmode: report that this is a Type-C connector
On Mon, Sep 04, 2023 at 12:41:45AM +0300, Dmitry Baryshkov wrote: > Set the bridge's path property to point out that this connector is > wrapped into the Type-C port. > > We can not really identify the exact Type-C port because it is > registered separately, by another driver, which is not mandatory and the > corresponding device is not even present on some of platforms, like > sc8180x or sm8350. Thus we use the shortened version of the PATH, which > includes just the 'typec:' part. How would a properly resolved path look like? As with the other patch, I'm okay with this going through the USB tree. Acked-by: Bjorn Andersson Regards, Bjorn
Re: [RFC PATCH v1 06/12] soc: qcom: pmic_glink_altmode: fix DRM connector type
On Mon, Sep 04, 2023 at 12:41:44AM +0300, Dmitry Baryshkov wrote: > During discussions regarding USB-C vs native DisplayPort it was pointed > out that other implementations (i915, AMD) are using > DRM_MODE_CONNECTOR_DisplayPort for both native and USB-C-wrapped DP > output. Follow this example and make the pmic_glink_altmode driver also > report DipslayPort connector rather than the USB one. I started off with this, but on devices with both USB Type-C and (native) DisplayPort, it seemed much more reasonable to make the distinction; and thereby get the outputs named "USB-n". Similarly, it looks quite reasonable that the output on my laptop are eDP-1, USB-1 and USB-2. But, if this is not the way its intended to be used, feel free to pick this together with the other patches. Acked-by: Bjorn Andersson Regards, Bjorn
Re: [RFC PATCH v1 00/12] drm,usb/typec: uABI for USB-C DisplayPort connectors
On Mon, Sep 04, 2023 at 12:41:38AM +0300, Dmitry Baryshkov wrote: > During the discussion regarding DisplayPort wrapped in the USB-C > connectors (via the USB-C altmode) it was pointed out that currently > there is no good way to let userspace know if the DRM connector in > question is the 'native' DP connector or if it is the USB-C connector. > > An attempt to use DRM_MODE_CONNECTOR_USB for such connectors was > declined, as existing DP drivers (i915, AMD) use > DRM_MODE_CONNECTOR_DisplayPort. New drivers should behave in the same > way. > Sorry, didn't see the commit message before posting my complaint about USB -> DisplayPort. > An attempt to use subconnector property was also declined. It is defined > to the type of the DP dongle connector rather than the host connector. > > This attempt targets reusing the connector's PATH property. Currently > this property is only used for the DP MST connectors. This patchset > reuses it to point out to the corresponding registered typec port > device. > Still interested in understanding how the path string should look like. Is the path expected to be consumed by machine, or is it only there for human convenience? Regards, Bjorn
Re: (subset) [PATCH v6 1/2] dt-bindings: display/msm: mdss-common: add memory-region property
On Wed, 26 Jul 2023 18:57:18 +0530, Amit Pundir wrote: > Add and document the reserved memory region property in the > mdss-common schema. > > For now (sdm845-db845c), it points to a framebuffer memory > region reserved by the bootloader for splash screen. > > > [...] Applied, thanks! [2/2] arm64: dts: qcom: sdm845-db845c: Mark cont splash memory region as reserved commit: 110e70fccce4f22b53986ae797d665ffb1950aa6 Best regards, -- Bjorn Andersson
Re: [PATCH -next] drm/msm/adreno: adreno_gpu: Switch to memdup_user_nul() helper
On Thu, Aug 10, 2023 at 08:04:24PM +0800, Ruan Jinjie wrote: > Use memdup_user_nul() helper instead of open-coding to simplify the code. > > Signed-off-by: Ruan Jinjie Reviewed-by: Bjorn Andersson Regards, Bjorn
Re: [PATCH 0/5] arm64: dts: qcom: qrb5165-rb5: enable DP support
On Tue, Jul 18, 2023 at 09:09:41AM +0300, Dmitry Baryshkov wrote: > On 18/07/2023 07:37, Bjorn Andersson wrote: > > On Sun, Jul 09, 2023 at 07:19:21AM +0300, Dmitry Baryshkov wrote: > > > Implement DisplayPort support for the Qualcomm RB5 platform. > > > > > > Note: while testing this, I had link training issues with several > > > dongles with DP connectors. Other DisplayPort-USB-C dongles (with HDMI > > > or VGA connectors) work perfectly. > > > > > > Dependencies: [1] > > > Soft-dependencies: [2], [3] > > > > > > [1] > > > https://lore.kernel.org/linux-arm-msm/20230515133643.3621656-1-bryan.odonog...@linaro.org/ > > > > I'm not able to find a version of this series ready to be merged, can > > you please help me find it? > > This = Bryan's? I have posted some (small) feedback regarding v8. You also > had issues with orientation switching bindings, etc. So there should be v9. > Right, Bryan's series. You linked to v8 which has requests for changes, and I can't find v9. Am I just bad at searching? Regards, Bjorn
Re: [Freedreno] [PATCH] drm/msm: Check for the GPU IOMMU during bind
On Mon, Jul 10, 2023 at 03:20:44AM +0530, Akhil P Oommen wrote: > On Fri, Jul 07, 2023 at 08:27:18PM +0300, Dmitry Baryshkov wrote: > > > > On 07/07/2023 18:03, Jordan Crouse wrote: > > > On Thu, Jul 06, 2023 at 09:55:13PM +0300, Dmitry Baryshkov wrote: > > > > > > > > On 10/03/2023 00:20, Jordan Crouse wrote: > > > > > While booting with amd,imageon on a headless target the GPU probe was > > > > > failing with -ENOSPC in get_pages() from msm_gem.c. > > > > > > > > > > Investigation showed that the driver was using the default 16MB VRAM > > > > > carveout because msm_use_mmu() was returning false since headless > > > > > devices > > > > > use a dummy parent device. Avoid this by extending the existing > > > > > is_a2xx > > > > > priv member to check the GPU IOMMU state on all platforms and use that > > > > > check in msm_use_mmu(). > > > > > > > > > > This works for memory allocations but it doesn't prevent the VRAM > > > > > carveout > > > > > from being created because that happens before we have a chance to > > > > > check > > > > > the GPU IOMMU state in adreno_bind. > > > > > > > > > > There are a number of possible options to resolve this but none of > > > > > them are > > > > > very clean. The easiest way is to likely specify vram=0 as module > > > > > parameter > > > > > on headless devices so that the memory doesn't get wasted. > > > > > > > > This patch was on my plate for quite a while, please excuse me for > > > > taking it so long. > > > > > > No worries. I'm also chasing a bunch of other stuff too. > > > > > > > I see the following problem with the current code. We have two different > > > > instances than can access memory: MDP/DPU and GPU. And each of them can > > > > either have or miss the MMU. > > > > > > > > For some time I toyed with the idea of determining whether the allocated > > > > BO is going to be used by display or by GPU, but then I abandoned it. We > > > > can have display BOs being filled by GPU, so handling it this way would > > > > complicate things a lot. > > > > > > > > This actually rings a tiny bell in my head with the idea of splitting > > > > the display and GPU parts to two different drivers, but I'm not sure > > > > what would be the overall impact. > > > > > > As I now exclusively work on headless devices I would be 100% for this, > > > but I'm sure that our laptop friends might not agree :) > > > > I do not know here. This is probably a question to Rob, as he better > > understands the interaction between GPU and display parts of the userspace. > > I fully support this if it is feasible. > I second this. > In our architecture, display and GPU are completely independent subsystems. > Like Jordan mentioned, there are IOT products without display. And I wouldn't > be surprised if there is a product with just display and uses software > rendering. > And we have SA8295P/SA8540P with two MDSS instances and one GPU. Regards, Bjorn
Re: [PATCH 0/5] arm64: dts: qcom: qrb5165-rb5: enable DP support
On Sun, Jul 09, 2023 at 07:19:21AM +0300, Dmitry Baryshkov wrote: > Implement DisplayPort support for the Qualcomm RB5 platform. > > Note: while testing this, I had link training issues with several > dongles with DP connectors. Other DisplayPort-USB-C dongles (with HDMI > or VGA connectors) work perfectly. > > Dependencies: [1] > Soft-dependencies: [2], [3] > > [1] > https://lore.kernel.org/linux-arm-msm/20230515133643.3621656-1-bryan.odonog...@linaro.org/ I'm not able to find a version of this series ready to be merged, can you please help me find it? Regards, Bjorn
Re: (subset) [PATCH v2 0/8] MDSS reg bus interconnect
On Wed, 12 Jul 2023 15:11:37 +0300, Dmitry Baryshkov wrote: > Per agreement with Konrad, picked up this patch series. > > Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's > another path that needs to be handled to ensure MDSS functions properly, > namely the "reg bus", a.k.a the CPU-MDSS interconnect. > > Gating that path may have a variety of effects. from none to otherwise > inexplicable DSI timeouts. > > [...] Applied, thanks! [8/8] arm64: dts: qcom: sm8450: provide MDSS cfg interconnect commit: 4e125191e6cb00d6c3f3a8e1b67fd242e639b3c3 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v3 0/7] Display support for MSM8226
On Thu, 01 Jun 2023 19:00:07 +0200, Luca Weiss wrote: > This series adds the required configs for MDP5 and DSI blocks that are > needed for MDSS on MSM8226. Finally we can add the new nodes into the > dts. > > Tested on apq8026-lg-lenok and msm8926-htc-memul. > > > [...] Applied, thanks! [7/7] ARM: dts: qcom: msm8226: Add mdss nodes commit: d5fb01ad5eb449ccfd950e946a882639cad168b3 Best regards, -- Bjorn Andersson
Re: [PATCH 1/5] dt-bindings: display: msm: dp-controller: document SM8250 compatible
On Sun, Jul 09, 2023 at 07:19:22AM +0300, Dmitry Baryshkov wrote: > It looks like DP controlled on SM8250 is the same as DP controller on > SM8350. Use the SM8350 compatible as fallback for SM8250. > > Signed-off-by: Dmitry Baryshkov Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > index 7a7cf3fb3e6d..a31ec9a4179f 100644 > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > @@ -28,6 +28,7 @@ properties: >- qcom,sm8350-dp >- items: >- enum: > + - qcom,sm8250-dp >- qcom,sm8450-dp >- qcom,sm8550-dp >- const: qcom,sm8350-dp > -- > 2.39.2 >
Re: [PATCH v1 4/5] drm/msm/dp: move relevant dp initialization code from bind() to probe()
On Fri, Jul 07, 2023 at 04:52:22PM -0700, Kuogee Hsieh wrote:
> In preparation of moving edp of_dp_aux_populate_bus() to
> dp_display_probe(), move dp_display_request_irq(),
> dp->parser->parse() and dp_power_client_init() to dp_display_probe()
> too.
>
> Signed-off-by: Kuogee Hsieh
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 48
> +
> drivers/gpu/drm/msm/dp/dp_display.h | 1 -
> 2 files changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 44580c2..185f1eb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev, struct
> device *master,
> goto end;
> }
>
> - rc = dp_power_client_init(dp->power);
> - if (rc) {
> - DRM_ERROR("Power client create failed\n");
> - goto end;
> - }
> -
> rc = dp_register_audio_driver(dev, dp->audio);
> if (rc) {
> DRM_ERROR("Audio registration Dp failed\n");
> @@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct dp_display_private
> *dp)
> goto error;
> }
>
> + rc = dp->parser->parse(dp->parser);
Today dp_init_sub_modules() just allocates memory for all the modules
and ties them together. While I don't fancy this way of structuring
device drivers in Linux, I think it's reasonable to retain that design
for now, and perform the parsing and power initialization in
dp_display_probe().
> + if (rc) {
> + DRM_ERROR("device tree parsing failed\n");
> + goto error;
> + }
> +
> dp->catalog = dp_catalog_get(dev, >parser->io);
> if (IS_ERR(dp->catalog)) {
> rc = PTR_ERR(dp->catalog);
> @@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct dp_display_private
> *dp)
> goto error;
> }
>
> + rc = dp_power_client_init(dp->power);
> + if (rc) {
> + DRM_ERROR("Power client create failed\n");
> + goto error;
> + }
> +
> dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
> if (IS_ERR(dp->aux)) {
> rc = PTR_ERR(dp->aux);
> @@ -1196,26 +1202,20 @@ static irqreturn_t dp_display_irq_handler(int irq,
> void *dev_id)
> return ret;
> }
>
> -int dp_display_request_irq(struct msm_dp *dp_display)
> +static int dp_display_request_irq(struct dp_display_private *dp)
> {
> int rc = 0;
> - struct dp_display_private *dp;
> -
> - if (!dp_display) {
> - DRM_ERROR("invalid input\n");
> - return -EINVAL;
> - }
Love this, but it's unrelated to the rest of the patch.
> -
> - dp = container_of(dp_display, struct dp_display_private, dp_display);
> + struct device *dev = >pdev->dev;
>
> - dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
> if (!dp->irq) {
> - DRM_ERROR("failed to get irq\n");
> - return -EINVAL;
> + dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
> + if (!dp->irq) {
> + DRM_ERROR("failed to get irq\n");
> + return -EINVAL;
> + }
> }
>
> - rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
> - dp_display_irq_handler,
> + rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
This is fixing a bug where currently the dp_display_irq_handler()
registration is tied to the DPU device's life cycle, while depending on
resources that are released as the DP device is torn down.
It would be nice if this was not hidden in a patch that claims to just
move calls around.
Regards,
Bjorn
> IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
> if (rc < 0) {
> DRM_ERROR("failed to request IRQ%u: %d\n",
> @@ -1290,6 +1290,8 @@ static int dp_display_probe(struct platform_device
> *pdev)
>
> platform_set_drvdata(pdev, >dp_display);
>
> + dp_display_request_irq(dp);
> +
> rc = component_add(>dev, _display_comp_ops);
> if (rc) {
> DRM_ERROR("component add failed, rc=%d\n", rc);
> @@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display,
> struct drm_device *dev,
>
> dp_priv = container_of(dp_display, struct dp_display_private,
> dp_display);
>
> - ret = dp_display_request_irq(dp_display);
> - if (ret) {
> - DRM_ERROR("request_irq failed, ret=%d\n", ret);
> - return ret;
> - }
> -
> ret = dp_display_get_next_bridge(dp_display);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
> b/drivers/gpu/drm/msm/dp/dp_display.h
> index 1e9415a..b3c08de 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -35,7 +35,6 @@ struct msm_dp {
> int
Re: [PATCH v1 2/5] drm/msm/dp: incorporate pm_runtime framework into DP driver
On Fri, Jul 07, 2023 at 04:52:20PM -0700, Kuogee Hsieh wrote:
> Incorporating pm runtime framework into DP driver so that power
> and clock resource handling can be centralized allowing easier
> control of these resources in preparation of registering aux bus
> uring probe.
>
> Signed-off-by: Kuogee Hsieh
> ---
> drivers/gpu/drm/msm/dp/dp_aux.c | 3 ++
> drivers/gpu/drm/msm/dp/dp_display.c | 75
> +
> 2 files changed, 63 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
> index 8e3b677..c592064 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.c
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.c
> @@ -291,6 +291,7 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
> return -EINVAL;
> }
>
> + pm_runtime_get_sync(dp_aux->dev);
> mutex_lock(>mutex);
> if (!aux->initted) {
> ret = -EIO;
> @@ -364,6 +365,8 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
>
> exit:
> mutex_unlock(>mutex);
> + pm_runtime_mark_last_busy(dp_aux->dev);
> + pm_runtime_put_autosuspend(dp_aux->dev);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 76f1395..2c5706a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -309,6 +309,10 @@ static int dp_display_bind(struct device *dev, struct
> device *master,
> goto end;
> }
>
> + pm_runtime_enable(dev);
> + pm_runtime_set_autosuspend_delay(dev, 1000);
> + pm_runtime_use_autosuspend(dev);
> +
> return 0;
> end:
> return rc;
> @@ -320,9 +324,8 @@ static void dp_display_unbind(struct device *dev, struct
> device *master,
> struct dp_display_private *dp = dev_get_dp_display_private(dev);
> struct msm_drm_private *priv = dev_get_drvdata(master);
>
> - /* disable all HPD interrupts */
> - if (dp->core_initialized)
> - dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK,
> false);
> + pm_runtime_dont_use_autosuspend(dev);
> + pm_runtime_disable(dev);
>
> kthread_stop(dp->ev_tsk);
>
> @@ -466,10 +469,12 @@ static void dp_display_host_init(struct
> dp_display_private *dp)
> dp->dp_display.connector_type, dp->core_initialized,
> dp->phy_initialized);
>
> - dp_power_init(dp->power);
> - dp_ctrl_reset_irq_ctrl(dp->ctrl, true);
> - dp_aux_init(dp->aux);
> - dp->core_initialized = true;
> + if (!dp->core_initialized) {
> + dp_power_init(dp->power);
> + dp_ctrl_reset_irq_ctrl(dp->ctrl, true);
> + dp_aux_init(dp->aux);
> + dp->core_initialized = true;
There are two cases that queries core_initialized, both of those are
done to avoid accessing the DP block without it first being powered up.
With the introduction of runtime PM, it seems reasonable to just power
up the block in those two code paths (and remove the variable).
> + }
> }
>
> static void dp_display_host_deinit(struct dp_display_private *dp)
> @@ -478,10 +483,12 @@ static void dp_display_host_deinit(struct
> dp_display_private *dp)
> dp->dp_display.connector_type, dp->core_initialized,
> dp->phy_initialized);
>
> - dp_ctrl_reset_irq_ctrl(dp->ctrl, false);
> - dp_aux_deinit(dp->aux);
> - dp_power_deinit(dp->power);
> - dp->core_initialized = false;
> + if (dp->core_initialized) {
> + dp_ctrl_reset_irq_ctrl(dp->ctrl, false);
> + dp_aux_deinit(dp->aux);
> + dp_power_deinit(dp->power);
> + dp->core_initialized = false;
> + }
> }
>
> static int dp_display_usbpd_configure_cb(struct device *dev)
> @@ -1304,6 +1311,39 @@ static int dp_display_remove(struct platform_device
> *pdev)
> dp_display_deinit_sub_modules(dp);
>
> platform_set_drvdata(pdev, NULL);
> + pm_runtime_put_sync_suspend(>dev);
> +
> + return 0;
> +}
> +
> +static int dp_pm_runtime_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct msm_dp *dp_display = platform_get_drvdata(pdev);
platform_get_drvdata() is a wrapper for dev_get_drvdata(>dev), so
there's no need to resolve the platform_device first...
> + struct dp_display_private *dp;
> +
> + dp = container_of(dp_display, struct dp_display_private, dp_display);
> +
> + dp_display_host_phy_exit(dp);
> + dp_catalog_ctrl_hpd_enable(dp->catalog);
> + dp_display_host_deinit(dp);
> +
> + return 0;
> +}
> +
> +static int dp_pm_runtime_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct msm_dp *dp_display = platform_get_drvdata(pdev);
> + struct dp_display_private *dp;
> +
> + dp = container_of(dp_display, struct dp_display_private, dp_display);
> +
Re: [PATCH v1 1/5] drm/msm/dp: remove pm_runtime_xxx() from dp_power.c
On Fri, Jul 07, 2023 at 04:52:19PM -0700, Kuogee Hsieh wrote: > Since both pm_runtime_resume() and pm_runtime_suspend() are not > populated at dp_pm_ops. Those pm_runtime_get/put() functions within > dp_power.c will not have any effects in addition to increase/decrease > power counter. Also pm_runtime_xxx() should be executed at top > layer. > Getting/putting the runtime PM state affects the vote for the GDSC. So I would suggest that you move this after patch 2, to not create a gap in the git history of lacking GDSC votes. Regards, Bjorn > Signed-off-by: Kuogee Hsieh > --- > drivers/gpu/drm/msm/dp/dp_power.c | 9 - > 1 file changed, 9 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_power.c > b/drivers/gpu/drm/msm/dp/dp_power.c > index 5cb84ca..ed2f62a 100644 > --- a/drivers/gpu/drm/msm/dp/dp_power.c > +++ b/drivers/gpu/drm/msm/dp/dp_power.c > @@ -152,8 +152,6 @@ int dp_power_client_init(struct dp_power *dp_power) > > power = container_of(dp_power, struct dp_power_private, dp_power); > > - pm_runtime_enable(power->dev); > - > return dp_power_clk_init(power); > } > > @@ -162,8 +160,6 @@ void dp_power_client_deinit(struct dp_power *dp_power) > struct dp_power_private *power; > > power = container_of(dp_power, struct dp_power_private, dp_power); > - > - pm_runtime_disable(power->dev); > } > > int dp_power_init(struct dp_power *dp_power) > @@ -173,11 +169,7 @@ int dp_power_init(struct dp_power *dp_power) > > power = container_of(dp_power, struct dp_power_private, dp_power); > > - pm_runtime_get_sync(power->dev); > - > rc = dp_power_clk_enable(dp_power, DP_CORE_PM, true); > - if (rc) > - pm_runtime_put_sync(power->dev); > > return rc; > } > @@ -189,7 +181,6 @@ int dp_power_deinit(struct dp_power *dp_power) > power = container_of(dp_power, struct dp_power_private, dp_power); > > dp_power_clk_enable(dp_power, DP_CORE_PM, false); > - pm_runtime_put_sync(power->dev); > return 0; > } > > -- > 2.7.4 >
Re: (subset) [PATCH v2 0/4] video: backlight: lp855x: modernize bindings
On Tue, Jun 13, 2023 at 03:30:10PM -0700, Bjorn Andersson wrote: > On Fri, 19 May 2023 20:07:24 +0200, Artur Weber wrote: > > Convert TI LP855X backlight controller bindings from TXT to YAML and, > > while we're at it, rework some of the code related to PWM handling. > > Also correct existing DTS files to avoid introducing new dtb_check > > errors. > > > > Signed-off-by: Artur Weber > > > > [...] > > Applied, thanks! > > [4/4] arm64: dts: adapt to LP855X bindings changes > commit: ebdcfc8c42c2b9d5ca1b27d8ee558eefb3e904d8 > Sorry, that was not for me to pick up. So I've dropped this change again. Please note that all other changes to the affected file is prefixed "arm64: tegra:". Following this is a good idea, and would have helped me not accidentally pick this change. Regards, Bjorn > Best regards, > -- > Bjorn Andersson
Re: [PATCH v2 0/3] drm/msm/adreno: GPU support on SC8280XP
On Wed, Jun 14, 2023 at 09:03:34AM -0700, Bjorn Andersson wrote: > On Mon, 22 May 2023 18:15:19 -0700, Bjorn Andersson wrote: > > This series introduces support for A690 in the DRM/MSM driver and > > enables it for the two SC8280XP laptops. > > > > Bjorn Andersson (3): > > drm/msm/adreno: Add Adreno A690 support > > arm64: dts: qcom: sc8280xp: Add GPU related nodes > > arm64: dts: qcom: sc8280xp: Enable GPU related nodes > > > > [...] > > Applied, thanks! > > [1/3] drm/msm/adreno: Add Adreno A690 support > (no commit info) > [2/3] arm64: dts: qcom: sc8280xp: Add GPU related nodes > commit: eec51ab2fd6f447a993c502364704d0cb5bc8cae > [3/3] arm64: dts: qcom: sc8280xp: Enable GPU related nodes > commit: 598a06afca5a2ab4850ce9ff8146ec728cca570c > Seems like I managed to confuse b4, only v4 of the DTS patches were merged, while Rob merged the driver change. Regards, Bjorn
Re: [PATCH 0/3] drm/msm/adreno: GPU support on SC8280XP
On Tue, 7 Feb 2023 19:40:49 -0800, Bjorn Andersson wrote: > This series introduces support for A690 in the DRM/MSM driver and > enables it for the two SC8280XP laptops. > > Bjorn Andersson (3): > drm/msm/adreno: Add Adreno A690 support > arm64: dts: qcom: sc8280xp: Add GPU related nodes > arm64: dts: qcom: sc8280xp: Enable GPU related nodes > > [...] Applied, thanks! [1/3] drm/msm/adreno: Add Adreno A690 support (no commit info) [2/3] arm64: dts: qcom: sc8280xp: Add GPU related nodes commit: eec51ab2fd6f447a993c502364704d0cb5bc8cae [3/3] arm64: dts: qcom: sc8280xp: Enable GPU related nodes commit: 598a06afca5a2ab4850ce9ff8146ec728cca570c Best regards, -- Bjorn Andersson
Re: [PATCH v4 0/2] drm/msm/adreno: GPU support on SC8280XP
On Wed, 14 Jun 2023 07:22:02 -0700, Bjorn Andersson wrote: > With the A690 support merged in the drm/msm driver, this series adds the > DeviceTree pieces to make it go on sc8280xp. > > Note that in order for the GPU driver to probe, the last change > requires (which is now in linux-next): > https://lore.kernel.org/linux-arm-msm/20230410185226.3240336-1-dmitry.barysh...@linaro.org/ > > [...] Applied, thanks! [1/2] arm64: dts: qcom: sc8280xp: Add GPU related nodes commit: eec51ab2fd6f447a993c502364704d0cb5bc8cae [2/2] arm64: dts: qcom: sc8280xp: Enable GPU related nodes commit: 598a06afca5a2ab4850ce9ff8146ec728cca570c Best regards, -- Bjorn Andersson
