Re: [PATCH 4/4] gpu: ipu-v3: Convert to platform remove callback returning void
On Di, 2024-04-09 at 19:02 +0200, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is ignored (apart > from emitting a warning) and this typically results in resource leaks. > > To improve here there is a quest to make the remove callback return > void. In the first step of this quest all drivers are converted to > .remove_new(), which already returns void. Eventually after all drivers > are converted, .remove_new() will be renamed to .remove(). > > Trivially convert the ipu-v3 platform drivers from always returning zero > in the remove callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH v5 11/16] drm/vkms: Add YUV support
Hi Louis, On Mi, 2024-03-13 at 18:45 +0100, Louis Chauvet wrote: > From: Arthur Grillo > > Add support to the YUV formats bellow: > > - NV12/NV16/NV24 > - NV21/NV61/NV42 > - YUV420/YUV422/YUV444 > - YVU420/YVU422/YVU444 > > The conversion from yuv to rgb is done with fixed-point arithmetic, using > 32.32 floats and the drm_fixed helpers. s/floats/fixed-point numbers/ Nothing floating here, the point is fixed. [...] > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index 23e1d247468d..f3116084de5a 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -99,6 +99,27 @@ typedef void (*pixel_read_line_t)(const struct > vkms_plane_state *plane, int x_st > int y_start, enum pixel_read_direction > direction, int count, > struct pixel_argb_u16 out_pixel[]); > > +/** > + * CONVERSION_MATRIX_FLOAT_DEPTH - Number of digits after the point for > conversion matrix values s/CONVERSION_MATRIX_FLOAT_DEPTH/CONVERSION_MATRIX_FRACTIONAL_BITS/ Just a suggestion, maybe there are better terms, but using "FLOAT" here is confusing. > + */ > +#define CONVERSION_MATRIX_FLOAT_DEPTH 32 > + > +/** > + * struct conversion_matrix - Matrix to use for a specific encoding and range > + * > + * @matrix: Conversion matrix from yuv to rgb. The matrix is stored in a > row-major manner and is > + * used to compute rgb values from yuv values: > + * [[r],[g],[b]] = @matrix * [[y],[u],[v]] > + * OR for yvu formats: > + * [[r],[g],[b]] = @matrix * [[y],[v],[u]] > + * The values of the matrix are fixed floats, > 32.CONVERSION_MATRIX_FLOAT_DEPTH s/fixed floats/fixed-point numbers/ regards Philipp
Re: [PATCH 5/8] drm/imx/ipuv3: do not return negative values from .get_modes()
On Fr, 2024-03-08 at 18:03 +0200, Jani Nikula wrote: > The .get_modes() hooks aren't supposed to return negative error > codes. Return 0 for no modes, whatever the reason. > > Cc: Philipp Zabel > Cc: sta...@vger.kernel.org > Signed-off-by: Jani Nikula Acked-by: Philipp Zabel regards Philipp
Re: [PATCH v2 1/3] drm/etnaviv: Update hardware headers from rnndb
On Do, 2024-01-25 at 17:14 +0100, Christian Gmeiner wrote:
> >
> > Update the state HI header from the rnndb commit
> > 8d7ee714cfe2 ("Merge pull request #24 from pH5/unknown-3950").
> >
> > Signed-off-by: Philipp Zabel
>
> You missed my R-b from the v1 series for this patch - please include
> it the next time!
>
> Reviewed-by: Christian Gmeiner
Sorry, I'm trying the b4 prep/send workflow and hadn't internalized yet
that "b4 trailers -u" is still a necessary manual step.
regards
Philipp
[PATCH v2 0/3] drm/etnaviv: Disable SH_EU clock gating on the i.MX8MP NPU
The vendor kernel sets a previously unknown clock gating bit in the VIVS_PM_MODULE_CONTROLS register to disable SH_EU clock gating. Import new headers from rnndb for the definition and set the bit for the VIPNano-Si+ NPU on i.MX8MP and other affected cores. Signed-off-by: Philipp Zabel --- Changes in v2: - Add patch to turn etnaviv_is_model_rev() into a function. - Use model numbers instead of made up GC model names. - Also disable SH_EU clock gating on other models/revisions listed in the vendor kernel. - Link to v1: https://lore.kernel.org/r/20240124-etnaviv-npu-v1-0-a5aaf64ae...@pengutronix.de --- Philipp Zabel (3): drm/etnaviv: Update hardware headers from rnndb drm/etnaviv: Turn etnaviv_is_model_rev() into a function drm/etnaviv: Disable SH_EU clock gating on VIPNano-Si+ drivers/gpu/drm/etnaviv/cmdstream.xml.h | 52 ++-- drivers/gpu/drm/etnaviv/common.xml.h| 12 ++-- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 72 +-- drivers/gpu/drm/etnaviv/state.xml.h | 101 +++- drivers/gpu/drm/etnaviv/state_blt.xml.h | 20 +++ drivers/gpu/drm/etnaviv/state_hi.xml.h | 28 + 6 files changed, 210 insertions(+), 75 deletions(-) --- base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d change-id: 20240124-etnaviv-npu-627f6881322c Best regards, -- Philipp Zabel
[PATCH v2 1/3] drm/etnaviv: Update hardware headers from rnndb
Update the state HI header from the rnndb commit
8d7ee714cfe2 ("Merge pull request #24 from pH5/unknown-3950").
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/etnaviv/cmdstream.xml.h | 52 ++--
drivers/gpu/drm/etnaviv/common.xml.h| 12 ++--
drivers/gpu/drm/etnaviv/state.xml.h | 101 +++-
drivers/gpu/drm/etnaviv/state_blt.xml.h | 20 +++
drivers/gpu/drm/etnaviv/state_hi.xml.h | 28 +
5 files changed, 170 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/cmdstream.xml.h
b/drivers/gpu/drm/etnaviv/cmdstream.xml.h
index 65f1ba1099bd..a96597a27ae2 100644
--- a/drivers/gpu/drm/etnaviv/cmdstream.xml.h
+++ b/drivers/gpu/drm/etnaviv/cmdstream.xml.h
@@ -8,11 +8,11 @@ This file was generated by the rules-ng-ng headergen tool in
this git repository
git clone git://0x04.net/rules-ng-ng
The rules-ng-ng source files this header was generated from are:
-- cmdstream.xml ( 14094 bytes, from 2016-11-11 06:55:14)
-- copyright.xml ( 1597 bytes, from 2016-10-29 07:29:22)
-- common.xml( 23344 bytes, from 2016-11-10 15:14:07)
+- cmdstream.xml ( 16933 bytes, from 2023-12-11 15:50:17)
+- copyright.xml ( 1597 bytes, from 2016-11-10 13:58:32)
+- common.xml( 35664 bytes, from 2023-12-06 10:55:32)
-Copyright (C) 2012-2016 by the following authors:
+Copyright (C) 2012-2023 by the following authors:
- Wladimir J. van der Laan
- Christian Gmeiner
- Lucas Stach
@@ -52,6 +52,9 @@ DEALINGS IN THE SOFTWARE.
#define FE_OPCODE_RETURN 0x000b
#define FE_OPCODE_DRAW_INSTANCED 0x000c
#define FE_OPCODE_CHIP_SELECT 0x000d
+#define FE_OPCODE_WAIT_FENCE 0x000f
+#define FE_OPCODE_DRAW_INDIRECT
0x0010
+#define FE_OPCODE_SNAP_PAGES 0x0013
#define PRIMITIVE_TYPE_POINTS 0x0001
#define PRIMITIVE_TYPE_LINES 0x0002
#define PRIMITIVE_TYPE_LINE_STRIP 0x0003
@@ -192,6 +195,9 @@ DEALINGS IN THE SOFTWARE.
#define VIV_FE_STALL_TOKEN_TO__MASK0x1f00
#define VIV_FE_STALL_TOKEN_TO__SHIFT 8
#define VIV_FE_STALL_TOKEN_TO(x) (((x) <<
VIV_FE_STALL_TOKEN_TO__SHIFT) & VIV_FE_STALL_TOKEN_TO__MASK)
+#define VIV_FE_STALL_TOKEN_UNK28__MASK 0x3000
+#define VIV_FE_STALL_TOKEN_UNK28__SHIFT28
+#define VIV_FE_STALL_TOKEN_UNK28(x)(((x) <<
VIV_FE_STALL_TOKEN_UNK28__SHIFT) & VIV_FE_STALL_TOKEN_UNK28__MASK)
#define VIV_FE_CALL0x
@@ -266,5 +272,43 @@ DEALINGS IN THE SOFTWARE.
#define VIV_FE_DRAW_INSTANCED_START_INDEX__SHIFT 0
#define VIV_FE_DRAW_INSTANCED_START_INDEX(x) (((x) <<
VIV_FE_DRAW_INSTANCED_START_INDEX__SHIFT) &
VIV_FE_DRAW_INSTANCED_START_INDEX__MASK)
+#define VIV_FE_WAIT_FENCE 0x
+
+#define VIV_FE_WAIT_FENCE_HEADER 0x
+#define VIV_FE_WAIT_FENCE_HEADER_OP__MASK 0xf800
+#define VIV_FE_WAIT_FENCE_HEADER_OP__SHIFT 27
+#define VIV_FE_WAIT_FENCE_HEADER_OP_WAIT_FENCE 0x7800
+#define VIV_FE_WAIT_FENCE_HEADER_UNK16__MASK 0x0003
+#define VIV_FE_WAIT_FENCE_HEADER_UNK16__SHIFT 16
+#define VIV_FE_WAIT_FENCE_HEADER_UNK16(x) (((x) <<
VIV_FE_WAIT_FENCE_HEADER_UNK16__SHIFT) & VIV_FE_WAIT_FENCE_HEADER_UNK16__MASK)
+#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__MASK 0x
+#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__SHIFT 0
+#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT(x) (((x) <<
VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__SHIFT) &
VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__MASK)
+
+#define VIV_FE_WAIT_FENCE_ADDRESS 0x0004
+
+#define VIV_FE_DRAW_INDIRECT 0x
+
+#define VIV_FE_DRAW_INDIRECT_HEADER0x
+#define VIV_FE_DRAW_INDIRECT_HEADER_OP__MASK 0xf800
+#define VIV_FE_DRAW_INDIRECT_HEADER_OP__SHIFT 27
+#define VIV_FE_DRAW_INDIRECT_HEADER_OP_DRAW_INDIRECT 0x8000
+#define VIV_FE_DRAW_INDIRECT_HEADER_INDEXED0x0100
+#define VIV_FE_DRAW_INDIRECT_HEADER_TYPE__MASK 0x000f
+#define VIV_FE_DRAW_INDIRECT_HEADER_TYPE__SHIFT0
+#define VIV_FE_DRAW_INDIRECT_HEADER_TYPE(x)(((x) <<
VIV_FE_DRAW_INDIRECT_HEADER_
[PATCH v2 3/3] drm/etnaviv: Disable SH_EU clock gating on VIPNano-Si+
Disable SH_EU clock gating for the VIPNano-Si+ NPU on i.MX8MP and for other affected core revisions. Taken from linux-imx lf-6.1.36-2.1.0, specifically [1]. [1] https://github.com/nxp-imx/linux-imx/blob/lf-6.1.36-2.1.0/drivers/mxc/gpu-viv/hal/kernel/arch/gc_hal_kernel_hardware.c#L2747-L2761 Signed-off-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index c61d50dd3829..c669419b3ae3 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -643,6 +643,12 @@ static void etnaviv_gpu_enable_mlcg(struct etnaviv_gpu *gpu) pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_SE | VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA; + /* Disable SH_EU clock gating on affected core revisions. */ + if (etnaviv_is_model_rev(gpu, 0x8000, 0x7200) || + etnaviv_is_model_rev(gpu, 0x8000, 0x8002) || + etnaviv_is_model_rev(gpu, 0x9200, 0x6304)) + pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_SH_EU; + pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA_HZ; pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA_EZ; -- 2.39.2
[PATCH v2 2/3] drm/etnaviv: Turn etnaviv_is_model_rev() into a function
Turn the etnaviv_is_model_rev() macro into a static inline function.
Use the raw model number as a parameter instead of the chipModel_GC
defines. This reduces synchronization requirements for the generated
headers. For newer hardware, the GC names are not the correct model
names anyway. For example, model 0x8000 NPUs are called VIPNano-QI/SI(+)
by VeriSilicon.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 66 ++-
1 file changed, 34 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 9b8445d2a128..c61d50dd3829 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -172,10 +172,12 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32
param, u64 *value)
return 0;
}
+static inline bool etnaviv_is_model_rev(struct etnaviv_gpu *gpu, u32 model,
u32 revision)
+{
+ return gpu->identity.model == model &&
+ gpu->identity.revision == revision;
+}
-#define etnaviv_is_model_rev(gpu, mod, rev) \
- ((gpu)->identity.model == chipModel_##mod && \
-(gpu)->identity.revision == rev)
#define etnaviv_field(val, field) \
(((val) & field##__MASK) >> field##__SHIFT)
@@ -281,7 +283,7 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
switch (gpu->identity.instruction_count) {
case 0:
- if (etnaviv_is_model_rev(gpu, GC2000, 0x5108) ||
+ if (etnaviv_is_model_rev(gpu, 0x2000, 0x5108) ||
gpu->identity.model == chipModel_GC880)
gpu->identity.instruction_count = 512;
else
@@ -315,17 +317,17 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
* For some cores, two varyings are consumed for position, so the
* maximum varying count needs to be reduced by one.
*/
- if (etnaviv_is_model_rev(gpu, GC5000, 0x5434) ||
- etnaviv_is_model_rev(gpu, GC4000, 0x5222) ||
- etnaviv_is_model_rev(gpu, GC4000, 0x5245) ||
- etnaviv_is_model_rev(gpu, GC4000, 0x5208) ||
- etnaviv_is_model_rev(gpu, GC3000, 0x5435) ||
- etnaviv_is_model_rev(gpu, GC2200, 0x5244) ||
- etnaviv_is_model_rev(gpu, GC2100, 0x5108) ||
- etnaviv_is_model_rev(gpu, GC2000, 0x5108) ||
- etnaviv_is_model_rev(gpu, GC1500, 0x5246) ||
- etnaviv_is_model_rev(gpu, GC880, 0x5107) ||
- etnaviv_is_model_rev(gpu, GC880, 0x5106))
+ if (etnaviv_is_model_rev(gpu, 0x5000, 0x5434) ||
+ etnaviv_is_model_rev(gpu, 0x4000, 0x5222) ||
+ etnaviv_is_model_rev(gpu, 0x4000, 0x5245) ||
+ etnaviv_is_model_rev(gpu, 0x4000, 0x5208) ||
+ etnaviv_is_model_rev(gpu, 0x3000, 0x5435) ||
+ etnaviv_is_model_rev(gpu, 0x2200, 0x5244) ||
+ etnaviv_is_model_rev(gpu, 0x2100, 0x5108) ||
+ etnaviv_is_model_rev(gpu, 0x2000, 0x5108) ||
+ etnaviv_is_model_rev(gpu, 0x1500, 0x5246) ||
+ etnaviv_is_model_rev(gpu, 0x880, 0x5107) ||
+ etnaviv_is_model_rev(gpu, 0x880, 0x5106))
gpu->identity.varyings_count -= 1;
}
@@ -351,7 +353,7 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
* Reading these two registers on GC600 rev 0x19 result in a
* unhandled fault: external abort on non-linefetch
*/
- if (!etnaviv_is_model_rev(gpu, GC600, 0x19)) {
+ if (!etnaviv_is_model_rev(gpu, 0x600, 0x19)) {
gpu->identity.product_id = gpu_read(gpu,
VIVS_HI_CHIP_PRODUCT_ID);
gpu->identity.eco_id = gpu_read(gpu,
VIVS_HI_CHIP_ECO_ID);
}
@@ -368,7 +370,7 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
}
/* Another special case */
- if (etnaviv_is_model_rev(gpu, GC300, 0x2201)) {
+ if (etnaviv_is_model_rev(gpu, 0x300, 0x2201)) {
u32 chipTime = gpu_read(gpu, VIVS_HI_CHIP_TIME);
if (chipDate == 0x20080814 && chipTime == 0x12051100) {
@@ -387,15 +389,15 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
* Fix model/rev here, so all other places can refer to this
* core by its real identity.
*/
- if (etnaviv_is_model_rev(gpu, GC2000, 0x5450)) {
+ if (etnaviv_is_model_rev(gpu, 0x2000, 0x5450)) {
gpu->identity.model = chipModel_GC3000;
gpu->identity.revision &= 0x;
}
- if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate ==
0x20120617))
+ if (etnaviv_is_model_rev(gpu, 0x1000, 0x50
[PATCH 1/2] drm/etnaviv: Update hardware headers from rnndb
Update the state HI header from the rnndb commit
8d7ee714cfe2 ("Merge pull request #24 from pH5/unknown-3950").
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/etnaviv/cmdstream.xml.h | 52 ++--
drivers/gpu/drm/etnaviv/common.xml.h| 12 ++--
drivers/gpu/drm/etnaviv/state.xml.h | 101 +++-
drivers/gpu/drm/etnaviv/state_blt.xml.h | 20 +++
drivers/gpu/drm/etnaviv/state_hi.xml.h | 28 +
5 files changed, 170 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/cmdstream.xml.h
b/drivers/gpu/drm/etnaviv/cmdstream.xml.h
index 65f1ba1099bd..a96597a27ae2 100644
--- a/drivers/gpu/drm/etnaviv/cmdstream.xml.h
+++ b/drivers/gpu/drm/etnaviv/cmdstream.xml.h
@@ -8,11 +8,11 @@ This file was generated by the rules-ng-ng headergen tool in
this git repository
git clone git://0x04.net/rules-ng-ng
The rules-ng-ng source files this header was generated from are:
-- cmdstream.xml ( 14094 bytes, from 2016-11-11 06:55:14)
-- copyright.xml ( 1597 bytes, from 2016-10-29 07:29:22)
-- common.xml( 23344 bytes, from 2016-11-10 15:14:07)
+- cmdstream.xml ( 16933 bytes, from 2023-12-11 15:50:17)
+- copyright.xml ( 1597 bytes, from 2016-11-10 13:58:32)
+- common.xml( 35664 bytes, from 2023-12-06 10:55:32)
-Copyright (C) 2012-2016 by the following authors:
+Copyright (C) 2012-2023 by the following authors:
- Wladimir J. van der Laan
- Christian Gmeiner
- Lucas Stach
@@ -52,6 +52,9 @@ DEALINGS IN THE SOFTWARE.
#define FE_OPCODE_RETURN 0x000b
#define FE_OPCODE_DRAW_INSTANCED 0x000c
#define FE_OPCODE_CHIP_SELECT 0x000d
+#define FE_OPCODE_WAIT_FENCE 0x000f
+#define FE_OPCODE_DRAW_INDIRECT
0x0010
+#define FE_OPCODE_SNAP_PAGES 0x0013
#define PRIMITIVE_TYPE_POINTS 0x0001
#define PRIMITIVE_TYPE_LINES 0x0002
#define PRIMITIVE_TYPE_LINE_STRIP 0x0003
@@ -192,6 +195,9 @@ DEALINGS IN THE SOFTWARE.
#define VIV_FE_STALL_TOKEN_TO__MASK0x1f00
#define VIV_FE_STALL_TOKEN_TO__SHIFT 8
#define VIV_FE_STALL_TOKEN_TO(x) (((x) <<
VIV_FE_STALL_TOKEN_TO__SHIFT) & VIV_FE_STALL_TOKEN_TO__MASK)
+#define VIV_FE_STALL_TOKEN_UNK28__MASK 0x3000
+#define VIV_FE_STALL_TOKEN_UNK28__SHIFT28
+#define VIV_FE_STALL_TOKEN_UNK28(x)(((x) <<
VIV_FE_STALL_TOKEN_UNK28__SHIFT) & VIV_FE_STALL_TOKEN_UNK28__MASK)
#define VIV_FE_CALL0x
@@ -266,5 +272,43 @@ DEALINGS IN THE SOFTWARE.
#define VIV_FE_DRAW_INSTANCED_START_INDEX__SHIFT 0
#define VIV_FE_DRAW_INSTANCED_START_INDEX(x) (((x) <<
VIV_FE_DRAW_INSTANCED_START_INDEX__SHIFT) &
VIV_FE_DRAW_INSTANCED_START_INDEX__MASK)
+#define VIV_FE_WAIT_FENCE 0x
+
+#define VIV_FE_WAIT_FENCE_HEADER 0x
+#define VIV_FE_WAIT_FENCE_HEADER_OP__MASK 0xf800
+#define VIV_FE_WAIT_FENCE_HEADER_OP__SHIFT 27
+#define VIV_FE_WAIT_FENCE_HEADER_OP_WAIT_FENCE 0x7800
+#define VIV_FE_WAIT_FENCE_HEADER_UNK16__MASK 0x0003
+#define VIV_FE_WAIT_FENCE_HEADER_UNK16__SHIFT 16
+#define VIV_FE_WAIT_FENCE_HEADER_UNK16(x) (((x) <<
VIV_FE_WAIT_FENCE_HEADER_UNK16__SHIFT) & VIV_FE_WAIT_FENCE_HEADER_UNK16__MASK)
+#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__MASK 0x
+#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__SHIFT 0
+#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT(x) (((x) <<
VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__SHIFT) &
VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__MASK)
+
+#define VIV_FE_WAIT_FENCE_ADDRESS 0x0004
+
+#define VIV_FE_DRAW_INDIRECT 0x
+
+#define VIV_FE_DRAW_INDIRECT_HEADER0x
+#define VIV_FE_DRAW_INDIRECT_HEADER_OP__MASK 0xf800
+#define VIV_FE_DRAW_INDIRECT_HEADER_OP__SHIFT 27
+#define VIV_FE_DRAW_INDIRECT_HEADER_OP_DRAW_INDIRECT 0x8000
+#define VIV_FE_DRAW_INDIRECT_HEADER_INDEXED0x0100
+#define VIV_FE_DRAW_INDIRECT_HEADER_TYPE__MASK 0x000f
+#define VIV_FE_DRAW_INDIRECT_HEADER_TYPE__SHIFT0
+#define VIV_FE_DRAW_INDIRECT_HEADER_TYPE(x)(((x) <<
VIV_FE_DRAW_INDIRECT_HEADER_
[PATCH 0/2] drm/etnaviv: Disable SH_EU clock gating on the i.MX8MP NPU
The vendor kernel sets a previously unknown clock gating bit in the VIVS_PM_MODULE_CONTROLS register to disable SH_EU clock gating. Import new headers from rnndb for the definition and set the bit for the VIPNano-Si+ NPU on i.MX8MP. Signed-off-by: Philipp Zabel --- Philipp Zabel (2): drm/etnaviv: Update hardware headers from rnndb drm/etnaviv: Disable SH_EU clock gating on VIPNano-Si+ drivers/gpu/drm/etnaviv/cmdstream.xml.h | 52 ++-- drivers/gpu/drm/etnaviv/common.xml.h| 12 ++-- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 ++ drivers/gpu/drm/etnaviv/state.xml.h | 101 +++- drivers/gpu/drm/etnaviv/state_blt.xml.h | 20 +++ drivers/gpu/drm/etnaviv/state_hi.xml.h | 28 + 6 files changed, 174 insertions(+), 43 deletions(-) --- base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d change-id: 20240124-etnaviv-npu-627f6881322c Best regards, -- Philipp Zabel
[PATCH 2/2] drm/etnaviv: Disable SH_EU clock gating on VIPNano-Si+
Disable SH_EU clock gating for the VIPNano-Si+ NPU on i.MX8MP. Taken from linux-imx lf-6.1.36-2.1.0, specifically [1]. [1] https://github.com/nxp-imx/linux-imx/blob/lf-6.1.36-2.1.0/drivers/mxc/gpu-viv/hal/kernel/arch/gc_hal_kernel_hardware.c#L2747-L2761 Signed-off-by: Philipp Zabel --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 9b8445d2a128..e28332a2560d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -641,6 +641,10 @@ static void etnaviv_gpu_enable_mlcg(struct etnaviv_gpu *gpu) pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_SE | VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA; + /* Disable SH_EU clock gating on affected core revisions. */ + if (etnaviv_is_model_rev(gpu, GC8000, 0x8002)) + pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_SH_EU; + pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA_HZ; pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA_EZ; -- 2.39.2
Re: [PATCH 4/6] drm/imx: prefer snprintf over sprintf
Hi Jani, On Mi, 2024-01-10 at 19:39 +0200, Jani Nikula wrote: > This will trade the W=1 warning -Wformat-overflow to > -Wformat-truncation. This lets us enable -Wformat-overflow subsystem > wide. > > Cc: Philipp Zabel > Signed-off-by: Jani Nikula Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH v2 2/2] gpu: drm: bridge: cadence: Add a driver and platform ops for StarFive JH7110 SoC
On Di, 2024-01-09 at 15:25 +0800, Shengyang Chen wrote:
> +static int cdns_dsi_get_reset(struct device *dev, struct cdns_dsi *dsi)
> +{
> + dsi->dpi_rst = devm_reset_control_get(dev, "dpi");
> + if (IS_ERR(dsi->dpi_rst))
> + return PTR_ERR(dsi->dpi_rst);
Please use devm_reset_control_get_exclusive() directly.
Also, consider using devm_reset_control_bulk_get_exclusive() instead,
to control "dpi"/"apb"/"txesc" resets together - if the hardware can
handle deasserting in reversed order.
> +
> + dsi->apb_rst = devm_reset_control_get(dev, "apb");
> + if (IS_ERR(dsi->apb_rst))
> + return PTR_ERR(dsi->apb_rst);
> +
> + dsi->txesc_rst = devm_reset_control_get(dev, "txesc");
> + if (IS_ERR(dsi->txesc_rst))
> + return PTR_ERR(dsi->txesc_rst);
> +
> + dsi->txbytehs_rst = devm_reset_control_get(dev, "txbytehs");
> + if (IS_ERR(dsi->txbytehs_rst))
> + return PTR_ERR(dsi->txbytehs_rst);
> +
> + return 0;
> +}
regards
Philipp
Re: [v3 3/6] drm/vs: Register DRM device
Hi Keith,
On Mo, 2023-12-04 at 20:33 +0800, Keith Zhao wrote:
> Implement drm device registration interface
>
> Signed-off-by: Keith Zhao
> ---
[...]
> diff --git a/drivers/gpu/drm/verisilicon/Kconfig
> b/drivers/gpu/drm/verisilicon/Kconfig
> new file mode 100644
> index ..e10fa97635aa
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0
> +config DRM_VERISILICON
> + tristate "DRM Support for VeriSilicon"
> + depends on DRM
> + select DRM_KMS_HELPER
> + select DRM_GEM_DMA_HELPER
> + select CMA
> + select DMA_CMA
> + help
> + Choose this option if you have a VeriSilicon soc chipset.
This seems a bit generic. Doesn't the VeriSilicon display controller IP
used on JH7110 have a product name?
[...]
> diff --git a/drivers/gpu/drm/verisilicon/vs_drv.c
> b/drivers/gpu/drm/verisilicon/vs_drv.c
> new file mode 100644
> index ..4fb1f29ef84b
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_drv.c
> @@ -0,0 +1,316 @@
> +// SPDX-License-Identifier: GPL-2.0
[...]
> +static void vs_drm_device_release_clocks(void *res)
> +{
> + struct vs_drm_device *priv = res;
> + unsigned int i;
> +
> + reset_control_bulk_assert(priv->nrsts, priv->rst_vout);
> +
> + for (i = 0; i < priv->clk_count; ++i) {
> + if (priv->clks[i]) {
> + clk_disable_unprepare(priv->clks[i]);
> + clk_put(priv->clks[i]);
> + }
> + }
Why not use the bulk API for clk as well?
[...]
> +static int vs_drm_device_init_clocks(struct vs_drm_device *priv)
> +{
> + struct drm_device *dev = >base;
> + struct platform_device *pdev = to_platform_device(dev->dev);
> + struct device_node *of_node = pdev->dev.of_node;
> + struct clk *clock;
> + unsigned int i;
> + int ret;
> +
> + if (dev_get_platdata(>dev) || !of_node)
> + return 0;
> +
> + priv->nrsts = ARRAY_SIZE(priv->rst_vout);
> + for (int i = 0; i < priv->nrsts; ++i)
> + priv->rst_vout[i].id = vout_resets[i];
> + ret = devm_reset_control_bulk_get_shared(dev->dev, priv->nrsts,
> + priv->rst_vout);
I would request resets and clocks in _probe().
If component_bind_all() returns -EPROBE_DEFER because of a still
missing DSI panel backlight or similar, this doesn't have to be done
multiple times.
> + if (ret) {
> + drm_err(dev, "Failed to get reset controls\n");
> + return ret;
> + }
> +
> + priv->clk_count = of_clk_get_parent_count(of_node);
> + if (!priv->clk_count)
> + return 0;
> +
> + priv->clks = drmm_kzalloc(dev, priv->clk_count * sizeof(priv->clks[0]),
> + GFP_KERNEL);
> + if (!priv->clks)
> + return -ENOMEM;
> +
> + for (i = 0; i < priv->clk_count; ++i) {
> + clock = of_clk_get(of_node, i);
> + if (IS_ERR(clock)) {
> + ret = PTR_ERR(clock);
> + if (ret == -EPROBE_DEFER)
> + goto err;
> + drm_err(dev, "clock %u not found: %d\n", i, ret);
> + continue;
> + }
> + ret = clk_prepare_enable(clock);
> + if (ret) {
> + drm_err(dev, "failed to enable clock %u: %d\n",
> + i, ret);
> + clk_put(clock);
> + continue;
> + }
> + priv->clks[i] = clock;
> + }
> +
> + ret = reset_control_bulk_deassert(priv->nrsts, priv->rst_vout);
> + if (ret)
> + return ret;
This should goto err, otherwise clocks are left enabled.
> +
> + return devm_add_action_or_reset(>dev,
> + vs_drm_device_release_clocks,
> + priv);
> +
> +err:
> + while (i) {
> + --i;
> + if (priv->clks[i]) {
> + clk_disable_unprepare(priv->clks[i]);
> + clk_put(priv->clks[i]);
> + }
> + }
> + return ret;
> +}
> +
> +static int vs_drm_bind(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct vs_drm_device *priv;
> + int ret;
> + struct drm_device *drm_dev;
> +
> + /* Remove existing drivers that may own the framebuffer memory. */
> + ret = drm_aperture_remove_framebuffers(_drm_driver);
> + if (ret)
> + return ret;
> +
> + priv = devm_drm_dev_alloc(dev, _drm_driver, struct vs_drm_device,
> base);
> + if (IS_ERR(priv))
> + return PTR_ERR(priv);
> +
> + priv->pitch_alignment = 64;
Why is this a variable instead of a constant?
> + ret = dma_set_coherent_mask(priv->base.dev, DMA_BIT_MASK(40));
> + if (ret)
> + return ret;
> +
> + drm_dev = >base;
> +
Re: [PATCH] drm/imx/lcdc: Fix double-free of driver data
On Do, 2023-07-06 at 11:27 +0200, Uwe Kleine-König wrote:
> The struct imx_lcdc driver data is allocated using devm_drm_dev_alloc()
> so it must not be explicitly kfree()d.
>
> Also drm_kms_helper_poll_fini() should not be called as there is no
> matching drm_kms_helper_poll_init(). So drop the release function
> completely.
>
> Fixes: c87e859cdeb5 ("drm/imx/lcdc: Implement DRM driver for imx25")
> Signed-off-by: Uwe Kleine-König
Reviewed-by: Philipp Zabel
regards
Philipp
[PATCH 2/3] dt-bindings: ili9881c: Add Ampire AM8001280G LCD panel
Document the compatible value for Ampire AM8001280G LCD panels. Signed-off-by: Philipp Zabel --- Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml index e7ab6224b52e..b1e624be3e33 100644 --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml @@ -16,6 +16,7 @@ properties: compatible: items: - enum: + - ampire,am8001280g - bananapi,lhr050h41 - feixin,k101-im2byl02 - tdo,tl050hdv35 -- 2.39.2
[PATCH 3/3] drm/panel: ilitek-ili9881c: Add Ampire AM8001280G LCD panel
Add support for Ampire AM8001280G LCD panels.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 223 ++
1 file changed, 223 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
index 0c911ed9141b..2ffe5f68a890 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
@@ -830,6 +830,203 @@ static const struct ili9881c_instr w552946ab_init[] = {
ILI9881C_SWITCH_PAGE_INSTR(0),
};
+static const struct ili9881c_instr am8001280g_init[] = {
+ ILI9881C_SWITCH_PAGE_INSTR(3),
+ ILI9881C_COMMAND_INSTR(0x01, 0x00),
+ ILI9881C_COMMAND_INSTR(0x02, 0x00),
+ ILI9881C_COMMAND_INSTR(0x03, 0x73),
+ ILI9881C_COMMAND_INSTR(0x04, 0xD3),
+ ILI9881C_COMMAND_INSTR(0x05, 0x00),
+ ILI9881C_COMMAND_INSTR(0x06, 0x0A),
+ ILI9881C_COMMAND_INSTR(0x07, 0x0E),
+ ILI9881C_COMMAND_INSTR(0x08, 0x00),
+ ILI9881C_COMMAND_INSTR(0x09, 0x01),
+ ILI9881C_COMMAND_INSTR(0x0a, 0x01),
+ ILI9881C_COMMAND_INSTR(0x0b, 0x01),
+ ILI9881C_COMMAND_INSTR(0x0c, 0x01),
+ ILI9881C_COMMAND_INSTR(0x0d, 0x01),
+ ILI9881C_COMMAND_INSTR(0x0e, 0x01),
+ ILI9881C_COMMAND_INSTR(0x0f, 0x01),
+ ILI9881C_COMMAND_INSTR(0x10, 0x01),
+ ILI9881C_COMMAND_INSTR(0x11, 0x00),
+ ILI9881C_COMMAND_INSTR(0x12, 0x00),
+ ILI9881C_COMMAND_INSTR(0x13, 0x00),
+ ILI9881C_COMMAND_INSTR(0x14, 0x00),
+ ILI9881C_COMMAND_INSTR(0x15, 0x00),
+ ILI9881C_COMMAND_INSTR(0x16, 0x00),
+ ILI9881C_COMMAND_INSTR(0x17, 0x00),
+ ILI9881C_COMMAND_INSTR(0x18, 0x00),
+ ILI9881C_COMMAND_INSTR(0x19, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1a, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1b, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1c, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1d, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1e, 0x40),
+ ILI9881C_COMMAND_INSTR(0x1f, 0x80),
+ ILI9881C_COMMAND_INSTR(0x20, 0x06),
+ ILI9881C_COMMAND_INSTR(0x21, 0x01),
+ ILI9881C_COMMAND_INSTR(0x22, 0x00),
+ ILI9881C_COMMAND_INSTR(0x23, 0x00),
+ ILI9881C_COMMAND_INSTR(0x24, 0x00),
+ ILI9881C_COMMAND_INSTR(0x25, 0x00),
+ ILI9881C_COMMAND_INSTR(0x26, 0x00),
+ ILI9881C_COMMAND_INSTR(0x27, 0x00),
+ ILI9881C_COMMAND_INSTR(0x28, 0x33),
+ ILI9881C_COMMAND_INSTR(0x29, 0x03),
+ ILI9881C_COMMAND_INSTR(0x2a, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2b, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2c, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2d, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2e, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2f, 0x00),
+ ILI9881C_COMMAND_INSTR(0x30, 0x00),
+ ILI9881C_COMMAND_INSTR(0x31, 0x00),
+ ILI9881C_COMMAND_INSTR(0x32, 0x00),
+ ILI9881C_COMMAND_INSTR(0x33, 0x00),
+ ILI9881C_COMMAND_INSTR(0x34, 0x03),
+ ILI9881C_COMMAND_INSTR(0x35, 0x00),
+ ILI9881C_COMMAND_INSTR(0x36, 0x03),
+ ILI9881C_COMMAND_INSTR(0x37, 0x00),
+ ILI9881C_COMMAND_INSTR(0x38, 0x00),
+ ILI9881C_COMMAND_INSTR(0x39, 0x00),
+ ILI9881C_COMMAND_INSTR(0x3a, 0x40),
+ ILI9881C_COMMAND_INSTR(0x3b, 0x40),
+ ILI9881C_COMMAND_INSTR(0x3c, 0x00),
+ ILI9881C_COMMAND_INSTR(0x3d, 0x00),
+ ILI9881C_COMMAND_INSTR(0x3e, 0x00),
+ ILI9881C_COMMAND_INSTR(0x3f, 0x00),
+ ILI9881C_COMMAND_INSTR(0x40, 0x00),
+ ILI9881C_COMMAND_INSTR(0x41, 0x00),
+ ILI9881C_COMMAND_INSTR(0x42, 0x00),
+ ILI9881C_COMMAND_INSTR(0x43, 0x00),
+ ILI9881C_COMMAND_INSTR(0x44, 0x00),
+
+ ILI9881C_COMMAND_INSTR(0x50, 0x01),
+ ILI9881C_COMMAND_INSTR(0x51, 0x23),
+ ILI9881C_COMMAND_INSTR(0x52, 0x45),
+ ILI9881C_COMMAND_INSTR(0x53, 0x67),
+ ILI9881C_COMMAND_INSTR(0x54, 0x89),
+ ILI9881C_COMMAND_INSTR(0x55, 0xab),
+ ILI9881C_COMMAND_INSTR(0x56, 0x01),
+ ILI9881C_COMMAND_INSTR(0x57, 0x23),
+ ILI9881C_COMMAND_INSTR(0x58, 0x45),
+ ILI9881C_COMMAND_INSTR(0x59, 0x67),
+ ILI9881C_COMMAND_INSTR(0x5a, 0x89),
+ ILI9881C_COMMAND_INSTR(0x5b, 0xab),
+ ILI9881C_COMMAND_INSTR(0x5c, 0xcd),
+ ILI9881C_COMMAND_INSTR(0x5d, 0xef),
+
+ ILI9881C_COMMAND_INSTR(0x5e, 0x11),
+ ILI9881C_COMMAND_INSTR(0x5f, 0x02),
+ ILI9881C_COMMAND_INSTR(0x60, 0x00),
+ ILI9881C_COMMAND_INSTR(0x61, 0x01),
+ ILI9881C_COMMAND_INSTR(0x62, 0x0D),
+ ILI9881C_COMMAND_INSTR(0x63, 0x0C),
+ ILI9881C_COMMAND_INSTR(0x64, 0x0F),
+ ILI9881C_COMMAND_INSTR(0x65, 0x0E),
+ ILI9881C_COMMAND_INSTR(0x66, 0x06),
+ ILI9881C_COMMAND_INSTR(0x67, 0x07),
+ ILI9881C_COMMAND_INSTR(0x68, 0x02),
+ ILI9881C_COMMAND_INSTR(0x69, 0x02),
+ ILI9881C_COMMAND_INSTR(0x6a, 0x08),
+ ILI9881C_COMMAND_INSTR(0x6b, 0x02),
+ ILI9881C_COMMAND_INSTR(0x6c, 0x02),
+ ILI9881C_COMMAND_INSTR(0x6d, 0x02
[PATCH 1/3] drm/panel: ilitek-ili9881c: make use of prepare_prev_first
From: Marco Felsch The panel.prepare() call requires an initialized MIPI-DSI host, so set the prepare_prev_first flag to indicate that the host must be initialized first. Signed-off-by: Marco Felsch Signed-off-by: Philipp Zabel --- drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c index 7838947a1bf3..0c911ed9141b 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c @@ -1094,6 +1094,8 @@ static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi) return ret; } + ctx->panel.prepare_prev_first = true; + ret = drm_panel_of_backlight(>panel); if (ret) return ret; -- 2.39.2
[PATCH 0/3] drm/panel: ilitek-ili9881c: Support Ampire AM8001280G LCD panel
Add support for Ampire AM8001280G LCD panels to the Ilitek ILI9881C
driver.
Also set prepare_prev_first, to make sure that the DSI host controller
is initialized to LP-11 before the panel is powered up. Tested to work
with samsung-dsim on i.MX8MM after commit 0c14d3130654 ("drm: bridge:
samsung-dsim: Fix i.MX8M enable flow to meet spec").
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: Rob Herring
To: Krzysztof Kozlowski
To: Conor Dooley
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc: devicet...@vger.kernel.org
Cc: Marco Felsch
Cc: ker...@pengutronix.de
Signed-off-by: Philipp Zabel
---
Marco Felsch (1):
drm/panel: ilitek-ili9881c: make use of prepare_prev_first
Philipp Zabel (2):
dt-bindings: ili9881c: Add Ampire AM8001280G LCD panel
drm/panel: ilitek-ili9881c: Add Ampire AM8001280G LCD panel
.../bindings/display/panel/ilitek,ili9881c.yaml| 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 225 +
2 files changed, 226 insertions(+)
---
base-commit: e4d983ac270ccee417445a69b9ed198658b1
change-id: 20231123-drm-panel-ili9881c-am8001280g-d2e2f4b30a2e
Best regards,
--
Philipp Zabel
Re: [PATCH] MAINTAINERS: Update DRM DRIVERS FOR FREESCALE IMX entry
On Mi, 2023-09-06 at 07:28 -0700, Douglas Anderson wrote: > As per the discussion on the lists [1], changes to this driver > generally flow through drm-misc. If they need to be coordinated with > v4l2 they sometimes go through Philipp Zabel's tree instead. List both > trees in MAINTAINERS. Also update the title of this driver to specify > that it's just for IMX 5/6 since, as per Philipp "There are a lot more > i.MX that do not use IPUv3 than those that do." > > [1] > https://lore.kernel.org/r/d56dfb568711b4b932edc9601010feda020c2c22.ca...@pengutronix.de > > Signed-off-by: Douglas Anderson Thank you, Acked-by: Philipp Zabel regards Philipp
Re: [RFT PATCH 13/15] drm/imx/ipuv3: Call drm_atomic_helper_shutdown() at shutdown/unbind time
Hi,
On Di, 2023-09-05 at 13:29 -0700, Doug Anderson wrote:
> Hi,
>
> On Mon, Sep 4, 2023 at 1:30 AM Philipp Zabel wrote:
> >
> > On Fr, 2023-09-01 at 16:41 -0700, Douglas Anderson wrote:
> > > Based on grepping through the source code this driver appears to be
> > > missing a call to drm_atomic_helper_shutdown() at system shutdown time
> > > and at driver unbind time. Among other things, this means that if a
> > > panel is in use that it won't be cleanly powered off at system
> > > shutdown time.
> > >
> > > The fact that we should call drm_atomic_helper_shutdown() in the case
> > > of OS shutdown/restart and at driver remove (or unbind) time comes
> > > straight out of the kernel doc "driver instance overview" in
> > > drm_drv.c.
> > >
> > > A few notes about this fix:
> > > - When adding drm_atomic_helper_shutdown() to the unbind path, I added
> > > it after drm_kms_helper_poll_fini() since that's when other drivers
> > > seemed to have it.
> > > - Technically with a previous patch, ("drm/atomic-helper:
> > > drm_atomic_helper_shutdown(NULL) should be a noop"), we don't
> > > actually need to check to see if our "drm" pointer is NULL before
> > > calling drm_atomic_helper_shutdown(). We'll leave the "if" test in,
> > > though, so that this patch can land without any dependencies. It
> > > could potentially be removed later.
> > > - This patch also makes sure to set the drvdata to NULL in the case of
> > > bind errors to make sure that shutdown can't access freed data.
> > >
> > > Suggested-by: Maxime Ripard
> > > Signed-off-by: Douglas Anderson
> >
> > Thank you,
> > Tested-by: Philipp Zabel
>
> Thanks! I notice that:
>
> ./scripts/get_maintainer.pl --scm -f drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
>
> Doesn't say drm-misc but also when I look at the MAINTAINERS file and
> find the section for "DRM DRIVERS FOR FREESCALE IMX"
That should probably say "IMX5/6" nowadays. There are a lot more i.MX
that do not use IPUv3 than those that do.
> it doesn't explicitly list a different git tree.
I used to send pull requests from git.pengutronix.de/git/pza/linux,
same as for the reset controller framework. I might still have to do
that for changes in drivers/gpu/ipu-v3 that need coordination between
drm and v4l2, but usually pure drm/imx/ipuv3 changes are pushed to drm-
misc.
> I guess the "shawnguo" git tree listed by get_maintainer.pl is just
> from regex matching?
The "N: imx" pattern in "ARM/FREESCALE IMX / MXC ARM ARCHITECTURE", I
think.
> Would you expect this to go through drm-misc? If so, I'll probably
> land it sooner rather than later. I can also post up a patch making it
> obvious that "DRM DRIVERS FOR FREESCALE IMX" goes through drm-misc if
> you don't object.
Yes, both would be great.
regards
Philipp
Re: [RFT PATCH 13/15] drm/imx/ipuv3: Call drm_atomic_helper_shutdown() at shutdown/unbind time
On Fr, 2023-09-01 at 16:41 -0700, Douglas Anderson wrote:
> Based on grepping through the source code this driver appears to be
> missing a call to drm_atomic_helper_shutdown() at system shutdown time
> and at driver unbind time. Among other things, this means that if a
> panel is in use that it won't be cleanly powered off at system
> shutdown time.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS shutdown/restart and at driver remove (or unbind) time comes
> straight out of the kernel doc "driver instance overview" in
> drm_drv.c.
>
> A few notes about this fix:
> - When adding drm_atomic_helper_shutdown() to the unbind path, I added
> it after drm_kms_helper_poll_fini() since that's when other drivers
> seemed to have it.
> - Technically with a previous patch, ("drm/atomic-helper:
> drm_atomic_helper_shutdown(NULL) should be a noop"), we don't
> actually need to check to see if our "drm" pointer is NULL before
> calling drm_atomic_helper_shutdown(). We'll leave the "if" test in,
> though, so that this patch can land without any dependencies. It
> could potentially be removed later.
> - This patch also makes sure to set the drvdata to NULL in the case of
> bind errors to make sure that shutdown can't access freed data.
>
> Suggested-by: Maxime Ripard
> Signed-off-by: Douglas Anderson
Thank you,
Tested-by: Philipp Zabel
regards
Philipp
Re: [PATCH 1/1] drm/imx/ipuv-v3: Fix front porch adjustment upon hactive aligning
On Fr, 2023-07-21 at 10:36 +0200, Alexander Stein wrote:
> Hi,
>
> Am Mittwoch, 3. Mai 2023, 13:14:56 CEST schrieb Alexander Stein:
> > When hactive is not aligned to 8 pixels, it is aligned accordingly and
> > hfront porch needs to be reduced the same amount. Unfortunately the front
> > porch is set to the difference rather than reducing it. There are some
> > Samsung TVs which can't cope with a front porch of instead of 70.
> >
> > Fixes: 94dfec48fca7 ("drm/imx: Add 8 pixel alignment fix")
> > Signed-off-by: Alexander Stein
> > ---
> > AFAICS ipu_di_adjust_videomode() checks that front porch is big enough to
> > reduce the alignment difference.
>
> A gentle ping. Is there anything to do? Or is someone picking this patch was
> tested by Ian?
Thank you, pushed to drm-misc-fixes with the subject changed to:
"drm/imx/ipuv3: Fix front porch adjustment upon hactive aligning".
regards
Philipp
Re: [PATCH v2 02/19] gpu: ipu-v3: pre: Convert to devm_platform_ioremap_resource()
On Mo, 2023-07-10 at 11:23 +0800, Yangtao Li wrote: > Use devm_platform_ioremap_resource() to simplify code. > > Signed-off-by: Yangtao Li > Reviewed-by: Philipp Zabel Thank you, pushed to drm-misc-next. regards Philipp
Re: [PATCH v2 08/19] gpu: ipu-v3: prg: Convert to devm_platform_ioremap_resource()
On Mo, 2023-07-10 at 11:23 +0800, Yangtao Li wrote: > Use devm_platform_ioremap_resource() to simplify code. > > Signed-off-by: Yangtao Li > Reviewed-by: Philipp Zabel Thank you, pushed to drm-misc-next. regards Philipp
Re: [PATCH] drm/imx: ipuv3-plane: reuse local variable height in atomic_update
On Di, 2023-01-10 at 19:21 +0100, Lucas Stach wrote: > Am Dienstag, dem 20.12.2022 um 10:44 +0100 schrieb Philipp Zabel: > > Use the already existing local variable height = drm_rect_height() >> 16 > > to replace other occurrences of the same value. > > > > Suggested-by: Lucas Stach > > Signed-off-by: Philipp Zabel > > Reviewed-by: Lucas Stach Thank you, pushed to drm-misc-next. regards Philipp
Re: [PATCH 1/1] drm/imx/ipuv-v3: Fix front porch adjustment upon hactive aligning
On Mi, 2023-05-03 at 13:14 +0200, Alexander Stein wrote:
> When hactive is not aligned to 8 pixels, it is aligned accordingly and
> hfront porch needs to be reduced the same amount. Unfortunately the front
> porch is set to the difference rather than reducing it. There are some
> Samsung TVs which can't cope with a front porch of instead of 70.
>
> Fixes: 94dfec48fca7 ("drm/imx: Add 8 pixel alignment fix")
> Signed-off-by: Alexander Stein
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH v3 1/1] drm/i915: Move abs_diff() to math.h
On Mo, 2023-07-24 at 11:25 +0300, Andy Shevchenko wrote: > abs_diff() belongs to math.h. Move it there. > This will allow others to use it. > > Signed-off-by: Andy Shevchenko > Reviewed-by: Jiri Slaby # tty/serial Reviewed-by: Philipp Zabel # gpu/ipu-v3 regards Philipp
Re: [PATCH 08/18] gpu: ipu-v3: prg: Convert to devm_platform_ioremap_resource()
On Fr, 2023-07-07 at 15:20 +0800, Yangtao Li wrote: > Use devm_platform_ioremap_resource() to simplify code. > > Signed-off-by: Yangtao Li Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 02/18] gpu: ipu-v3: pre: Convert to devm_platform_ioremap_resource()
On Fr, 2023-07-07 at 15:20 +0800, Yangtao Li wrote: > Use devm_platform_ioremap_resource() to simplify code. > > Signed-off-by: Yangtao Li Reviewed-by: Philipp Zabel regards Philipp
[PATCH] backlight: pwm_bl: Avoid backlight flicker applying initial PWM state
The initial PWM state returned by pwm_init_state() has a duty cycle
of 0 ns. To avoid backlight flicker when taking over an enabled
display from the bootloader, skip the initial pwm_apply_state()
and leave the PWM be until backlight_update_state() will apply the
state with the desired brightness.
Signed-off-by: Philipp Zabel
---
With a PWM driver that allows to inherit PWM state from the bootloader,
postponing the initial pwm_apply_state() with 0 ns duty cycle allows to
set the desired duty cycle before the PWM is set, avoiding a short flicker
if the backlight was previously enabled and will be enabled again.
---
drivers/video/backlight/pwm_bl.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index fce412234d10..47a917038f58 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -531,12 +531,10 @@ static int pwm_backlight_probe(struct platform_device
*pdev)
if (!state.period && (data->pwm_period_ns > 0))
state.period = data->pwm_period_ns;
- ret = pwm_apply_state(pb->pwm, );
- if (ret) {
- dev_err(>dev, "failed to apply initial PWM state: %d\n",
- ret);
- goto err_alloc;
- }
+ /*
+* No need to apply initial state, except in the error path.
+* State will be applied by backlight_update_status() on success.
+*/
memset(, 0, sizeof(struct backlight_properties));
@@ -573,7 +571,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (ret < 0) {
dev_err(>dev,
"failed to setup default brightness table\n");
- goto err_alloc;
+ goto err_apply;
}
for (i = 0; i <= data->max_brightness; i++) {
@@ -602,7 +600,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (IS_ERR(bl)) {
dev_err(>dev, "failed to register backlight\n");
ret = PTR_ERR(bl);
- goto err_alloc;
+ goto err_apply;
}
if (data->dft_brightness > data->max_brightness) {
@@ -619,6 +617,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bl);
return 0;
+err_apply:
+ pwm_apply_state(pb->pwm, );
err_alloc:
if (data->exit)
data->exit(>dev);
---
base-commit: ac9a78681b921877518763ba0e89202254349d1b
change-id: 20230608-backlight-pwm-avoid-flicker-d2dd8d12f826
Best regards,
--
Philipp Zabel
Re: [PATCH 9/9] drm/verisilicon: Add starfive hdmi driver
Hi Keith,
On Fri, Jun 02, 2023 at 03:40:43PM +0800, Keith Zhao wrote:
> Add HDMI dirver for StarFive SoC JH7110.
>
> Signed-off-by: Keith Zhao
> ---
> drivers/gpu/drm/verisilicon/Kconfig | 11 +
> drivers/gpu/drm/verisilicon/Makefile| 1 +
> drivers/gpu/drm/verisilicon/starfive_hdmi.c | 928
> drivers/gpu/drm/verisilicon/starfive_hdmi.h | 296 +++
> drivers/gpu/drm/verisilicon/vs_drv.c| 6 +
> drivers/gpu/drm/verisilicon/vs_drv.h| 4 +
> 6 files changed, 1246 insertions(+)
> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c
> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h
>
[...]
> diff --git a/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> new file mode 100644
> index ..128ecca03309
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> @@ -0,0 +1,928 @@
[...]
> +static int starfive_hdmi_enable_clk_deassert_rst(struct device *dev, struct
> starfive_hdmi *hdmi)
> +{
> + int ret;
> +
> + ret = clk_prepare_enable(hdmi->sys_clk);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "Cannot enable HDMI sys clock: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(hdmi->mclk);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "Cannot enable HDMI mclk clock: %d\n", ret);
> + return ret;
> + }
> + ret = clk_prepare_enable(hdmi->bclk);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "Cannot enable HDMI bclk clock: %d\n", ret);
> + return ret;
> + }
> + ret = reset_control_deassert(hdmi->tx_rst);
> + if (ret < 0) {
> + dev_err(dev, "failed to deassert tx_rst\n");
The error paths should clk_disable_unprepare() enabled clocks.
> + return ret;
> + }
> + return 0;
> +}
> +
[...]
> +static int starfive_hdmi_get_clk_rst(struct device *dev, struct
> starfive_hdmi *hdmi)
> +{
> + hdmi->sys_clk = devm_clk_get(dev, "sysclk");
> + if (IS_ERR(hdmi->sys_clk)) {
> + DRM_DEV_ERROR(dev, "Unable to get HDMI sysclk clk\n");
> + return PTR_ERR(hdmi->sys_clk);
> + }
> + hdmi->mclk = devm_clk_get(dev, "mclk");
> + if (IS_ERR(hdmi->mclk)) {
> + DRM_DEV_ERROR(dev, "Unable to get HDMI mclk clk\n");
> + return PTR_ERR(hdmi->mclk);
> + }
> + hdmi->bclk = devm_clk_get(dev, "bclk");
> + if (IS_ERR(hdmi->bclk)) {
> + DRM_DEV_ERROR(dev, "Unable to get HDMI bclk clk\n");
> + return PTR_ERR(hdmi->bclk);
> + }
> + hdmi->tx_rst = reset_control_get_shared(dev, "hdmi_tx");
Use devm_reset_control_get_shared() for consistency, otherwise this is missing
a reset_control_put() somewhere.
regards
Philipp
Re: [PATCH 22/53] drm/imx/ipuv3: Convert to platform remove callback returning void
On So, 2023-05-07 at 18:25 +0200, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is (mostly) ignored > and this typically results in resource leaks. To improve here there is a > quest to make the remove callback return void. In the first step of this > quest all drivers are converted to .remove_new() which already returns > void. > > Trivially convert the ipuv3 imx drivers from always returning zero in > the remove callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH v8 4/5] drm: Add RZ/G2L DU Support
Hi Biju,
On Mon, Apr 24, 2023 at 05:10:23PM +0100, Biju Das wrote:
> The LCD controller is composed of Frame Compression Processor (FCPVD),
> Video Signal Processor (VSPD), and Display Unit (DU).
>
> It has DPI/DSI interfaces and supports a maximum resolution of 1080p
> along with 2 RPFs to support the blending of two picture layers and
> raster operations (ROPs).
>
> The DU module is connected to VSPD. Add RZ/G2L DU support for RZ/G2L
> alike SoCs.
>
> Signed-off-by: Biju Das
[...]
> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> new file mode 100644
> index ..af877d0dadc2
> --- /dev/null
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> @@ -0,0 +1,716 @@
[...]
> +static int rzg2l_du_crtc_get(struct rzg2l_du_crtc *rcrtc)
> +{
> + int ret;
> +
> + /*
> + * Guard against double-get, as the function is called from both the
> + * .atomic_enable() and .atomic_begin() handlers.
> + */
> + if (rcrtc->initialized)
> + return 0;
> +
> + ret = clk_prepare_enable(rcrtc->rzg2l_clocks.aclk);
> + if (ret < 0)
> + return ret;
> +
> + ret = clk_prepare_enable(rcrtc->rzg2l_clocks.pclk);
> + if (ret < 0)
> + goto error_clock;
> +
> + ret = reset_control_deassert(rcrtc->rstc);
> + if (ret < 0)
> + goto error_reset;
> +
> + rzg2l_du_crtc_setup(rcrtc);
> + rcrtc->initialized = true;
> +
> + return 0;
> +
> +error_reset:
> + reset_control_assert(rcrtc->rstc);
If deassertion did not succeed, there is no need to assert.
Worse, for shared reset controls this messes up the deassert_count.
You can just drop the reset_control_assert() here.
regards
Philipp
Re: [PATCH v6 0/2] drm/imx/lcdc: Implement DRM driver for imx25
On Mo, 2023-03-06 at 12:52 +0100, Uwe Kleine-König wrote: > Hello, > > in response to Philipp's reply to v5 here comes a v6. The code changes are: Thank you, pushed to drm-misc-next. regards Philipp
Re: [PATCH] drm: Drop ARCH_MULTIPLATFORM from dependencies
On Fr, 2022-12-09 at 23:05 +0100, Uwe Kleine-König wrote:
> Some of these dependencies used to be sensible when only a small part of
> the platforms supported by ARCH=arm could be compiled together in a
> single kernel image. Nowadays ARCH_MULTIPLATFORM is only used as a guard
> for kernel options incompatible with a multiplatform image. See commit
> 84fc86360623 ("ARM: make ARCH_MULTIPLATFORM user-visible") for some more
> details.
>
> Signed-off-by: Uwe Kleine-König
> ---
> drivers/gpu/drm/imx/Kconfig | 2 +-
> drivers/gpu/ipu-v3/Kconfig | 2 +-
For i.MX / IPUv3,
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH v5 2/2] drm/imx/lcdc: Implement DRM driver for imx25
Hi Uwe,
just a few nitpicks, see below.
On Fri, Feb 10, 2023 at 07:00:14PM +0100, Uwe Kleine-König wrote:
> From: Marian Cichy
>
> Add support for the LCD Controller found on i.MX21 and i.MX25.
>
> It targets to be a drop in replacement for the imx-fb driver.
>
> Signed-off-by: Marian Cichy
> [ukl: Rebase to a newer kernel version, various smaller fixes and
> improvements]
> Signed-off-by: Uwe Kleine-König
> ---
> drivers/gpu/drm/imx/Kconfig | 1 +
> drivers/gpu/drm/imx/Makefile| 1 +
> drivers/gpu/drm/imx/lcdc/Kconfig| 7 +
> drivers/gpu/drm/imx/lcdc/Makefile | 1 +
> drivers/gpu/drm/imx/lcdc/imx-lcdc.c | 553
> 5 files changed, 563 insertions(+)
> create mode 100644 drivers/gpu/drm/imx/lcdc/Kconfig
> create mode 100644 drivers/gpu/drm/imx/lcdc/Makefile
> create mode 100644 drivers/gpu/drm/imx/lcdc/imx-lcdc.c
>
[...]
> diff --git a/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
> b/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
> new file mode 100644
> index ..c2197fc50306
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
> @@ -0,0 +1,553 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// SPDX-FileCopyrightText: 2020 Marian Cichy
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Choose one, remove the other.
[...]
> +struct imx_lcdc {
> + struct drm_device drm;
> + struct drm_simple_display_pipe pipe;
> + const struct drm_display_mode *mode;
The mode pointer appears to be unused.
> + struct drm_bridge *bridge;
The bridge could be a local variable in _probe().
> + struct drm_connector *connector;
> + void __iomem *base;
> +
> + struct clk *clk_ipg;
> + struct clk *clk_ahb;
> + struct clk *clk_per;
> +};
> +
> +static const u32 imx_lcdc_formats[] = {
> + DRM_FORMAT_RGB565, DRM_FORMAT_XRGB,
> +};
> +
> +static inline struct imx_lcdc *imx_lcdc_from_drmdev(struct drm_device *drm)
> +{
> + return container_of(drm, struct imx_lcdc, drm);
> +}
> +
> +static unsigned int imx_lcdc_get_format(unsigned int drm_format)
> +{
> + unsigned int bpp;
> +
> + switch (drm_format) {
> + default:
> + DRM_WARN("Format not supported - fallback to XRGB\n");
> + fallthrough;
> +
> + case DRM_FORMAT_XRGB:
> + bpp = BPP_XRGB;
> + break;
This could just return directly, no need for the local bpp variable.
> +
> + case DRM_FORMAT_RGB565:
> + bpp = BPP_RGB565;
> + break;
> + }
> +
> + return bpp;
> +}
> +
[...]
> +
> +static int imx_lcdc_probe(struct platform_device *pdev)
> +{
> + struct imx_lcdc *lcdc;
> + struct drm_device *drm;
> + int irq;
> + int ret;
> + struct device *dev = >dev;
> +
> + lcdc = devm_drm_dev_alloc(dev, _lcdc_drm_driver,
> + struct imx_lcdc, drm);
> + if (!lcdc)
> + return -ENOMEM;
> +
> + drm = >drm;
> +
> + lcdc->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(lcdc->base))
> + return dev_err_probe(dev, PTR_ERR(lcdc->base), "Cannot get IO
> memory\n");
> +
> + lcdc->bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
> + if (IS_ERR(lcdc->bridge))
> + return dev_err_probe(dev, PTR_ERR(lcdc->bridge), "Failed to
> find bridge\n");
[...]
> + ret = drm_bridge_attach(>pipe.encoder, lcdc->bridge, NULL,
> DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> + if (ret)
> + return dev_err_probe(drm->dev, ret, "Cannot attach bridge\n");
The bridge could be a local variable.
With those addressed,
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH v3 3/3] drm/etnaviv: export client GPU usage statistics via fdinfo
On Wed, Feb 01, 2023 at 04:26:09PM +0100, Lucas Stach wrote: > This exposes a accumulated GPU active time per client via the > fdinfo infrastructure. > > Signed-off-by: Lucas Stach > --- > v3: handle NPU cores Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH v3 2/3] drm/etnaviv: allocate unique ID per drm_file
On Wed, Feb 01, 2023 at 04:26:08PM +0100, Lucas Stach wrote: > Allows to easily track if several fd are pointing to the same > execution context due to being dup'ed. > > Signed-off-by: Lucas Stach > --- > v3: use xarray to track the active contexts to avoid issues > on rollover Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 1/2] gpu: ipu-v3: pre: move state into struct
On Thu, Jan 26, 2023 at 07:47:43PM +0100, Lucas Stach wrote: > Move the variables tracking the current dynamic state into a struct > to separate it a bit better from the static device properties. > > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 2/2] gpu: ipu-v3: pre: add dynamic buffer layout reconfiguration
On Thu, Jan 26, 2023 at 07:47:44PM +0100, Lucas Stach wrote:
> imx-drm doesn't mandate a modeset when the framebuffer modifier changes,
> but currently the tile prefetch and resolve (TPR) configuration of the
> PRE is only set up on the initial modeset.
>
> As the TPR configuration is double buffered, same as all the other PRE
> states, we can support dynamic reconfiguration of the buffer layout from
> one frame to another. As switching between (super-)tiled and linear
> prefetch needs to touch the CTRL register make sure to do the
> reconfiguration inside the safe window.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/ipu-v3/ipu-pre.c | 59 +---
> drivers/gpu/ipu-v3/ipu-prg.c | 2 +-
> drivers/gpu/ipu-v3/ipu-prv.h | 2 +-
> 3 files changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
> index befffc85a146..e8d9792827dd 100644
> --- a/drivers/gpu/ipu-v3/ipu-pre.c
> +++ b/drivers/gpu/ipu-v3/ipu-pre.c
> @@ -99,8 +99,12 @@ struct ipu_pre {
>
> struct {
> boolin_use;
> + uint64_tmodifier;
> + unsigned intheight;
> unsigned intsafe_window_end;
> unsigned intbufaddr;
> + u32 ctrl;
> + u8 cpp;
> } cur;
> };
>
> @@ -173,6 +177,11 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int
> width,
> u32 active_bpp = info->cpp[0] >> 1;
> u32 val;
>
> + pre->cur.bufaddr = bufaddr;
> + pre->cur.height = height;
> + pre->cur.modifier = modifier;
> + pre->cur.cpp = info->cpp[0];
> +
> /* calculate safe window for ctrl register updates */
> if (modifier == DRM_FORMAT_MOD_LINEAR)
> pre->cur.safe_window_end = height - 2;
> @@ -181,7 +190,6 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int
> width,
>
> writel(bufaddr, pre->regs + IPU_PRE_CUR_BUF);
> writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
> - pre->cur.bufaddr = bufaddr;
>
> val = IPU_PRE_PREF_ENG_CTRL_INPUT_PIXEL_FORMAT(0) |
> IPU_PRE_PREF_ENG_CTRL_INPUT_ACTIVE_BPP(active_bpp) |
> @@ -223,28 +231,56 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned
> int width,
> }
> writel(val, pre->regs + IPU_PRE_TPR_CTRL);
>
> - val = readl(pre->regs + IPU_PRE_CTRL);
> - val |= IPU_PRE_CTRL_EN_REPEAT | IPU_PRE_CTRL_ENABLE |
> -IPU_PRE_CTRL_SDW_UPDATE;
> + pre->cur.ctrl = readl(pre->regs + IPU_PRE_CTRL);
> + pre->cur.ctrl |= IPU_PRE_CTRL_EN_REPEAT | IPU_PRE_CTRL_ENABLE;
> if (modifier == DRM_FORMAT_MOD_LINEAR)
> - val &= ~IPU_PRE_CTRL_BLOCK_EN;
> + pre->cur.ctrl &= ~IPU_PRE_CTRL_BLOCK_EN;
> else
> - val |= IPU_PRE_CTRL_BLOCK_EN;
> - writel(val, pre->regs + IPU_PRE_CTRL);
> + pre->cur.ctrl |= IPU_PRE_CTRL_BLOCK_EN;
> + writel(pre->cur.ctrl | IPU_PRE_CTRL_SDW_UPDATE,
> +pre->regs + IPU_PRE_CTRL);
> }
>
> -void ipu_pre_update(struct ipu_pre *pre, unsigned int bufaddr)
> +void ipu_pre_update(struct ipu_pre *pre, uint64_t modifier, unsigned int
> bufaddr)
> {
> unsigned long timeout = jiffies + msecs_to_jiffies(5);
> unsigned short current_yblock;
> + unsigned int safe_window_end = pre->cur.safe_window_end;
> u32 val;
>
> - if (bufaddr == pre->cur.bufaddr)
> + if (bufaddr == pre->cur.bufaddr &&
> + modifier == pre->cur.modifier)
> return;
>
> writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
> pre->cur.bufaddr = bufaddr;
>
> + if (modifier != pre->cur.modifier) {
> + val = readl(pre->regs + IPU_PRE_TPR_CTRL);
> + val &= ~IPU_PRE_TPR_CTRL_TILE_FORMAT_MASK;
> + if (modifier != DRM_FORMAT_MOD_LINEAR) {
> + /* only support single buffer formats for now */
> + val |= IPU_PRE_TPR_CTRL_TILE_FORMAT_SINGLE_BUF;
> + if (modifier == DRM_FORMAT_MOD_VIVANTE_SUPER_TILED)
> + val |= IPU_PRE_TPR_CTRL_TILE_FORMAT_SUPER_TILED;
> + if (pre->cur.cpp == 2)
> + val |= IPU_PRE_TPR_CTRL_TILE_FORMAT_16_BIT;
> + }
> + writel(val, pre->regs + IPU_PRE_TPR_CTRL);
> +
> + if (modifier == DRM_FORMAT_MOD_LINEAR)
> + pre->cur.ctrl &= ~IPU_PRE_CTRL_BLOCK_EN;
> + else
> + pre->cur.ctrl |= IPU_PRE_CTRL_BLOCK_EN;
> +
> + if (modifier == DRM_FORMAT_MOD_LINEAR)
> + pre->cur.safe_window_end = pre->cur.height - 2;
> + else
> + pre->cur.safe_window_end =
> DIV_ROUND_UP(pre->cur.height, 4) - 1;
Could you extract the same code from ipu_pre_configure() into a separate
function, say ipu_pre_configure_modifier(), instead, and call that
Re: [PATCH 52/86] drm/imx: Direct include headers from drm_atomic_helper
On Sat, Jan 21, 2023 at 09:08:30PM +0100, Sam Ravnborg via B4 Submission Endpoint wrote: > From: Sam Ravnborg > > Direct include the headers that otherwise comes indirect from > drm_atomic_helper, because drm_atomic_helper will be reduced to > include only the minimal set of headers. > > Signed-off-by: Sam Ravnborg Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH v5 4/5] drm/msm/a6xx: Remove cx gdsc polling using 'reset'
On Mon, Jan 02, 2023 at 04:18:30PM +0530, Akhil P Oommen wrote:
> Remove the unused 'reset' interface which was supposed to help to ensure
> that cx gdsc has collapsed during gpu recovery. This is was not enabled
> so far due to missing gpucc driver support. Similar functionality using
> genpd framework will be implemented in the upcoming patch.
>
> This effectively reverts commit 1f6cca404918
> ("drm/msm/a6xx: Ensure CX collapse during gpu recovery").
>
> Signed-off-by: Akhil P Oommen
> Reviewed-by: Ulf Hansson
Reviewed-by: Philipp Zabel
regards
Philipp
[PATCH] drm/imx: ipuv3-plane: reuse local variable height in atomic_update
Use the already existing local variable height = drm_rect_height() >> 16
to replace other occurrences of the same value.
Suggested-by: Lucas Stach
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
index 80142d9a4a55..dade8b59feae 100644
--- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
@@ -618,6 +618,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
width = ipu_src_rect_width(new_state);
else
width = drm_rect_width(_state->src) >> 16;
+ height = drm_rect_height(_state->src) >> 16;
eba = drm_plane_state_to_eba(new_state, 0);
@@ -628,9 +629,9 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
if (ipu_state->use_pre) {
axi_id = ipu_chan_assign_axi_id(ipu_plane->dma);
ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id, width,
- drm_rect_height(_state->src) >>
16,
- fb->pitches[0], fb->format->format,
- fb->modifier, );
+ height, fb->pitches[0],
+ fb->format->format, fb->modifier,
+ );
}
if (!old_state->fb ||
@@ -684,7 +685,6 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
ipu_dmfc_config_wait4eot(ipu_plane->dmfc, width);
- height = drm_rect_height(_state->src) >> 16;
info = drm_format_info(fb->format->format);
ipu_calculate_bursts(width, info->cpp[0], fb->pitches[0],
, _bursts);
@@ -747,8 +747,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
ipu_cpmem_set_burstsize(ipu_plane->ipu_ch, 16);
ipu_cpmem_zero(ipu_plane->alpha_ch);
- ipu_cpmem_set_resolution(ipu_plane->alpha_ch, width,
-drm_rect_height(_state->src) >>
16);
+ ipu_cpmem_set_resolution(ipu_plane->alpha_ch, width, height);
ipu_cpmem_set_format_passthrough(ipu_plane->alpha_ch, 8);
ipu_cpmem_set_high_priority(ipu_plane->alpha_ch);
ipu_idmac_set_double_buffer(ipu_plane->alpha_ch, 1);
base-commit: 4b6cb2b67da883bc5095ee6d77f951f1cd7a1c24
--
2.30.2
Re: [PATCH v3 2/2] drm/imx/lcdc: Implement DRM driver for imx21
On Fr, 2022-12-16 at 18:50 +0100, Uwe Kleine-König wrote:
[...]
> +static int imx_lcdc_pipe_check(struct drm_simple_display_pipe *pipe,
> +struct drm_plane_state *plane_state,
> +struct drm_crtc_state *crtc_state)
> +{
> + const struct drm_display_mode *mode = _state->mode;
> + const struct drm_display_mode *old_mode = >crtc.state->mode;
> +
> + if (mode->hdisplay < LCDC_MIN_XRES || mode->hdisplay > LCDC_MAX_XRES ||
> + mode->vdisplay < LCDC_MIN_YRES || mode->vdisplay > LCDC_MAX_YRES ||
> + mode->hdisplay & 0x10) { /* must be multiple of 16 */
> + drm_err(pipe->crtc.dev, "unsupported display mode (%u x %u)\n",
> + mode->hdisplay, mode->vdisplay);
^^
Nitpick: now superfluous whitespace.
regards
Philipp
Re: [PATCH v3 2/2] drm/imx/lcdc: Implement DRM driver for imx21
On Fr, 2022-12-16 at 22:00 +0100, Uwe Kleine-König wrote:
> Hallo Philipp,
>
> On Fri, Dec 16, 2022 at 07:05:13PM +0100, Philipp Zabel wrote:
> > On Fr, 2022-12-16 at 18:50 +0100, Uwe Kleine-König wrote:
> > > From: Marian Cichy
> > >
> > > Add support for the LCD Controller found on i.MX21 and i.MX25.
> > >
> > > It targets to be a drop in replacement for the imx-fb driver.
> > >
> > > Signed-off-by: Marian Cichy
> > > [ukl: Rebase to v6.1, various smaller fixes]
> > > Signed-off-by: Uwe Kleine-König
> > > ---
> > > drivers/gpu/drm/imx/Kconfig | 1 +
> > > drivers/gpu/drm/imx/Makefile| 1 +
> >
> > I miss drivers/gpu/drm/imx/lcdc/{Kconfig,Makefile}.
>
> Their content is:
>
> - Kconfig -
> config DRM_IMX_LCDC
> tristate "Freescale i.MX LCDC displays"
> depends on DRM && (ARCH_MXC || COMPILE_TEST)
> select DRM_GEM_DMA_HELPER
> select DRM_KMS_HELPER
> help
> Found on i.MX1, i.MX21, i.MX25 and i.MX27.
> - Makefile -
> obj-y += imx-lcdc.o
> - >8 ---
>
> will resend the series once both dependent patches are in Linus' tree.
> Until then this v3 should be good enough to give reviewers a chance to
> look.
Please also rebase past 8ab59da26bc0 ("drm/fb-helper: Move generic
fbdev emulation into separate source file") and 00b5497d642b
("drm/simple-kms: Remove drm_gem_simple_display_pipe_prepare_fb()").
These changes let me compile imx-lcdc.c on top of drm-misc-next:
--8<--
diff --git a/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
b/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
index 79fbb7956374..1bb46a346377 100644
--- a/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
+++ b/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
@@ -4,7 +4,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -368,7 +368,6 @@ static const struct drm_simple_display_pipe_funcs
imx_lcdc_pipe_funcs = {
.disable = imx_lcdc_pipe_disable,
.check = imx_lcdc_pipe_check,
.update = imx_lcdc_pipe_update,
- .prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
};
static const struct drm_mode_config_funcs imx_lcdc_mode_config_funcs = {
-->8--
regards
Philipp
Re: [PATCH] gpu: ipu-v3: common: Add of_node_put() for reference returned by of_graph_get_port_by_id()
On Mi, 2022-07-20 at 23:22 +0800, Liang He wrote:
> In ipu_add_client_devices(), we need to call of_node_put() for
> reference returned by of_graph_get_port_by_id() in fail path.
>
> Fixes: 17e052175039 ("gpu: ipu-v3: Do not bail out on missing optional port
> nodes")
> Signed-off-by: Liang He
Applied to drm-misc-next.
regards
Philipp
Re: [PATCH] drm/imx: ipuv3-plane: Fix overlay plane width
On Fr, 2022-12-02 at 16:43 +0100, Lucas Stach wrote:
> Am Dienstag, dem 08.11.2022 um 15:14 +0100 schrieb Philipp Zabel:
> > ipu_src_rect_width() was introduced to support odd screen resolutions
> > such as 1366x768 by internally rounding up primary plane width to a
> > multiple of 8 and compensating with reduced horizontal blanking.
> > This also caused overlay plane width to be rounded up, which was not
> > intended. Fix overlay plane width by limiting the rounding up to the
> > primary plane.
> >
> > drm_rect_width(_state->src) >> 16 is the same value as
> > drm_rect_width(dst) because there is no plane scaling support.
>
> Heh, I stumbled at exactly this point while reading the code changes
> and was about to suggest it be added to the changelog until I realized
> that it's already here. :)
> >
> > Fixes: 94dfec48fca7 ("drm/imx: Add 8 pixel alignment fix")
> > Signed-off-by: Philipp Zabel
>
> Reviewed-by: Lucas Stach
Thank you, applied to drm-misc-next.
[...]
> There's a opportunity for a follow-up cleanup here:
> "drm_rect_height(_state->src) >> 16" is used multiple times in this
> function, which could be replaced by using this local height variable
> instead.
Will create a follow-up patch.
regards
Philipp
Re: [PATCH] drm/imx: move IPUv3 driver into separate subdirectory
On Fr, 2022-11-25 at 12:25 +0100, Lucas Stach wrote: > The IPUv3 and DCSS driver are two totally separate DRM drivers. Having > one of them live in the drivers/gpu/drm/imx toplevel directory and the > other one in the dcss/ subdirectory is confusing. Move the IPUv3 driver > into its own subdirectory to make the separation more clear. > > Signed-off-by: Lucas Stach Applied to drm-misc-next. regards Philipp
Re: [PATCH v3 0/2] drm/imx/lcdc: Implement DRM driver for imx21
On Fr, 2022-12-16 at 18:50 +0100, Uwe Kleine-König wrote:
> Hello,
>
> Changes since v2:
>
> - added allOf as Krzysztof requested
> - reworked driver based on Philipp's comments
> (improved error handling, different selects, moved driver to a
> subdirectory,
> header sorting, drm_err instead of DRM_ERROR, inlined
> imx_lcdc_check_mode_change, make use of dev_err_probe())
>
>
>
>
> Krzysztof also pointed out that we're now having two compatibles for a
> single hardware. Admittedly this is unusual, but this is the chance that
> the (bad) compatible identifier imx21-fb gets deprecated. The hardware
> is called LCDC and only the linux (framebuffer) driver is called imxfb.
>
> The two prerequisite commits on top of v6.1 are:
>
> - 93266da2409b ("dt-bindings: display: Convert fsl,imx-fb.txt to
> dt-schema") which is currently in next via branch 'for-next' of
> git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git .
>
> - "drm/imx: move IPUv3 driver into separate subdirectory"
> from
> https://lore.kernel.org/r/20221125112519.3849636-1-l.st...@pengutronix.de
This is on drm-misc-next now, so patch 2 applies there.
regards
Philipp
Re: [PATCH v3 2/2] drm/imx/lcdc: Implement DRM driver for imx21
Hi Uwe,
On Fr, 2022-12-16 at 18:50 +0100, Uwe Kleine-König wrote:
> From: Marian Cichy
>
> Add support for the LCD Controller found on i.MX21 and i.MX25.
>
> It targets to be a drop in replacement for the imx-fb driver.
>
> Signed-off-by: Marian Cichy
> [ukl: Rebase to v6.1, various smaller fixes]
> Signed-off-by: Uwe Kleine-König
> ---
> drivers/gpu/drm/imx/Kconfig | 1 +
> drivers/gpu/drm/imx/Makefile| 1 +
I miss drivers/gpu/drm/imx/lcdc/{Kconfig,Makefile}.
> drivers/gpu/drm/imx/lcdc/imx-lcdc.c | 587
> 3 files changed, 589 insertions(+)
> create mode 100644 drivers/gpu/drm/imx/lcdc/imx-lcdc.c
regards
Philipp
Re: [PATCH] drm/imx: move IPUv3 driver into separate subdirectory
On Fr, 2022-12-16 at 13:03 +0100, Uwe Kleine-König wrote: > On Fri, Nov 25, 2022 at 12:25:19PM +0100, Lucas Stach wrote: > > diff --git a/drivers/gpu/drm/imx/Makefile b/drivers/gpu/drm/imx/Makefile > > index b644deffe948..909622864716 100644 > > --- a/drivers/gpu/drm/imx/Makefile > > +++ b/drivers/gpu/drm/imx/Makefile > > @@ -1,12 +1,4 @@ > > [...] > > obj-$(CONFIG_DRM_IMX_DCSS) += dcss/ > > +obj-$(CONFIG_DRM_IMX) += ipuv3/ > > I wonder if it would make sense to rename DRM_IMX to DRM_IMX_IPUV3 ?! > > > diff --git a/drivers/gpu/drm/imx/ipuv3/Kconfig > > b/drivers/gpu/drm/imx/ipuv3/Kconfig > > new file mode 100644 > > index ..f518eb47a18e > > --- /dev/null > > +++ b/drivers/gpu/drm/imx/ipuv3/Kconfig > > [...] > > +config DRM_IMX_HDMI > > + tristate "Freescale i.MX DRM HDMI" > > + select DRM_DW_HDMI > > + depends on DRM_IMX && OF > > + help > > + Choose this if you want to use HDMI on i.MX6. > > + > > Trailing empty line could be dropped. I'll do that when applying. regards Philipp
Re: [PATCH] drm/imx: move IPUv3 driver into separate subdirectory
On Fr, 2022-11-25 at 12:25 +0100, Lucas Stach wrote: > The IPUv3 and DCSS driver are two totally separate DRM drivers. Having > one of them live in the drivers/gpu/drm/imx toplevel directory and the > other one in the dcss/ subdirectory is confusing. Move the IPUv3 driver > into its own subdirectory to make the separation more clear. > > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH] gpu: ipu-v3: common: Add of_node_put() for reference returned by of_graph_get_port_by_id()
On Mi, 2022-07-20 at 23:22 +0800, Liang He wrote:
> In ipu_add_client_devices(), we need to call of_node_put() for
> reference returned by of_graph_get_port_by_id() in fail path.
>
> Fixes: 17e052175039 ("gpu: ipu-v3: Do not bail out on missing optional port
> nodes")
> Signed-off-by: Liang He
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH v2 4/5] drm/msm/a6xx: Remove cx gdsc polling using 'reset'
On Fr, 2022-12-16 at 15:51 +0530, Akhil P Oommen wrote:
> Remove the unused 'reset' interface which was supposed to help to ensure
> that cx gdsc has collapsed during gpu recovery. This is was not enabled
> so far due to missing gpucc driver support. Similar functionality using
> genpd framework will be implemented in the upcoming patch.
Maybe mention that this effectively reverts commit 1f6cca404918
("drm/msm/a6xx: Ensure CX collapse during gpu recovery").
regards
Philipp
Re: [PATCH v2 2/2] drm/imx/lcdc: Implement DRM driver for imx21
On Wed, Dec 14, 2022 at 12:59:21PM +0100, Uwe Kleine-König wrote:
> From: Marian Cichy
>
> Add support for the LCD Controller found on i.MX21 and i.MX25.
>
> It targets to be a drop in replacement for the imx-fb driver.
>
> Signed-off-by: Marian Cichy
> [ukl: Rebase to v6.1, various smaller fixes]
> Signed-off-by: Uwe Kleine-König
> ---
> drivers/gpu/drm/imx/Kconfig| 7 +
> drivers/gpu/drm/imx/Makefile | 2 +
> drivers/gpu/drm/imx/imx-lcdc.c | 610 +
We are in the process of placing imx drivers in subdirectories,
could you move this into drivers/gpu/drm/imx/lcdc/ ?
> 3 files changed, 619 insertions(+)
> create mode 100644 drivers/gpu/drm/imx/imx-lcdc.c
>
> diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
> index fd5b2471fdf0..af5c6cb8c445 100644
> --- a/drivers/gpu/drm/imx/Kconfig
> +++ b/drivers/gpu/drm/imx/Kconfig
> @@ -41,3 +41,10 @@ config DRM_IMX_HDMI
> Choose this if you want to use HDMI on i.MX6.
>
> source "drivers/gpu/drm/imx/dcss/Kconfig"
> +
> +config DRM_IMX_LCDC
> + tristate "Freescale i.MX LCDC displays"
> + depends on DRM && (ARCH_MXC || COMPILE_TEST)
Select DRM_GEM_DMA_HELPER for DEFINE_DRM_GEM_DMA_FOPS().
> + select DRM_KMS_CMA_HELPER
Select DRM_KMS_HELPER instead, see commit 09717af7d13d ("drm: Remove
CONFIG_DRM_KMS_CMA_HELPER option").
> + help
> + Found on i.MX1, i.MX21, i.MX25 and i.MX27.
> diff --git a/drivers/gpu/drm/imx/Makefile b/drivers/gpu/drm/imx/Makefile
> index b644deffe948..1f96de7f15b4 100644
> --- a/drivers/gpu/drm/imx/Makefile
> +++ b/drivers/gpu/drm/imx/Makefile
> @@ -10,3 +10,5 @@ obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
>
> obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
> obj-$(CONFIG_DRM_IMX_DCSS) += dcss/
> +
> +obj-$(CONFIG_DRM_IMX_LCDC) += imx-lcdc.o
> diff --git a/drivers/gpu/drm/imx/imx-lcdc.c b/drivers/gpu/drm/imx/imx-lcdc.c
> new file mode 100644
> index ..14d4962cecfd
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/imx-lcdc.c
> @@ -0,0 +1,610 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// SPDX-FileCopyrightText: 2020 Marian Cichy
> +
> +#include "drm/drm_fourcc.h"
#include
and sort alphabetically?
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define IMX21LCDC_LSSAR 0x /* LCDC Screen Start Address Register
> */
> +#define IMX21LCDC_LSR 0x0004 /* LCDC Size Register */
> +#define IMX21LCDC_LVPWR 0x0008 /* LCDC Virtual Page Width Register */
> +#define IMX21LCDC_LCPR 0x000C /* LCDC Cursor Position Register */
> +#define IMX21LCDC_LCWHB 0x0010 /* LCDC Cursor Width Height and Blink
> Register*/
> +#define IMX21LCDC_LCCMR 0x0014 /* LCDC Color Cursor Mapping Register
> */
> +#define IMX21LCDC_LPCR 0x0018 /* LCDC Panel Configuration Register
> */
> +#define IMX21LCDC_LHCR 0x001C /* LCDC Horizontal Configuration
> Register */
> +#define IMX21LCDC_LVCR 0x0020 /* LCDC Vertical Configuration
> Register */
> +#define IMX21LCDC_LPOR 0x0024 /* LCDC Panning Offset Register */
> +#define IMX21LCDC_LSCR 0x0028 /* LCDC Sharp Configuration Register
> */
> +#define IMX21LCDC_LPCCR 0x002C /* LCDC PWM Contrast Control Register
> */
> +#define IMX21LCDC_LDCR 0x0030 /* LCDC DMA Control Register */
> +#define IMX21LCDC_LRMCR 0x0034 /* LCDC Refresh Mode Control Register
> */
> +#define IMX21LCDC_LICR 0x0038 /* LCDC Interrupt Configuration
> Register */
> +#define IMX21LCDC_LIER 0x003C /* LCDC Interrupt Enable Register */
> +#define IMX21LCDC_LISR 0x0040 /* LCDC Interrupt Status Register */
> +#define IMX21LCDC_LGWSAR0x0050 /* LCDC Graphic Window Start Address
> Register */
> +#define IMX21LCDC_LGWSR 0x0054 /* LCDC Graph Window Size Register */
> +#define IMX21LCDC_LGWVPWR 0x0058 /* LCDC Graphic Window Virtual Page
> Width Register */
> +#define IMX21LCDC_LGWPOR0x005C /* LCDC Graphic Window Panning Offset
> Register */
> +#define IMX21LCDC_LGWPR 0x0060 /* LCDC Graphic Window Position
> Register */
> +#define IMX21LCDC_LGWCR 0x0064 /* LCDC Graphic Window Control
> Register */
> +#define IMX21LCDC_LGWDCR0x0068 /* LCDC Graphic Window DMA Control
> Register */
> +#define IMX21LCDC_LAUSCR0x0080 /* LCDC AUS Mode Control Register */
> +#define IMX21LCDC_LAUSCCR 0x0084 /* LCDC AUS Mode Cursor Control
> Register */
> +#define IMX21LCDC_BGLUT 0x0800 /* Background Lookup Table */
> +#define IMX21LCDC_GWLUT 0x0C00 /* Graphic Window Lookup Table */
> +
> +#define IMX21LCDC_LCPR_CC0 BIT(30) /* Cursor Control Bit 0 */
> +#define IMX21LCDC_LCPR_CC1 BIT(31) /* Cursor Control Bit 1 */
> +
> +/* Values HSYNC,
Re: [PATCH 2/2] drm/etnaviv: convert user fence tracking to XArray
On Thu, Dec 01, 2022 at 06:48:46PM +0100, Lucas Stach wrote: > This simplifies the driver code a bit, as XArray already provides > internal locking. IDRs are implemented using XArrays anyways, so > this drops one level of unneeded abstraction. > > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 1/2] drm/etnaviv: split fence lock
On Thu, Dec 01, 2022 at 06:48:45PM +0100, Lucas Stach wrote: > The fence lock currently protects two distinct things. It protects the fence > IDR from concurrent inserts and removes and also keeps drm_sched_job_arm and > drm_sched_entity_push_job in one atomic section to guarantee the fence seqno > monotonicity. Split the lock into those two functions. > > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH v3 2/2] drm/etnaviv: print MMU exception cause
On Fri, Dec 02, 2022 at 10:19:29AM +0100, Lucas Stach wrote: > From: Christian Gmeiner > > The MMU tells us the fault status. While the raw register value is > already printed, it's a bit more user friendly to translate the > fault reasons into human readable format. > > Signed-off-by: Christian Gmeiner > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH] drm/etnaviv: print MMU exception cause
On Mi, 2022-11-30 at 19:53 +0100, Lucas Stach wrote:
From: Christian Gmeiner
The MMU tells us the fault status. While the raw register value is
already printed, it's a bit more user friendly to translate the
fault reasons into human readable format.
Signed-off-by: Christian Gmeiner
Signed-off-by: Lucas Stach
---
I've rewritten parts of the patch to properly cover multiple
MMUs and squashed the reason into the existing message. Christian,
please tell me if you are fine with having your name attached to
this patch.
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..f79203b774d9 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1426,6 +1426,15 @@ static void sync_point_worker(struct work_struct *work)
static void dump_mmu_fault(struct etnaviv_gpu *gpu)
{
+ static const char *fault_reasons[] = {
+ "slave not present",
+ "page not present",
+ "write violation",
+ "out of bounds",
+ "read security violation",
+ "write security violation",
+ };
+
u32 status_reg, status;
int i;
@@ -1438,18 +1447,25 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu)
dev_err_ratelimited(gpu->dev, "MMU fault status 0x%08x\n", status);
for (i = 0; i < 4; i++) {
+ const char *reason = "unknown";
u32 address_reg;
+ u32 mmu_status;
- if (!(status & (VIVS_MMUv2_STATUS_EXCEPTION0__MASK << (i * 4
+ mmu_status = (status >> (i * 4)) &
VIVS_MMUv2_STATUS_EXCEPTION0__MASK;
VIVS_MMUv2_STATUS_EXCEPTION0__MASK is 0x3 ...
+ if (!mmu_status)
continue;
+ if ((mmu_status - 1) < ARRAY_SIZE(fault_reasons))
+ reason = fault_reasons[mmu_status - 1];
... so (mmu_status - 1) can be 2 at most. This leaves me wondering how
"out of bounds" and the "security violation" errors can be reached. I
think this requires the exception bitfield masks to be extended to 0x7.
regards
Philipp
Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema
On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote: [...] > new file mode 100644 > index ..c3cf6f92a766 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml > @@ -0,0 +1,110 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and > i.MX27 > + > +maintainers: > + - Sascha Hauer > + - Pengutronix Kernel Team > + > +properties: > + compatible: > +oneOf: > + - items: > + - enum: > + - fsl,imx1-fb > + - fsl,imx21-fb Are the items/enum keywords superfluous here? Couldn't this just be two - const: fsl,imx1-fb - const: fsl,imx21-fb entries? > + - items: > + - enum: > + - fsl,imx25-fb > + - fsl,imx27-fb > + - const: fsl,imx21-fb > + > + clocks: > +maxItems: 3 > + > + clock-names: > +items: > + - const: ipg > + - const: ahb > + - const: per clocks and clock-names are new, so this is a little bit more than a straight forward conversion. I'd mention this in the commit description. regards Philipp
Re: [PATCH i-g-t 8/8] gputop: Basic vendor agnostic GPU top tool
On Fr, 2022-11-11 at 15:58 +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Rudimentary vendor agnostic example of how lib_igt_drm_clients can be used > to display a sorted by card and usage list of processes using GPUs. > > Borrows a bit of code from intel_gpu_top but for now omits the fancy > features like interactive functionality, card selection, client > aggregation, sort modes, JSON output and pretty engine names. Also no > support for global GPU or system metrics. > > On the other hand it shows clients from all DRM cards which > intel_gpu_top does not do. > > Signed-off-by: Tvrtko Ursulin > Cc: Rob Clark > Cc: Christian König > Acked-by: Christian König Tested-by: Philipp Zabel on etnaviv with [1]. [1] https://lore.kernel.org/dri-devel/20220916151205.165687-3-l.st...@pengutronix.de/ regards Philipp
Re: [PATCH v2 3/3] drm/etnaviv: export client GPU usage statistics via fdinfo
On Fri, Sep 16, 2022 at 05:12:05PM +0200, Lucas Stach wrote:
> This exposes a accumulated GPU active time per client via the
> fdinfo infrastructure.
>
> Signed-off-by: Lucas Stach
> ---
> v2:
> - fix code style
> - switch to raw seq_printf
> - leave some breadcrumbs about the output format
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 40 ++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index b69edb40ae2a..c08748472f74 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -22,6 +22,7 @@
> #include "etnaviv_gem.h"
> #include "etnaviv_mmu.h"
> #include "etnaviv_perfmon.h"
> +#include "common.xml.h"
>
> /*
> * DRM operations:
> @@ -471,7 +472,44 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> };
>
> -DEFINE_DRM_GEM_FOPS(fops);
> +static void etnaviv_fop_show_fdinfo(struct seq_file *m, struct file *f)
> +{
> + struct drm_file *file = f->private_data;
> + struct drm_device *dev = file->minor->dev;
> + struct etnaviv_drm_private *priv = dev->dev_private;
> + struct etnaviv_file_private *ctx = file->driver_priv;
> +
> + /*
> + * For a description of the text output format used here, see
> + * Documentation/gpu/drm-usage-stats.rst.
> + */
> + seq_printf(m, "drm-driver:\t%s\n", dev->driver->name);
> + seq_printf(m, "drm-client-id:\t%u\n", ctx->id);
> +
> + for (int i = 0; i < ETNA_MAX_PIPES; i++) {
> + struct etnaviv_gpu *gpu = priv->gpu[i];
> + char engine[8];
Maybe initialize this as well? See below.
> + int cur = 0;
> +
> + if (!gpu)
> + continue;
> +
> + if (gpu->identity.features & chipFeatures_PIPE_2D)
> + cur = snprintf(engine, sizeof(engine), "2D");
> + if (gpu->identity.features & chipFeatures_PIPE_3D)
> + cur = snprintf(engine + cur, sizeof(engine) - cur,
> +"%s3D", cur ? "/" : "");
Does the NPU have either bit set? If not, this must not be forgotten to
be changed when NPU support is added, to avoid uninitalized use of the
engine variable.
Reviewed-by: Philipp Zabel
Tested-by: Philipp Zabel
with gputop [1].
[1]
https://lore.kernel.org/dri-devel/2022155844.3290531-1-tvrtko.ursu...@linux.intel.com/
regards
Philipp
Re: [PATCH v2 2/3] drm/etnaviv: allocate unique ID per drm_file
On Fri, Sep 16, 2022 at 05:12:04PM +0200, Lucas Stach wrote:
> Allows to easily track if several fd are pointing to the same
> execution context due to being dup'ed.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
> drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 1d2b4fb4bcf8..b69edb40ae2a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -49,6 +49,7 @@ static void load_gpu(struct drm_device *dev)
> static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
> {
> struct etnaviv_drm_private *priv = dev->dev_private;
> + static atomic_t ident = ATOMIC_INIT(0);
> struct etnaviv_file_private *ctx;
> int ret, i;
>
> @@ -56,6 +57,8 @@ static int etnaviv_open(struct drm_device *dev, struct
> drm_file *file)
> if (!ctx)
> return -ENOMEM;
>
> + ctx->id = atomic_inc_return();
I suppose we can ignore that this could theoretically wrap around.
Reviewed-by: Philipp Zabel
regards
Philipp
[PATCH] drm/imx: ipuv3-plane: Fix overlay plane width
ipu_src_rect_width() was introduced to support odd screen resolutions
such as 1366x768 by internally rounding up primary plane width to a
multiple of 8 and compensating with reduced horizontal blanking.
This also caused overlay plane width to be rounded up, which was not
intended. Fix overlay plane width by limiting the rounding up to the
primary plane.
drm_rect_width(_state->src) >> 16 is the same value as
drm_rect_width(dst) because there is no plane scaling support.
Fixes: 94dfec48fca7 ("drm/imx: Add 8 pixel alignment fix")
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/imx/ipuv3-plane.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
b/drivers/gpu/drm/imx/ipuv3-plane.c
index dba4f7d81d69..80142d9a4a55 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -614,6 +614,11 @@ static void ipu_plane_atomic_update(struct drm_plane
*plane,
break;
}
+ if (ipu_plane->dp_flow == IPU_DP_FLOW_SYNC_BG)
+ width = ipu_src_rect_width(new_state);
+ else
+ width = drm_rect_width(_state->src) >> 16;
+
eba = drm_plane_state_to_eba(new_state, 0);
/*
@@ -622,8 +627,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
*/
if (ipu_state->use_pre) {
axi_id = ipu_chan_assign_axi_id(ipu_plane->dma);
- ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id,
- ipu_src_rect_width(new_state),
+ ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id, width,
drm_rect_height(_state->src) >>
16,
fb->pitches[0], fb->format->format,
fb->modifier, );
@@ -678,9 +682,8 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
break;
}
- ipu_dmfc_config_wait4eot(ipu_plane->dmfc, ALIGN(drm_rect_width(dst),
8));
+ ipu_dmfc_config_wait4eot(ipu_plane->dmfc, width);
- width = ipu_src_rect_width(new_state);
height = drm_rect_height(_state->src) >> 16;
info = drm_format_info(fb->format->format);
ipu_calculate_bursts(width, info->cpp[0], fb->pitches[0],
@@ -744,8 +747,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
ipu_cpmem_set_burstsize(ipu_plane->ipu_ch, 16);
ipu_cpmem_zero(ipu_plane->alpha_ch);
- ipu_cpmem_set_resolution(ipu_plane->alpha_ch,
-ipu_src_rect_width(new_state),
+ ipu_cpmem_set_resolution(ipu_plane->alpha_ch, width,
drm_rect_height(_state->src) >>
16);
ipu_cpmem_set_format_passthrough(ipu_plane->alpha_ch, 8);
ipu_cpmem_set_high_priority(ipu_plane->alpha_ch);
base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
--
2.30.2
Re: [PATCH] drm/imx: imx-tve: Fix return type of imx_tve_connector_mode_valid
On Di, 2022-09-13 at 13:55 -0700, Nathan Huckleberry wrote:
> The mode_valid field in drm_connector_helper_funcs is expected to be of
> type:
> enum drm_mode_status (* mode_valid) (struct drm_connector *connector,
> struct drm_display_mode *mode);
>
> The mismatched return type breaks forward edge kCFI since the underlying
> function definition does not match the function hook definition.
>
> The return type of imx_tve_connector_mode_valid should be changed from
> int to enum drm_mode_status.
>
> Reported-by: Dan Carpenter
> Link: https://github.com/ClangBuiltLinux/linux/issues/1703
> Cc: l...@lists.linux.dev
> Signed-off-by: Nathan Huckleberry
> ---
> drivers/gpu/drm/imx/imx-tve.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
> index 6b34fac3f73a..ab4d1c878fda 100644
> --- a/drivers/gpu/drm/imx/imx-tve.c
> +++ b/drivers/gpu/drm/imx/imx-tve.c
> @@ -218,8 +218,9 @@ static int imx_tve_connector_get_modes(struct
> drm_connector *connector)
> return ret;
> }
>
>
> -static int imx_tve_connector_mode_valid(struct drm_connector *connector,
> - struct drm_display_mode *mode)
> +static enum drm_mode_status
> +imx_tve_connector_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> {
> struct imx_tve *tve = con_to_tve(connector);
> unsigned long rate;
Reviewed-by: Philipp Zabel
and pushed to drm-misc-fixes.
regards
Philipp
Re: [PATCH] drm/imx: Kconfig: Remove duplicated 'select DRM_KMS_HELPER' line
On So, 2022-10-09 at 10:35 +0800, Liu Ying wrote:
> A duplicated line 'select DRM_KMS_HELPER' was introduced in Kconfig file
> by commit 09717af7d13d ("drm: Remove CONFIG_DRM_KMS_CMA_HELPER option"),
> so remove it.
>
> Fixes: 09717af7d13d ("drm: Remove CONFIG_DRM_KMS_CMA_HELPER option")
> Signed-off-by: Liu Ying
> ---
> drivers/gpu/drm/imx/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
> index 975de4ff7313..fd5b2471fdf0 100644
> --- a/drivers/gpu/drm/imx/Kconfig
> +++ b/drivers/gpu/drm/imx/Kconfig
> @@ -4,7 +4,6 @@ config DRM_IMX
> select DRM_KMS_HELPER
> select VIDEOMODE_HELPERS
> select DRM_GEM_DMA_HELPER
> - select DRM_KMS_HELPER
> depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM || COMPILE_TEST)
> depends on IMX_IPUV3_CORE
> help
Reviewed-by: Philipp Zabel
and pushed to drm-misc-fixes.
regards
Philipp
Re: [PATCH v2] drm/etnaviv: don't truncate physical page address
On Fr, 2022-09-16 at 12:40 +0200, Lucas Stach wrote:
> While the interface for the MMU mapping takes phys_addr_t to hold a
> full 64bit address when necessary and MMUv2 is able to map physical
> addresses with up to 40bit, etnaviv_iommu_map() truncates the address
> to 32bits. Fix this by using the correct type.
>
> Fixes: 931e97f3afd8 ("drm/etnaviv: mmuv2: support 40 bit phys address")
> Signed-off-by: Lucas Stach
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH] drm/etnaviv: don't truncate physical page address
Hi Lucas,
On Do, 2022-09-15 at 16:19 +0200, Lucas Stach wrote:
> While the interface for the MMU mapping takes phys_addr_t to hold a
> full 64bit address when necessary and MMUv2 is able to map physical
> addresses with up to 40bit, etnaviv_iommu_map() truncates the address
> to 32bits. Fix this by using the correct type.
>
> Fixes: 931e97f3afd8 ("drm/etnaviv: mmuv2: support 40 bit phys address")
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> index dc1aa738c4f1..2ff80d5ccf07 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> @@ -80,7 +80,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context
> *context, u32 iova,
> return -EINVAL;
>
>
> for_each_sgtable_dma_sg(sgt, sg, i) {
> - u32 pa = sg_dma_address(sg) - sg->offset;
> + phys_addr_t pa = sg_dma_address(sg) - sg->offset;
> size_t bytes = sg_dma_len(sg) + sg->offset;
>
>
> VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes);
^^
Use %pap, here?
regards
Philipp
Re: [PATCH v6 4/6] clk: qcom: gpucc-sc7280: Add cx collapse reset support
On Wed, Aug 31, 2022 at 10:48:25AM +0530, Akhil P Oommen wrote:
> Allow a consumer driver to poll for cx gdsc collapse through Reset
> framework.
>
> Signed-off-by: Akhil P Oommen
> Reviewed-by: Dmitry Baryshkov
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Convert 'struct qcom_reset_ops cx_gdsc_reset' to 'static const' (Krzysztof)
>
> Changes in v2:
> - Minor update to use the updated custom reset ops implementation
>
> drivers/clk/qcom/gpucc-sc7280.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
> index 9a832f2..fece3f4 100644
> --- a/drivers/clk/qcom/gpucc-sc7280.c
> +++ b/drivers/clk/qcom/gpucc-sc7280.c
> @@ -433,12 +433,22 @@ static const struct regmap_config
> gpu_cc_sc7280_regmap_config = {
> .fast_io = true,
> };
>
> +static const struct qcom_reset_ops cx_gdsc_reset = {
> + .reset = gdsc_wait_for_collapse,
This should be accompanied by a comment explaining the not-quite-reset
nature of this workaround, i.e. what is the prerequisite for this to
actually work as expected?
> +};
> +
> +static const struct qcom_reset_map gpucc_sc7280_resets[] = {
> + [GPU_CX_COLLAPSE] = { .ops = _gdsc_reset, .priv = _gdsc },
> +};
> +
> static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
> .config = _cc_sc7280_regmap_config,
> .clks = gpu_cc_sc7280_clocks,
> .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
> .gdscs = gpu_cc_sc7180_gdscs,
> .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
> + .resets = gpucc_sc7280_resets,
> + .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
See my comment on patch 2. I think instead of adding a const struct
qcom_reset_ops * to gpucc_sc7280_resets, this should just add a const
struct reset_control * to gpu_cc_sc7280_desc.
regards
Philipp
Re: [PATCH v6 3/6] clk: qcom: gdsc: Add a reset op to poll gdsc collapse
On Wed, Aug 31, 2022 at 10:48:24AM +0530, Akhil P Oommen wrote:
> Add a reset op compatible function to poll for gdsc collapse.
>
> Signed-off-by: Akhil P Oommen
> Reviewed-by: Dmitry Baryshkov
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Minor update to function prototype
>
> drivers/clk/qcom/gdsc.c | 23 +++
> drivers/clk/qcom/gdsc.h | 7 +++
> 2 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 44520ef..2d0f1d1 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include "gdsc.h"
> +#include "reset.h"
>
> #define PWR_ON_MASK BIT(31)
> #define EN_REST_WAIT_MASKGENMASK_ULL(23, 20)
> @@ -116,7 +117,8 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en)
> return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val);
> }
>
> -static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
> +static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status,
> + s64 timeout_us, unsigned int interval_ms)
> {
> ktime_t start;
>
> @@ -124,7 +126,9 @@ static int gdsc_poll_status(struct gdsc *sc, enum
> gdsc_status status)
> do {
> if (gdsc_check_status(sc, status))
> return 0;
> - } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US);
> + if (interval_ms)
> + msleep(interval_ms);
> + } while (ktime_us_delta(ktime_get(), start) < timeout_us);
Could this loop be implemented with read_poll_timeout()?
> if (gdsc_check_status(sc, status))
> return 0;
> @@ -172,7 +176,7 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum
> gdsc_status status)
> udelay(1);
> }
>
> - ret = gdsc_poll_status(sc, status);
> + ret = gdsc_poll_status(sc, status, TIMEOUT_US, 0);
> WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
>
> if (!ret && status == GDSC_OFF && sc->rsupply) {
> @@ -343,7 +347,7 @@ static int _gdsc_disable(struct gdsc *sc)
>*/
> udelay(1);
>
> - ret = gdsc_poll_status(sc, GDSC_ON);
> + ret = gdsc_poll_status(sc, GDSC_ON, TIMEOUT_US, 0);
> if (ret)
> return ret;
> }
> @@ -565,3 +569,14 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain
> *domain)
> return 0;
> }
> EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
> +
> +int gdsc_wait_for_collapse(void *priv)
> +{
> + struct gdsc *sc = priv;
> + int ret;
> +
> + ret = gdsc_poll_status(sc, GDSC_OFF, 50, 5);
> + WARN(ret, "%s status stuck at 'on'", sc->pd.name);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gdsc_wait_for_collapse);
Superficially, using this as a reset op seems like abuse of the reset
controller API. Calling reset_control_reset() on this in the GPU driver
will not trigger a reset signal on the GPU's "cx_collapse" reset input.
So at the very least, this patchset should contain an explanation why
this is a good idea regardless, and how this is almost a reset control.
I have read the linked discussion, and I'm not sure I understand all
of it, so please correct me if I'm wrong: There is some other way to
force the GDSC into a state that will eventually cause a GPU reset, and
this is just the remaining part to make sure that the workaround dance
is finished?
If so, it should be explained that this depends on something else to
actually indirectly trigger the reset, and where this happens.
regards
Philipp
Re: [PATCH v6 2/6] clk: qcom: Allow custom reset ops
Hi Akhil,
On Wed, Aug 31, 2022 at 10:48:23AM +0530, Akhil P Oommen wrote:
> Allow soc specific clk drivers to specify a custom reset operation. We
> will use this in an upcoming patch to allow gpucc driver to specify a
> differet reset operation for cx_gdsc.
>
> Signed-off-by: Akhil P Oommen
> Reviewed-by: Dmitry Baryshkov
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Use pointer to const for "struct qcom_reset_ops" in qcom_reset_map
> (Krzysztof)
>
> Changes in v2:
> - Return error when a particular custom reset op is not implemented. (Dmitry)
>
> drivers/clk/qcom/reset.c | 27 +++
> drivers/clk/qcom/reset.h | 8
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c
> index 819d194..b7ae4a3 100644
> --- a/drivers/clk/qcom/reset.c
> +++ b/drivers/clk/qcom/reset.c
> @@ -13,6 +13,21 @@
>
> static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
> {
> + struct qcom_reset_controller *rst;
> + const struct qcom_reset_map *map;
> +
> + rst = to_qcom_reset_controller(rcdev);
> + map = >reset_map[id];
> +
> + if (map->ops && map->ops->reset)
> + return map->ops->reset(map->priv);
> + /*
> + * If custom ops is implemented but just not this callback, return
> + * error
> + */
> + else if (map->ops)
> + return -EOPNOTSUPP;
> +
It doesn't seem necessary to stack reset_ops -> qcom_reset_ops for what
you need here.
Just add an optional const struct reset_ops * to qcom_cc_desc and feed
that into qcom_cc_really_probe to replace _reset_ops.
[...]
> +struct qcom_reset_ops {
> + int (*reset)(void *priv);
> + int (*assert)(void *priv);
> + int (*deassert)(void *priv);
Why add assert and deassert ops? There doesn't seem to be any user.
> +};
> +
> struct qcom_reset_map {
> unsigned int reg;
> u8 bit;
> + const struct qcom_reset_ops *ops;
> + void *priv;
Adding per-reset ops + priv counters seems excessive to me.
Are you expecting different reset controls in the same reset controller
to have completely different ops at some point? If so, I would wonder
whether it wouldn't be better to split the reset controller in that
case. Either way, for the GDSC collapse workaround this does not seem
to be required at all.
regards
Philipp
Re: [PATCH v4 5/7] drm/msm/a6xx: Ensure CX collapse during gpu recovery
Hi Akhil, On Wed, Aug 17, 2022 at 08:44:18PM +0530, Akhil P Oommen wrote: > Because there could be transient votes from other drivers/tz/hyp which > may keep the cx gdsc enabled, we should poll until cx gdsc collapses. > We can use the reset framework to poll for cx gdsc collapse from gpucc > clk driver. > > This feature requires support from the platform's gpucc driver. > > Signed-off-by: Akhil P Oommen > Reviewed-by: Dmitry Baryshkov > --- > > (no changes since v3) > > Changes in v3: > - Use reset interface from gpucc driver to poll for cx gdsc collapse > https://patchwork.freedesktop.org/series/106860/ > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 > drivers/gpu/drm/msm/msm_gpu.c | 4 > drivers/gpu/drm/msm/msm_gpu.h | 4 > 3 files changed, 12 insertions(+) > [...] > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c > index 07e55a6..4a57627 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.c > +++ b/drivers/gpu/drm/msm/msm_gpu.c [...] > @@ -903,6 +904,9 @@ int msm_gpu_init(struct drm_device *drm, struct > platform_device *pdev, > if (IS_ERR(gpu->gpu_cx)) > gpu->gpu_cx = NULL; > > + gpu->cx_collapse = devm_reset_control_get_optional(>dev, > + "cx_collapse"); Please use devm_reset_control_get_optional_exclusive() instead. With that, Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH] drm: bridge: dw_hdmi: only trigger hotplug event on real link state change
On Mo, 2022-06-27 at 14:47 +0200, Lucas Stach wrote:
There are two events that signal a real change of the link state: HPD going
high means the sink is newly connected or wants the source to re-read the
EDID, RX sense going low is a indication that the link has been disconnected.
Ignore the other two events that also trigger interrupts, but don't need
immediate attention: HPD going low does not necessarily mean the link has
been lost and should not trigger a immediate read of the status. RX sense
going high also does not require a detect cycle, as HPD going high is the
right point in time to read the EDID.
Ignoring the negative HPD edge does make the detection much more robust
against spurious link status changes due to EMI or marginal signal levels.
Signed-off-by: Lucas Stach
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 3e1be9894ed1..24f991b5248d 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3095,6 +3095,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
{
struct dw_hdmi *hdmi = dev_id;
u8 intr_stat, phy_int_pol, phy_pol_mask, phy_stat;
+ enum drm_connector_status status = connector_status_unknown;
intr_stat = hdmi_readb(hdmi, HDMI_IH_PHY_STAT0);
phy_int_pol = hdmi_readb(hdmi, HDMI_PHY_POL0);
@@ -3133,13 +3134,15 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
mutex_unlock(>cec_notifier_mutex);
}
- }
- if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
- enum drm_connector_status status = phy_int_pol & HDMI_PHY_HPD
-? connector_status_connected
-:
connector_status_disconnected;
+ if (phy_stat & HDMI_PHY_HPD)
+ status = connector_status_connected;
+
+ if (!( phy_stat & HDMI_PHY_RX_SENSE))
Too much space: ^
Also, would it make sense to check
if (!(phy_stat & (HDMI_PHY_HPD | HDMI_PHY_RX_SENSE)))
instead?
regards
Philipp
Re: [PATCH v24 01/10] dt-bindings: reset: mt8195: add vdosys1 reset control bit
On Mi, 2022-06-22 at 21:08 +0800, Nancy.Lin wrote: > Add vdosys1 reset control bit for MT8195 platform. > > Signed-off-by: Nancy.Lin > Reviewed-by: Chun-Kuang Hu > Reviewed-by: AngeloGioacchino Del Regno > > Reviewed-by: Rex-BC Chen > Acked-by: Krzysztof Kozlowski > Tested-by: AngeloGioacchino Del Regno > Acked-by: Philipp Zabel regards Philipp
Re: [PATCH v3 03/71] drm/encoder: Introduce drmm_encoder_init
On Mi, 2022-06-29 at 14:34 +0200, Maxime Ripard wrote: > The DRM-managed function to register an encoder is > drmm_encoder_alloc() and its variants, which will allocate the underlying > structure and initialisation the encoder. > > However, we might want to separate the structure creation and the encoder > initialisation, for example if the structure is shared across multiple DRM > entities, for example an encoder and a connector. > > Let's create an helper to only initialise an encoder that would be passed > as an argument. Daniel pointed out here [1], that it might be good to check the passed encoders are actually in drmm managed memory. [1] https://lore.kernel.org/dri-devel/CAKMK7uGaAtk4AY5y=jbc7ndduryfbflsdhe8wykj602lk-3...@mail.gmail.com/ regards Philipp
[GIT PULL] drm/imx: various cleanups
Hi Dave, Daniel, The following changes since commit 3123109284176b1532874591f7c81f3837bbdc17: Linux 5.18-rc1 (2022-04-03 14:08:21 -0700) are available in the Git repository at: git://git.pengutronix.de/pza/linux.git tags/imx-drm-next-2022-05-04 for you to fetch changes up to 927d8fd465adbaaad6cce82f840d489d7c378f29: drm/imx: ipuv3-plane: Remove redundant color encoding and range initialisation (2022-04-04 09:34:21 +0200) drm/imx: various cleanups - Use swap() instead of open-coding in ipu-image-convert. - Use devm_platform_ioremap_resource() helper in imx-tve. - Make static channel_offsets array const in ipu-dc. - Remove redundant zpos, color encoding and range initialization. Cai Huoqing (1): drm/imx: imx-tve: Make use of the helper function devm_platform_ioremap_resource() Colin Ian King (1): drm/imx: make static read-only array channel_offsets const Maxime Ripard (2): drm/imx: ipuv3-plane: Remove redundant zpos initialisation drm/imx: ipuv3-plane: Remove redundant color encoding and range initialisation Salah Triki (1): gpu: ipu-v3: image-convert: use swap() drivers/gpu/drm/imx/imx-tve.c | 4 +--- drivers/gpu/drm/imx/ipuv3-plane.c | 8 +--- drivers/gpu/ipu-v3/ipu-dc.c| 5 +++-- drivers/gpu/ipu-v3/ipu-image-convert.c | 9 +++-- 4 files changed, 8 insertions(+), 18 deletions(-)
Re: [PATCH v17 14/21] drm/mediatek: add ETHDR support for MT8195
On Sa, 2022-04-16 at 10:07 +0800, Nancy.Lin wrote:
> ETHDR is a part of ovl_adaptor.
> ETHDR is designed for HDR video and graphics conversion in the external
> display path. It handles multiple HDR input types and performs tone
> mapping, color space/color format conversion, and then combine
> different layers, output the required HDR or SDR signal to the
> subsequent display path.
>
> Signed-off-by: Nancy.Lin
> Reviewed-by: Chun-Kuang Hu
> Reviewed-by: AngeloGioacchino Del Regno
>
> ---
[...]
> +static int mtk_ethdr_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct mtk_ethdr *priv;
> + int ret;
> + int i;
> +
> + dev_info(dev, "%s+\n", __func__);
Left-over debug statements?
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + for (i = 0; i < ETHDR_ID_MAX; i++) {
> + priv->ethdr_comp[i].dev = dev;
> + priv->ethdr_comp[i].regs = of_iomap(dev->of_node, i);
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> + ret = cmdq_dev_get_client_reg(dev,
> + >ethdr_comp[i].cmdq_base,
> i);
> + if (ret)
> + dev_dbg(dev, "get mediatek,gce-client-reg fail!\n");
> +#endif
> + dev_dbg(dev, "[DRM]regs:0x%p, node:%d\n",
> priv->ethdr_comp[i].regs, i);
> + }
> +
> + for (i = 0; i < ETHDR_CLK_NUM; i++)
> + priv->ethdr_clk[i].id = ethdr_clk_str[i];
> + ret = devm_clk_bulk_get_optional(dev, ETHDR_CLK_NUM, priv->ethdr_clk);
> + if (ret)
> + return ret;
> +
> + priv->irq = platform_get_irq(pdev, 0);
> + if (priv->irq < 0)
> + priv->irq = 0;
> +
> + if (priv->irq) {
> + ret = devm_request_irq(dev, priv->irq, mtk_ethdr_irq_handler,
> +IRQF_TRIGGER_NONE, dev_name(dev), priv);
> + if (ret < 0) {
> + dev_err(dev, "Failed to request irq %d: %d\n",
> priv->irq, ret);
> + return ret;
> + }
> + }
> +
> + priv->reset_ctl = devm_reset_control_array_get_optional_exclusive(dev);
This is missing error handling. You could use dev_err_probe() here.
regards
Philipp
Re: [PATCH v17 13/21] drm/mediatek: add display merge async reset control
On Sa, 2022-04-16 at 10:07 +0800, Nancy.Lin wrote: > Add merge async reset control in mtk_merge_stop. Async hw doesn't do self > reset on each sof signal(start of frame), so need to reset the async to > clear the hw status for the next merge start. > > Signed-off-by: Nancy.Lin > Reviewed-by: CK Hu > Reviewed-by: AngeloGioacchino Del Regno > > --- > drivers/gpu/drm/mediatek/mtk_disp_merge.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_merge.c > b/drivers/gpu/drm/mediatek/mtk_disp_merge.c > index 9dca145cfb71..177473fa8160 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_merge.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_merge.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > #include > > > > > #include "mtk_drm_ddp_comp.h" > @@ -79,6 +80,9 @@ void mtk_merge_stop(struct device *dev) > struct mtk_disp_merge *priv = dev_get_drvdata(dev); > > > > > mtk_merge_stop_cmdq(dev, NULL); > + > + if (priv->async_clk) > + device_reset_optional(dev); To avoid the overhead of looking up the reset control in the device tree every time, it would be better to request a reset control during probe using devm_reset_control_get_optional_exclusive(). Here you'd just call reset_control_reset(). regards Philipp
Re: [PATCH v17 03/21] dt-bindings: mediatek: add ethdr definition for mt8195
Hi Nancy, On Sa, 2022-04-16 at 10:07 +0800, Nancy.Lin wrote: > Add vdosys1 ETHDR definition. > > Signed-off-by: Nancy.Lin > Reviewed-by: Chun-Kuang Hu > Reviewed-by: AngeloGioacchino Del Regno > > --- > .../display/mediatek/mediatek,ethdr.yaml | 158 ++ > 1 file changed, 158 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml > b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml > new file mode 100644 > index ..e8303c28a361 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml > @@ -0,0 +1,158 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/mediatek/mediatek,ethdr.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek Ethdr Device Tree Bindings > + > +maintainers: > + - Chun-Kuang Hu > + - Philipp Zabel > + > +description: | > + ETHDR is designed for HDR video and graphics conversion in the external > display path. > + It handles multiple HDR input types and performs tone mapping, color > space/color > + format conversion, and then combine different layers, output the required > HDR or > + SDR signal to the subsequent display path. This engine is composed of two > video > + frontends, two graphic frontends, one video backend and a mixer. ETHDR has > two > + DMA function blocks, DS and ADL. These two function blocks read the > pre-programmed > + registers from DRAM and set them to HW in the v-blanking period. > + > +properties: > + compatible: > +items: > + - const: mediatek,mt8195-disp-ethdr > + reg: > +maxItems: 7 > + reg-names: > +items: > + - const: mixer > + - const: vdo_fe0 > + - const: vdo_fe1 > + - const: gfx_fe0 > + - const: gfx_fe1 > + - const: vdo_be > + - const: adl_ds > + interrupts: > +minItems: 1 > + iommus: > +description: The compatible property is DMA function blocks. > + Should point to the respective IOMMU block with master port as > argument, > + see Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for > + details. > +minItems: 1 > +maxItems: 2 > + clocks: > +items: > + - description: mixer clock > + - description: video frontend 0 clock > + - description: video frontend 1 clock > + - description: graphic frontend 0 clock > + - description: graphic frontend 1 clock > + - description: video backend clock > + - description: autodownload and menuload clock > + - description: video frontend 0 async clock > + - description: video frontend 1 async clock > + - description: graphic frontend 0 async clock > + - description: graphic frontend 1 async clock > + - description: video backend async clock > + - description: ethdr top clock > + clock-names: > +items: > + - const: mixer > + - const: vdo_fe0 > + - const: vdo_fe1 > + - const: gfx_fe0 > + - const: gfx_fe1 > + - const: vdo_be > + - const: adl_ds > + - const: vdo_fe0_async > + - const: vdo_fe1_async > + - const: gfx_fe0_async > + - const: gfx_fe1_async > + - const: vdo_be_async > + - const: ethdr_top > + power-domains: > +maxItems: 1 > + resets: > +maxItems: 5 Please add a reset-names property and name these resets. regards Philipp
Re: [PATCH v0 05/10] drm/imx: add driver for HDMI TX Parallel Video Interface
On Mi, 2022-04-06 at 18:01 +0200, Lucas Stach wrote:
> This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a
> full timing generator and can switch between different video sources. On
> the i.MX8MP however the only supported source is the LCDIF. The block
> just needs to be powered up and told about the polarity of the video
> sync signals to act in bypass mode.
>
> Signed-off-by: Lucas Stach
> ---
[...]
> +static void imx_hdmi_pvi_bridge_enable(struct drm_bridge *bridge,
> +struct drm_bridge_state *bridge_state)
> +{
> + struct drm_atomic_state *state = bridge_state->base.state;
> + struct imx_hdmi_pvi *pvi = to_imx_hdmi_pvi(bridge);
> + struct drm_connector_state *conn_state;
> + const struct drm_display_mode *mode;
> + struct drm_crtc_state *crtc_state;
> + struct drm_connector *connector;
> + u32 bus_flags, val;
> +
> + connector = drm_atomic_get_new_connector_for_encoder(state,
> bridge->encoder);
> + if (WARN_ON(!connector))
> + return;
> +
> + conn_state = drm_atomic_get_new_connector_state(state, connector);
> + if (WARN_ON(!conn_state))
> + return;
> +
> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> + if (WARN_ON(!crtc_state))
> + return;
Can those happen at all, and if so, should they be caught at
atomic_check time?
> +
> + if (WARN_ON(pm_runtime_resume_and_get(pvi->dev)))
> + return;
Should be pm_runtime_get_sync(), since the error is ignored.
Otherwise the pm_runtime_put() in imx_hdmi_pvi_bridge_disable() will
double-decrement the usage counter in case this failed.
regards
Philipp
Re: [PATCH v0 03/10] drm/imx: add bridge wrapper driver for i.MX8MP DWC HDMI
On Mi, 2022-04-06 at 18:01 +0200, Lucas Stach wrote: > Add a simple wrapper driver for the DWC HDMI bridge driver that > implements the few bits that are necessary to abstract the i.MX8MP > SoC integration. > > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH v0 07/10] phy: freescale: add Samsung HDMI PHY
Hi Lucas,
On Mi, 2022-04-06 at 18:01 +0200, Lucas Stach wrote:
> This adds the driver for the Samsung HDMI PHY found on the
> i.MX8MP SoC.
>
> Heavily based on the PHY implementation in the downstream kernel
> written by Sandor Yu , but also cleaned up
> quite a bit and extended to support runtime PM.
>
> Signed-off-by: Lucas Stach
> ---
> FIXME: The PHY configuration could be cleaned up further, it
> currently has a lot of register writes that are same across
> all supported modes.
Agreed.
[...]
> ---
> drivers/phy/freescale/Kconfig|7 +
> drivers/phy/freescale/Makefile |1 +
> drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1145 ++
> 3 files changed, 1153 insertions(+)
> create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c
>
> diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
> index f9c54cd02036..f80c92eb7c55 100644
> --- a/drivers/phy/freescale/Kconfig
> +++ b/drivers/phy/freescale/Kconfig
> @@ -26,6 +26,13 @@ config PHY_FSL_IMX8M_PCIE
> Enable this to add support for the PCIE PHY as found on
> i.MX8M family of SOCs.
>
> +config PHY_FSL_SAMSUNG_HDMI_PHY
> + tristate "Samsung HDMI PHY support"
> + depends on OF && HAS_IOMEM
> + select GENERIC_PHY
Why select GENERIC_PHY when all the driver does is register a clock?
[...]
> +struct fsl_samsung_hdmi_phy {
> + struct device *dev;
> + void __iomem *regs;
> + struct clk *apbclk;
> + struct clk *refclk;
refclk isn't really used beyond phy_clk_register, it doesn't have to be
stored in struct fsl_samsung_hdmi_phy.
> +
> + /* clk provider */
> + struct clk_hw hw;
> + const struct phy_config *cur_cfg;
> +};
> +
> +static inline struct fsl_samsung_hdmi_phy *
> +to_fsl_samsung_hdmi_phy(struct clk_hw *hw)
> +{
> + return container_of(hw, struct fsl_samsung_hdmi_phy, hw);
> +}
> +
> +static void fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
> + const struct phy_config *cfg)
> +{
> + int i;
> +
> + /* HDMI PHY init */
> + writeb(REG33_FIX_DA, phy->regs + PHY_REG_33);
> +
> + for (i = 0; i < PHY_PLL_REGS_NUM; i++)
> + writeb(cfg->regs[i], phy->regs + i * 4);
> +
> + writeb(REG33_FIX_DA | REG33_MODE_SET_DONE , phy->regs + PHY_REG_33);
> +}
> +
> +static int phy_clk_prepare(struct clk_hw *hw)
> +{
> + struct fsl_samsung_hdmi_phy *phy = to_fsl_samsung_hdmi_phy(hw);
> + int ret = 0;
> + u8 val;
> +
> + return 0;
I'd say remove this line or pyh_clk_prepare().
> + ret = readb_poll_timeout(phy->regs + PHY_REG_34, val,
> + val & REG34_PLL_LOCK,
> + 20, 2);
> + if (ret)
> + dev_err(phy->dev, "PLL failed to lock\n");
> +
> + return ret;
> +}
> +
> +static unsigned long phy_clk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct fsl_samsung_hdmi_phy *phy = to_fsl_samsung_hdmi_phy(hw);
> +
> + if (!phy->cur_cfg)
> + return 0;
> +
> + return phy->cur_cfg->clk_rate;
> +}
> +
> +static long phy_clk_round_rate(struct clk_hw *hw,
> +unsigned long rate, unsigned long *parent_rate)
> +{
> + const struct phy_config *phy_cfg = phy_pll_cfg;
> +
> + for (; phy_cfg->clk_rate != 0; phy_cfg++)
> + if (phy_cfg->clk_rate == rate)
* @round_rate: Given a target rate as input, returns the closest rate actually
* supported by the clock. The parent rate is an input/output
* parameter.
This should round, not -EINVAL on unsupported rates.
> + break;
> +
> + if (phy_cfg->clk_rate == 0)
> + return -EINVAL;
> +
> + return phy_cfg->clk_rate;
> +}
> +
> +static int phy_clk_set_rate(struct clk_hw *hw,
> + unsigned long rate, unsigned long parent_rate)
> +{
> + struct fsl_samsung_hdmi_phy *phy = to_fsl_samsung_hdmi_phy(hw);
> + const struct phy_config *phy_cfg = phy_pll_cfg;
> + int ret = 0;
Unnecessary initialization.
> + u8 val;
> +
> + for (; phy_cfg->clk_rate != 0; phy_cfg++)
> + if (phy_cfg->clk_rate == rate)
> + break;
> +
> + if (phy_cfg->clk_rate == 0)
> + return -EINVAL;
> +
> + phy->cur_cfg = phy_cfg;
> +
> + fsl_samsung_hdmi_phy_configure(phy, phy_cfg);
> +
> + ret = readb_poll_timeout(phy->regs + PHY_REG_34, val,
> + val & REG34_PLL_LOCK,
> + 50, 2);
> + if (ret)
> + dev_err(phy->dev, "PLL failed to lock\n");
> +
> + return ret;
> +}
> +
> +static const struct clk_ops phy_clk_ops = {
> + .prepare = phy_clk_prepare,
> + .recalc_rate = phy_clk_recalc_rate,
> + .round_rate = phy_clk_round_rate,
> + .set_rate = phy_clk_set_rate,
> +};
> +
> +static
[GIT PULL] drm/imx: error handling and debug output fixes
Hi Dave, Daniel, The following changes since commit 3123109284176b1532874591f7c81f3837bbdc17: Linux 5.18-rc1 (2022-04-03 14:08:21 -0700) are available in the Git repository at: git://git.pengutronix.de/pza/linux.git tags/imx-drm-fixes-2022-04-06 for you to fetch changes up to 070a88fd4a03f921b73a2059e97d55faaa447dab: gpu: ipu-v3: Fix dev_dbg frequency output (2022-04-04 09:37:42 +0200) drm/imx: error handling and debug output fixes Catch an EDID allocation failure in imx-ldb, fix a leaked drm display mode on DT parsing error in parallel-display, properly remove the dw_hdmi bridge in case the component_add fails in dw_hdmi-imx, and fix the IPU clock frequency debug printout in ipu-di. Jiasheng Jiang (1): drm/imx: imx-ldb: Check for null pointer after calling kmemdup José Expósito (1): drm/imx: Fix memory leak in imx_pd_connector_get_modes Leo Ruan (1): gpu: ipu-v3: Fix dev_dbg frequency output Liu Ying (1): drm/imx: dw_hdmi-imx: Fix bailout in error cases of probe drivers/gpu/drm/imx/dw_hdmi-imx.c | 8 +++- drivers/gpu/drm/imx/imx-ldb.c | 2 ++ drivers/gpu/drm/imx/parallel-display.c | 4 +++- drivers/gpu/ipu-v3/ipu-di.c| 5 +++-- 4 files changed, 15 insertions(+), 4 deletions(-)
Re: [PATCH] drm/imx: make static read-only array channel_offsets const
On So, 2022-01-23 at 22:34 +, Colin Ian King wrote:
> The static array channel_offsets is read-only so it make sense to
> make
> it const.
>
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/ipu-v3/ipu-dc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-
> dc.c
> index ca96b235491a..b038a6d7307b 100644
> --- a/drivers/gpu/ipu-v3/ipu-dc.c
> +++ b/drivers/gpu/ipu-v3/ipu-dc.c
> @@ -344,8 +344,9 @@ int ipu_dc_init(struct ipu_soc *ipu, struct
> device *dev,
> unsigned long base, unsigned long template_base)
> {
> struct ipu_dc_priv *priv;
> - static int channel_offsets[] = { 0, 0x1c, 0x38, 0x54, 0x58,
> 0x5c,
> - 0x78, 0, 0x94, 0xb4};
> + static const int channel_offsets[] = {
> + 0, 0x1c, 0x38, 0x54, 0x58, 0x5c, 0x78, 0, 0x94, 0xb4
> + };
> int i;
>
> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
Thank you, applied to imx-drm/next.
regards
Philipp
Re: [PATCH] gpu: ipu-v3: Fix dev_dbg frequency output
On Mo, 2022-02-07 at 16:14 +0100, Mark Jonas wrote:
> From: Leo Ruan
>
> This commit corrects the printing of the IPU clock error percentage
> if
> it is between -0.1% to -0.9%. For example, if the pixel clock
> requested
> is 27.2 MHz but only 27.0 MHz can be achieved the deviation is -0.8%.
> But the fixed point math had a flaw and calculated error of 0.2%.
>
> Before:
> Clocks: IPU 27000Hz DI 24716667Hz Needed 2720Hz
> IPU clock can give 2700 with divider 10, error 0.2%
> Want 2720Hz IPU 27000Hz DI 24716667Hz using IPU, 2700Hz
>
> After:
> Clocks: IPU 27000Hz DI 24716667Hz Needed 2720Hz
> IPU clock can give 2700 with divider 10, error -0.8%
> Want 2720Hz IPU 27000Hz DI 24716667Hz using IPU, 2700Hz
>
> Signed-off-by: Leo Ruan
> Signed-off-by: Mark Jonas
> ---
> drivers/gpu/ipu-v3/ipu-di.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-
> di.c
> index b4a31d506fcc..74eca68891ad 100644
> --- a/drivers/gpu/ipu-v3/ipu-di.c
> +++ b/drivers/gpu/ipu-v3/ipu-di.c
> @@ -451,8 +451,9 @@ static void ipu_di_config_clock(struct ipu_di
> *di,
>
> error = rate / (sig->mode.pixelclock / 1000);
>
> - dev_dbg(di->ipu->dev, " IPU clock can give %lu with
> divider %u, error %d.%u%%\n",
> - rate, div, (signed)(error - 1000) / 10, error
> % 10);
> + dev_dbg(di->ipu->dev, " IPU clock can give %lu with
> divider %u, error %c%d.%d%%\n",
> + rate, div, error < 1000 ? '-' : '+',
> + abs(error - 1000) / 10, abs(error - 1000) %
> 10);
>
> /* Allow a 1% error */
> if (error < 1010 && error >= 990) {
Thank you, applied to imx-drm/fixes.
regards
Philipp
Re: [PATCH] drm/imx: dw_hdmi-imx: Fix bailout in error cases of probe
On Fr, 2022-01-28 at 17:19 +0800, Liu Ying wrote:
> In dw_hdmi_imx_probe(), if error happens after dw_hdmi_probe()
> returns
> successfully, dw_hdmi_remove() should be called where necessary as
> bailout.
>
> Fixes: c805ec7eb210 ("drm/imx: dw_hdmi-imx: move initialization into
> probe")
> Cc: Philipp Zabel
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Shawn Guo
> Cc: Sascha Hauer
> Cc: Pengutronix Kernel Team
> Cc: Fabio Estevam
> Cc: NXP Linux Team
> Signed-off-by: Liu Ying
> ---
> drivers/gpu/drm/imx/dw_hdmi-imx.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index 87428fb23d9f..a2277a0d6d06 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -222,6 +222,7 @@ static int dw_hdmi_imx_probe(struct
> platform_device *pdev)
> struct device_node *np = pdev->dev.of_node;
> const struct of_device_id *match =
> of_match_node(dw_hdmi_imx_dt_ids, np);
> struct imx_hdmi *hdmi;
> + int ret;
>
> hdmi = devm_kzalloc(>dev, sizeof(*hdmi), GFP_KERNEL);
> if (!hdmi)
> @@ -243,10 +244,15 @@ static int dw_hdmi_imx_probe(struct
> platform_device *pdev)
> hdmi->bridge = of_drm_find_bridge(np);
> if (!hdmi->bridge) {
> dev_err(hdmi->dev, "Unable to find bridge\n");
> + dw_hdmi_remove(hdmi->hdmi);
> return -ENODEV;
> }
>
> - return component_add(>dev, _hdmi_imx_ops);
> + ret = component_add(>dev, _hdmi_imx_ops);
> + if (ret)
> + dw_hdmi_remove(hdmi->hdmi);
> +
> + return ret;
> }
>
> static int dw_hdmi_imx_remove(struct platform_device *pdev)
Thank you, applied to imx-drm/fixes.
regards
Philipp
Re: [PATCH] drm/imx: imx-ldb: Check for null pointer after calling kmemdup
On Mi, 2022-01-05 at 15:47 +0800, Jiasheng Jiang wrote:
> As the possible failure of the allocation, kmemdup() may return NULL
> pointer.
> Therefore, it should be better to check the return value of kmemdup()
> and return error if fails.
>
> Fixes: dc80d7038883 ("drm/imx-ldb: Add support to drm-bridge")
> Signed-off-by: Jiasheng Jiang
> ---
> drivers/gpu/drm/imx/imx-ldb.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-
> ldb.c
> index 53132ddf9587..f9880a779678 100644
> --- a/drivers/gpu/drm/imx/imx-ldb.c
> +++ b/drivers/gpu/drm/imx/imx-ldb.c
> @@ -574,6 +574,8 @@ static int imx_ldb_panel_ddc(struct device *dev,
> edidp = of_get_property(child, "edid", _len);
> if (edidp) {
> channel->edid = kmemdup(edidp, edid_len,
> GFP_KERNEL);
> + if (!channel->edid)
> + return -ENOMEM;
> } else if (!channel->panel) {
> /* fallback to display-timings node */
> ret = of_get_drm_display_mode(child,
Thank you, applied to imx-drm/fixes.
regards
Philipp
Re: [PATCH v2] drm/imx: Fix memory leak in imx_pd_connector_get_modes
On Sa, 2022-01-08 at 17:52 +0100, José Expósito wrote:
> Avoid leaking the display mode variable if of_get_drm_display_mode
> fails.
>
> Fixes: 76ecd9c9fb24 ("drm/imx: parallel-display: check return code
> from of_get_drm_display_mode()")
> Addresses-Coverity-ID: 1443943 ("Resource leak")
> Signed-off-by: José Expósito
>
> ---
>
> v2: Improve commit message
> ---
> drivers/gpu/drm/imx/parallel-display.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/parallel-display.c
> b/drivers/gpu/drm/imx/parallel-display.c
> index a8aba0141ce7..3bf8e0a4803a 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -75,8 +75,10 @@ static int imx_pd_connector_get_modes(struct
> drm_connector *connector)
> ret = of_get_drm_display_mode(np, >mode,
> >bus_flags,
> OF_USE_NATIVE_MODE);
> - if (ret)
> + if (ret) {
> + drm_mode_destroy(connector->dev, mode);
> return ret;
> + }
>
> drm_mode_copy(mode, >mode);
> mode->type |= DRM_MODE_TYPE_DRIVER |
> DRM_MODE_TYPE_PREFERRED;
Thank you, applied to imx-drm/fixes.
regards
Philipp
Re: [PATCH 3/4] drm/etnaviv: move flush_seq increment into etnaviv_iommu_map/unmap
On Mi, 2022-03-23 at 17:08 +0100, Lucas Stach wrote: The flush sequence is a marker that the page tables have been changed and any affected TLBs need to be flushed. Move the flush_seq increment a little further down the call stack to place it next to the actual page table manipulation. Not functional change. Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 2/4] drm/etnaviv: move MMU context ref/unref into map/unmap_gem
On Mi, 2022-03-23 at 17:08 +0100, Lucas Stach wrote: > This makes it a little more clear that the mapping holds a reference > to the context once the buffer has been successfully mapped into that > context and simplifies the error handling a bit. > > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 1/4] drm/etnaviv: check for reaped mapping in etnaviv_iommu_unmap_gem
On Mi, 2022-03-23 at 17:08 +0100, Lucas Stach wrote: > When the mapping is already reaped the unmap must be a no-op, as we > would otherwise try to remove the mapping twice, corrupting the > involved > data structures. > > Cc: sta...@vger.kernel.org # 5.4 > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 09/22] drm/imx: Use drm_mode_duplicate()
On Fri, 2022-02-18 at 12:03 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Replace the hand rolled drm_mode_duplicate() with the
> real thing.
>
> @is_dup@
> @@
> drm_mode_duplicate(...)
> { ... }
>
> @depends on !is_dup@
> expression dev, oldmode;
> identifier newmode;
> @@
> - newmode = drm_mode_create(dev);
> + newmode = drm_mode_duplicate(dev, oldmode);
> ...
> - drm_mode_copy(newmode, oldmode);
>
> Cc: Philipp Zabel
> Signed-off-by: Ville Syrjälä
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH] gpu: ipu-v3: Fix dev_dbg frequency output
Hi Mark,
On Mon, 2022-02-07 at 16:14 +0100, Mark Jonas wrote:
> From: Leo Ruan
>
> This commit corrects the printing of the IPU clock error percentage if
> it is between -0.1% to -0.9%. For example, if the pixel clock requested
> is 27.2 MHz but only 27.0 MHz can be achieved the deviation is -0.8%.
> But the fixed point math had a flaw and calculated error of 0.2%.
>
> Before:
> Clocks: IPU 27000Hz DI 24716667Hz Needed 2720Hz
> IPU clock can give 2700 with divider 10, error 0.2%
> Want 2720Hz IPU 27000Hz DI 24716667Hz using IPU, 2700Hz
>
> After:
> Clocks: IPU 27000Hz DI 24716667Hz Needed 2720Hz
> IPU clock can give 2700 with divider 10, error -0.8%
> Want 2720Hz IPU 27000Hz DI 24716667Hz using IPU, 2700Hz
>
> Signed-off-by: Leo Ruan
> Signed-off-by: Mark Jonas
> ---
> drivers/gpu/ipu-v3/ipu-di.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
> index b4a31d506fcc..74eca68891ad 100644
> --- a/drivers/gpu/ipu-v3/ipu-di.c
> +++ b/drivers/gpu/ipu-v3/ipu-di.c
> @@ -451,8 +451,9 @@ static void ipu_di_config_clock(struct ipu_di *di,
>
> error = rate / (sig->mode.pixelclock / 1000);
>
> - dev_dbg(di->ipu->dev, " IPU clock can give %lu with divider
> %u, error %d.%u%%\n",
> - rate, div, (signed)(error - 1000) / 10, error % 10);
> + dev_dbg(di->ipu->dev, " IPU clock can give %lu with divider
> %u, error %c%d.%d%%\n",
> + rate, div, error < 1000 ? '-' : '+',
> + abs(error - 1000) / 10, abs(error - 1000) % 10);
>
> /* Allow a 1% error */
> if (error < 1010 && error >= 990) {
Rounding (always down) is still a bit unintuitive, but this certainly
improves things.
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH] drm/edid: improve non-desktop quirk logging
On Tue, Dec 28, 2021 at 11:10 AM Jani Nikula wrote: > > Improve non-desktop quirk logging if the EDID indicates non-desktop. If > both are set, note about redundant quirk. If there's no quirk but the > EDID indicates non-desktop, don't log non-desktop is set to 0. > > Cc: Philipp Zabel > Signed-off-by: Jani Nikula Thank you, Reviewed-by: Philipp Zabel Tested-by: Philipp Zabel regards Philipp
[PATCH v2 2/2] drm/edid: remove non_desktop quirk for HPN-3515 and LEN-B800.
Now that there is support for the Microsoft VSDB for HMDs, remove the
non-desktop quirk for two devices that are verified to contain it in
their EDID: HPN-3515 and LEN-B800.
Presumably most of the other Windows Mixed Reality headsets contain it
as well, but there are ACR-7FCE and SEC-5194 devices without it.
Tested with LEN-B800.
Signed-off-by: Philipp Zabel
Reviewed-by: Jani Nikula
---
Changes since v1 [1]:
- Quirk removal split out of patch 1.
[1] https://lore.kernel.org/all/20211213184706.5776-1-philipp.za...@gmail.com/
---
drivers/gpu/drm/drm_edid.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 271b5616cfaf..c23ad8f0b3e9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -214,9 +214,7 @@ static const struct edid_quirk {
/* Windows Mixed Reality Headsets */
EDID_QUIRK('A', 'C', 'R', 0x7fce, EDID_QUIRK_NON_DESKTOP),
- EDID_QUIRK('H', 'P', 'N', 0x3515, EDID_QUIRK_NON_DESKTOP),
EDID_QUIRK('L', 'E', 'N', 0x0408, EDID_QUIRK_NON_DESKTOP),
- EDID_QUIRK('L', 'E', 'N', 0xb800, EDID_QUIRK_NON_DESKTOP),
EDID_QUIRK('F', 'U', 'J', 0x1970, EDID_QUIRK_NON_DESKTOP),
EDID_QUIRK('D', 'E', 'L', 0x7fce, EDID_QUIRK_NON_DESKTOP),
EDID_QUIRK('S', 'E', 'C', 0x144a, EDID_QUIRK_NON_DESKTOP),
--
2.34.1
[PATCH v2 1/2] drm/edid: support Microsoft extension for HMDs and specialized monitors
Add minimal support for parsing VSDBs documented in Microsoft's "EDID
extension for head-mounted and specialized monitors" [1]. The version
field and the desktop usage flag can be used to set the non_desktop
connector property.
[1]
https://docs.microsoft.com/en-us/windows-hardware/drivers/display/specialized-monitors-edid-extension
Tested with HPN-36C1 and LEN-B800.
Signed-off-by: Philipp Zabel
Reviewed-by: Jani Nikula
---
Changes since v1 [2]:
- Split out quirk removal into a separate patch.
- Set non_desktop to true instead of 1.
[2] https://lore.kernel.org/all/20211213184706.5776-1-philipp.za...@gmail.com/
---
drivers/gpu/drm/drm_edid.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 12893e7be89b..271b5616cfaf 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -93,6 +93,8 @@ static int oui(u8 first, u8 second, u8 third)
/* Non desktop display (i.e. HMD) */
#define EDID_QUIRK_NON_DESKTOP (1 << 12)
+#define MICROSOFT_IEEE_OUI 0xca125c
+
struct detailed_mode_closure {
struct drm_connector *connector;
struct edid *edid;
@@ -4222,6 +4224,17 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
return oui(db[3], db[2], db[1]) == HDMI_FORUM_IEEE_OUI;
}
+static bool cea_db_is_microsoft_vsdb(const u8 *db)
+{
+ if (cea_db_tag(db) != VENDOR_BLOCK)
+ return false;
+
+ if (cea_db_payload_len(db) != 21)
+ return false;
+
+ return oui(db[3], db[2], db[1]) == MICROSOFT_IEEE_OUI;
+}
+
static bool cea_db_is_vcdb(const u8 *db)
{
if (cea_db_tag(db) != USE_EXTENDED_TAG)
@@ -5149,6 +5162,25 @@ drm_parse_hdmi_vsdb_video(struct drm_connector
*connector, const u8 *db)
drm_parse_hdmi_deep_color_info(connector, db);
}
+/*
+ * See EDID extension for head-mounted and specialized monitors, specified at:
+ *
https://docs.microsoft.com/en-us/windows-hardware/drivers/display/specialized-monitors-edid-extension
+ */
+static void drm_parse_microsoft_vsdb(struct drm_connector *connector,
+const u8 *db)
+{
+ struct drm_display_info *info = >display_info;
+ u8 version = db[4];
+ bool desktop_usage = db[5] & BIT(6);
+
+ /* Version 1 and 2 for HMDs, version 3 flags desktop usage explicitly */
+ if (version == 1 || version == 2 || (version == 3 && !desktop_usage))
+ info->non_desktop = true;
+
+ drm_dbg_kms(connector->dev, "HMD or specialized display VSDB version
%u: 0x%02x\n",
+ version, db[5]);
+}
+
static void drm_parse_cea_ext(struct drm_connector *connector,
const struct edid *edid)
{
@@ -5179,6 +5211,8 @@ static void drm_parse_cea_ext(struct drm_connector
*connector,
drm_parse_hdmi_vsdb_video(connector, db);
if (cea_db_is_hdmi_forum_vsdb(db))
drm_parse_hdmi_forum_vsdb(connector, db);
+ if (cea_db_is_microsoft_vsdb(db))
+ drm_parse_microsoft_vsdb(connector, db);
if (cea_db_is_y420cmdb(db))
drm_parse_y420cmdb_bitmap(connector, db);
if (cea_db_is_vcdb(db))
base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07
--
2.34.1
Re: [RFC 22/28] drm: rcar-du: Add RZ/G2L DSI driver
Hi Biju,
On Wed, 2022-01-12 at 17:46 +, Biju Das wrote:
[...]
> +static int rzg2l_mipi_dsi_probe(struct platform_device *pdev)
> +{
[...]
> + dsi->rstc = devm_reset_control_get(dsi->dev, "rst");
[...]
> + dsi->arstc = devm_reset_control_get(dsi->dev, "arst");
[...]
> + dsi->prstc = devm_reset_control_get(dsi->dev, "prst");
Please use devm_reset_control_get_exclusive() instead.
regards
Philipp
[PATCH] drm/edid: support Microsoft extension for HMDs and specialized monitors
Add minimal support for parsing VSDBs documented in Microsoft's "EDID
extension for head-mounted and specialized monitors" [1]. The version
field and the desktop usage flag can be used to set the non_desktop
connector property.
Remove the non-desktop quirk for devices that are verified to contain
the VSDB: HPN-3515 and LEN-B800. Presumably most of the other Windows
Mixed Reality headsets contain it as well, but there are ACR-7FCE and
SEC-5194 devices without it.
[1]
https://docs.microsoft.com/en-us/windows-hardware/drivers/display/specialized-monitors-edid-extension
Tested with HPN-36C1 and LEN-B800.
Signed-off-by: Philipp Zabel
---
drivers/gpu/drm/drm_edid.c | 36 ++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 12893e7be89b..baea65dfff7d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -93,6 +93,8 @@ static int oui(u8 first, u8 second, u8 third)
/* Non desktop display (i.e. HMD) */
#define EDID_QUIRK_NON_DESKTOP (1 << 12)
+#define MICROSOFT_IEEE_OUI 0xca125c
+
struct detailed_mode_closure {
struct drm_connector *connector;
struct edid *edid;
@@ -212,9 +214,7 @@ static const struct edid_quirk {
/* Windows Mixed Reality Headsets */
EDID_QUIRK('A', 'C', 'R', 0x7fce, EDID_QUIRK_NON_DESKTOP),
- EDID_QUIRK('H', 'P', 'N', 0x3515, EDID_QUIRK_NON_DESKTOP),
EDID_QUIRK('L', 'E', 'N', 0x0408, EDID_QUIRK_NON_DESKTOP),
- EDID_QUIRK('L', 'E', 'N', 0xb800, EDID_QUIRK_NON_DESKTOP),
EDID_QUIRK('F', 'U', 'J', 0x1970, EDID_QUIRK_NON_DESKTOP),
EDID_QUIRK('D', 'E', 'L', 0x7fce, EDID_QUIRK_NON_DESKTOP),
EDID_QUIRK('S', 'E', 'C', 0x144a, EDID_QUIRK_NON_DESKTOP),
@@ -4222,6 +4222,17 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
return oui(db[3], db[2], db[1]) == HDMI_FORUM_IEEE_OUI;
}
+static bool cea_db_is_microsoft_vsdb(const u8 *db)
+{
+ if (cea_db_tag(db) != VENDOR_BLOCK)
+ return false;
+
+ if (cea_db_payload_len(db) != 21)
+ return false;
+
+ return oui(db[3], db[2], db[1]) == MICROSOFT_IEEE_OUI;
+}
+
static bool cea_db_is_vcdb(const u8 *db)
{
if (cea_db_tag(db) != USE_EXTENDED_TAG)
@@ -5149,6 +5160,25 @@ drm_parse_hdmi_vsdb_video(struct drm_connector
*connector, const u8 *db)
drm_parse_hdmi_deep_color_info(connector, db);
}
+/*
+ * See EDID extension for head-mounted and specialized monitors, specified at:
+ *
https://docs.microsoft.com/en-us/windows-hardware/drivers/display/specialized-monitors-edid-extension
+ */
+static void drm_parse_microsoft_vsdb(struct drm_connector *connector,
+const u8 *db)
+{
+ struct drm_display_info *info = >display_info;
+ u8 version = db[4];
+ bool desktop_usage = db[5] & BIT(6);
+
+ /* Version 1 and 2 for HMDs, version 3 flags desktop usage explicitly */
+ if (version == 1 || version == 2 || (version == 3 && !desktop_usage))
+ info->non_desktop = 1;
+
+ drm_dbg_kms(connector->dev, "HMD or specialized display VSDB version
%u: 0x%02x\n",
+ version, db[5]);
+}
+
static void drm_parse_cea_ext(struct drm_connector *connector,
const struct edid *edid)
{
@@ -5179,6 +5209,8 @@ static void drm_parse_cea_ext(struct drm_connector
*connector,
drm_parse_hdmi_vsdb_video(connector, db);
if (cea_db_is_hdmi_forum_vsdb(db))
drm_parse_hdmi_forum_vsdb(connector, db);
+ if (cea_db_is_microsoft_vsdb(db))
+ drm_parse_microsoft_vsdb(connector, db);
if (cea_db_is_y420cmdb(db))
drm_parse_y420cmdb_bitmap(connector, db);
if (cea_db_is_vcdb(db))
base-commit: fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf
--
2.34.1
Re: [PATCH v9 16/22] drm/mediatek: add ETHDR support for MT8195
Hi Nancy,
On Tue, 2021-11-30 at 11:35 +0800, Nancy.Lin wrote:
[...]
> +void mtk_ethdr_stop(struct device *dev)
> +{
> + struct mtk_ethdr *priv = dev_get_drvdata(dev);
> + struct mtk_ethdr_comp *mixer = >ethdr_comp[ETHDR_MIXER];
> +
> + writel(0, mixer->regs + MIX_EN);
> + writel(1, mixer->regs + MIX_RST);
> + reset_control_reset(devm_reset_control_array_get(dev, true, true));
Are these reset lines really shared with other hardware units, and more
specifically, are there other drivers that also try to control them?
If so, please use devm_reset_control_array_get_optional_shared().
Otherwise use devm_reset_control_array_get_optional_exclusive() instead.
If you really need to share the reset line with other hardware and have
to trigger it more than once, the only use case supported by the reset
framework is to use reset_control_reset() before starting the hardware
for each user and then reset_control_rearm() after stopping it for all
users (see [1]). That would both stop the reset from being triggered
while the hardware is active (before last _rearm) and allow another
reset once after all hardware is stopped.
[1] https://www.kernel.org/doc/html/latest/driver-api/reset.htm#triggering
Also, request the reset control once in the probe function, not every
time ETHDR is stopped. Otherwise you'll end up leaking devres memory
regards
Philipp
