Re: [PATCH 2/7] drm/msm/dpu: convert vsync source defines to the enum
On 5/22/2024 1:01 PM, Dmitry Baryshkov wrote:
On Wed, 22 May 2024 at 21:41, Abhinav Kumar wrote:
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
Add enum dpu_vsync_source instead of a series of defines. Use this enum
to pass vsync information.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +-
5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 66759623fc42..a2eff36a2224 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -54,18 +54,20 @@
#define DPU_BLEND_BG_INV_MOD_ALPHA (1 << 12)
#define DPU_BLEND_BG_TRANSP_EN (1 << 13)
-#define DPU_VSYNC0_SOURCE_GPIO 0
-#define DPU_VSYNC1_SOURCE_GPIO 1
-#define DPU_VSYNC2_SOURCE_GPIO 2
-#define DPU_VSYNC_SOURCE_INTF_0 3
-#define DPU_VSYNC_SOURCE_INTF_1 4
-#define DPU_VSYNC_SOURCE_INTF_2 5
-#define DPU_VSYNC_SOURCE_INTF_3 6
-#define DPU_VSYNC_SOURCE_WD_TIMER_4 11
-#define DPU_VSYNC_SOURCE_WD_TIMER_3 12
-#define DPU_VSYNC_SOURCE_WD_TIMER_2 13
-#define DPU_VSYNC_SOURCE_WD_TIMER_1 14
-#define DPU_VSYNC_SOURCE_WD_TIMER_0 15
+enum dpu_vsync_source {
+ DPU_VSYNC_SOURCE_GPIO_0,
+ DPU_VSYNC_SOURCE_GPIO_1,
+ DPU_VSYNC_SOURCE_GPIO_2,
+ DPU_VSYNC_SOURCE_INTF_0 = 3,
Do we need this assignment to 3?
It is redundant, but it points out that if at some point another GPIO
is added, it should go to the end (or to some other place, having the
proper value).
Ack, makes sense.
Rest LGTM,
Reviewed-by: Abhinav Kumar
Re: [PATCH 1/7] dt-bindings: display/msm/dsi: allow specifying TE source
On 5/22/2024 1:05 PM, Dmitry Baryshkov wrote:
On Wed, 22 May 2024 at 21:38, Abhinav Kumar wrote:
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
Command mode panels provide TE signal back to the DSI host to signal
that the frame display has completed and update of the image will not
cause tearing. Usually it is connected to the first GPIO with the
mdp_vsync function, which is the default. In such case the property can
be skipped.
This is a good addition overall. Some comments below.
Signed-off-by: Dmitry Baryshkov
---
.../bindings/display/msm/dsi-controller-main.yaml| 16
1 file changed, 16 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index 1fa28e976559..c1771c69b247 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -162,6 +162,21 @@ properties:
items:
enum: [ 0, 1, 2, 3 ]
+ qcom,te-source:
+$ref: /schemas/types.yaml#/definitions/string
+description:
+ Specifies the source of vsync signal from the panel used for
+ tearing elimination. The default is mdp_gpio0.
panel --> command mode panel?
+enum:
+ - mdp_gpio0
+ - mdp_gpio1
+ - mdp_gpio2
are gpio0, gpio1 and gpio2 referring to the vsync_p, vsync_s and vsync_e
sources?
No idea, unfortunately. They are gpioN or just mdp_vsync all over the
place. For the reference, in case of the SDM845 and Pixel3 the signal
is routed through SoC GPIO12.
GPIO12 on sdm845 is mdp_vsync_e.
Thats why I think its better we use mdp_vsync_p/s/e instead of mdp_gpio0/1/2
In that case wouldnt it be better to name it like that?
+ - timer0
+ - timer1
+ - timer2
+ - timer3
+ - timer4
+
These are indicating watchdog timer sources right?
Yes.
required:
- port@0
- port@1
@@ -452,6 +467,7 @@ examples:
dsi0_out: endpoint {
remote-endpoint = <_in>;
data-lanes = <0 1 2 3>;
+ qcom,te-source = "mdp_gpio2";
I have a basic doubt on this. Should te-source should be in the input
port or the output one for the controller? Because TE is an input to the
DSI. And if the source is watchdog timer then it aligns even more as a
property of the input endpoint.
I don't really want to split this. Both data-lanes and te-source are
properties of the link between the DSI and panel. You can not really
say which side has which property.
TE is an input to the DSI from the panel. Between input and output port,
I think it belongs more to the input port.
I didnt follow why this is a link property. Sorry , I didnt follow the
split part.
If we are unsure about input vs output port, do you think its better we
make it a property of the main dsi node instead?
};
};
};
[PATCH v3 2/2] drm/msm: Extend gpu devcore dumps with pgtbl info
From: Rob Clark
In the case of iova fault triggered devcore dumps, include additional
debug information based on what we think is the current page tables,
including the TTBR0 value (which should match what we have in
adreno_smmu_fault_info unless things have gone horribly wrong), and
the pagetable entries traversed in the process of resolving the
faulting iova.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 10 ++
drivers/gpu/drm/msm/msm_gpu.c | 22 ++
drivers/gpu/drm/msm/msm_gpu.h | 8
drivers/gpu/drm/msm/msm_iommu.c | 18 ++
drivers/gpu/drm/msm/msm_mmu.h | 5 -
5 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index a00241e3373b..3b4c75df0a5f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -861,6 +861,16 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state
*state,
drm_printf(p, " - dir=%s\n", info->flags & IOMMU_FAULT_WRITE ?
"WRITE" : "READ");
drm_printf(p, " - type=%s\n", info->type);
drm_printf(p, " - source=%s\n", info->block);
+
+ /* Information extracted from what we think are the current
+* pgtables. Hopefully the TTBR0 matches what we've extracted
+* from the SMMU registers in smmu_info!
+*/
+ drm_puts(p, "pgtable-fault-info:\n");
+ drm_printf(p, " - ttbr0: %.16llx\n", (u64)info->pgtbl_ttbr0);
+ drm_printf(p, " - asid: %d\n", info->asid);
+ drm_printf(p, " - ptes: %.16llx %.16llx %.16llx %.16llx\n",
+ info->ptes[0], info->ptes[1], info->ptes[2],
info->ptes[3]);
}
drm_printf(p, "rbbm-status: 0x%08x\n", state->rbbm_status);
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 43cde0590250..647bddc897f2 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -256,6 +256,18 @@ static void msm_gpu_crashstate_get_bo(struct msm_gpu_state
*state,
state->nr_bos++;
}
+static int pgtable_walk_cb(void *cb_data, void *pte, int level)
+{
+ struct msm_gpu_fault_info *info = cb_data;
+
+ if (level > ARRAY_SIZE(info->ptes))
+ return -EINVAL;
+
+ info->ptes[level] = *(u64 *)pte;
+
+ return 0;
+}
+
static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
struct msm_gem_submit *submit, char *comm, char *cmd)
{
@@ -281,6 +293,16 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
if (submit) {
int i;
+ if (state->fault_info.ttbr0) {
+ struct msm_gpu_fault_info *info = >fault_info;
+ struct msm_mmu *mmu = submit->aspace->mmu;
+
+ msm_iommu_pagetable_params(mmu, >pgtbl_ttbr0,
+ >asid);
+ msm_iommu_pagetable_walk(mmu, info->iova,
+pgtable_walk_cb, info);
+ }
+
state->bos = kcalloc(submit->nr_bos,
sizeof(struct msm_gpu_state_bo), GFP_KERNEL);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 04a696ac4626..82fbb626461a 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -101,6 +101,14 @@ struct msm_gpu_fault_info {
int flags;
const char *type;
const char *block;
+
+ /* Information about what we think/expect is the current SMMU state,
+* for example expected_ttbr0 should match smmu_info.ttbr0 which
+* was read back from SMMU registers.
+*/
+ phys_addr_t pgtbl_ttbr0;
+ u64 ptes[4];
+ int asid;
};
/**
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index d5512037c38b..f46ed4667475 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -195,6 +195,24 @@ struct iommu_domain_geometry
*msm_iommu_get_geometry(struct msm_mmu *mmu)
return >domain->geometry;
}
+int msm_iommu_pagetable_walk(struct msm_mmu *mmu, unsigned long iova,
+int (*cb)(void *cb_data, void *pte, int level),
+void *cb_data)
+{
+ struct msm_iommu_pagetable *pagetable;
+
+ if (mmu->type != MSM_MMU_IOMMU_PAGETABLE)
+ return -EINVAL;
+
+ pagetable = to_pagetable(mmu);
+
+ if (!pagetable->pgtbl_ops->pgtable_walk)
+ return -EINVAL;
+
+ return pagetable->pgtbl_ops->pgtable_walk(pagetable->pgtbl_ops, iova,
+ cb, cb_data);
+}
+
static const struct msm_mmu_funcs pagetable_funcs = {
.map
[PATCH v3 1/2] iommu/io-pgtable-arm: Add way to debug pgtable walk
From: Rob Clark
Add an io-pgtable method to walk the pgtable returning the raw PTEs that
would be traversed for a given iova access.
Signed-off-by: Rob Clark
---
drivers/iommu/io-pgtable-arm.c | 50 --
include/linux/io-pgtable.h | 4 +++
2 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index f7828a7aad41..86d2b34d6f95 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -693,17 +693,19 @@ static size_t arm_lpae_unmap_pages(struct io_pgtable_ops
*ops, unsigned long iov
data->start_level, ptep);
}
-static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
-unsigned long iova)
+static int pgtable_walk(struct io_pgtable_ops *ops, unsigned long iova,
+ int (*cb)(void *cb_data, void *pte, int level),
+ void *cb_data)
{
struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
arm_lpae_iopte pte, *ptep = data->pgd;
int lvl = data->start_level;
+ int ret;
do {
/* Valid IOPTE pointer? */
if (!ptep)
- return 0;
+ return -EFAULT;
/* Grab the IOPTE we're interested in */
ptep += ARM_LPAE_LVL_IDX(iova, lvl, data);
@@ -711,22 +713,52 @@ static phys_addr_t arm_lpae_iova_to_phys(struct
io_pgtable_ops *ops,
/* Valid entry? */
if (!pte)
- return 0;
+ return -EFAULT;
+
+ ret = cb(cb_data, , lvl);
+ if (ret)
+ return ret;
- /* Leaf entry? */
+ /* Leaf entry? If so, we've found the translation */
if (iopte_leaf(pte, lvl, data->iop.fmt))
- goto found_translation;
+ return 0;
/* Take it to the next level */
ptep = iopte_deref(pte, data);
} while (++lvl < ARM_LPAE_MAX_LEVELS);
/* Ran out of page tables to walk */
+ return -EFAULT;
+}
+
+struct iova_to_phys_walk_data {
+ arm_lpae_iopte pte;
+ int level;
+};
+
+static int iova_to_phys_walk_cb(void *cb_data, void *pte, int level)
+{
+ struct iova_to_phys_walk_data *d = cb_data;
+
+ d->pte = *(arm_lpae_iopte *)pte;
+ d->level = level;
+
return 0;
+}
+
+static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
+unsigned long iova)
+{
+ struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
+ struct iova_to_phys_walk_data d;
+ int ret;
+
+ ret = pgtable_walk(ops, iova, iova_to_phys_walk_cb, );
+ if (ret)
+ return 0;
-found_translation:
- iova &= (ARM_LPAE_BLOCK_SIZE(lvl, data) - 1);
- return iopte_to_paddr(pte, data) | iova;
+ iova &= (ARM_LPAE_BLOCK_SIZE(d.level, data) - 1);
+ return iopte_to_paddr(d.pte, data) | iova;
}
static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index 86cf1f7ae389..261b48af068a 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -177,6 +177,7 @@ struct io_pgtable_cfg {
* @map_pages:Map a physically contiguous range of pages of the same size.
* @unmap_pages: Unmap a range of virtually contiguous pages of the same size.
* @iova_to_phys: Translate iova to physical address.
+ * @pgtable_walk: (optional) Perform a page table walk for a given iova.
*
* These functions map directly onto the iommu_ops member functions with
* the same names.
@@ -190,6 +191,9 @@ struct io_pgtable_ops {
struct iommu_iotlb_gather *gather);
phys_addr_t (*iova_to_phys)(struct io_pgtable_ops *ops,
unsigned long iova);
+ int (*pgtable_walk)(struct io_pgtable_ops *ops, unsigned long iova,
+ int (*cb)(void *cb_data, void *pte, int level),
+ void *cb_data);
int (*read_and_clear_dirty)(struct io_pgtable_ops *ops,
unsigned long iova, size_t size,
unsigned long flags,
--
2.45.1
[PATCH v3 0/2] io-pgtable-arm + drm/msm: Extend iova fault debugging
From: Rob Clark This series extends io-pgtable-arm with a method to retrieve the page table entries traversed in the process of address translation, and then beefs up drm/msm gpu devcore dump to include this (and additional info) in the devcore dump. This is a respin of https://patchwork.freedesktop.org/series/94968/ (minus a patch that was already merged) v2: Fix an armv7/32b build error in the last patch v3: Incorperate Will Deacon's suggestion to make the interface callback based. Rob Clark (2): iommu/io-pgtable-arm: Add way to debug pgtable walk drm/msm: Extend gpu devcore dumps with pgtbl info drivers/gpu/drm/msm/adreno/adreno_gpu.c | 10 + drivers/gpu/drm/msm/msm_gpu.c | 22 +++ drivers/gpu/drm/msm/msm_gpu.h | 8 drivers/gpu/drm/msm/msm_iommu.c | 18 + drivers/gpu/drm/msm/msm_mmu.h | 5 ++- drivers/iommu/io-pgtable-arm.c | 50 - include/linux/io-pgtable.h | 4 ++ 7 files changed, 107 insertions(+), 10 deletions(-) -- 2.45.1
Re: [PATCH 1/7] dt-bindings: display/msm/dsi: allow specifying TE source
On Wed, 22 May 2024 at 21:38, Abhinav Kumar wrote:
>
>
>
> On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
> > Command mode panels provide TE signal back to the DSI host to signal
> > that the frame display has completed and update of the image will not
> > cause tearing. Usually it is connected to the first GPIO with the
> > mdp_vsync function, which is the default. In such case the property can
> > be skipped.
> >
>
> This is a good addition overall. Some comments below.
>
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > .../bindings/display/msm/dsi-controller-main.yaml| 16
> >
> > 1 file changed, 16 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > index 1fa28e976559..c1771c69b247 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > @@ -162,6 +162,21 @@ properties:
> > items:
> > enum: [ 0, 1, 2, 3 ]
> >
> > + qcom,te-source:
> > +$ref: /schemas/types.yaml#/definitions/string
> > +description:
> > + Specifies the source of vsync signal from the panel used
> > for
> > + tearing elimination. The default is mdp_gpio0.
>
> panel --> command mode panel?
>
> > +enum:
> > + - mdp_gpio0
> > + - mdp_gpio1
> > + - mdp_gpio2
>
> are gpio0, gpio1 and gpio2 referring to the vsync_p, vsync_s and vsync_e
> sources?
No idea, unfortunately. They are gpioN or just mdp_vsync all over the
place. For the reference, in case of the SDM845 and Pixel3 the signal
is routed through SoC GPIO12.
> In that case wouldnt it be better to name it like that?
>
> > + - timer0
> > + - timer1
> > + - timer2
> > + - timer3
> > + - timer4
> > +
>
> These are indicating watchdog timer sources right?
Yes.
>
> > required:
> > - port@0
> > - port@1
> > @@ -452,6 +467,7 @@ examples:
> > dsi0_out: endpoint {
> > remote-endpoint = <_in>;
> > data-lanes = <0 1 2 3>;
> > + qcom,te-source = "mdp_gpio2";
>
> I have a basic doubt on this. Should te-source should be in the input
> port or the output one for the controller? Because TE is an input to the
> DSI. And if the source is watchdog timer then it aligns even more as a
> property of the input endpoint.
I don't really want to split this. Both data-lanes and te-source are
properties of the link between the DSI and panel. You can not really
say which side has which property.
> > };
> > };
> > };
> >
--
With best wishes
Dmitry
Re: [PATCH 2/7] drm/msm/dpu: convert vsync source defines to the enum
On Wed, 22 May 2024 at 21:41, Abhinav Kumar wrote:
>
>
>
> On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
> > Add enum dpu_vsync_source instead of a series of defines. Use this enum
> > to pass vsync information.
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26
> > ++
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +-
> > 5 files changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > index 66759623fc42..a2eff36a2224 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > @@ -54,18 +54,20 @@
> > #define DPU_BLEND_BG_INV_MOD_ALPHA (1 << 12)
> > #define DPU_BLEND_BG_TRANSP_EN (1 << 13)
> >
> > -#define DPU_VSYNC0_SOURCE_GPIO 0
> > -#define DPU_VSYNC1_SOURCE_GPIO 1
> > -#define DPU_VSYNC2_SOURCE_GPIO 2
> > -#define DPU_VSYNC_SOURCE_INTF_0 3
> > -#define DPU_VSYNC_SOURCE_INTF_1 4
> > -#define DPU_VSYNC_SOURCE_INTF_2 5
> > -#define DPU_VSYNC_SOURCE_INTF_3 6
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_4 11
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_3 12
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_2 13
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_1 14
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_0 15
> > +enum dpu_vsync_source {
> > + DPU_VSYNC_SOURCE_GPIO_0,
> > + DPU_VSYNC_SOURCE_GPIO_1,
> > + DPU_VSYNC_SOURCE_GPIO_2,
> > + DPU_VSYNC_SOURCE_INTF_0 = 3,
>
> Do we need this assignment to 3?
It is redundant, but it points out that if at some point another GPIO
is added, it should go to the end (or to some other place, having the
proper value).
>
> Rest LGTM,
>
> Reviewed-by: Abhinav Kumar
--
With best wishes
Dmitry
Re: [PATCH 0/7] drm/msm/dpu: handle non-default TE source pins
On Wed, 22 May 2024 at 21:39, Abhinav Kumar wrote: > > > > On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote: > > Command-mode DSI panels need to signal the display controlller when > > vsync happens, so that the device can start sending the next frame. Some > > devices (Google Pixel 3) use a non-default pin, so additional > > configuration is required. Add a way to specify this information in DT > > and handle it in the DSI and DPU drivers. > > > > Which pin is the pixel 3 using? Just wanted to know .. is it gpio0 or gpio1? gpio2. If it was gpio0 then there were no issues at all. > > > Signed-off-by: Dmitry Baryshkov > > --- > > Dmitry Baryshkov (7): > >dt-bindings: display/msm/dsi: allow specifying TE source > >drm/msm/dpu: convert vsync source defines to the enum > >drm/msm/dsi: drop unused GPIOs handling > >drm/msm/dpu: pull the is_cmd_mode out of > > _dpu_encoder_update_vsync_source() > >drm/msm/dpu: rework vsync_source handling > >drm/msm/dsi: parse vsync source from device tree > >drm/msm/dpu: support setting the TE source > > > > .../bindings/display/msm/dsi-controller-main.yaml | 16 > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 11 ++--- > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 5 +-- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 2 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 2 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 26 ++-- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 44 > > > > drivers/gpu/drm/msm/dsi/dsi.h | 1 + > > drivers/gpu/drm/msm/dsi/dsi_host.c | 48 > > +- > > drivers/gpu/drm/msm/dsi/dsi_manager.c | 5 +++ > > drivers/gpu/drm/msm/msm_drv.h | 6 +++ > > 12 files changed, 106 insertions(+), 62 deletions(-) > > --- > > base-commit: 75fa778d74b786a1608d55d655d42b480a6fa8bd > > change-id: 20240514-dpu-handle-te-signal-82663c0211bd > > > > Best regards, -- With best wishes Dmitry
Re: [PATCH 5/7] drm/msm/dpu: rework vsync_source handling
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote: The struct msm_display_info has is_te_using_watchdog_timer field which is neither used anywhere nor is flexible enough to specify different sources. Replace it with the field specifying the vsync source using enum dpu_vsync_source. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 5 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ 3 files changed, 5 insertions(+), 7 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH 4/7] drm/msm/dpu: pull the is_cmd_mode out of _dpu_encoder_update_vsync_source()
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote: Setting vsync source makes sense only for DSI CMD panels. Pull the is_cmd_mode condition out of the function into the calling code, so that it becomes more explicit. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH 3/7] drm/msm/dsi: drop unused GPIOs handling
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote: Neither disp-enable-gpios nor disp-te-gpios are defined in the schema. None of the board DT files use those GPIO pins. Drop them from the driver. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/dsi_host.c | 37 - 1 file changed, 37 deletions(-) Reviewed-by: Abhinav Kumar
Re: [PATCH 2/7] drm/msm/dpu: convert vsync source defines to the enum
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
Add enum dpu_vsync_source instead of a series of defines. Use this enum
to pass vsync information.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +-
5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..4988a1029431 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -747,7 +747,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
if (disp_info->is_te_using_watchdog_timer)
vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
else
- vsync_cfg.vsync_source = DPU_VSYNC0_SOURCE_GPIO;
+ vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
hw_mdptop->ops.setup_vsync_source(hw_mdptop, _cfg);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 225c1c7768ff..96f6160cf607 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -462,7 +462,7 @@ static int dpu_hw_intf_get_vsync_info(struct dpu_hw_intf
*intf,
}
static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
- u32 vsync_source)
+ enum dpu_vsync_source vsync_source)
{
struct dpu_hw_blk_reg_map *c;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index f9015c67a574..ac244f0b33fb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -107,7 +107,7 @@ struct dpu_hw_intf_ops {
int (*connect_external_te)(struct dpu_hw_intf *intf, bool enable_external_te);
- void (*vsync_sel)(struct dpu_hw_intf *intf, u32 vsync_source);
+ void (*vsync_sel)(struct dpu_hw_intf *intf, enum dpu_vsync_source
vsync_source);
/**
* Disable autorefresh if enabled
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 66759623fc42..a2eff36a2224 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -54,18 +54,20 @@
#define DPU_BLEND_BG_INV_MOD_ALPHA(1 << 12)
#define DPU_BLEND_BG_TRANSP_EN(1 << 13)
-#define DPU_VSYNC0_SOURCE_GPIO 0
-#define DPU_VSYNC1_SOURCE_GPIO 1
-#define DPU_VSYNC2_SOURCE_GPIO 2
-#define DPU_VSYNC_SOURCE_INTF_03
-#define DPU_VSYNC_SOURCE_INTF_14
-#define DPU_VSYNC_SOURCE_INTF_25
-#define DPU_VSYNC_SOURCE_INTF_36
-#define DPU_VSYNC_SOURCE_WD_TIMER_411
-#define DPU_VSYNC_SOURCE_WD_TIMER_312
-#define DPU_VSYNC_SOURCE_WD_TIMER_213
-#define DPU_VSYNC_SOURCE_WD_TIMER_114
-#define DPU_VSYNC_SOURCE_WD_TIMER_015
+enum dpu_vsync_source {
+ DPU_VSYNC_SOURCE_GPIO_0,
+ DPU_VSYNC_SOURCE_GPIO_1,
+ DPU_VSYNC_SOURCE_GPIO_2,
+ DPU_VSYNC_SOURCE_INTF_0 = 3,
Do we need this assignment to 3?
Rest LGTM,
Reviewed-by: Abhinav Kumar
Re: [PATCH 0/7] drm/msm/dpu: handle non-default TE source pins
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote: Command-mode DSI panels need to signal the display controlller when vsync happens, so that the device can start sending the next frame. Some devices (Google Pixel 3) use a non-default pin, so additional configuration is required. Add a way to specify this information in DT and handle it in the DSI and DPU drivers. Which pin is the pixel 3 using? Just wanted to know .. is it gpio0 or gpio1? Signed-off-by: Dmitry Baryshkov --- Dmitry Baryshkov (7): dt-bindings: display/msm/dsi: allow specifying TE source drm/msm/dpu: convert vsync source defines to the enum drm/msm/dsi: drop unused GPIOs handling drm/msm/dpu: pull the is_cmd_mode out of _dpu_encoder_update_vsync_source() drm/msm/dpu: rework vsync_source handling drm/msm/dsi: parse vsync source from device tree drm/msm/dpu: support setting the TE source .../bindings/display/msm/dsi-controller-main.yaml | 16 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 11 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 5 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 26 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 44 drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 48 +- drivers/gpu/drm/msm/dsi/dsi_manager.c | 5 +++ drivers/gpu/drm/msm/msm_drv.h | 6 +++ 12 files changed, 106 insertions(+), 62 deletions(-) --- base-commit: 75fa778d74b786a1608d55d655d42b480a6fa8bd change-id: 20240514-dpu-handle-te-signal-82663c0211bd Best regards,
Re: [PATCH 1/7] dt-bindings: display/msm/dsi: allow specifying TE source
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
Command mode panels provide TE signal back to the DSI host to signal
that the frame display has completed and update of the image will not
cause tearing. Usually it is connected to the first GPIO with the
mdp_vsync function, which is the default. In such case the property can
be skipped.
This is a good addition overall. Some comments below.
Signed-off-by: Dmitry Baryshkov
---
.../bindings/display/msm/dsi-controller-main.yaml| 16
1 file changed, 16 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index 1fa28e976559..c1771c69b247 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -162,6 +162,21 @@ properties:
items:
enum: [ 0, 1, 2, 3 ]
+ qcom,te-source:
+$ref: /schemas/types.yaml#/definitions/string
+description:
+ Specifies the source of vsync signal from the panel used for
+ tearing elimination. The default is mdp_gpio0.
panel --> command mode panel?
+enum:
+ - mdp_gpio0
+ - mdp_gpio1
+ - mdp_gpio2
are gpio0, gpio1 and gpio2 referring to the vsync_p, vsync_s and vsync_e
sources?
In that case wouldnt it be better to name it like that?
+ - timer0
+ - timer1
+ - timer2
+ - timer3
+ - timer4
+
These are indicating watchdog timer sources right?
required:
- port@0
- port@1
@@ -452,6 +467,7 @@ examples:
dsi0_out: endpoint {
remote-endpoint = <_in>;
data-lanes = <0 1 2 3>;
+ qcom,te-source = "mdp_gpio2";
I have a basic doubt on this. Should te-source should be in the input
port or the output one for the controller? Because TE is an input to the
DSI. And if the source is watchdog timer then it aligns even more as a
property of the input endpoint.
};
};
};
Re: [PATCH 1/7] dt-bindings: display/msm/dsi: allow specifying TE source
On Mon, May 20, 2024 at 03:12:43PM +0300, Dmitry Baryshkov wrote: > Command mode panels provide TE signal back to the DSI host to signal > that the frame display has completed and update of the image will not > cause tearing. Usually it is connected to the first GPIO with the > mdp_vsync function, which is the default. In such case the property can > be skipped. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dsi-controller-main.yaml| 16 > > 1 file changed, 16 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml > b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml > index 1fa28e976559..c1771c69b247 100644 > --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml > @@ -162,6 +162,21 @@ properties: > items: >enum: [ 0, 1, 2, 3 ] > > + qcom,te-source: > +$ref: /schemas/types.yaml#/definitions/string > +description: > + Specifies the source of vsync signal from the panel used > for > + tearing elimination. The default is mdp_gpio0. default: mdp_gpio0 With that, Reviewed-by: Rob Herring (Arm)
Re: [PATCH v3 2/3] drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER
On 2024-05-22 09:25:54, Dmitry Baryshkov wrote:
> This panel driver uses DSC PPS functions and as such depends on the
> DRM_DISPLAY_DP_HELPER. Select this symbol to make required functions
Here and in the title: maybe this is a remnant from v2, but you split out a
DRM_DISPLAY_DSC_HELPER and shouldn't be enabling DP for a DSI panel now.
- Marijn
> available to the driver.
>
> Reported-by: kernel test robot
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202404200800.kysryyli-...@intel.com/
> Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/panel/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 4a2f621433ef..3e3f63479544 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -340,6 +340,8 @@ config DRM_PANEL_LG_SW43408
> depends on OF
> depends on DRM_MIPI_DSI
> depends on BACKLIGHT_CLASS_DEVICE
> + select DRM_DISPLAY_DSC_HELPER
> + select DRM_DISPLAY_HELPER
> help
> Say Y here if you want to enable support for LG sw43408 panel.
> The panel has a 1080x2160@60Hz resolution and uses 24 bit RGB per
>
> --
> 2.39.2
>
Re: [PATCH v3 2/3] drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER
On 22/05/2024 08:25, Dmitry Baryshkov wrote:
This panel driver uses DSC PPS functions and as such depends on the
DRM_DISPLAY_DP_HELPER. Select this symbol to make required functions
available to the driver.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202404200800.kysryyli-...@intel.com/
Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/panel/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 4a2f621433ef..3e3f63479544 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -340,6 +340,8 @@ config DRM_PANEL_LG_SW43408
depends on OF
depends on DRM_MIPI_DSI
depends on BACKLIGHT_CLASS_DEVICE
+ select DRM_DISPLAY_DSC_HELPER
+ select DRM_DISPLAY_HELPER
help
Say Y here if you want to enable support for LG sw43408 panel.
The panel has a 1080x2160@60Hz resolution and uses 24 bit RGB per
Reviewed-by: Neil Armstrong
Re: [PATCH v3 3/3] drm/panel/lg-sw43408: mark sw43408_backlight_ops as static
On 22/05/2024 08:25, Dmitry Baryshkov wrote:
Fix sparse warning regarding symbol 'sw43408_backlight_ops' not being
declared.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202404200739.hbwzvohr-...@intel.com/
Reviewed-by: Neil Armstrong
Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/panel/panel-lg-sw43408.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c
b/drivers/gpu/drm/panel/panel-lg-sw43408.c
index 115f4702d59f..2b3a73696dce 100644
--- a/drivers/gpu/drm/panel/panel-lg-sw43408.c
+++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
@@ -182,7 +182,7 @@ static int sw43408_backlight_update_status(struct
backlight_device *bl)
return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
}
-const struct backlight_ops sw43408_backlight_ops = {
+static const struct backlight_ops sw43408_backlight_ops = {
.update_status = sw43408_backlight_update_status,
};
Reviewed-by: Neil Armstrong
[PATCH v2 13/14] drm/msm/hdmi: ensure that HDMI is one if HPD is requested
The HDMI block needs to be enabled to properly generate HPD events. Make
sure it is not turned off in the disable paths if HPD delivery is enabled.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c| 1 +
drivers/gpu/drm/msm/hdmi/hdmi.h| 2 ++
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 8 +++-
drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 9 -
4 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index a9437054c015..2890196857f8 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -409,6 +409,7 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
hdmi->pdev = pdev;
hdmi->config = config;
spin_lock_init(>reg_lock);
+ mutex_init(>state_mutex);
ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL,
>next_bridge);
if (ret && ret != -ENODEV)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 268ff8604423..7f0ca5252018 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -42,6 +42,8 @@ struct hdmi {
/* video state: */
bool power_on;
+ bool hpd_enabled;
+ struct mutex state_mutex; /* protects two booleans */
unsigned long int pixclock;
void __iomem *mmio;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index cddba640d292..104107ed47d0 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -117,11 +117,13 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct
drm_bridge *bridge,
DBG("power up");
+ mutex_lock(>state_mutex);
if (!hdmi->power_on) {
msm_hdmi_phy_resource_enable(phy);
msm_hdmi_power_on(bridge);
hdmi->power_on = true;
}
+ mutex_unlock(>state_mutex);
if (hdmi->hdmi_mode) {
msm_hdmi_config_avi_infoframe(hdmi);
@@ -147,7 +149,10 @@ static void msm_hdmi_bridge_atomic_post_disable(struct
drm_bridge *bridge,
msm_hdmi_hdcp_off(hdmi->hdcp_ctrl);
DBG("power down");
- msm_hdmi_set_mode(hdmi, false);
+
+ /* Keep the HDMI enabled if the HPD is enabled */
+ mutex_lock(>state_mutex);
+ msm_hdmi_set_mode(hdmi, hdmi->hpd_enabled);
msm_hdmi_phy_powerdown(phy);
@@ -158,6 +163,7 @@ static void msm_hdmi_bridge_atomic_post_disable(struct
drm_bridge *bridge,
msm_hdmi_audio_update(hdmi);
msm_hdmi_phy_resource_disable(phy);
}
+ mutex_unlock(>state_mutex);
}
static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge,
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index d3353c6148ed..cb89e9e2c6ea 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -73,10 +73,14 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
if (ret)
return ret;
+ mutex_lock(>state_mutex);
msm_hdmi_set_mode(hdmi, false);
msm_hdmi_phy_reset(hdmi);
msm_hdmi_set_mode(hdmi, true);
+ hdmi->hpd_enabled = true;
+ mutex_unlock(>state_mutex);
+
hdmi_write(hdmi, REG_HDMI_USEC_REFTIMER, 0x0001001b);
/* enable HPD events: */
@@ -106,7 +110,10 @@ void msm_hdmi_hpd_disable(struct hdmi *hdmi)
/* Disable HPD interrupt */
hdmi_write(hdmi, REG_HDMI_HPD_INT_CTRL, 0);
- msm_hdmi_set_mode(hdmi, false);
+ mutex_lock(>state_mutex);
+ hdmi->hpd_enabled = false;
+ msm_hdmi_set_mode(hdmi, hdmi->power_on);
+ mutex_unlock(>state_mutex);
pm_runtime_put(dev);
}
--
2.39.2
[PATCH v2 11/14] drm/msm/hdmi: expand the HDMI_CFG macro
Expand the HDMI_CFG() macro in HDMI config description. It has no added
value other than hiding some boilerplate declarations.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 16
drivers/gpu/drm/msm/hdmi/hdmi.h | 2 +-
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index c39a1f3a7505..e160a23e962e 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -223,24 +223,24 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
* The hdmi device:
*/
-#define HDMI_CFG(item, entry) \
- .item ## _names = item ##_names_ ## entry, \
- .item ## _cnt = ARRAY_SIZE(item ## _names_ ## entry)
-
static const char *pwr_reg_names_8960[] = {"core-vdda"};
static const char *pwr_clk_names_8960[] = {"core", "master_iface",
"slave_iface"};
static const struct hdmi_platform_config hdmi_tx_8960_config = {
- HDMI_CFG(pwr_reg, 8960),
- HDMI_CFG(pwr_clk, 8960),
+ .pwr_reg_names = pwr_reg_names_8960,
+ .pwr_reg_cnt = ARRAY_SIZE(pwr_reg_names_8960),
+ .pwr_clk_names = pwr_clk_names_8960,
+ .pwr_clk_cnt = ARRAY_SIZE(pwr_clk_names_8960),
};
static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
static const char *pwr_clk_names_8x74[] = {"iface", "core", "mdp_core",
"alt_iface"};
static const struct hdmi_platform_config hdmi_tx_8974_config = {
- HDMI_CFG(pwr_reg, 8x74),
- HDMI_CFG(pwr_clk, 8x74),
+ .pwr_reg_names = pwr_reg_names_8x74,
+ .pwr_reg_cnt = ARRAY_SIZE(pwr_reg_names_8x74),
+ .pwr_clk_names = pwr_clk_names_8x74,
+ .pwr_clk_cnt = ARRAY_SIZE(pwr_clk_names_8x74),
};
/*
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 1e346e697f8e..2a98efa8b6bd 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -89,7 +89,7 @@ struct hdmi_platform_config {
const char **pwr_reg_names;
int pwr_reg_cnt;
- /* clks that need to be on for hpd: */
+ /* clks that need to be on: */
const char **pwr_clk_names;
int pwr_clk_cnt;
};
--
2.39.2
[PATCH v2 14/14] drm/msm/hdmi: wire in hpd_enable/hpd_disable bridge ops
The HDMI driver already has msm_hdmi_hpd_enable() and
msm_hdmi_hpd_disable() functions. Wire them into the
msm_hdmi_bridge_funcs, so that HPD can be enabled and disabled
dynamically rather than always having HPD events generation enabled.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c| 9 -
drivers/gpu/drm/msm/hdmi/hdmi.h| 4 ++--
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 3 +++
drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 12 ++--
4 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 2890196857f8..06adcf4a6544 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -202,12 +202,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
goto fail;
}
- ret = msm_hdmi_hpd_enable(hdmi->bridge);
- if (ret < 0) {
- DRM_DEV_ERROR(>pdev->dev, "failed to enable HPD: %d\n",
ret);
- goto fail;
- }
-
return 0;
fail:
@@ -377,9 +371,6 @@ static void msm_hdmi_unbind(struct device *dev, struct
device *master,
if (priv->hdmi->audio_pdev)
platform_device_unregister(priv->hdmi->audio_pdev);
- if (priv->hdmi->bridge)
- msm_hdmi_hpd_disable(priv->hdmi);
-
msm_hdmi_destroy(priv->hdmi);
priv->hdmi = NULL;
}
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 7f0ca5252018..c6519e6f7f2c 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -219,8 +219,8 @@ int msm_hdmi_bridge_init(struct hdmi *hdmi);
void msm_hdmi_hpd_irq(struct drm_bridge *bridge);
enum drm_connector_status msm_hdmi_bridge_detect(
struct drm_bridge *bridge);
-int msm_hdmi_hpd_enable(struct drm_bridge *bridge);
-void msm_hdmi_hpd_disable(struct hdmi *hdmi);
+void msm_hdmi_hpd_enable(struct drm_bridge *bridge);
+void msm_hdmi_hpd_disable(struct drm_bridge *bridge);
/*
* i2c adapter for ddc:
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 104107ed47d0..41722b2e6b44 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -300,6 +300,9 @@ static const struct drm_bridge_funcs msm_hdmi_bridge_funcs
= {
.mode_valid = msm_hdmi_bridge_mode_valid,
.edid_read = msm_hdmi_bridge_edid_read,
.detect = msm_hdmi_bridge_detect,
+
+ .hpd_enable = msm_hdmi_hpd_enable,
+ .hpd_disable = msm_hdmi_hpd_disable,
};
static void
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index cb89e9e2c6ea..04d00b6f36fd 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -60,7 +60,7 @@ static void msm_hdmi_phy_reset(struct hdmi *hdmi)
}
}
-int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
+void msm_hdmi_hpd_enable(struct drm_bridge *bridge)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
@@ -70,8 +70,8 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
unsigned long flags;
ret = pm_runtime_resume_and_get(dev);
- if (ret)
- return ret;
+ if (WARN_ON(ret))
+ return;
mutex_lock(>state_mutex);
msm_hdmi_set_mode(hdmi, false);
@@ -99,12 +99,12 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
hdmi_write(hdmi, REG_HDMI_HPD_CTRL,
HDMI_HPD_CTRL_ENABLE | hpd_ctrl);
spin_unlock_irqrestore(>reg_lock, flags);
-
- return 0;
}
-void msm_hdmi_hpd_disable(struct hdmi *hdmi)
+void msm_hdmi_hpd_disable(struct drm_bridge *bridge)
{
+ struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
+ struct hdmi *hdmi = hdmi_bridge->hdmi;
struct device *dev = >pdev->dev;
/* Disable HPD interrupt */
--
2.39.2
[PATCH v2 12/14] drm/msm/hdmi: drop hpd-gpios support
Supporting simultaneous check of native HPD and the external GPIO proved
to be less stable than just native HPD. Drop the hpd-gpios support,
leaving just the native HPD support. In case the native HPD doesn't work
the user is urged to switch to specifying the HPD property to the
hdmi-connector device.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 14 +++---
drivers/gpu/drm/msm/hdmi/hdmi.h | 2 --
drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 53 +++--
3 files changed, 7 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index e160a23e962e..a9437054c015 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -468,17 +468,9 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(hdmi->extp_clk),
"failed to get extp clock\n");
- hdmi->hpd_gpiod = devm_gpiod_get_optional(>dev, "hpd", GPIOD_IN);
- /* This will catch e.g. -EPROBE_DEFER */
- if (IS_ERR(hdmi->hpd_gpiod))
- return dev_err_probe(dev, PTR_ERR(hdmi->hpd_gpiod),
-"failed to get hpd gpio\n");
-
- if (!hdmi->hpd_gpiod)
- DBG("failed to get HPD gpio");
-
- if (hdmi->hpd_gpiod)
- gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
+ if (of_find_property(dev->of_node, "hpd-gpios", NULL) ||
+ of_find_property(dev->of_node, "hpd-gpio", NULL))
+ dev_warn(dev, "hpd-gpios is not supported anymore, please
migrate to the hdmi-connector\n");
ret = msm_hdmi_get_phy(hdmi);
if (ret) {
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 2a98efa8b6bd..268ff8604423 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -52,8 +52,6 @@ struct hdmi {
struct clk_bulk_data *pwr_clks;
struct clk *extp_clk;
- struct gpio_desc *hpd_gpiod;
-
struct hdmi_phy *phy;
struct device *phy_dev;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 32e447267e3b..d3353c6148ed 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -69,9 +69,6 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
int ret;
unsigned long flags;
- if (hdmi->hpd_gpiod)
- gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
-
ret = pm_runtime_resume_and_get(dev);
if (ret)
return ret;
@@ -144,8 +141,11 @@ void msm_hdmi_hpd_irq(struct drm_bridge *bridge)
}
}
-static enum drm_connector_status detect_reg(struct hdmi *hdmi)
+enum drm_connector_status msm_hdmi_bridge_detect(
+ struct drm_bridge *bridge)
{
+ struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
+ struct hdmi *hdmi = hdmi_bridge->hdmi;
uint32_t hpd_int_status = 0;
int ret;
@@ -161,48 +161,3 @@ static enum drm_connector_status detect_reg(struct hdmi
*hdmi)
return (hpd_int_status & HDMI_HPD_INT_STATUS_CABLE_DETECTED) ?
connector_status_connected :
connector_status_disconnected;
}
-
-#define HPD_GPIO_INDEX 2
-static enum drm_connector_status detect_gpio(struct hdmi *hdmi)
-{
- return gpiod_get_value(hdmi->hpd_gpiod) ?
- connector_status_connected :
- connector_status_disconnected;
-}
-
-enum drm_connector_status msm_hdmi_bridge_detect(
- struct drm_bridge *bridge)
-{
- struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
- struct hdmi *hdmi = hdmi_bridge->hdmi;
- enum drm_connector_status stat_gpio, stat_reg;
- int retry = 20;
-
- /*
-* some platforms may not have hpd gpio. Rely only on the status
-* provided by REG_HDMI_HPD_INT_STATUS in this case.
-*/
- if (!hdmi->hpd_gpiod)
- return detect_reg(hdmi);
-
- do {
- stat_gpio = detect_gpio(hdmi);
- stat_reg = detect_reg(hdmi);
-
- if (stat_gpio == stat_reg)
- break;
-
- mdelay(10);
- } while (--retry);
-
- /* the status we get from reading gpio seems to be more reliable,
-* so trust that one the most if we didn't manage to get hdmi and
-* gpio status to agree:
-*/
- if (stat_gpio != stat_reg) {
- DBG("HDMI_HPD_INT_STATUS tells us: %d", stat_reg);
- DBG("hpd gpio tells us: %d", stat_gpio);
- }
-
- return stat_gpio;
-}
--
2.39.2
[PATCH v2 10/14] drm/msm/hdmi: rename hpd_clks to pwr_clks
As these clocks are now used in the runtime PM callbacks, they have no
connection to 'HPD'. Rename corresponding fields to follow clocks
purpose, to power up the HDMI controller.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 26 +-
drivers/gpu/drm/msm/hdmi/hdmi.h | 6 +++---
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index cc671baad87b..c39a1f3a7505 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -228,19 +228,19 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
.item ## _cnt = ARRAY_SIZE(item ## _names_ ## entry)
static const char *pwr_reg_names_8960[] = {"core-vdda"};
-static const char *hpd_clk_names_8960[] = {"core", "master_iface",
"slave_iface"};
+static const char *pwr_clk_names_8960[] = {"core", "master_iface",
"slave_iface"};
static const struct hdmi_platform_config hdmi_tx_8960_config = {
HDMI_CFG(pwr_reg, 8960),
- HDMI_CFG(hpd_clk, 8960),
+ HDMI_CFG(pwr_clk, 8960),
};
static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
-static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core",
"alt_iface"};
+static const char *pwr_clk_names_8x74[] = {"iface", "core", "mdp_core",
"alt_iface"};
static const struct hdmi_platform_config hdmi_tx_8974_config = {
HDMI_CFG(pwr_reg, 8x74),
- HDMI_CFG(hpd_clk, 8x74),
+ HDMI_CFG(pwr_clk, 8x74),
};
/*
@@ -449,17 +449,17 @@ static int msm_hdmi_dev_probe(struct platform_device
*pdev)
if (ret)
return dev_err_probe(dev, ret, "failed to get pwr
regulators\n");
- hdmi->hpd_clks = devm_kcalloc(>dev,
- config->hpd_clk_cnt,
- sizeof(hdmi->hpd_clks[0]),
+ hdmi->pwr_clks = devm_kcalloc(>dev,
+ config->pwr_clk_cnt,
+ sizeof(hdmi->pwr_clks[0]),
GFP_KERNEL);
- if (!hdmi->hpd_clks)
+ if (!hdmi->pwr_clks)
return -ENOMEM;
- for (i = 0; i < config->hpd_clk_cnt; i++)
- hdmi->hpd_clks[i].id = config->hpd_clk_names[i];
+ for (i = 0; i < config->pwr_clk_cnt; i++)
+ hdmi->pwr_clks[i].id = config->pwr_clk_names[i];
- ret = devm_clk_bulk_get(>dev, config->hpd_clk_cnt,
hdmi->hpd_clks);
+ ret = devm_clk_bulk_get(>dev, config->pwr_clk_cnt,
hdmi->pwr_clks);
if (ret)
return ret;
@@ -517,7 +517,7 @@ static int msm_hdmi_runtime_suspend(struct device *dev)
struct hdmi *hdmi = dev_get_drvdata(dev);
const struct hdmi_platform_config *config = hdmi->config;
- clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
+ clk_bulk_disable_unprepare(config->pwr_clk_cnt, hdmi->pwr_clks);
pinctrl_pm_select_sleep_state(dev);
@@ -540,7 +540,7 @@ static int msm_hdmi_runtime_resume(struct device *dev)
if (ret)
goto fail;
- ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
+ ret = clk_bulk_prepare_enable(config->pwr_clk_cnt, hdmi->pwr_clks);
if (ret)
goto fail;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index ee5463eb41b6..1e346e697f8e 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -49,7 +49,7 @@ struct hdmi {
phys_addr_t mmio_phy_addr;
struct regulator_bulk_data *pwr_regs;
- struct clk_bulk_data *hpd_clks;
+ struct clk_bulk_data *pwr_clks;
struct clk *extp_clk;
struct gpio_desc *hpd_gpiod;
@@ -90,8 +90,8 @@ struct hdmi_platform_config {
int pwr_reg_cnt;
/* clks that need to be on for hpd: */
- const char **hpd_clk_names;
- int hpd_clk_cnt;
+ const char **pwr_clk_names;
+ int pwr_clk_cnt;
};
struct hdmi_bridge {
--
2.39.2
[PATCH v2 06/14] drm/msm/hdmi: switch to clk_bulk API
The last platform using legacy clock names for HDMI block (APQ8064)
switched to new clock names in 5.16. It's time to stop caring about old
DT, drop hand-coded helpers and switch to clk_bulk_* API.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 15 +-
drivers/gpu/drm/msm/hdmi/hdmi.h | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 39 +
3 files changed, 19 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index c14e009f38b1..7ec4ca3b7597 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -469,17 +469,12 @@ static int msm_hdmi_dev_probe(struct platform_device
*pdev)
if (!hdmi->hpd_clks)
return -ENOMEM;
- for (i = 0; i < config->hpd_clk_cnt; i++) {
- struct clk *clk;
+ for (i = 0; i < config->hpd_clk_cnt; i++)
+ hdmi->hpd_clks[i].id = config->hpd_clk_names[i];
- clk = msm_clk_get(pdev, config->hpd_clk_names[i]);
- if (IS_ERR(clk))
- return dev_err_probe(dev, PTR_ERR(clk),
-"failed to get hpd clk: %s\n",
-config->hpd_clk_names[i]);
-
- hdmi->hpd_clks[i] = clk;
- }
+ ret = devm_clk_bulk_get(>dev, config->hpd_clk_cnt,
hdmi->hpd_clks);
+ if (ret)
+ return ret;
hdmi->extp_clk = devm_clk_get_optional(>dev, "extp");
if (IS_ERR(hdmi->extp_clk))
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index c0d60ed23b75..eeba85ffef09 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -50,7 +50,7 @@ struct hdmi {
struct regulator_bulk_data *hpd_regs;
struct regulator_bulk_data *pwr_regs;
- struct clk **hpd_clks;
+ struct clk_bulk_data *hpd_clks;
struct clk *extp_clk;
struct gpio_desc *hpd_gpiod;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 7ae69b14e953..36266aa626dc 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -60,27 +60,6 @@ static void msm_hdmi_phy_reset(struct hdmi *hdmi)
}
}
-static void enable_hpd_clocks(struct hdmi *hdmi, bool enable)
-{
- const struct hdmi_platform_config *config = hdmi->config;
- struct device *dev = >pdev->dev;
- int i, ret;
-
- if (enable) {
- for (i = 0; i < config->hpd_clk_cnt; i++) {
- ret = clk_prepare_enable(hdmi->hpd_clks[i]);
- if (ret) {
- DRM_DEV_ERROR(dev,
- "failed to enable hpd clk: %s (%d)\n",
- config->hpd_clk_names[i], ret);
- }
- }
- } else {
- for (i = config->hpd_clk_cnt - 1; i >= 0; i--)
- clk_disable_unprepare(hdmi->hpd_clks[i]);
- }
-}
-
int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
@@ -107,7 +86,9 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
pm_runtime_get_sync(dev);
- enable_hpd_clocks(hdmi, true);
+ ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
+ if (ret)
+ goto fail;
msm_hdmi_set_mode(hdmi, false);
msm_hdmi_phy_reset(hdmi);
@@ -149,7 +130,7 @@ void msm_hdmi_hpd_disable(struct hdmi *hdmi)
msm_hdmi_set_mode(hdmi, false);
- enable_hpd_clocks(hdmi, false);
+ clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
pm_runtime_put(dev);
ret = pinctrl_pm_select_sleep_state(dev);
@@ -193,14 +174,20 @@ void msm_hdmi_hpd_irq(struct drm_bridge *bridge)
static enum drm_connector_status detect_reg(struct hdmi *hdmi)
{
- uint32_t hpd_int_status;
+ const struct hdmi_platform_config *config = hdmi->config;
+ uint32_t hpd_int_status = 0;
+ int ret;
pm_runtime_get_sync(>pdev->dev);
- enable_hpd_clocks(hdmi, true);
+ ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
+ if (ret)
+ goto out;
hpd_int_status = hdmi_read(hdmi, REG_HDMI_HPD_INT_STATUS);
- enable_hpd_clocks(hdmi, false);
+ clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
+
+out:
pm_runtime_put(>pdev->dev);
return (hpd_int_status & HDMI_HPD_INT_STATUS_CABLE_DETECTED) ?
--
2.39.2
[PATCH v2 01/14] drm/msm/hdmi: move the alt_iface clock to the hpd list
According to the vendor kernel [1] , the alt_iface clock should be
enabled together with the rest of HPD clocks, to make HPD to work
properly.
[1]
https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/commit/e07a5487e521e57f76083c0a6e2f995414ac6d03
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 24abcb7254cc..108c86925780 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -235,9 +235,9 @@ static const struct hdmi_platform_config
hdmi_tx_8960_config = {
};
static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
-static const char *pwr_clk_names_8x74[] = {"extp", "alt_iface"};
-static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core"};
-static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0};
+static const char *pwr_clk_names_8x74[] = {"extp"};
+static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core",
"alt_iface"};
+static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0, 0};
static const struct hdmi_platform_config hdmi_tx_8974_config = {
HDMI_CFG(pwr_reg, 8x74),
--
2.39.2
[PATCH v2 09/14] drm/msm/hdmi: implement proper runtime PM handling
It is completely not obvious, but the so-called 'hpd' clocks and
regulators are required for the HDMI host to function properly. Merge
pwr and hpd regulators. Use regulators, clocks and pinctrl to implement
proper runtime PM callbacks.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c| 62 +-
drivers/gpu/drm/msm/hdmi/hdmi.h| 7 +---
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 12 ---
drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 42 +--
4 files changed, 48 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 7ec4ca3b7597..cc671baad87b 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
@@ -226,11 +227,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
.item ## _names = item ##_names_ ## entry, \
.item ## _cnt = ARRAY_SIZE(item ## _names_ ## entry)
-static const char *hpd_reg_names_8960[] = {"core-vdda"};
+static const char *pwr_reg_names_8960[] = {"core-vdda"};
static const char *hpd_clk_names_8960[] = {"core", "master_iface",
"slave_iface"};
static const struct hdmi_platform_config hdmi_tx_8960_config = {
- HDMI_CFG(hpd_reg, 8960),
+ HDMI_CFG(pwr_reg, 8960),
HDMI_CFG(hpd_clk, 8960),
};
@@ -434,20 +435,6 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
if (hdmi->irq < 0)
return hdmi->irq;
- hdmi->hpd_regs = devm_kcalloc(>dev,
- config->hpd_reg_cnt,
- sizeof(hdmi->hpd_regs[0]),
- GFP_KERNEL);
- if (!hdmi->hpd_regs)
- return -ENOMEM;
-
- for (i = 0; i < config->hpd_reg_cnt; i++)
- hdmi->hpd_regs[i].supply = config->hpd_reg_names[i];
-
- ret = devm_regulator_bulk_get(>dev, config->hpd_reg_cnt,
hdmi->hpd_regs);
- if (ret)
- return dev_err_probe(dev, ret, "failed to get hpd
regulators\n");
-
hdmi->pwr_regs = devm_kcalloc(>dev,
config->pwr_reg_cnt,
sizeof(hdmi->pwr_regs[0]),
@@ -525,6 +512,48 @@ static void msm_hdmi_dev_remove(struct platform_device
*pdev)
msm_hdmi_put_phy(hdmi);
}
+static int msm_hdmi_runtime_suspend(struct device *dev)
+{
+ struct hdmi *hdmi = dev_get_drvdata(dev);
+ const struct hdmi_platform_config *config = hdmi->config;
+
+ clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
+
+ pinctrl_pm_select_sleep_state(dev);
+
+ regulator_bulk_disable(config->pwr_reg_cnt, hdmi->pwr_regs);
+
+ return 0;
+}
+
+static int msm_hdmi_runtime_resume(struct device *dev)
+{
+ struct hdmi *hdmi = dev_get_drvdata(dev);
+ const struct hdmi_platform_config *config = hdmi->config;
+ int ret;
+
+ ret = regulator_bulk_enable(config->pwr_reg_cnt, hdmi->pwr_regs);
+ if (ret)
+ return ret;
+
+ ret = pinctrl_pm_select_default_state(dev);
+ if (ret)
+ goto fail;
+
+ ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
+ if (ret)
+ goto fail;
+
+ return 0;
+
+fail:
+ pinctrl_pm_select_sleep_state(dev);
+
+ return ret;
+}
+
+DEFINE_RUNTIME_DEV_PM_OPS(msm_hdmi_pm_ops, msm_hdmi_runtime_suspend,
msm_hdmi_runtime_resume, NULL);
+
static const struct of_device_id msm_hdmi_dt_match[] = {
{ .compatible = "qcom,hdmi-tx-8996", .data = _tx_8974_config },
{ .compatible = "qcom,hdmi-tx-8994", .data = _tx_8974_config },
@@ -541,6 +570,7 @@ static struct platform_driver msm_hdmi_driver = {
.driver = {
.name = "hdmi_msm",
.of_match_table = msm_hdmi_dt_match,
+ .pm = _hdmi_pm_ops,
},
};
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index eeba85ffef09..ee5463eb41b6 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -48,7 +48,6 @@ struct hdmi {
void __iomem *qfprom_mmio;
phys_addr_t mmio_phy_addr;
- struct regulator_bulk_data *hpd_regs;
struct regulator_bulk_data *pwr_regs;
struct clk_bulk_data *hpd_clks;
struct clk *extp_clk;
@@ -86,11 +85,7 @@ struct hdmi {
/* platform config data (ie. from DT, or pdata) */
struct hdmi_platform_config {
- /* regulators that need to be on for hpd: */
- const char **hpd_reg_names;
- int hpd_reg_cnt;
-
- /* regulators that need to be on for screen pwr: */
+ /* regulators that need to be on: */
const char **pwr_reg_names;
int pwr_reg_cnt;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index
[PATCH v2 08/14] drm/msm/hdmi: add runtime PM calls to DDC transfer function
We must be sure that the HDMI controller is powered on, while performing
the DDC transfer. Add corresponding runtime PM calls to
msm_hdmi_i2c_xfer().
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi_i2c.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
b/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
index 7aa500d24240..ebefea4fb408 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
@@ -107,11 +107,15 @@ static int msm_hdmi_i2c_xfer(struct i2c_adapter *i2c,
if (num == 0)
return num;
+ ret = pm_runtime_resume_and_get(>pdev->dev);
+ if (ret)
+ return ret;
+
init_ddc(hdmi_i2c);
ret = ddc_clear_irq(hdmi_i2c);
if (ret)
- return ret;
+ goto fail;
for (i = 0; i < num; i++) {
struct i2c_msg *p = [i];
@@ -169,7 +173,7 @@ static int msm_hdmi_i2c_xfer(struct i2c_adapter *i2c,
hdmi_read(hdmi, REG_HDMI_DDC_SW_STATUS),
hdmi_read(hdmi, REG_HDMI_DDC_HW_STATUS),
hdmi_read(hdmi, REG_HDMI_DDC_INT_CTRL));
- return ret;
+ goto fail;
}
ddc_status = hdmi_read(hdmi, REG_HDMI_DDC_SW_STATUS);
@@ -202,7 +206,13 @@ static int msm_hdmi_i2c_xfer(struct i2c_adapter *i2c,
}
}
+ pm_runtime_put(>pdev->dev);
+
return i;
+
+fail:
+ pm_runtime_put(>pdev->dev);
+ return ret;
}
static u32 msm_hdmi_i2c_func(struct i2c_adapter *adapter)
--
2.39.2
[PATCH v2 05/14] drm/msm/hdmi: drop clock frequency assignment
The only clock which has frequency being set through hpd_freqs is the
"core" aka MDSS_HDMI_CLK clock. It always has the specified frequency,
so we can drop corresponding clk_set_rate() call together with the
hpd_freq infrastructure.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 2 --
drivers/gpu/drm/msm/hdmi/hdmi.h | 1 -
drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 9 -
3 files changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 681265e29aa0..c14e009f38b1 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -236,12 +236,10 @@ static const struct hdmi_platform_config
hdmi_tx_8960_config = {
static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core",
"alt_iface"};
-static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0, 0};
static const struct hdmi_platform_config hdmi_tx_8974_config = {
HDMI_CFG(pwr_reg, 8x74),
HDMI_CFG(hpd_clk, 8x74),
- .hpd_freq = hpd_clk_freq_8x74,
};
/*
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index abdbe4779cf9..c0d60ed23b75 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -96,7 +96,6 @@ struct hdmi_platform_config {
/* clks that need to be on for hpd: */
const char **hpd_clk_names;
- const long unsigned *hpd_freq;
int hpd_clk_cnt;
};
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 9ce0ffa35417..7ae69b14e953 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -68,15 +68,6 @@ static void enable_hpd_clocks(struct hdmi *hdmi, bool enable)
if (enable) {
for (i = 0; i < config->hpd_clk_cnt; i++) {
- if (config->hpd_freq && config->hpd_freq[i]) {
- ret = clk_set_rate(hdmi->hpd_clks[i],
- config->hpd_freq[i]);
- if (ret)
- dev_warn(dev,
-"failed to set clk %s (%d)\n",
-config->hpd_clk_names[i], ret);
- }
-
ret = clk_prepare_enable(hdmi->hpd_clks[i]);
if (ret) {
DRM_DEV_ERROR(dev,
--
2.39.2
[PATCH v2 07/14] drm/msm/hdmi: switch to pm_runtime_resume_and_get()
The pm_runtime_get_sync() function is a bad choise for runtime power
management. Switch HDMI driver to pm_runtime_resume_and_get() and add
proper error handling, while we are at it.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 12 ++--
drivers/gpu/drm/msm/hdmi/hdmi_phy.c| 6 +-
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index fb99328107dd..d1b35328b6e8 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -19,7 +19,7 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
const struct hdmi_platform_config *config = hdmi->config;
int ret;
- pm_runtime_get_sync(>pdev->dev);
+ pm_runtime_resume_and_get(>pdev->dev);
ret = regulator_bulk_enable(config->pwr_reg_cnt, hdmi->pwr_regs);
if (ret)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 36266aa626dc..fc21ad3b01dc 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -85,7 +85,12 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
if (hdmi->hpd_gpiod)
gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
- pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "runtime resume failed: %d\n", ret);
+ goto fail;
+ }
+
ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
if (ret)
goto fail;
@@ -178,7 +183,10 @@ static enum drm_connector_status detect_reg(struct hdmi
*hdmi)
uint32_t hpd_int_status = 0;
int ret;
- pm_runtime_get_sync(>pdev->dev);
+ ret = pm_runtime_resume_and_get(>pdev->dev);
+ if (ret)
+ goto out;
+
ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
index 88a3423b7f24..d5acae752300 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
@@ -58,7 +58,11 @@ int msm_hdmi_phy_resource_enable(struct hdmi_phy *phy)
struct device *dev = >pdev->dev;
int i, ret = 0;
- pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "runtime resume failed: %d\n", ret);
+ return ret;
+ }
ret = regulator_bulk_enable(cfg->num_regs, phy->regs);
if (ret) {
--
2.39.2
[PATCH v2 04/14] drm/msm/hdmi: set infoframes on all pre_enable calls
In consequent modeset calls, the atomic_pre_enable() will be called
several times without calling atomic_post_disable() inbetween. Thus
iframes will not be updated for the next mode. Fix this by setting the
iframe outside of the !power_on check.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 3c6121c57b01..fb99328107dd 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -133,10 +133,11 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct
drm_bridge *bridge,
msm_hdmi_phy_resource_enable(phy);
msm_hdmi_power_on(bridge);
hdmi->power_on = true;
- if (hdmi->hdmi_mode) {
- msm_hdmi_config_avi_infoframe(hdmi);
- msm_hdmi_audio_update(hdmi);
- }
+ }
+
+ if (hdmi->hdmi_mode) {
+ msm_hdmi_config_avi_infoframe(hdmi);
+ msm_hdmi_audio_update(hdmi);
}
msm_hdmi_phy_powerup(phy, hdmi->pixclock);
--
2.39.2
[PATCH v2 03/14] drm/msm/hdmi: switch to atomic_pre_enable/post_disable
In preparation of reworking the HDMI mode setting, switch pre_enable and
post_disable callbacks to their atomic variants.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 9eb4d06bdc0e..3c6121c57b01 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -120,7 +120,8 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi)
hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val);
}
-static void msm_hdmi_bridge_pre_enable(struct drm_bridge *bridge)
+static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*old_bridge_state)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
@@ -146,7 +147,8 @@ static void msm_hdmi_bridge_pre_enable(struct drm_bridge
*bridge)
msm_hdmi_hdcp_on(hdmi->hdcp_ctrl);
}
-static void msm_hdmi_bridge_post_disable(struct drm_bridge *bridge)
+static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*old_bridge_state)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
@@ -292,8 +294,13 @@ static enum drm_mode_status
msm_hdmi_bridge_mode_valid(struct drm_bridge *bridge
}
static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = {
- .pre_enable = msm_hdmi_bridge_pre_enable,
- .post_disable = msm_hdmi_bridge_post_disable,
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+ .atomic_reset = drm_atomic_helper_bridge_reset,
+
+ .atomic_pre_enable = msm_hdmi_bridge_atomic_pre_enable,
+ .atomic_post_disable = msm_hdmi_bridge_atomic_post_disable,
+
.mode_set = msm_hdmi_bridge_mode_set,
.mode_valid = msm_hdmi_bridge_mode_valid,
.edid_read = msm_hdmi_bridge_edid_read,
--
2.39.2
[PATCH v2 02/14] drm/msm/hdmi: simplify extp clock handling
With the extp being the only "power" clock left, remove the surrounding
loops and handle the extp clock directly.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/hdmi/hdmi.c| 24
drivers/gpu/drm/msm/hdmi/hdmi.h| 6 +-
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 33 +
3 files changed, 18 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 108c86925780..681265e29aa0 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -235,13 +235,11 @@ static const struct hdmi_platform_config
hdmi_tx_8960_config = {
};
static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
-static const char *pwr_clk_names_8x74[] = {"extp"};
static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core",
"alt_iface"};
static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0, 0};
static const struct hdmi_platform_config hdmi_tx_8974_config = {
HDMI_CFG(pwr_reg, 8x74),
- HDMI_CFG(pwr_clk, 8x74),
HDMI_CFG(hpd_clk, 8x74),
.hpd_freq = hpd_clk_freq_8x74,
};
@@ -485,24 +483,10 @@ static int msm_hdmi_dev_probe(struct platform_device
*pdev)
hdmi->hpd_clks[i] = clk;
}
- hdmi->pwr_clks = devm_kcalloc(>dev,
- config->pwr_clk_cnt,
- sizeof(hdmi->pwr_clks[0]),
- GFP_KERNEL);
- if (!hdmi->pwr_clks)
- return -ENOMEM;
-
- for (i = 0; i < config->pwr_clk_cnt; i++) {
- struct clk *clk;
-
- clk = msm_clk_get(pdev, config->pwr_clk_names[i]);
- if (IS_ERR(clk))
- return dev_err_probe(dev, PTR_ERR(clk),
-"failed to get pwr clk: %s\n",
-config->pwr_clk_names[i]);
-
- hdmi->pwr_clks[i] = clk;
- }
+ hdmi->extp_clk = devm_clk_get_optional(>dev, "extp");
+ if (IS_ERR(hdmi->extp_clk))
+ return dev_err_probe(dev, PTR_ERR(hdmi->extp_clk),
+"failed to get extp clock\n");
hdmi->hpd_gpiod = devm_gpiod_get_optional(>dev, "hpd", GPIOD_IN);
/* This will catch e.g. -EPROBE_DEFER */
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 4586baf36415..abdbe4779cf9 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -51,7 +51,7 @@ struct hdmi {
struct regulator_bulk_data *hpd_regs;
struct regulator_bulk_data *pwr_regs;
struct clk **hpd_clks;
- struct clk **pwr_clks;
+ struct clk *extp_clk;
struct gpio_desc *hpd_gpiod;
@@ -98,10 +98,6 @@ struct hdmi_platform_config {
const char **hpd_clk_names;
const long unsigned *hpd_freq;
int hpd_clk_cnt;
-
- /* clks that need to be on for screen pwr (ie pixel clk): */
- const char **pwr_clk_names;
- int pwr_clk_cnt;
};
struct hdmi_bridge {
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 4a5b5112227f..9eb4d06bdc0e 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -17,7 +17,7 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
const struct hdmi_platform_config *config = hdmi->config;
- int i, ret;
+ int ret;
pm_runtime_get_sync(>pdev->dev);
@@ -25,21 +25,15 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
if (ret)
DRM_DEV_ERROR(dev->dev, "failed to enable pwr regulator: %d\n",
ret);
- if (config->pwr_clk_cnt > 0) {
+ if (hdmi->extp_clk) {
DBG("pixclock: %lu", hdmi->pixclock);
- ret = clk_set_rate(hdmi->pwr_clks[0], hdmi->pixclock);
- if (ret) {
- DRM_DEV_ERROR(dev->dev, "failed to set pixel clk: %s
(%d)\n",
- config->pwr_clk_names[0], ret);
- }
- }
+ ret = clk_set_rate(hdmi->extp_clk, hdmi->pixclock);
+ if (ret)
+ DRM_DEV_ERROR(dev->dev, "failed to set extp clk rate:
%d\n", ret);
- for (i = 0; i < config->pwr_clk_cnt; i++) {
- ret = clk_prepare_enable(hdmi->pwr_clks[i]);
- if (ret) {
- DRM_DEV_ERROR(dev->dev, "failed to enable pwr clk: %s
(%d)\n",
- config->pwr_clk_names[i], ret);
- }
+ ret = clk_prepare_enable(hdmi->extp_clk);
+ if (ret)
+ DRM_DEV_ERROR(dev->dev,
[PATCH v2 00/14] drm/msm/hdmi: rework and fix the HPD even generation
The MSM HDMI driver is plagued with the long-standing bug. If HDMI cable is disconnected, in most of the cases cable reconnection will not be detected properly. We have been carrying the patch from [1] in our integration tree for ages. The time has come to fix the long-standing bug and implement proper HPD handling. This series was tested on msm8996 and apq8064 boards. Previously HPD handling sometimes could trigger in the CRTC event handling, however I can no longer reproduce it now. [1] https://lore.kernel.org/linux-arm-msm/20171027105732.19235-2-arch...@codeaurora.org/ --- Dmitry Baryshkov (14): drm/msm/hdmi: move the alt_iface clock to the hpd list drm/msm/hdmi: simplify extp clock handling drm/msm/hdmi: switch to atomic_pre_enable/post_disable drm/msm/hdmi: set infoframes on all pre_enable calls drm/msm/hdmi: drop clock frequency assignment drm/msm/hdmi: switch to clk_bulk API drm/msm/hdmi: switch to pm_runtime_resume_and_get() drm/msm/hdmi: add runtime PM calls to DDC transfer function drm/msm/hdmi: implement proper runtime PM handling drm/msm/hdmi: rename hpd_clks to pwr_clks drm/msm/hdmi: expand the HDMI_CFG macro drm/msm/hdmi: drop hpd-gpios support drm/msm/hdmi: ensure that HDMI is one if HPD is requested drm/msm/hdmi: wire in hpd_enable/hpd_disable bridge ops drivers/gpu/drm/msm/hdmi/hdmi.c| 145 - drivers/gpu/drm/msm/hdmi/hdmi.h| 26 ++ drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 80 +- drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 142 ++-- drivers/gpu/drm/msm/hdmi/hdmi_i2c.c| 14 +++- drivers/gpu/drm/msm/hdmi/hdmi_phy.c| 6 +- 6 files changed, 157 insertions(+), 256 deletions(-) --- base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b change-id: 20240522-fd-hdmi-hpd-e3868deb6ae0 Best regards, -- Dmitry Baryshkov
[PATCH v2] Revert "drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set"
In the DPU driver blank IRQ handling is called from a vblank worker and
can happen outside of the irq_enable / irq_disable pair. Using the
worker makes that completely asynchronous with the rest of the code.
Revert commit d13f638c9b88 ("drm/msm/dpu: drop
dpu_encoder_phys_ops.atomic_mode_set") to fix vblank IRQ assignment for
CMD DSI panels.
Call trace:
dpu_encoder_phys_cmd_control_vblank_irq+0x218/0x294
dpu_encoder_toggle_vblank_for_crtc+0x160/0x194
dpu_crtc_vblank+0xbc/0x228
dpu_kms_enable_vblank+0x18/0x24
vblank_ctrl_worker+0x34/0x6c
process_one_work+0x218/0x620
worker_thread+0x1ac/0x37c
kthread+0x114/0x118
ret_from_fork+0x10/0x20
Fixes: d13f638c9b88 ("drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set")
Signed-off-by: Dmitry Baryshkov
---
Changes in v2:
- Expanded commit message to describe the reason for revert and added a
call trace (Abhinav)
- Link to v1:
https://lore.kernel.org/r/20240514-dpu-revert-ams-v1-1-b13623d6c...@linaro.org
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 5
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 32 --
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 13 +++--
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 11 +++-
5 files changed, 46 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..a7d8ecf3f5be 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1200,6 +1200,8 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
phys->cached_mode = crtc_state->adjusted_mode;
+ if (phys->ops.atomic_mode_set)
+ phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
}
}
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 002e89cc1705..30470cd15a48 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -69,6 +69,8 @@ struct dpu_encoder_phys;
* @is_master: Whether this phys_enc is the current master
* encoder. Can be switched at enable time. Based
* on split_role and current mode (CMD/VID).
+ * @atomic_mode_set: DRM Call. Set a DRM mode.
+ * This likely caches the mode, for use at enable.
* @enable:DRM Call. Enable a DRM mode.
* @disable: DRM Call. Disable mode.
* @control_vblank_irq Register/Deregister for VBLANK IRQ
@@ -93,6 +95,9 @@ struct dpu_encoder_phys;
struct dpu_encoder_phys_ops {
void (*prepare_commit)(struct dpu_encoder_phys *encoder);
bool (*is_master)(struct dpu_encoder_phys *encoder);
+ void (*atomic_mode_set)(struct dpu_encoder_phys *encoder,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state);
void (*enable)(struct dpu_encoder_phys *encoder);
void (*disable)(struct dpu_encoder_phys *encoder);
int (*control_vblank_irq)(struct dpu_encoder_phys *enc, bool enable);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 489be1c0c704..95cd39b49668 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -142,6 +142,23 @@ static void dpu_encoder_phys_cmd_underrun_irq(void *arg)
dpu_encoder_underrun_callback(phys_enc->parent, phys_enc);
}
+static void dpu_encoder_phys_cmd_atomic_mode_set(
+ struct dpu_encoder_phys *phys_enc,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ phys_enc->irq[INTR_IDX_CTL_START] = phys_enc->hw_ctl->caps->intr_start;
+
+ phys_enc->irq[INTR_IDX_PINGPONG] = phys_enc->hw_pp->caps->intr_done;
+
+ if (phys_enc->has_intf_te)
+ phys_enc->irq[INTR_IDX_RDPTR] =
phys_enc->hw_intf->cap->intr_tear_rd_ptr;
+ else
+ phys_enc->irq[INTR_IDX_RDPTR] =
phys_enc->hw_pp->caps->intr_rdptr;
+
+ phys_enc->irq[INTR_IDX_UNDERRUN] =
phys_enc->hw_intf->cap->intr_underrun;
+}
+
static int _dpu_encoder_phys_cmd_handle_ppdone_timeout(
struct dpu_encoder_phys *phys_enc)
{
@@ -280,14 +297,6 @@ static void dpu_encoder_phys_cmd_irq_enable(struct
dpu_encoder_phys *phys_enc)
phys_enc->hw_pp->idx - PINGPONG_0,
phys_enc->vblank_refcount);
- phys_enc->irq[INTR_IDX_CTL_START] = phys_enc->hw_ctl->caps->intr_start;
-
[PATCH v3 3/3] drm/panel/lg-sw43408: mark sw43408_backlight_ops as static
Fix sparse warning regarding symbol 'sw43408_backlight_ops' not being
declared.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202404200739.hbwzvohr-...@intel.com/
Reviewed-by: Neil Armstrong
Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/panel/panel-lg-sw43408.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c
b/drivers/gpu/drm/panel/panel-lg-sw43408.c
index 115f4702d59f..2b3a73696dce 100644
--- a/drivers/gpu/drm/panel/panel-lg-sw43408.c
+++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
@@ -182,7 +182,7 @@ static int sw43408_backlight_update_status(struct
backlight_device *bl)
return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
}
-const struct backlight_ops sw43408_backlight_ops = {
+static const struct backlight_ops sw43408_backlight_ops = {
.update_status = sw43408_backlight_update_status,
};
--
2.39.2
[PATCH v3 1/3] drm/display: split DSC helpers from DP helpers
Currently the DRM DSC functions are selected by the DRM_DISPLAY_DP_HELPER Kconfig symbol. This is not optimal, since the DSI code (both panel and host drivers) end up selecting the seemingly irrelevant DP helpers. Split the DSC code to be guarded by the separate DRM_DISPLAY_DSC_HELPER Kconfig symbol. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig| 6 ++ drivers/gpu/drm/display/Makefile | 3 ++- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig| 1 + drivers/gpu/drm/panel/Kconfig | 4 ++-- 6 files changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 22d88f8ef527..b69d5c4a5367 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -6,6 +6,7 @@ config DRM_AMDGPU depends on !UML select FW_LOADER select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 864a6488bfdf..f524cf95dec3 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -59,6 +59,12 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG If in doubt, say "N". +config DRM_DISPLAY_DSC_HELPER + bool + depends on DRM_DISPLAY_HELPER + help + DRM display helpers for VESA DSC (used by DSI and DisplayPort). + config DRM_DISPLAY_HDCP_HELPER bool depends on DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile index 17d2cc73ff56..2ec71e15c3cb 100644 --- a/drivers/gpu/drm/display/Makefile +++ b/drivers/gpu/drm/display/Makefile @@ -6,7 +6,8 @@ drm_display_helper-y := drm_display_helper_mod.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \ drm_dp_dual_mode_helper.o \ drm_dp_helper.o \ - drm_dp_mst_topology.o \ + drm_dp_mst_topology.o +drm_display_helper-$(CONFIG_DRM_DISPLAY_DSC_HELPER) += \ drm_dsc_helper.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_TUNNEL) += \ drm_dp_tunnel.o diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 5932024f8f95..117b84260b1c 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -11,6 +11,7 @@ config DRM_I915 select SHMEM select TMPFS select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 1931ecf73e32..6dcd26180611 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -111,6 +111,7 @@ config DRM_MSM_DSI depends on DRM_MSM select DRM_PANEL select DRM_MIPI_DSI + select DRM_DISPLAY_DSC_HELPER default y help Choose this option if you have a need for MIPI DSI connector diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 982324ef5a41..4a2f621433ef 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -547,7 +547,7 @@ config DRM_PANEL_RAYDIUM_RM692E5 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for Raydium RM692E5-based @@ -905,7 +905,7 @@ config DRM_PANEL_VISIONOX_R66451 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for Visionox -- 2.39.2
[PATCH v3 2/3] drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER
This panel driver uses DSC PPS functions and as such depends on the
DRM_DISPLAY_DP_HELPER. Select this symbol to make required functions
available to the driver.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202404200800.kysryyli-...@intel.com/
Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/panel/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 4a2f621433ef..3e3f63479544 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -340,6 +340,8 @@ config DRM_PANEL_LG_SW43408
depends on OF
depends on DRM_MIPI_DSI
depends on BACKLIGHT_CLASS_DEVICE
+ select DRM_DISPLAY_DSC_HELPER
+ select DRM_DISPLAY_HELPER
help
Say Y here if you want to enable support for LG sw43408 panel.
The panel has a 1080x2160@60Hz resolution and uses 24 bit RGB per
--
2.39.2
[PATCH v3 0/3] drm/panel: two fixes for lg-sw43408
Fix two issues with the panel-lg-sw43408 driver reported by the kernel test robot. To: Neil Armstrong To: Jessica Zhang To: Sam Ravnborg To: Maarten Lankhorst To: Maxime Ripard To: Thomas Zimmermann To: David Airlie To: Daniel Vetter To: Sumit Semwal To: Caleb Connolly To: Alex Deucher To: Christian König To: Pan, Xinhui To: Jani Nikula To: Joonas Lahtinen To: Rodrigo Vivi To: Tvrtko Ursulin To: Rob Clark To: Abhinav Kumar To: Sean Paul To: Marijn Suijten To: Vinod Koul To: Caleb Connolly Cc: dri-de...@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Cc: amd-...@lists.freedesktop.org Cc: intel-...@lists.freedesktop.org Cc: linux-arm-...@vger.kernel.org Cc: freedreno@lists.freedesktop.org Signed-off-by: Dmitry Baryshkov Changes in v3: - Split DRM_DISPLAY_DSC_HELPER from DRM_DISPLAY_DP_HELPER - Added missing Fixes tags - Link to v2: https://lore.kernel.org/r/20240510-panel-sw43408-fix-v2-0-d1ef91ee1...@linaro.org Changes in v2: - use SELECT instead of DEPEND to follow the reverted Kconfig changes - Link to v1: https://lore.kernel.org/r/20240420-panel-sw43408-fix-v1-0-b282ff725...@linaro.org --- Dmitry Baryshkov (3): drm/display: split DSC helpers from DP helpers drm/panel/lg-sw43408: select CONFIG_DRM_DISPLAY_DP_HELPER drm/panel/lg-sw43408: mark sw43408_backlight_ops as static drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig | 6 ++ drivers/gpu/drm/display/Makefile | 3 ++- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig | 1 + drivers/gpu/drm/panel/Kconfig| 6 -- drivers/gpu/drm/panel/panel-lg-sw43408.c | 2 +- 7 files changed, 16 insertions(+), 4 deletions(-) --- base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b change-id: 20240420-panel-sw43408-fix-ff6549c121be Best regards, -- Dmitry Baryshkov
