Re: [PATCH 1/9] drm/amdgpu: generally allow over-commit during BO allocation
On 2022-11-25 05:21, Christian König wrote:
We already fallback to a dummy BO with no backing store when we
allocate GDS,GWS and OA resources and to GTT when we allocate VRAM.
Drop all those workarounds and generalize this for GTT as well. This
fixes ENOMEM issues with runaway applications which try to allocate/free
GTT in a loop and are otherwise only limited by the CPU speed.
The CS will wait for the cleanup of freed up BOs to satisfy the
various domain specific limits and so effectively throttle those
buggy applications down to a sane allocation behavior again.
Signed-off-by: Christian König
This patch causes some regressions in KFDTest. KFDMemoryTest.MMBench
sees a huge VRAM allocation slow-down. And
KFDMemoryTest.LargestVramBufferTest can only allocate half the available
memory.
This seems to be caused by initially validating VRAM BOs in the CPU
domain, which allocates a ttm_tt. A subsequent validation in the VRAM
domain involves a copy from GTT to VRAM.
After that, freeing of BOs can get delayed by the ghost object of a
previous migration, which delays calling release notifiers and causes
problems for KFDs available memory accounting.
I experimented with a workaround that validates BOs immediately after
allocation, but that only moves around the delays and doesn't solve the
problem. During those experiments I may also have stumbled over a bug in
ttm_buffer_object_transfer: It calls ttm_bo_set_bulk_move before
initializing and locking fbo->base.base._resv. This results in a flood
of warnings because ttm_bo_set_bulk_move expects the reservation to be
locked.
Right now I'd like to remove the bp.domain = initial_domain |
AMDGPU_GEM_DOMAIN_CPU change in amdgpu_gem_object_create to fix this.
Regards,
Felix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 16 +++-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 +-
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index a0780a4e3e61..62e98f1ad770 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -113,7 +113,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev,
unsigned long size,
bp.resv = resv;
bp.preferred_domain = initial_domain;
bp.flags = flags;
- bp.domain = initial_domain;
+ bp.domain = initial_domain | AMDGPU_GEM_DOMAIN_CPU;
bp.bo_ptr_size = sizeof(struct amdgpu_bo);
r = amdgpu_bo_create_user(adev, , );
@@ -332,20 +332,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void
*data,
}
initial_domain = (u32)(0x & args->in.domains);
-retry:
r = amdgpu_gem_object_create(adev, size, args->in.alignment,
-initial_domain,
-flags, ttm_bo_type_device, resv, );
+initial_domain, flags, ttm_bo_type_device,
+resv, );
if (r && r != -ERESTARTSYS) {
- if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
- flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
- goto retry;
- }
-
- if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
- initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
- goto retry;
- }
DRM_DEBUG("Failed to allocate GEM object (%llu, %d, %llu,
%d)\n",
size, initial_domain, args->in.alignment, r);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 974e85d8b6cc..919bbea2e3ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -581,11 +581,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
- if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
- AMDGPU_GEM_DOMAIN_GDS))
- amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
- else
- amdgpu_bo_placement_from_domain(bo, bp->domain);
+ amdgpu_bo_placement_from_domain(bo, bp->domain);
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 1;
Re: [PATCH] dma-buf: fix dma_buf_export init order v2
Hi Christian,
On Fri, 9 Dec 2022 at 12:45, Christian König
wrote:
>
> The init order and resulting error handling in dma_buf_export
> was pretty messy.
>
> Subordinate objects like the file and the sysfs kernel objects
> were initializing and wiring itself up with the object in the
> wrong order resulting not only in complicating and partially
> incorrect error handling, but also in publishing only halve
> initialized DMA-buf objects.
>
> Clean this up thoughtfully by allocating the file independent
> of the DMA-buf object. Then allocate and initialize the DMA-buf
> object itself, before publishing it through sysfs. If everything
> works as expected the file is then connected with the DMA-buf
> object and publish it through debugfs.
>
> Also adds the missing dma_resv_fini() into the error handling.
>
> v2: add some missing changes to dma_bug_getfile() and a missing NULL
> check in dma_buf_file_release()
>
> Signed-off-by: Christian König
Thank you for this nice cleanup.
Acked-by: Sumit Semwal
Best,
Sumit.
> ---
> drivers/dma-buf/dma-buf-sysfs-stats.c | 7 +--
> drivers/dma-buf/dma-buf-sysfs-stats.h | 4 +-
> drivers/dma-buf/dma-buf.c | 84 +--
> 3 files changed, 43 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c
> b/drivers/dma-buf/dma-buf-sysfs-stats.c
> index 2bba0babcb62..4b680e10c15a 100644
> --- a/drivers/dma-buf/dma-buf-sysfs-stats.c
> +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
> @@ -168,14 +168,11 @@ void dma_buf_uninit_sysfs_statistics(void)
> kset_unregister(dma_buf_stats_kset);
> }
>
> -int dma_buf_stats_setup(struct dma_buf *dmabuf)
> +int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
> {
> struct dma_buf_sysfs_entry *sysfs_entry;
> int ret;
>
> - if (!dmabuf || !dmabuf->file)
> - return -EINVAL;
> -
> if (!dmabuf->exp_name) {
> pr_err("exporter name must not be empty if stats needed\n");
> return -EINVAL;
> @@ -192,7 +189,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
>
> /* create the directory for buffer stats */
> ret = kobject_init_and_add(_entry->kobj, _buf_ktype, NULL,
> - "%lu", file_inode(dmabuf->file)->i_ino);
> + "%lu", file_inode(file)->i_ino);
> if (ret)
> goto err_sysfs_dmabuf;
>
> diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h
> b/drivers/dma-buf/dma-buf-sysfs-stats.h
> index a49c6e2650cc..7a8a995b75ba 100644
> --- a/drivers/dma-buf/dma-buf-sysfs-stats.h
> +++ b/drivers/dma-buf/dma-buf-sysfs-stats.h
> @@ -13,7 +13,7 @@
> int dma_buf_init_sysfs_statistics(void);
> void dma_buf_uninit_sysfs_statistics(void);
>
> -int dma_buf_stats_setup(struct dma_buf *dmabuf);
> +int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file);
>
> void dma_buf_stats_teardown(struct dma_buf *dmabuf);
> #else
> @@ -25,7 +25,7 @@ static inline int dma_buf_init_sysfs_statistics(void)
>
> static inline void dma_buf_uninit_sysfs_statistics(void) {}
>
> -static inline int dma_buf_stats_setup(struct dma_buf *dmabuf)
> +static inline int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file
> *file)
> {
> return 0;
> }
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e6f36c014c4c..eb6b59363c4f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -95,10 +95,11 @@ static int dma_buf_file_release(struct inode *inode,
> struct file *file)
> return -EINVAL;
>
> dmabuf = file->private_data;
> -
> - mutex_lock(_list.lock);
> - list_del(>list_node);
> - mutex_unlock(_list.lock);
> + if (dmabuf) {
> + mutex_lock(_list.lock);
> + list_del(>list_node);
> + mutex_unlock(_list.lock);
> + }
>
> return 0;
> }
> @@ -523,17 +524,17 @@ static inline int is_dma_buf_file(struct file *file)
> return file->f_op == _buf_fops;
> }
>
> -static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
> +static struct file *dma_buf_getfile(size_t size, int flags)
> {
> static atomic64_t dmabuf_inode = ATOMIC64_INIT(0);
> - struct file *file;
> struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
> + struct file *file;
>
> if (IS_ERR(inode))
> return ERR_CAST(inode);
>
> - inode->i_size = dmabuf->size;
> - inode_set_bytes(inode, dmabuf->size);
> + inode->i_size = size;
> + inode_set_bytes(inode, size);
>
> /*
> * The ->i_ino acquired from get_next_ino() is not unique thus
> @@ -547,8 +548,6 @@ static struct file *dma_buf_getfile(struct dma_buf
> *dmabuf, int flags)
> flags, _buf_fops);
> if (IS_ERR(file))
> goto err_alloc_file;
> - file->private_data = dmabuf;
> -
Re: [PATCH] [next] drm/radeon: Replace 1-element arrays with flexible-array members
Applied. Thanks!
Alex
On Fri, Dec 9, 2022 at 3:24 AM Paulo Miguel Almeida
wrote:
>
> One-element arrays are deprecated, and we are replacing them with
> flexible array members instead. So, replace one-element array with
> flexible-array member in structs _ATOM_DISPLAY_OBJECT_PATH,
> _ATOM_DISPLAY_OBJECT_PATH_TABLE, _ATOM_OBJECT_TABLE, GOP_VBIOS_CONTENT
> _ATOM_GPIO_VOLTAGE_OBJECT_V3 and refactor the rest of the code accordingly.
>
> It's worth mentioning that doing a build before/after this patch
> results in no binary output differences.
>
> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> routines on memcpy() and help us make progress towards globally
> enabling -fstrict-flex-arrays=3 [1].
>
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://github.com/KSPP/linux/issues/239
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
>
> Signed-off-by: Paulo Miguel Almeida
> ---
> Notes for the maintainer:
>
> - These are all fake-flexible arrays with references in source code for
> the radeon driver. Given the way they are used, no change to *.c files
> were required.
> ---
> drivers/gpu/drm/radeon/atombios.h | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios.h
> b/drivers/gpu/drm/radeon/atombios.h
> index 235e59b547a1..8a6621f1e82c 100644
> --- a/drivers/gpu/drm/radeon/atombios.h
> +++ b/drivers/gpu/drm/radeon/atombios.h
> @@ -4020,7 +4020,7 @@ typedef struct _ATOM_DISPLAY_OBJECT_PATH
>USHORTusSize;//the size of
> ATOM_DISPLAY_OBJECT_PATH
>USHORTusConnObjectId;//Connector
> Object ID
>USHORTusGPUObjectId; //GPU ID
> - USHORTusGraphicObjIds[1]; //1st Encoder
> Obj source from GPU to last Graphic Obj destinate to connector.
> + USHORTusGraphicObjIds[]; //1st Encoder Obj
> source from GPU to last Graphic Obj destinate to connector.
> }ATOM_DISPLAY_OBJECT_PATH;
>
> typedef struct _ATOM_DISPLAY_EXTERNAL_OBJECT_PATH
> @@ -4037,7 +4037,7 @@ typedef struct _ATOM_DISPLAY_OBJECT_PATH_TABLE
>UCHAR ucNumOfDispPath;
>UCHAR ucVersion;
>UCHAR ucPadding[2];
> - ATOM_DISPLAY_OBJECT_PATHasDispPath[1];
> + ATOM_DISPLAY_OBJECT_PATHasDispPath[];
> }ATOM_DISPLAY_OBJECT_PATH_TABLE;
>
>
> @@ -4053,7 +4053,7 @@ typedef struct _ATOM_OBJECT_TABLE
> //Above 4 object table
> {
>UCHAR ucNumberOfObjects;
>UCHAR ucPadding[3];
> - ATOM_OBJECT asObjects[1];
> + ATOM_OBJECT asObjects[];
> }ATOM_OBJECT_TABLE;
>
> typedef struct _ATOM_SRC_DST_TABLE_FOR_ONE_OBJECT
> //usSrcDstTableOffset pointing to this structure
> @@ -4615,7 +4615,7 @@ typedef struct _ATOM_GPIO_VOLTAGE_OBJECT_V3
> UCHARucPhaseDelay;// phase delay in unit of micro
> second
> UCHARucReserved;
> ULONGulGpioMaskVal; // GPIO Mask value
> - VOLTAGE_LUT_ENTRY_V2 asVolGpioLut[1];
> + VOLTAGE_LUT_ENTRY_V2 asVolGpioLut[];
> }ATOM_GPIO_VOLTAGE_OBJECT_V3;
>
> typedef struct _ATOM_LEAKAGE_VOLTAGE_OBJECT_V3
> @@ -7964,7 +7964,7 @@ typedef struct {
>
> typedef struct {
>VFCT_IMAGE_HEADERVbiosHeader;
> - UCHARVbiosContent[1];
> + UCHARVbiosContent[];
> }GOP_VBIOS_CONTENT;
>
> typedef struct {
> --
> 2.38.1
>
Re: [PATCH] drm: Drop ARCH_MULTIPLATFORM from dependencies
On Fri, Dec 9, 2022, at 23:05, 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
Makes sense,
Acked-by: Arnd Bergmann
> diff --git a/drivers/gpu/drm/omapdrm/Kconfig
> b/drivers/gpu/drm/omapdrm/Kconfig
> index 455e1a91f0e5..76ded1568bd0 100644
> --- a/drivers/gpu/drm/omapdrm/Kconfig
> +++ b/drivers/gpu/drm/omapdrm/Kconfig
> @@ -2,7 +2,7 @@
> config DRM_OMAP
> tristate "OMAP DRM"
> depends on DRM && OF
> - depends on ARCH_OMAP2PLUS || ARCH_MULTIPLATFORM
> + depends on ARCH_OMAP2PLUS
> select DRM_KMS_HELPER
> select VIDEOMODE_HELPERS
> select HDMI
Since the original purpose of the ||ARCH_MULTIPLATFORM was to allow
building the driver on more targets, I wonder if we should instead
make that ||COMPILE_TEST, which would also allow building it on
x86 and others.
> diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
> index f2a880c48485..3c7a5feff8de 100644
> --- a/drivers/gpu/drm/sti/Kconfig
> +++ b/drivers/gpu/drm/sti/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> config DRM_STI
> tristate "DRM Support for STMicroelectronics SoC stiH4xx Series"
> - depends on OF && DRM && (ARCH_STI || ARCH_MULTIPLATFORM)
> + depends on OF && DRM && ARCH_STI
> select RESET_CONTROLLER
> select DRM_KMS_HELPER
> select DRM_GEM_DMA_HELPER
> diff --git a/drivers/gpu/drm/stm/Kconfig b/drivers/gpu/drm/stm/Kconfig
> index ded72f879482..fa49cde43bb2 100644
> --- a/drivers/gpu/drm/stm/Kconfig
> +++ b/drivers/gpu/drm/stm/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> config DRM_STM
> tristate "DRM Support for STMicroelectronics SoC Series"
> - depends on DRM && (ARCH_STM32 || ARCH_MULTIPLATFORM)
> + depends on DRM && ARCH_STM32
> select DRM_KMS_HELPER
> select DRM_GEM_DMA_HELPER
> select DRM_PANEL_BRIDGE
Same here.
arnd
[PATCH] drm: Drop ARCH_MULTIPLATFORM from dependencies
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/exynos/Kconfig | 2 +-
drivers/gpu/drm/imx/Kconfig | 2 +-
drivers/gpu/drm/omapdrm/Kconfig | 2 +-
drivers/gpu/drm/sti/Kconfig | 2 +-
drivers/gpu/drm/stm/Kconfig | 2 +-
drivers/gpu/ipu-v3/Kconfig | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 3d2f025d4fd4..4049fa4273ab 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -2,7 +2,7 @@
config DRM_EXYNOS
tristate "DRM Support for Samsung SoC Exynos Series"
depends on OF && DRM && COMMON_CLK
- depends on ARCH_S3C64XX || ARCH_S5PV210 || ARCH_EXYNOS ||
ARCH_MULTIPLATFORM || COMPILE_TEST
+ depends on ARCH_S3C64XX || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
depends on MMU
select DRM_DISPLAY_HELPER if DRM_EXYNOS_DP
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
index fd5b2471fdf0..77339497 100644
--- a/drivers/gpu/drm/imx/Kconfig
+++ b/drivers/gpu/drm/imx/Kconfig
@@ -4,7 +4,7 @@ config DRM_IMX
select DRM_KMS_HELPER
select VIDEOMODE_HELPERS
select DRM_GEM_DMA_HELPER
- depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM || COMPILE_TEST)
+ depends on DRM && (ARCH_MXC || COMPILE_TEST)
depends on IMX_IPUV3_CORE
help
enable i.MX graphics support
diff --git a/drivers/gpu/drm/omapdrm/Kconfig b/drivers/gpu/drm/omapdrm/Kconfig
index 455e1a91f0e5..76ded1568bd0 100644
--- a/drivers/gpu/drm/omapdrm/Kconfig
+++ b/drivers/gpu/drm/omapdrm/Kconfig
@@ -2,7 +2,7 @@
config DRM_OMAP
tristate "OMAP DRM"
depends on DRM && OF
- depends on ARCH_OMAP2PLUS || ARCH_MULTIPLATFORM
+ depends on ARCH_OMAP2PLUS
select DRM_KMS_HELPER
select VIDEOMODE_HELPERS
select HDMI
diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index f2a880c48485..3c7a5feff8de 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config DRM_STI
tristate "DRM Support for STMicroelectronics SoC stiH4xx Series"
- depends on OF && DRM && (ARCH_STI || ARCH_MULTIPLATFORM)
+ depends on OF && DRM && ARCH_STI
select RESET_CONTROLLER
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
diff --git a/drivers/gpu/drm/stm/Kconfig b/drivers/gpu/drm/stm/Kconfig
index ded72f879482..fa49cde43bb2 100644
--- a/drivers/gpu/drm/stm/Kconfig
+++ b/drivers/gpu/drm/stm/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config DRM_STM
tristate "DRM Support for STMicroelectronics SoC Series"
- depends on DRM && (ARCH_STM32 || ARCH_MULTIPLATFORM)
+ depends on DRM && ARCH_STM32
select DRM_KMS_HELPER
select DRM_GEM_DMA_HELPER
select DRM_PANEL_BRIDGE
diff --git a/drivers/gpu/ipu-v3/Kconfig b/drivers/gpu/ipu-v3/Kconfig
index 061fb990c120..7dece2a53c5c 100644
--- a/drivers/gpu/ipu-v3/Kconfig
+++ b/drivers/gpu/ipu-v3/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config IMX_IPUV3_CORE
tristate "IPUv3 core support"
- depends on SOC_IMX5 || SOC_IMX6Q || ARCH_MULTIPLATFORM || COMPILE_TEST
+ depends on SOC_IMX5 || SOC_IMX6Q || COMPILE_TEST
depends on DRM || !DRM # if DRM=m, this can't be 'y'
select BITREVERSE
select GENERIC_ALLOCATOR if DRM
base-commit: 0d1409e4ff08aa4a9a254d3f723410db32aa7552
--
2.38.1
[PATCH v2] drm/display: Add missing Adaptive Sync DPCD definitions
The missing DPCD defintions from DP2.0 spec is as follows: DOWNSPREAD_CTRL (107h): FIXED_VTOTAL_AS_SDP_EN_IN_PR_ACTIVE (bit 6) For sink devices that support Adaptive-Sync operation and Panel Replay DPRX_FEATURE_ENUMERATION_LIST_CONT_1 (2214h): ADAPTIVE_SYNC_SDP_SUPPORTED (bit 0) Bit to check sink device has Adaptive-Sync capability AS_SDP_FIRST_HALF_LINE_OR_3840_PIXEL_CYCLE_WINDOW_NOT_SUPPORTED (bit 1) A sink device that clears this bit will generate VSync pulse leading edge of the HDMI output on the line count at which Adaptive-Sync SDP is received as long as source device transmits Adaptive-Sync SDP either in first line or first 3840 pixel cycles of the line whichever occurs first. VSC_EXT_SDP_FRAMEWORK_VERSION_1_SUPPORTED (bit 4) Bit to check sink device has SDP framework version 1 capability --- include/drm/display/drm_dp.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 4d0abe4c7ea9..1bd6f9af0b46 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -603,6 +603,7 @@ #define DP_DOWNSPREAD_CTRL 0x107 # define DP_SPREAD_AMP_0_5 (1 << 4) +# define DP_FIXED_VTOTAL_AS_SDP_EN_IN_PR_ACTIVE (1 << 6) # define DP_MSA_TIMING_PAR_IGNORE_EN (1 << 7) /* eDP */ #define DP_MAIN_LINK_CHANNEL_CODING_SET0x108 @@ -1105,6 +1106,11 @@ # define DP_VSC_EXT_CEA_SDP_SUPPORTED (1 << 6) /* DP 1.4 */ # define DP_VSC_EXT_CEA_SDP_CHAINING_SUPPORTED (1 << 7) /* DP 1.4 */ +#define DP_DPRX_FEATURE_ENUMERATION_LIST_CONT_1 0x2214 /* 2.0 E11 */ +# define DP_ADAPTIVE_SYNC_SDP_SUPPORTED(1 << 0) +# define DP_AS_SDP_FIRST_HALF_LINE_OR_3840_PIXEL_CYCLE_WINDOW_NOT_SUPPORTED (1 << 1) +# define DP_VSC_EXT_SDP_FRAMEWORK_VERSION_1_SUPPORTED (1 << 4) + #define DP_128B132B_SUPPORTED_LINK_RATES 0x2215 /* 2.0 */ # define DP_UHBR10 (1 << 0) # define DP_UHBR20 (1 << 1) -- 2.20.1
Re: [PATCH] drm/display: Include missing DPCD definitions from DP2.0 spec
The commit subject is very generic. A better one might be "Add missing Adaptive Sync DPCD definitions" On 12/8/22 14:25, Sung Joon Kim wrote: > The missing DPCD defintions from DP2.0 spec is as follows: > > DOWNSPREAD_CTRL (107h): > ADAPTIVE_SYNC_SDP_EN (bit 6) > For sink devices that support Adaptive-Sync operation > and Panel Replay > > DPRX_FEATURE_ENUMERATION_LIST_CONT_1 (2214h): > ADAPTIVE_SYNC_SDP_SUPPORTED (bit 0) > Bit to check sink device has Adaptive-Sync capability > AS_SDP_FIRST_HALF_LINE_OR_3840_PIXEL_CYCLE_WINDOW_NOT_SUPPORTED (bit 1) > A sink device that clears this bit will generate VSync pulse > leading edge of the HDMI output on the line count at which > Adaptive-Sync SDP is received as long as source device transmits > Adaptive-Sync SDP either in first line or first 3840 pixel > cycles > of the line whichever occurs first. > VSC_EXT_SDP_FRAMEWORK_VERSION_1_SUPPORTED (bit 4) > Bit to check sink device has SDP framework version 1 capability > --- > include/drm/display/drm_dp.h | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 4d0abe4c7ea9..4f33b6aeb91e 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -603,6 +603,7 @@ > > #define DP_DOWNSPREAD_CTRL 0x107 > # define DP_SPREAD_AMP_0_5 (1 << 4) > +# define DP_ADAPTIVE_SYNC_SDP_EN(1 << 6) We tend to stick with the naming from the spec to allow people to find things more easily. Please change this to: DP_FIXED_VTOTAL_AS_SDP_EN_IN_PR_ACTIVE Harry > # define DP_MSA_TIMING_PAR_IGNORE_EN (1 << 7) /* eDP */ > > #define DP_MAIN_LINK_CHANNEL_CODING_SET 0x108 > @@ -1105,6 +1106,11 @@ > # define DP_VSC_EXT_CEA_SDP_SUPPORTED(1 << 6) /* DP > 1.4 */ > # define DP_VSC_EXT_CEA_SDP_CHAINING_SUPPORTED (1 << 7) /* DP > 1.4 */ > > +#define DP_DPRX_FEATURE_ENUMERATION_LIST_CONT_1 0x2214 /* 2.0 E11 */ > +# define DP_ADAPTIVE_SYNC_SDP_SUPPORTED(1 << 0) > +# define DP_AS_SDP_FIRST_HALF_LINE_OR_3840_PIXEL_CYCLE_WINDOW_NOT_SUPPORTED > (1 << 1) > +# define DP_VSC_EXT_SDP_FRAMEWORK_VERSION_1_SUPPORTED (1 << 4) > + > #define DP_128B132B_SUPPORTED_LINK_RATES 0x2215 /* 2.0 */ > # define DP_UHBR10 (1 << 0) > # define DP_UHBR20 (1 << 1)
Re: [PATCH v2 00/11] pwm: Allow .get_state to fail
On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-König wrote: > Hello, > > I forgot about this series and was remembered when I talked to Conor > Dooley about how .get_state() should behave in an error case. > > Compared to (implicit) v1, sent with Message-Id: > 20220916151506.298488-1-u.kleine-koe...@pengutronix.de > I changed: > > - Patch #1 which does the prototype change now just adds "return 0" to >all implementations and so gets simpler and doesn't change behaviour. >The adaptions to the different .get_state() implementations are split >out into individual patches to ease review. > - One minor inconsistency fixed in "pwm: Handle .get_state() failures" >that I noticed while looking into this patch. > - I skipped changing sun4i.c as I don't know how to handle the error >there. Someone might want to have a look. (That's not ideal, but it's >not worse than the same issue before this series.) > > In v1 Thierry had the concern: > > | That raises the question about what to do in these cases. If we return > | an error, that could potentially throw off consumers. So perhaps the > | closest would be to return a disabled PWM? Or perhaps it'd be up to the > | consumer to provide some fallback configuration for invalidly configured > | or unconfigured PWMs. > > .get_state() is only called in pwm_device_request on a pwm_state that a > consumer might see. Before my series a consumer might have seen a > partial modified pwm_state (because .get_state() might have modified > .period, then stumbled and returned silently). The last patch ensures > that this partial modification isn't given out to the consumer. Instead > they now see the same as if .get_state wasn't implemented at all. I'm wondering why we didn't see a compiler warning about mistyped function prototypes in some drivers. P.S. The series is good thing to do, thank you. -- With Best Regards, Andy Shevchenko
Re: [PATCH v3 1/1] dt-bindings: lcdif: Fix constraints for imx8mp
On Thu, 08 Dec 2022 15:08:40 +0100, Alexander Stein wrote:
> i.MX8MP uses 3 clocks, so soften the restrictions for clocks & clock-names.
> This SoC requires a power-domain for this peripheral to use. Add it as
> a required property.
>
> Fixes: f5419cb0743f ("dt-bindings: lcdif: Add compatible for i.MX8MP")
> Signed-off-by: Alexander Stein
> ---
> Changes in v3:
> * Removed power-domains minItems: 1 constraint
>
> Changes in v2:
> * Squash both patches into one
> * Split the conditionals from fsl,imx6sx-lcdif
> * Mark power-domains as required for fsl,imx8mp-lcdif
> * Ignored the A-b & R-b due to reorganization
>
> .../bindings/display/fsl,lcdif.yaml | 29 ++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
Applied, thanks!
Re: [PATCH] dma-buf: fix dma_buf_export init order v2
On Fri, Dec 9, 2022 at 8:29 AM Ruhl, Michael J wrote:
>
> >-Original Message-
> >From: dri-devel On Behalf Of
> >Christian König
> >Sent: Friday, December 9, 2022 2:16 AM
> >To: quic_chara...@quicinc.com; cuigaoshe...@huawei.com;
> >sumit.sem...@linaro.org
> >Cc: linaro-mm-...@lists.linaro.org; dri-devel@lists.freedesktop.org; linux-
> >me...@vger.kernel.org
> >Subject: [PATCH] dma-buf: fix dma_buf_export init order v2
> >
> >The init order and resulting error handling in dma_buf_export
> >was pretty messy.
> >
> >Subordinate objects like the file and the sysfs kernel objects
> >were initializing and wiring itself up with the object in the
> >wrong order resulting not only in complicating and partially
> >incorrect error handling, but also in publishing only halve
> >initialized DMA-buf objects.
> >
> >Clean this up thoughtfully by allocating the file independent
> >of the DMA-buf object. Then allocate and initialize the DMA-buf
> >object itself, before publishing it through sysfs. If everything
> >works as expected the file is then connected with the DMA-buf
> >object and publish it through debugfs.
> >
> >Also adds the missing dma_resv_fini() into the error handling.
> >
> >v2: add some missing changes to dma_bug_getfile() and a missing NULL
> >check in dma_buf_file_release()
>
> Looks good.
>
> Reviewed-by: Michael J. Ruhl
>
> Mike
>
+1
Reviewed-by: T.J. Mercier
> >Signed-off-by: Christian König
> >---
> > drivers/dma-buf/dma-buf-sysfs-stats.c | 7 +--
> > drivers/dma-buf/dma-buf-sysfs-stats.h | 4 +-
> > drivers/dma-buf/dma-buf.c | 84 +--
> > 3 files changed, 43 insertions(+), 52 deletions(-)
> >
> >diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-
> >buf-sysfs-stats.c
> >index 2bba0babcb62..4b680e10c15a 100644
> >--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
> >+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
> >@@ -168,14 +168,11 @@ void dma_buf_uninit_sysfs_statistics(void)
> > kset_unregister(dma_buf_stats_kset);
> > }
> >
> >-int dma_buf_stats_setup(struct dma_buf *dmabuf)
> >+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
> > {
> > struct dma_buf_sysfs_entry *sysfs_entry;
> > int ret;
> >
> >- if (!dmabuf || !dmabuf->file)
> >- return -EINVAL;
> >-
> > if (!dmabuf->exp_name) {
> > pr_err("exporter name must not be empty if stats
> >needed\n");
> > return -EINVAL;
> >@@ -192,7 +189,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
> >
> > /* create the directory for buffer stats */
> > ret = kobject_init_and_add(_entry->kobj, _buf_ktype,
> >NULL,
> >- "%lu", file_inode(dmabuf->file)->i_ino);
> >+ "%lu", file_inode(file)->i_ino);
> > if (ret)
> > goto err_sysfs_dmabuf;
> >
> >diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h b/drivers/dma-buf/dma-
> >buf-sysfs-stats.h
> >index a49c6e2650cc..7a8a995b75ba 100644
> >--- a/drivers/dma-buf/dma-buf-sysfs-stats.h
> >+++ b/drivers/dma-buf/dma-buf-sysfs-stats.h
> >@@ -13,7 +13,7 @@
> > int dma_buf_init_sysfs_statistics(void);
> > void dma_buf_uninit_sysfs_statistics(void);
> >
> >-int dma_buf_stats_setup(struct dma_buf *dmabuf);
> >+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file);
> >
> > void dma_buf_stats_teardown(struct dma_buf *dmabuf);
> > #else
> >@@ -25,7 +25,7 @@ static inline int dma_buf_init_sysfs_statistics(void)
> >
> > static inline void dma_buf_uninit_sysfs_statistics(void) {}
> >
> >-static inline int dma_buf_stats_setup(struct dma_buf *dmabuf)
> >+static inline int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file
> >*file)
> > {
> > return 0;
> > }
> >diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> >index e6f36c014c4c..eb6b59363c4f 100644
> >--- a/drivers/dma-buf/dma-buf.c
> >+++ b/drivers/dma-buf/dma-buf.c
> >@@ -95,10 +95,11 @@ static int dma_buf_file_release(struct inode *inode,
> >struct file *file)
> > return -EINVAL;
> >
> > dmabuf = file->private_data;
> >-
> >- mutex_lock(_list.lock);
> >- list_del(>list_node);
> >- mutex_unlock(_list.lock);
> >+ if (dmabuf) {
> >+ mutex_lock(_list.lock);
> >+ list_del(>list_node);
> >+ mutex_unlock(_list.lock);
> >+ }
> >
> > return 0;
> > }
> >@@ -523,17 +524,17 @@ static inline int is_dma_buf_file(struct file *file)
> > return file->f_op == _buf_fops;
> > }
> >
> >-static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
> >+static struct file *dma_buf_getfile(size_t size, int flags)
> > {
> > static atomic64_t dmabuf_inode = ATOMIC64_INIT(0);
> >- struct file *file;
> > struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
> >+ struct file *file;
> >
> > if (IS_ERR(inode))
> > return ERR_CAST(inode);
> >
> >- inode->i_size =
Re: [PATCH 1/1] crypto: ccp - Allocate TEE ring and cmd buffer using DMA APIs
On 12/6/22 06:30, Rijo Thomas wrote:
For AMD Secure Processor (ASP) to map and access TEE ring buffer, the
ring buffer address sent by host to ASP must be a real physical
address and the pages must be physically contiguous.
In a virtualized environment though, when the driver is running in a
guest VM, the pages allocated by __get_free_pages() may not be
contiguous in the host (or machine) physical address space. Guests
will see a guest (or pseudo) physical address and not the actual host
(or machine) physical address. The TEE running on ASP cannot decipher
pseudo physical addresses. It needs host or machine physical address.
To resolve this problem, use DMA APIs for allocating buffers that must
be shared with TEE. This will ensure that the pages are contiguous in
host (or machine) address space. If the DMA handle is an IOVA,
translate it into a physical address before sending it to ASP.
This patch also exports two APIs (one for buffer allocation and
another to free the buffer). This API can be used by AMD-TEE driver to
share buffers with TEE.
Fixes: 33960acccfbd ("crypto: ccp - add TEE support for Raven Ridge")
Cc: Tom Lendacky
Cc: sta...@vger.kernel.org
Signed-off-by: Rijo Thomas
Co-developed-by: Jeshwanth
Signed-off-by: Jeshwanth
Reviewed-by: Devaraj Rangasamy
---
drivers/crypto/ccp/psp-dev.c | 6 +-
drivers/crypto/ccp/tee-dev.c | 116 ++-
drivers/crypto/ccp/tee-dev.h | 9 +--
include/linux/psp-tee.h | 47 ++
4 files changed, 127 insertions(+), 51 deletions(-)
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index c9c741ac8442..2b86158d7435 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -161,13 +161,13 @@ int psp_dev_init(struct sp_device *sp)
goto e_err;
}
+ if (sp->set_psp_master_device)
+ sp->set_psp_master_device(sp);
+
This worries me a bit... if psp_init() fails, it may still be referenced
as the master device. What's the reason for moving it?
ret = psp_init(psp);
if (ret)
goto e_irq;
- if (sp->set_psp_master_device)
- sp->set_psp_master_device(sp);
-
/* Enable interrupt */
iowrite32(-1, psp->io_regs + psp->vdata->inten_reg);
diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 5c9d47f3be37..1631d9851e54 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -12,8 +12,9 @@
#include
#include
#include
+#include
+#include
#include
-#include
#include
#include "psp-dev.h"
@@ -21,25 +22,64 @@
static bool psp_dead;
+struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp)
It looks like both calls to this use the same gfp_t values, you can
probably eliminate from the call and just specify them in here.
+{
+ struct psp_device *psp = psp_get_master_device();
+ struct dma_buffer *dma_buf;
+ struct iommu_domain *dom;
+
+ if (!psp || !size)
+ return NULL;
+
+ dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL);
+ if (!dma_buf)
+ return NULL;
+
+ dma_buf->vaddr = dma_alloc_coherent(psp->dev, size, _buf->dma, gfp);
+ if (!dma_buf->vaddr || !dma_buf->dma) {
I don't think you can have one of these be NULL without both being NULL,
but I guess it doesn't hurt to check.
+ kfree(dma_buf);
+ return NULL;
+ }
+
+ dma_buf->size = size;
+ > +dom = iommu_get_domain_for_dev(psp->dev);
+ if (dom)
You need a nice comment above this all explaining that. I guess you're
using the presence of a domain to determine whether you're running on
bare-metal vs within a hypervisor? I'm not sure what will happen if the
guest ever gets an emulated IOMMU...
+ dma_buf->paddr = iommu_iova_to_phys(dom, dma_buf->dma);
If you're just looking to get the physical address, why not just to an
__pa(dma_buf->vaddr)?
Also, paddr might not be the best name, since it isn't always a physical
address, but I can't really think of something right now.
Thanks,
Tom
+ else
+ dma_buf->paddr = dma_buf->dma;
+
+ return dma_buf;
+}
+EXPORT_SYMBOL(psp_tee_alloc_dmabuf);
+
+void psp_tee_free_dmabuf(struct dma_buffer *dma_buf)
+{
+ struct psp_device *psp = psp_get_master_device();
+
+ if (!psp || !dma_buf)
+ return;
+
+ dma_free_coherent(psp->dev, dma_buf->size,
+ dma_buf->vaddr, dma_buf->dma);
+
+ kfree(dma_buf);
+}
+EXPORT_SYMBOL(psp_tee_free_dmabuf);
+
static int tee_alloc_ring(struct psp_tee_device *tee, int ring_size)
{
struct ring_buf_manager *rb_mgr = >rb_mgr;
- void *start_addr;
if (!ring_size)
return -EINVAL;
- /* We need actual physical address instead of DMA address, since
-* Trusted OS running on AMD Secure
Re: [PATCH v3 2/2] gpu/drm/panel: Add Sony TD4353 JDI panel driver
On 2022-09-30 20:08:11, Konrad Dybcio wrote:
> Add support for the Sony TD4353 JDI 2160x1080 display panel used in
This is a vertical panel of 1080x2160 right?
> some Sony Xperia XZ2 and XZ2 Compact smartphones. Due to the specifics
> of smartphone manufacturing, it is impossible to retrieve a better name
> for this panel.
>
> This revision adds support for the default 60 Hz configuration, however
> there could possibly be some room for expansion, as the display panels
> used on Sony devices have historically been capable of >2x refresh rate
> overclocking.
>
> Signed-off-by: Konrad Dybcio
Reviewed-by: Marijn Suijten
Apart from some minor nits about how we can change seemingly magical hex
constants to a decimal represntation, which makes it more obvious (to
the reader) that these are just width-1 and height-1.
> ---
> Changes since v2:
> - "GPL v2" -> "GPL"
> - add missing S-o-b (how embarassing)
> - move { after sony_td4353_assert_reset_gpios() to a new line
>
> drivers/gpu/drm/panel/Kconfig | 10 +
> drivers/gpu/drm/panel/Makefile| 1 +
> drivers/gpu/drm/panel/panel-sony-td4353-jdi.c | 329 ++
> 3 files changed, 340 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index a582ddd583c2..6ef1b48169b5 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -637,6 +637,16 @@ config DRM_PANEL_SONY_ACX565AKM
> Say Y here if you want to enable support for the Sony ACX565AKM
> 800x600 3.5" panel (found on the Nokia N900).
>
> +config DRM_PANEL_SONY_TD4353_JDI
> + tristate "Sony TD4353 JDI panel"
> + depends on GPIOLIB && OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to enable support for the Sony Tama
> + TD4353 JDI command mode panel as found on some Sony Xperia
> + XZ2 and XZ2 Compact smartphones.
> +
> config DRM_PANEL_SONY_TULIP_TRULY_NT35521
> tristate "Sony Tulip Truly NT35521 panel"
> depends on GPIOLIB && OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 8e71aa7581b8..8ef27bc86f94 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) +=
> panel-sitronix-st7701.o
> obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7703) += panel-sitronix-st7703.o
> obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
> +obj-$(CONFIG_DRM_PANEL_SONY_TD4353_JDI) += panel-sony-td4353-jdi.o
> obj-$(CONFIG_DRM_PANEL_SONY_TULIP_TRULY_NT35521) +=
> panel-sony-tulip-truly-nt35521.o
> obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) += panel-tdo-tl070wsh30.o
> obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
> diff --git a/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
> b/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
> new file mode 100644
> index ..11db62992b8b
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
> @@ -0,0 +1,329 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022 Konrad Dybcio
> + *
> + * Generated with linux-mdss-dsi-panel-driver-generator with a
> + * substantial amount of manual adjustments.
> + *
> + * SONY Downstream kernel calls this one:
> + * - "JDI ID3" for Akari (XZ2)
> + * - "JDI ID4" for Apollo (XZ2 Compact)
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +#include
> +#include
> +
> +enum {
> + TYPE_TAMA_60HZ,
> + /*
> + * Leaving room for expansion - SONY very often uses
> + * *truly reliably* overclockable panels on their flagships!
> + */
> +};
> +
> +struct sony_td4353_jdi {
> + struct drm_panel panel;
> + struct mipi_dsi_device *dsi;
> + struct regulator_bulk_data supplies[3];
> + struct gpio_desc *panel_reset_gpio;
> + struct gpio_desc *touch_reset_gpio;
> + bool prepared;
> + int type;
> +};
> +
> +static inline struct sony_td4353_jdi *to_sony_td4353_jdi(struct drm_panel
> *panel)
> +{
> + return container_of(panel, struct sony_td4353_jdi, panel);
> +}
> +
> +static int sony_td4353_jdi_on(struct sony_td4353_jdi *ctx)
> +{
> + struct mipi_dsi_device *dsi = ctx->dsi;
> + struct device *dev = >dev;
> + int ret;
> +
> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> + ret = mipi_dsi_dcs_set_column_address(dsi, 0x, 0x0437);
As discussed offline, can we just use 0, 1079 here? That way it's a bit
more clear that our column address is just width-1.
> + if (ret < 0) {
> + dev_err(dev, "Failed to set column address: %d\n", ret);
> + return ret;
> + }
> +
> + ret = mipi_dsi_dcs_set_page_address(dsi, 0x, 0x086f);
Re: [PATCH 1/5] linux/minmax.h: add non-atomic version of xchg
On Fri, Dec 09, 2022 at 08:56:28PM +0200, Andy Shevchenko wrote: > On Fri, Dec 09, 2022 at 04:48:39PM +0100, Andrzej Hajda wrote: ... > > I hope there will be place for such tiny helper in kernel. > > Quick cocci analyze shows there is probably few thousands places > > where it could be used, of course I do not intend to do it :). > > > > I was not sure where to put this macro, I hope near swap definition > > is the most suitable place. > > Ah, swap() in this context is not the same. minmax.h hosts it because > it's often related to the swap function in the sort-type algorithms. > > > Moreover sorry if to/cc is not correct - get_maintainers.pl was > > more confused than me, to who address this patch. > > ... > > > include/linux/minmax.h | 14 ++ > > Does it really suit this header? I would expect something else. > Maybe include/linux/non-atomic/xchg.h, dunno. It may become a candidate to host io-64 non-atomic versions and other non-atomic generic headers... > Btw, have you looked if Ingo's gigantic series have done anything to cmpxchg.h > and related headers? Maybe some ideas can be taken from there? -- With Best Regards, Andy Shevchenko
Re: [PATCH 1/5] linux/minmax.h: add non-atomic version of xchg
On Fri, Dec 09, 2022 at 04:48:39PM +0100, Andrzej Hajda wrote: > The pattern of setting variable with new value and returning old > one is very common in kernel. Usually atomicity of the operation > is not required, so xchg seems to be suboptimal and confusing in > such cases. Since name xchg is already in use and __xchg is used > in architecture code, proposition is to name the macro exchange. > > Signed-off-by: Andrzej Hajda > --- > Hi, > > I hope there will be place for such tiny helper in kernel. > Quick cocci analyze shows there is probably few thousands places > where it could be used, of course I do not intend to do it :). > > I was not sure where to put this macro, I hope near swap definition > is the most suitable place. Ah, swap() in this context is not the same. minmax.h hosts it because it's often related to the swap function in the sort-type algorithms. > Moreover sorry if to/cc is not correct - get_maintainers.pl was > more confused than me, to who address this patch. ... > include/linux/minmax.h | 14 ++ Does it really suit this header? I would expect something else. Maybe include/linux/non-atomic/xchg.h, dunno. Btw, have you looked if Ingo's gigantic series have done anything to cmpxchg.h and related headers? Maybe some ideas can be taken from there? -- With Best Regards, Andy Shevchenko
Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
On Sun, 14 Aug 2022 16:46:54 -0700, Vinay Belgaumkar wrote: > > Host Turbo operates at efficient frequency when GT is not idle unless > the user or workload has forced it to a higher level. Replicate the same > behavior in SLPC by allowing the algorithm to use efficient frequency. > We had disabled it during boot due to concerns that it might break > kernel ABI for min frequency. However, this is not the case since > SLPC will still abide by the (min,max) range limits. This change seems to have broken the i915 kernel ABI for min frequency for DG2. Tvrtko pointed this out here: https://patchwork.freedesktop.org/patch/512274/?series=110574=3 These bugs are the result of that ABI break: Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/6806 Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/6786 On DG2 when we set min == max freq, we see the GPU running not at the set min == max freq but at efficient freq (different from the set freq). We are still trying to see if the ABI can be salvaged but something is definitely wrong at present. Thanks. -- Ashutosh
Re: [PATCH v7 0/6] clk/qcom: Support gdsc collapse polling using 'reset' interface
On Thu, 8 Dec 2022 at 22:06, Bjorn Andersson wrote: > > On Thu, Dec 08, 2022 at 02:40:55PM +0100, Ulf Hansson wrote: > > On Wed, 7 Dec 2022 at 17:55, Bjorn Andersson wrote: > > > > > > On Wed, Dec 07, 2022 at 05:00:51PM +0100, Ulf Hansson wrote: > > > > On Thu, 1 Dec 2022 at 23:57, Bjorn Andersson > > > > wrote: > > > > > > > > > > On Wed, Oct 05, 2022 at 02:36:58PM +0530, Akhil P Oommen wrote: > > > > > > > > > > > > > > > > @Ulf, Akhil has a power-domain for a piece of hardware which may be > > > > > voted active by multiple different subsystems (co-processors/execution > > > > > contexts) in the system. > > > > > > > > > > As such, during the powering down sequence we don't wait for the > > > > > power-domain to turn off. But in the event of an error, the recovery > > > > > mechanism relies on waiting for the hardware to settle in a powered > > > > > off > > > > > state. > > > > > > > > > > The proposal here is to use the reset framework to wait for this state > > > > > to be reached, before continuing with the recovery mechanism in the > > > > > client driver. > > > > > > > > I tried to review the series (see my other replies), but I am not sure > > > > I fully understand the consumer part. > > > > > > > > More exactly, when and who is going to pull the reset and at what point? > > > > > > > > > > > > > > Given our other discussions on quirky behavior, do you have any > > > > > input/suggestions on this? > > > > > > > > > > > Some clients like adreno gpu driver would like to ensure that its > > > > > > gdsc > > > > > > is collapsed at hardware during a gpu reset sequence. This is > > > > > > because it > > > > > > has a votable gdsc which could be ON due to a vote from another > > > > > > subsystem > > > > > > like tz, hyp etc or due to an internal hardware signal. To allow > > > > > > this, gpucc driver can expose an interface to the client driver > > > > > > using > > > > > > reset framework. Using this the client driver can trigger a polling > > > > > > within > > > > > > the gdsc driver. > > > > > > > > > > @Akhil, this description is fairly generic. As we've reached the state > > > > > where the hardware has settled and we return to the client, what > > > > > prevents it from being powered up again? > > > > > > > > > > Or is it simply a question of it hitting the powered-off state, not > > > > > necessarily staying there? > > > > > > > > Okay, so it's indeed the GPU driver that is going to assert/de-assert > > > > the reset at some point. Right? > > > > > > > > That seems like a reasonable approach to me, even if it's a bit > > > > unclear under what conditions that could happen. > > > > > > > > > > Generally the disable-path of the power-domain does not check that the > > > power-domain is actually turned off, because the status might indicate > > > that the hardware is voting for the power-domain to be on. > > > > Is there a good reason why the HW needs to vote too, when the GPU > > driver is already in control? > > > > Or perhaps that depends on the running use case? > > > > > > > > As part of the recovery of the GPU after some fatal fault, the GPU > > > driver does something which will cause the hardware votes for the > > > power-domain to be let go, and then the driver does pm_runtime_put(). > > > > Okay. That "something", sounds like a device specific setting for the > > corresponding gdsc, right? > > > > So somehow the GPU driver needs to manage that setting, right? > > > > > > > > But in this case the GPU driver wants to ensure that the power-domain is > > > actually powered down, before it does pm_runtime_get() again. To ensure > > > that the hardware lost its state... > > > > I see. > > > > > > > > The proposal here is to use a reset to reach into the power-domain > > > provider and wait for the hardware to be turned off, before the GPU > > > driver attempts turning the power-domain on again. > > > > > > > > > In other words, there is no reset. This is a hack to make a normally > > > asynchronous pd.power_off() to be synchronous in this particular case. > > > > Alright, assuming I understood your clarifications above correctly > > (thanks!), I think I have got a much better picture now. > > > > Rather than abusing the reset interface, I think we should manage this > > through the genpd's power on/off notifiers (GENPD_NOTIFY_OFF). The GPU > > driver should register its corresponding device for them > > (dev_pm_genpd_add_notifier()). > > > > The trick however, is to make the behaviour of the power-domain for > > the gdsc (the genpd->power_off() callback) conditional on whether the > > HW is configured to vote or not. If the HW can vote, it should not > > poll for the state - and vice versa when the HW can't vote. > > > > Per Akhil's description I misunderstood who the other voters are; but > either way it's not the same "HW configured" mechanism as the one we're > already discussing. Okay, so this is another thing then. > > > But if we based on similar means could control if
Re: [PATCH 1/5] linux/minmax.h: add non-atomic version of xchg
On Fri, Dec 9, 2022, at 16:48, Andrzej Hajda wrote:
> The pattern of setting variable with new value and returning old
> one is very common in kernel. Usually atomicity of the operation
> is not required, so xchg seems to be suboptimal and confusing in
> such cases. Since name xchg is already in use and __xchg is used
> in architecture code, proposition is to name the macro exchange.
>
> Signed-off-by: Andrzej Hajda
While I generally don't like type invariant calling conventions
of xchg() and cmpxchg(), having a new function that has a similar
name without being able to tell which one is which from the
name seems more confusing.
Since __xchg() is only used on 11 architectures as an internal
name for the backing of arch_xchg() or arch_xchg_relaxed(),
maybe we can instead rename those to __arch_xchg() and use the
__xchg() name for the new non-atomic version?
> +/**
> + * exchange - set variable pointed by @ptr to @val, return old value
> + * @ptr: pointer to affected variable
> + * @val: value to be written
> + *
> + * This is non-atomic variant of xchg.
> + */
> +#define exchange(ptr, val) ({\
> + typeof(ptr) __ptr = ptr;\
> + typeof(*__ptr) __t = *__ptr;\
I think you can better express this using __auto_type than typeof(),
it is now provided by all supported compilers now.
Arnd
Re: Try to address the DMA-buf coherency problem
On Fri, Dec 9, 2022 at 4:32 AM Pekka Paalanen wrote: > > On Fri, 9 Dec 2022 17:26:06 +0900 > Tomasz Figa wrote: > > > On Mon, Dec 5, 2022 at 5:29 PM Christian König > > wrote: > > > > > > Hi Tomasz, > > > > > > Am 05.12.22 um 07:41 schrieb Tomasz Figa: > > > > [SNIP] > > > >> In other words explicit ownership transfer is not something we would > > > >> want as requirement in the framework, cause otherwise we break tons of > > > >> use cases which require concurrent access to the underlying buffer. > > > >> > > > >> When a device driver needs explicit ownership transfer it's perfectly > > > >> possible to implement this using the dma_fence objects mentioned above. > > > >> E.g. drivers can already look at who is accessing a buffer currently > > > >> and > > > >> can even grab explicit ownership of it by adding their own dma_fence > > > >> objects. > > > >> > > > >> The only exception is CPU based access, e.g. when something is written > > > >> with the CPU a cache flush might be necessary and when something is > > > >> read > > > >> with the CPU a cache invalidation might be necessary. > > > >> > > > > Okay, that's much clearer now, thanks for clarifying this. So we > > > > should be covered for the cache maintenance needs originating from CPU > > > > accesses already, +/- the broken cases which don't call the begin/end > > > > CPU access routines that I mentioned above. > > > > > > > > Similarly, for any ownership transfer between different DMA engines, > > > > we should be covered either by the userspace explicitly flushing the > > > > hardware pipeline or attaching a DMA-buf fence to the buffer. > > > > > > > > But then, what's left to be solved? :) (Besides the cases of missing > > > > begin/end CPU access calls.) > > > > > > Well there are multiple problems here: > > > > > > 1. A lot of userspace applications/frameworks assume that it can > > > allocate the buffer anywhere and it just works. > > > > > > This isn't true at all, we have tons of cases where device can only > > > access their special memory for certain use cases. > > > Just look at scanout for displaying on dGPU, neither AMD nor NVidia > > > supports system memory here. Similar cases exists for audio/video codecs > > > where intermediate memory is only accessible by certain devices because > > > of content protection. > > > > Ack. > > > > Although I think the most common case on mainstream Linux today is > > properly allocating for device X (e.g. V4L2 video decoder or DRM-based > > GPU) and hoping that other devices would accept the buffers just fine, > > which isn't a given on most platforms (although often it's just about > > pixel format, width/height/stride alignment, tiling, etc. rather than > > the memory itself). That's why ChromiumOS has minigbm and Android has > > gralloc that act as the central point of knowledge on buffer > > allocation. > > Hi, > > as an anecdote, when I was improving Mutter's cross-DRM-device handling > (for DisplayLink uses) a few years ago, I implemented several different > approaches of where to allocate, to try until going for the slowest but > guaranteed to work case of copying every update into and out of sysram. > > It seems there are two different approaches in general for allocation > and sharing: > > 1. Try different things until it works or you run out of options > > pro: > - no need for a single software component to know everything about > every device in the system > > con: > - who bothers with fallbacks, if the first try works on my system for > my use case I test with? I.e. cost of code in users. > - trial-and-error can be very laborious (allocate, share with all > devices, populate, test) > - the search space might be huge > Even that is fraught with difficulty. We had a ton of bug reports over the years claiming amdgpu was broken when users tried to use displaylink devices in combination with AMD dGPUs because the performance was so slow. The problem was that rather than using dma-buf, the compositor was just mmaping the the dGPU BAR, which happens to be uncached write combined and across the PCI bus, and copying the data to the displaylink device. Read access to a PCI BAR with the CPU is on the order of 10s of MB per second. Alex > > 2. Have a central component that knows what to do > > pro: > - It might work on the first attempt, so no fallbacks in users. > - It might be optimal. > > con: > - You need a software component that knows everything about every > single combination of hardware in existence, multiplied by use cases. > > > Neither seems good, which brings us back to > https://github.com/cubanismo/allocator . > > > > > 2. We don't properly communicate allocation requirements to userspace. > > > > > > E.g. even if you allocate from DMA-Heaps userspace can currently only > > > guess if normal, CMA or even device specific memory is needed. > > > > DMA-buf heaps actually make it even more difficult for the userspace, > > because now it needs to pick the right
RE: [PATCH] dma-buf: fix dma_buf_export init order v2
>-Original Message-
>From: dri-devel On Behalf Of
>Christian König
>Sent: Friday, December 9, 2022 2:16 AM
>To: quic_chara...@quicinc.com; cuigaoshe...@huawei.com;
>sumit.sem...@linaro.org
>Cc: linaro-mm-...@lists.linaro.org; dri-devel@lists.freedesktop.org; linux-
>me...@vger.kernel.org
>Subject: [PATCH] dma-buf: fix dma_buf_export init order v2
>
>The init order and resulting error handling in dma_buf_export
>was pretty messy.
>
>Subordinate objects like the file and the sysfs kernel objects
>were initializing and wiring itself up with the object in the
>wrong order resulting not only in complicating and partially
>incorrect error handling, but also in publishing only halve
>initialized DMA-buf objects.
>
>Clean this up thoughtfully by allocating the file independent
>of the DMA-buf object. Then allocate and initialize the DMA-buf
>object itself, before publishing it through sysfs. If everything
>works as expected the file is then connected with the DMA-buf
>object and publish it through debugfs.
>
>Also adds the missing dma_resv_fini() into the error handling.
>
>v2: add some missing changes to dma_bug_getfile() and a missing NULL
>check in dma_buf_file_release()
Looks good.
Reviewed-by: Michael J. Ruhl
Mike
>Signed-off-by: Christian König
>---
> drivers/dma-buf/dma-buf-sysfs-stats.c | 7 +--
> drivers/dma-buf/dma-buf-sysfs-stats.h | 4 +-
> drivers/dma-buf/dma-buf.c | 84 +--
> 3 files changed, 43 insertions(+), 52 deletions(-)
>
>diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-
>buf-sysfs-stats.c
>index 2bba0babcb62..4b680e10c15a 100644
>--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
>+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
>@@ -168,14 +168,11 @@ void dma_buf_uninit_sysfs_statistics(void)
> kset_unregister(dma_buf_stats_kset);
> }
>
>-int dma_buf_stats_setup(struct dma_buf *dmabuf)
>+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
> {
> struct dma_buf_sysfs_entry *sysfs_entry;
> int ret;
>
>- if (!dmabuf || !dmabuf->file)
>- return -EINVAL;
>-
> if (!dmabuf->exp_name) {
> pr_err("exporter name must not be empty if stats
>needed\n");
> return -EINVAL;
>@@ -192,7 +189,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
>
> /* create the directory for buffer stats */
> ret = kobject_init_and_add(_entry->kobj, _buf_ktype,
>NULL,
>- "%lu", file_inode(dmabuf->file)->i_ino);
>+ "%lu", file_inode(file)->i_ino);
> if (ret)
> goto err_sysfs_dmabuf;
>
>diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h b/drivers/dma-buf/dma-
>buf-sysfs-stats.h
>index a49c6e2650cc..7a8a995b75ba 100644
>--- a/drivers/dma-buf/dma-buf-sysfs-stats.h
>+++ b/drivers/dma-buf/dma-buf-sysfs-stats.h
>@@ -13,7 +13,7 @@
> int dma_buf_init_sysfs_statistics(void);
> void dma_buf_uninit_sysfs_statistics(void);
>
>-int dma_buf_stats_setup(struct dma_buf *dmabuf);
>+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file);
>
> void dma_buf_stats_teardown(struct dma_buf *dmabuf);
> #else
>@@ -25,7 +25,7 @@ static inline int dma_buf_init_sysfs_statistics(void)
>
> static inline void dma_buf_uninit_sysfs_statistics(void) {}
>
>-static inline int dma_buf_stats_setup(struct dma_buf *dmabuf)
>+static inline int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file
>*file)
> {
> return 0;
> }
>diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>index e6f36c014c4c..eb6b59363c4f 100644
>--- a/drivers/dma-buf/dma-buf.c
>+++ b/drivers/dma-buf/dma-buf.c
>@@ -95,10 +95,11 @@ static int dma_buf_file_release(struct inode *inode,
>struct file *file)
> return -EINVAL;
>
> dmabuf = file->private_data;
>-
>- mutex_lock(_list.lock);
>- list_del(>list_node);
>- mutex_unlock(_list.lock);
>+ if (dmabuf) {
>+ mutex_lock(_list.lock);
>+ list_del(>list_node);
>+ mutex_unlock(_list.lock);
>+ }
>
> return 0;
> }
>@@ -523,17 +524,17 @@ static inline int is_dma_buf_file(struct file *file)
> return file->f_op == _buf_fops;
> }
>
>-static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
>+static struct file *dma_buf_getfile(size_t size, int flags)
> {
> static atomic64_t dmabuf_inode = ATOMIC64_INIT(0);
>- struct file *file;
> struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
>+ struct file *file;
>
> if (IS_ERR(inode))
> return ERR_CAST(inode);
>
>- inode->i_size = dmabuf->size;
>- inode_set_bytes(inode, dmabuf->size);
>+ inode->i_size = size;
>+ inode_set_bytes(inode, size);
>
> /*
>* The ->i_ino acquired from get_next_ino() is not unique thus
>@@ -547,8 +548,6 @@ static struct file *dma_buf_getfile(struct dma_buf
>*dmabuf, int flags)
>flags, _buf_fops);
[PATCH 5/5] drm/i915: kill fetch_and_zero
Better use recently introduced kernel core helper.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 4 ++--
drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 6 +++---
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 4 ++--
drivers/gpu/drm/i915/i915_hwmon.c | 2 +-
drivers/gpu/drm/i915/i915_perf.c | 2 +-
drivers/gpu/drm/i915/i915_query.c | 2 +-
drivers/gpu/drm/i915/i915_request.c | 4 ++--
drivers/gpu/drm/i915/i915_vma.c | 2 +-
drivers/gpu/drm/i915/intel_memory_region.c| 2 +-
drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
drivers/gpu/drm/i915/intel_wakeref.c | 4 ++--
drivers/gpu/drm/i915/pxp/intel_pxp.c | 2 +-
drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 4 ++--
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 2 +-
15 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 05a27723ebb8cb..5498bd00ffd5e1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -209,7 +209,7 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object
*obj)
assert_object_held_shared(obj);
- pages = fetch_and_zero(>mm.pages);
+ pages = exchange(>mm.pages, NULL);
if (IS_ERR_OR_NULL(pages))
return pages;
@@ -515,7 +515,7 @@ void __i915_gem_object_release_map(struct
drm_i915_gem_object *obj)
* Furthermore, since this is an unsafe operation reserved only
* for construction time manipulation, we ignore locking prudence.
*/
- unmap_object(obj, page_mask_bits(fetch_and_zero(>mm.mapping)));
+ unmap_object(obj, page_mask_bits(exchange(>mm.mapping, 0)));
i915_gem_object_unpin_map(obj);
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index bc952107880734..6901b2529d1b29 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -652,7 +652,7 @@ static void
i915_gem_object_release_stolen(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
- struct drm_mm_node *stolen = fetch_and_zero(>stolen);
+ struct drm_mm_node *stolen = exchange(>stolen, NULL);
GEM_BUG_ON(!stolen);
i915_gem_stolen_remove_node(i915, stolen);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 5247d88b3c13e6..075112ebe30247 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -447,7 +447,7 @@ int i915_ttm_purge(struct drm_i915_gem_object *obj)
*/
shmem_truncate_range(file_inode(i915_tt->filp),
0, (loff_t)-1);
- fput(fetch_and_zero(_tt->filp));
+ fput(exchange(_tt->filp, 0));
}
obj->write_domain = 0;
@@ -779,7 +779,7 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object
*obj,
int ret;
/* First try only the requested placement. No eviction. */
- real_num_busy = fetch_and_zero(>num_busy_placement);
+ real_num_busy = exchange(>num_busy_placement, 0);
ret = ttm_bo_validate(bo, placement, );
if (ret) {
ret = i915_ttm_err_to_gem(ret);
@@ -905,7 +905,7 @@ static void i915_ttm_put_pages(struct drm_i915_gem_object
*obj,
*/
if (obj->mm.rsgt)
- i915_refct_sgt_put(fetch_and_zero(>mm.rsgt));
+ i915_refct_sgt_put(exchange(>mm.rsgt, 0));
}
/**
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index ac02fb03659208..4d456f74d5d901 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -615,7 +615,7 @@ static void throttle_release(struct i915_request **q, int
count)
if (IS_ERR_OR_NULL(q[i]))
continue;
- i915_request_put(fetch_and_zero([i]));
+ i915_request_put(exchange([i], 0));
}
}
@@ -1072,7 +1072,7 @@ __sseu_prepare(const char *name,
err_fini:
igt_spinner_fini(*spin);
err_free:
- kfree(fetch_and_zero(spin));
+ kfree(exchange(spin, 0));
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c
b/drivers/gpu/drm/i915/i915_hwmon.c
index cca7a4350ec8fd..c9d02f1124a067 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -732,5 +732,5 @@ void i915_hwmon_register(struct drm_i915_private *i915)
void i915_hwmon_unregister(struct
[PATCH 4/5] drm/i915/gvt: kill fetch_and_zero usage
Better use recently introduced kernel core helper.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
drivers/gpu/drm/i915/gvt/scheduler.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 077892a9aa8fdc..061302abb0a189 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1831,7 +1831,7 @@ static int init_service_thread(struct intel_gvt *gvt)
*/
static void intel_gvt_clean_device(struct drm_i915_private *i915)
{
- struct intel_gvt *gvt = fetch_and_zero(>gvt);
+ struct intel_gvt *gvt = exchange(>gvt, NULL);
if (drm_WARN_ON(>drm, !gvt))
return;
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 9cd8fcbf7cad16..6699135f366f3f 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -826,7 +826,7 @@ static int dispatch_workload(struct intel_vgpu_workload
*workload)
/* We might still need to add request with
* clean ctx to retire it properly..
*/
- rq = fetch_and_zero(>req);
+ rq = exchange(>req, NULL);
i915_request_put(rq);
}
@@ -1103,7 +1103,7 @@ static void complete_current_workload(struct intel_gvt
*gvt, int ring_id)
intel_vgpu_trigger_virtual_event(vgpu, event);
}
- i915_request_put(fetch_and_zero(>req));
+ i915_request_put(exchange(>req, 0));
}
gvt_dbg_sched("ring id %d complete workload %p status %d\n",
--
2.34.1
[PATCH 2/5] drm/i915/display: kill fetch_and_zero usage
Better use recently introduced kernel core helper.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/i915/display/icl_dsi.c| 2 +-
drivers/gpu/drm/i915/display/intel_ddi.c | 6 ++---
drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
.../drm/i915/display/intel_display_power.c| 22 +--
drivers/gpu/drm/i915/display/intel_dmc.c | 2 +-
drivers/gpu/drm/i915/display/intel_fb_pin.c | 6 ++---
drivers/gpu/drm/i915/display/intel_fbdev.c| 3 ++-
drivers/gpu/drm/i915/display/intel_overlay.c | 4 ++--
drivers/gpu/drm/i915/display/intel_pps.c | 4 ++--
drivers/gpu/drm/i915/display/intel_tc.c | 4 ++--
10 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
b/drivers/gpu/drm/i915/display/icl_dsi.c
index d16b30a2dded33..629b51ef7bfcce 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1425,7 +1425,7 @@ static void gen11_dsi_disable_io_power(struct
intel_encoder *encoder)
for_each_dsi_port(port, intel_dsi->ports) {
intel_wakeref_t wakeref;
- wakeref = fetch_and_zero(_dsi->io_wakeref[port]);
+ wakeref = exchange(_dsi->io_wakeref[port], 0);
intel_display_power_put(dev_priv,
port == PORT_A ?
POWER_DOMAIN_PORT_DDI_IO_A :
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 5f9a2410fc4c35..9486768fb9d38e 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -902,7 +902,7 @@ main_link_aux_power_domain_put(struct intel_digital_port
*dig_port,
intel_ddi_main_link_aux_domain(dig_port, crtc_state);
intel_wakeref_t wf;
- wf = fetch_and_zero(_port->aux_wakeref);
+ wf = exchange(_port->aux_wakeref, 0);
if (!wf)
return;
@@ -2678,7 +2678,7 @@ static void intel_ddi_post_disable_dp(struct
intel_atomic_state *state,
if (!intel_tc_port_in_tbt_alt_mode(dig_port))
intel_display_power_put(dev_priv,
dig_port->ddi_io_power_domain,
-
fetch_and_zero(_port->ddi_io_wakeref));
+ exchange(_port->ddi_io_wakeref, 0));
intel_ddi_disable_clock(encoder);
}
@@ -2705,7 +2705,7 @@ static void intel_ddi_post_disable_hdmi(struct
intel_atomic_state *state,
intel_display_power_put(dev_priv,
dig_port->ddi_io_power_domain,
- fetch_and_zero(_port->ddi_io_wakeref));
+ exchange(_port->ddi_io_wakeref, 0));
intel_ddi_disable_clock(encoder);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 32b25715718644..fd9f7ab71ee84c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -964,7 +964,7 @@ void intel_display_finish_reset(struct drm_i915_private
*i915)
if (!test_bit(I915_RESET_MODESET, _gt(i915)->reset.flags))
return;
- state = fetch_and_zero(>display.restore.modeset_state);
+ state = exchange(>display.restore.modeset_state, NULL);
if (!state)
goto unlock;
@@ -7591,7 +7591,7 @@ static void intel_atomic_commit_tail(struct
intel_atomic_state *state)
* cleanup. So copy and reset the dsb structure to sync with
* commit_done and later do dsb cleanup in cleanup_work.
*/
- old_crtc_state->dsb = fetch_and_zero(_crtc_state->dsb);
+ old_crtc_state->dsb = exchange(_crtc_state->dsb, 0);
}
/* Underruns don't always raise interrupts, so check manually */
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 3adba64937de68..34a155bf825c87 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -474,7 +474,7 @@ intel_display_power_grab_async_put_ref(struct
drm_i915_private *dev_priv,
cancel_delayed_work(_domains->async_put_work);
intel_runtime_pm_put_raw(_priv->runtime_pm,
-
fetch_and_zero(_domains->async_put_wakeref));
+exchange(_domains->async_put_wakeref,
0));
out_verify:
verify_async_put_domains_state(power_domains);
@@ -660,7 +660,7 @@ intel_display_power_put_async_work(struct work_struct *work)
* Bail out if all the domain refs pending to be released were grabbed
* by subsequent gets or a flush_work.
*/
- old_work_wakeref = fetch_and_zero(_domains->async_put_wakeref);
+ old_work_wakeref =
[PATCH 3/5] drm/i915/gt: kill fetch_and_zero usage
Better use recently introduced kernel core helper.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c| 2 +-
drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_ggtt.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gsc.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gt_pm.c| 2 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 6 +++---
drivers/gpu/drm/i915/gt/intel_migrate.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rps.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_context.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_ring_submission.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_timeline.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c| 2 +-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 2 +-
16 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index c33e0d72d6702b..de318d96d52abd 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1024,7 +1024,7 @@ static void cleanup_status_page(struct intel_engine_cs
*engine)
/* Prevent writes into HWSP after returning the page to the system */
intel_engine_set_hwsp_writemask(engine, ~0u);
- vma = fetch_and_zero(>status_page.vma);
+ vma = exchange(>status_page.vma, NULL);
if (!vma)
return;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 9a527e1f5be655..6029fafaaa674f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -229,7 +229,7 @@ static void heartbeat(struct work_struct *wrk)
mutex_unlock(>timeline->mutex);
out:
if (!engine->i915->params.enable_hangcheck || !next_heartbeat(engine))
- i915_request_put(fetch_and_zero(>heartbeat.systole));
+ i915_request_put(exchange(>heartbeat.systole, 0));
intel_engine_pm_put(engine);
}
@@ -244,7 +244,7 @@ void intel_engine_unpark_heartbeat(struct intel_engine_cs
*engine)
void intel_engine_park_heartbeat(struct intel_engine_cs *engine)
{
if (cancel_delayed_work(>heartbeat.work))
- i915_request_put(fetch_and_zero(>heartbeat.systole));
+ i915_request_put(exchange(>heartbeat.systole, 0));
}
void intel_gt_unpark_heartbeats(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 49a8f10d76c77b..29e78078d55a8b 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -3197,7 +3197,7 @@ static void execlists_reset_cancel(struct intel_engine_cs
*engine)
RB_CLEAR_NODE(rb);
spin_lock(>base.sched_engine->lock);
- rq = fetch_and_zero(>request);
+ rq = exchange(>request, NULL);
if (rq) {
if (i915_request_mark_eio(rq)) {
rq->engine = engine;
@@ -3602,7 +3602,7 @@ static void rcu_virtual_context_destroy(struct
work_struct *wrk)
spin_lock_irq(>base.sched_engine->lock);
- old = fetch_and_zero(>request);
+ old = exchange(>request, NULL);
if (old) {
GEM_BUG_ON(!__i915_request_is_complete(old));
__i915_request_submit(old);
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 0c7fe360f87331..2eb0173c6e968c 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -684,7 +684,7 @@ static void fini_aliasing_ppgtt(struct i915_ggtt *ggtt)
{
struct i915_ppgtt *ppgtt;
- ppgtt = fetch_and_zero(>alias);
+ ppgtt = exchange(>alias, NULL);
if (!ppgtt)
return;
@@ -1238,7 +1238,7 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
was_bound);
if (obj) { /* only used during resume => exclusive access */
- write_domain_objs |= fetch_and_zero(>write_domain);
+ write_domain_objs |= exchange(>write_domain, 0);
obj->read_domains |= I915_GEM_DOMAIN_GTT;
}
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c
b/drivers/gpu/drm/i915/gt/intel_gsc.c
index bcc3605158dbde..7226b42bb70b2a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gsc.c
+++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
@@ -70,7 +70,7
[PATCH 1/5] linux/minmax.h: add non-atomic version of xchg
The pattern of setting variable with new value and returning old
one is very common in kernel. Usually atomicity of the operation
is not required, so xchg seems to be suboptimal and confusing in
such cases. Since name xchg is already in use and __xchg is used
in architecture code, proposition is to name the macro exchange.
Signed-off-by: Andrzej Hajda
---
Hi,
I hope there will be place for such tiny helper in kernel.
Quick cocci analyze shows there is probably few thousands places
where it could be used, of course I do not intend to do it :).
I was not sure where to put this macro, I hope near swap definition
is the most suitable place.
Moreover sorry if to/cc is not correct - get_maintainers.pl was
more confused than me, to who address this patch.
Regards
Andrzej
---
include/linux/minmax.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 5433c08fcc6858..17d48769203bd5 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -144,4 +144,18 @@
#define swap(a, b) \
do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
+/**
+ * exchange - set variable pointed by @ptr to @val, return old value
+ * @ptr: pointer to affected variable
+ * @val: value to be written
+ *
+ * This is non-atomic variant of xchg.
+ */
+#define exchange(ptr, val) ({ \
+ typeof(ptr) __ptr = ptr;\
+ typeof(*__ptr) __t = *__ptr;\
+ *(__ptr) = (val); \
+ __t;\
+})
+
#endif /* _LINUX_MINMAX_H */
--
2.34.1
[PATCH v9 18/18] drm: bridge: samsung-dsim: Add i.MX8M Plus support
Add extras to support i.MX8M Plus. The main change is the removal of
HS/VS/DE signal inversion in the LCDIFv3-DSIM glue logic, otherwise
the implementation of this IP in i.MX8M Plus is very much compatible
with the i.MX8M Mini/Nano one.
Signed-off-by: Marek Vasut
Signed-off-by: Jagan Teki
---
Changes for v9:
- added im8mp in DSIM_STATE_REINITIALIZED check
drivers/gpu/drm/bridge/samsung-dsim.c | 26 +-
include/drm/bridge/samsung-dsim.h | 1 +
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 7ff10308a7ad..6e9ad955ebd3 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -479,6 +479,7 @@ samsung_dsim_types[SAMSUNG_DSIM_TYPE_COUNT] = {
[SAMSUNG_DSIM_TYPE_EXYNOS5422] = _dsi_driver_data,
[SAMSUNG_DSIM_TYPE_EXYNOS5433] = _dsi_driver_data,
[SAMSUNG_DSIM_TYPE_IMX8MM] = _dsi_driver_data,
+ [SAMSUNG_DSIM_TYPE_IMX8MP] = _dsi_driver_data,
};
static inline struct samsung_dsim *host_to_dsi(struct mipi_dsi_host *h)
@@ -1305,7 +1306,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi,
unsigned int flag)
* This host reinitialization is handled via DSIM_STATE_REINITIALIZED
* flag and triggers from host transfer. Do this exclusively for Exynos.
*/
- if ((dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM) &&
+ if ((dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM ||
+ dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MP) &&
dsi->state & DSIM_STATE_REINITIALIZED)
return 0;
@@ -1468,10 +1470,17 @@ static int samsung_dsim_atomic_check(struct drm_bridge
*bridge,
* 13.6.2.7.2 RGB interface
* both claim "Vsync, Hsync, and VDEN are active high signals.", the
* LCDIF must generate inverted HS/VS/DE signals, i.e. active LOW.
+*
+* The i.MX8M Plus glue logic between LCDIFv3 and DSIM does not
+* implement the same behavior, therefore LCDIFv3 must generate
+* HS/VS/DE signals active HIGH.
*/
if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM) {
adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC |
DRM_MODE_FLAG_NVSYNC);
adjusted_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC |
DRM_MODE_FLAG_PVSYNC);
+ } else if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MP) {
+ adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC |
DRM_MODE_FLAG_NVSYNC);
+ adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC |
DRM_MODE_FLAG_PVSYNC);
}
return 0;
@@ -1689,6 +1698,10 @@ static const struct samsung_dsim_host_ops
samsung_dsim_generic_host_ops = {
.unregister_host = samsung_dsim_unregister_host,
};
+static const struct drm_bridge_timings samsung_dsim_bridge_timings_de_high = {
+ .input_bus_flags = DRM_BUS_FLAG_DE_HIGH,
+};
+
static const struct drm_bridge_timings samsung_dsim_bridge_timings_de_low = {
.input_bus_flags = DRM_BUS_FLAG_DE_LOW,
};
@@ -1778,6 +1791,8 @@ int samsung_dsim_probe(struct platform_device *pdev)
/* DE_LOW: i.MX8M Mini/Nano LCDIF-DSIM glue logic inverts HS/VS/DE */
if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM)
dsi->bridge.timings = _dsim_bridge_timings_de_low;
+ else
+ dsi->bridge.timings = _dsim_bridge_timings_de_high;
if (dsi->plat_data->host_ops && dsi->plat_data->host_ops->register_host)
ret = dsi->plat_data->host_ops->register_host(dsi);
@@ -1883,11 +1898,20 @@ static const struct samsung_dsim_plat_data
samsung_dsim_imx8mm_pdata = {
.host_ops = _dsim_generic_host_ops,
};
+static const struct samsung_dsim_plat_data samsung_dsim_imx8mp_pdata = {
+ .hw_type = SAMSUNG_DSIM_TYPE_IMX8MP,
+ .host_ops = _dsim_generic_host_ops,
+};
+
static const struct of_device_id samsung_dsim_of_match[] = {
{
.compatible = "fsl,imx8mm-mipi-dsim",
.data = _dsim_imx8mm_pdata,
},
+ {
+ .compatible = "fsl,imx8mp-mipi-dsim",
+ .data = _dsim_imx8mp_pdata,
+ },
{ /* sentinel. */ }
};
MODULE_DEVICE_TABLE(of, samsung_dsim_of_match);
diff --git a/include/drm/bridge/samsung-dsim.h
b/include/drm/bridge/samsung-dsim.h
index df3d030daec6..3789f9dbb238 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -28,6 +28,7 @@ enum samsung_dsim_type {
SAMSUNG_DSIM_TYPE_EXYNOS5422,
SAMSUNG_DSIM_TYPE_EXYNOS5433,
SAMSUNG_DSIM_TYPE_IMX8MM,
+ SAMSUNG_DSIM_TYPE_IMX8MP,
SAMSUNG_DSIM_TYPE_COUNT,
};
--
2.25.1
[PATCH v9 17/18] dt-bindings: display: exynos: dsim: Add NXP i.MX8M Plus support
Samsung MIPI DSIM bridge can also be found in i.MX8M Plus SoC. Add dt-bingings for it. Signed-off-by: Jagan Teki --- Changes for v9: - none Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt index 5133d4d39190..2a5f0889ec32 100644 --- a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt +++ b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt @@ -8,6 +8,7 @@ Required properties: "samsung,exynos5422-mipi-dsi" /* for Exynos5422/5800 SoCs */ "samsung,exynos5433-mipi-dsi" /* for Exynos5433 SoCs */ "fsl,imx8mm-mipi-dsim" /* for i.MX8M Mini/Nano SoCs */ + "fsl,imx8mp-mipi-dsim" /* for i.MX8M Plus SoCs */ - reg: physical base address and length of the registers set for the device - interrupts: should contain DSI interrupt - clocks: list of clock specifiers, must contain an entry for each required -- 2.25.1
[PATCH v9 16/18] drm: bridge: samsung-dsim: Add i.MX8M Mini/Nano support
Samsung MIPI DSIM master can also be found in i.MX8M Mini/Nano SoC.
Add compatible and associated driver_data for it.
v9:
* none
v8:
* fix and update the comment
v7, v6:
* none
v3:
* enable DSIM_QUIRK_FIXUP_SYNC_POL quirk
v5:
* [mszyprow] rebased and adjusted to the new driver initialization
* drop quirk
v4:
* none
v3:
* enable DSIM_QUIRK_FIXUP_SYNC_POL quirk
v2:
* collect Laurent r-b
v1:
* none
Reviewed-by: Laurent Pinchart
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 44 +++
1 file changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 4de1294f29b3..7ff10308a7ad 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -376,6 +376,24 @@ static const unsigned int exynos5433_reg_values[] = {
[PHYTIMING_HS_TRAIL] = DSIM_PHYTIMING2_HS_TRAIL(0x0c),
};
+static const unsigned int imx8mm_dsim_reg_values[] = {
+ [RESET_TYPE] = DSIM_SWRST,
+ [PLL_TIMER] = 500,
+ [STOP_STATE_CNT] = 0xf,
+ [PHYCTRL_ULPS_EXIT] = 0,
+ [PHYCTRL_VREG_LP] = 0,
+ [PHYCTRL_SLEW_UP] = 0,
+ [PHYTIMING_LPX] = DSIM_PHYTIMING_LPX(0x06),
+ [PHYTIMING_HS_EXIT] = DSIM_PHYTIMING_HS_EXIT(0x0b),
+ [PHYTIMING_CLK_PREPARE] = DSIM_PHYTIMING1_CLK_PREPARE(0x07),
+ [PHYTIMING_CLK_ZERO] = DSIM_PHYTIMING1_CLK_ZERO(0x26),
+ [PHYTIMING_CLK_POST] = DSIM_PHYTIMING1_CLK_POST(0x0d),
+ [PHYTIMING_CLK_TRAIL] = DSIM_PHYTIMING1_CLK_TRAIL(0x08),
+ [PHYTIMING_HS_PREPARE] = DSIM_PHYTIMING2_HS_PREPARE(0x08),
+ [PHYTIMING_HS_ZERO] = DSIM_PHYTIMING2_HS_ZERO(0x0d),
+ [PHYTIMING_HS_TRAIL] = DSIM_PHYTIMING2_HS_TRAIL(0x0b),
+};
+
static const struct samsung_dsim_driver_data exynos3_dsi_driver_data = {
.reg_ofs = exynos_reg_ofs,
.plltmr_reg = 0x50,
@@ -437,6 +455,22 @@ static const struct samsung_dsim_driver_data
exynos5422_dsi_driver_data = {
.reg_values = exynos5422_reg_values,
};
+static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
+ .reg_ofs = exynos5433_reg_ofs,
+ .plltmr_reg = 0xa0,
+ .has_clklane_stop = 1,
+ .num_clks = 2,
+ .max_freq = 2100,
+ .wait_for_reset = 0,
+ .num_bits_resol = 12,
+ /*
+* Unlike Exynos, PLL_P(PMS_P) offset 14 is used in i.MX8M
Mini/Nano/Plus
+* downstream driver - drivers/gpu/drm/bridge/sec-dsim.c
+*/
+ .pll_p_offset = 14,
+ .reg_values = imx8mm_dsim_reg_values,
+};
+
static const struct samsung_dsim_driver_data *
samsung_dsim_types[SAMSUNG_DSIM_TYPE_COUNT] = {
[SAMSUNG_DSIM_TYPE_EXYNOS3250] = _dsi_driver_data,
@@ -444,6 +478,7 @@ samsung_dsim_types[SAMSUNG_DSIM_TYPE_COUNT] = {
[SAMSUNG_DSIM_TYPE_EXYNOS5410] = _dsi_driver_data,
[SAMSUNG_DSIM_TYPE_EXYNOS5422] = _dsi_driver_data,
[SAMSUNG_DSIM_TYPE_EXYNOS5433] = _dsi_driver_data,
+ [SAMSUNG_DSIM_TYPE_IMX8MM] = _dsi_driver_data,
};
static inline struct samsung_dsim *host_to_dsi(struct mipi_dsi_host *h)
@@ -1843,7 +1878,16 @@ const struct dev_pm_ops samsung_dsim_pm_ops = {
};
EXPORT_SYMBOL_GPL(samsung_dsim_pm_ops);
+static const struct samsung_dsim_plat_data samsung_dsim_imx8mm_pdata = {
+ .hw_type = SAMSUNG_DSIM_TYPE_IMX8MM,
+ .host_ops = _dsim_generic_host_ops,
+};
+
static const struct of_device_id samsung_dsim_of_match[] = {
+ {
+ .compatible = "fsl,imx8mm-mipi-dsim",
+ .data = _dsim_imx8mm_pdata,
+ },
{ /* sentinel. */ }
};
MODULE_DEVICE_TABLE(of, samsung_dsim_of_match);
--
2.25.1
[PATCH v9 15/18] dt-bindings: display: exynos: dsim: Add NXP i.MX8M Mini/Nano support
Samsung MIPI DSIM bridge can also be found in i.MX8M Mini/Nano SoC. Add dt-bingings for it. v9: * none v8: * add comment to include i.MX8M Nano. v7, v6, v5, v4: * none v3: * collect Rob Acked-by v2: * updated comments v1: * new patch Acked-by: Rob Herring Signed-off-by: Jagan Teki --- Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt index be377786e8cd..5133d4d39190 100644 --- a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt +++ b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt @@ -7,6 +7,7 @@ Required properties: "samsung,exynos5410-mipi-dsi" /* for Exynos5410/5420/5440 SoCs */ "samsung,exynos5422-mipi-dsi" /* for Exynos5422/5800 SoCs */ "samsung,exynos5433-mipi-dsi" /* for Exynos5433 SoCs */ + "fsl,imx8mm-mipi-dsim" /* for i.MX8M Mini/Nano SoCs */ - reg: physical base address and length of the registers set for the device - interrupts: should contain DSI interrupt - clocks: list of clock specifiers, must contain an entry for each required -- 2.25.1
[PATCH v9 14/18] drm: bridge: samsung-dsim: Add input_bus_flags
LCDIF-DSIM glue logic inverts the HS/VS/DE signals and expecting
the i.MX8M Mini/Nano DSI host to add additional Data Enable signal
active low (DE_LOW). This makes the valid data transfer on each
horizontal line.
So, add additional bus flags DE_LOW setting via input_bus_flags for
i.MX8M Mini/Nano platforms.
v9:
* none
v8:
* add DE_LOW for i.MX8M Mini/Nano platforms.
v7, v6:
* none
v5:
* rebased based on updated bridge changes
v4, v3, v2, v1:
* none
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index ef0d802d25bf..4de1294f29b3 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1654,6 +1654,10 @@ static const struct samsung_dsim_host_ops
samsung_dsim_generic_host_ops = {
.unregister_host = samsung_dsim_unregister_host,
};
+static const struct drm_bridge_timings samsung_dsim_bridge_timings_de_low = {
+ .input_bus_flags = DRM_BUS_FLAG_DE_LOW,
+};
+
int samsung_dsim_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
@@ -1736,6 +1740,10 @@ int samsung_dsim_probe(struct platform_device *pdev)
dsi->bridge.of_node = dev->of_node;
dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
+ /* DE_LOW: i.MX8M Mini/Nano LCDIF-DSIM glue logic inverts HS/VS/DE */
+ if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM)
+ dsi->bridge.timings = _dsim_bridge_timings_de_low;
+
if (dsi->plat_data->host_ops && dsi->plat_data->host_ops->register_host)
ret = dsi->plat_data->host_ops->register_host(dsi);
--
2.25.1
[PATCH v9 13/18] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts
Finding the right input bus format throughout the pipeline is hard
so add atomic_get_input_bus_fmts callback and initialize with the
proper input format from list of supported output formats.
This format can be used in pipeline for negotiating bus format between
the DSI-end of this bridge and the other component closer to pipeline
components.
List of Pixel formats are taken from,
AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
3.7.4 Pixel formats
Table 14. DSI pixel packing formats
v9:
* added MEDIA_BUS_FMT_FIXED
* return MEDIA_BUS_FMT_RGB888_1X24 output_fmt if supported output_fmt
list is unsupported.
* added MEDIA_BUS_FMT_YUYV10_1X20, MEDIA_BUS_FMT_YUYV12_1X24
v8:
* added pixel formats supported by NXP AN13573 i.MX 8/RT MIPI DSI/CSI-2
v7, v6, v5, v4:
* none
v3:
* include media-bus-format.h
v2:
* none
v1:
* new patch
Tested-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 69 +++
1 file changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 9203116f1efb..ef0d802d25bf 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1348,6 +1349,73 @@ static void samsung_dsim_atomic_post_disable(struct
drm_bridge *bridge,
pm_runtime_put_sync(dsi->dev);
}
+/*
+ * This pixel output formats list referenced from,
+ * AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
+ * 3.7.4 Pixel formats
+ * Table 14. DSI pixel packing formats
+ */
+static const u32 samsung_dsim_pixel_output_fmts[] = {
+ MEDIA_BUS_FMT_YUYV10_1X20,
+ MEDIA_BUS_FMT_YUYV12_1X24,
+ MEDIA_BUS_FMT_UYVY8_1X16,
+ MEDIA_BUS_FMT_RGB101010_1X30,
+ MEDIA_BUS_FMT_RGB121212_1X36,
+ MEDIA_BUS_FMT_RGB565_1X16,
+ MEDIA_BUS_FMT_RGB666_1X18,
+ MEDIA_BUS_FMT_RGB888_1X24,
+
+ MEDIA_BUS_FMT_FIXED,
+};
+
+static bool samsung_dsim_pixel_output_fmt_supported(u32 fmt)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(samsung_dsim_pixel_output_fmts); i++) {
+ if (samsung_dsim_pixel_output_fmts[i] == fmt)
+ return true;
+ }
+
+ return false;
+}
+
+static u32 *
+samsung_dsim_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
+{
+ u32 *input_fmts;
+
+ if (!samsung_dsim_pixel_output_fmt_supported(output_fmt))
+ /*
+* Some bridge/display drivers are still not able to pass the
+* correct format, so handle those pipelines by falling back
+* to the default format till the supported formats finalized.
+*/
+ output_fmt = MEDIA_BUS_FMT_RGB888_1X24;
+
+ input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
+ if (!input_fmts)
+ return NULL;
+
+ switch (output_fmt) {
+ case MEDIA_BUS_FMT_FIXED:
+ input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+ break;
+ default:
+ input_fmts[0] = output_fmt;
+ break;
+ }
+
+ *num_input_fmts = 1;
+
+ return input_fmts;
+}
+
static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
struct drm_bridge_state *bridge_state,
struct drm_crtc_state *crtc_state,
@@ -1396,6 +1464,7 @@ static const struct drm_bridge_funcs
samsung_dsim_bridge_funcs = {
.atomic_duplicate_state =
drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state =
drm_atomic_helper_bridge_destroy_state,
.atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_get_input_bus_fmts =
samsung_dsim_atomic_get_input_bus_fmts,
.atomic_check = samsung_dsim_atomic_check,
.atomic_pre_enable = samsung_dsim_atomic_pre_enable,
.atomic_enable = samsung_dsim_atomic_enable,
--
2.25.1
[PATCH v9 12/18] drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset
Look like PLL PMS_P offset value varies between platforms that have
Samsung DSIM IP.
However, there is no clear evidence for it as both Exynos and i.MX
8M Mini Application Processor Reference Manual is still referring
the PMS_P offset as 13.
The offset 13 is not working for i.MX8M Mini SoCs but the downstream
NXP sec-dsim.c driver is using offset 14 for i.MX8M Mini SoC platforms
[1] [2].
PMS_P value set in sec_mipi_dsim_check_pll_out using PLLCTRL_SET_P()
with offset 13 and then an additional offset of one bit added in
sec_mipi_dsim_config_pll via PLLCTRL_SET_PMS().
Not sure whether it is reference manual documentation or something else
but this patch trusts the downstream code and handle PLL_P offset via
platform driver data so-that imx8mm driver data shall use pll_p_offset
to 14.
Similar to Mini the i.MX8M Nano/Plus also has P=14, unlike Exynos.
[1]
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n210
[2]
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n211
v9:
* none
v8:
* updated commit message for 8M Nano/Plus
v7, v6:
* none
v5:
* updated clear commit message
v4, v3, v2:
* none
v1:
* updated commit message
* add downstream driver link
Reviewed-by: Marek Vasut
Signed-off-by: Frieder Schrempf
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 10 --
include/drm/bridge/samsung-dsim.h | 1 +
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index c79f7dc49e17..9203116f1efb 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -184,7 +184,7 @@
/* DSIM_PLLCTRL */
#define DSIM_FREQ_BAND(x) ((x) << 24)
#define DSIM_PLL_EN(1 << 23)
-#define DSIM_PLL_P(x) ((x) << 13)
+#define DSIM_PLL_P(x, offset) ((x) << (offset))
#define DSIM_PLL_M(x) ((x) << 4)
#define DSIM_PLL_S(x) ((x) << 1)
@@ -384,6 +384,7 @@ static const struct samsung_dsim_driver_data
exynos3_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+ .pll_p_offset = 13,
.reg_values = reg_values,
};
@@ -396,6 +397,7 @@ static const struct samsung_dsim_driver_data
exynos4_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+ .pll_p_offset = 13,
.reg_values = reg_values,
};
@@ -406,6 +408,7 @@ static const struct samsung_dsim_driver_data
exynos5_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+ .pll_p_offset = 13,
.reg_values = reg_values,
};
@@ -417,6 +420,7 @@ static const struct samsung_dsim_driver_data
exynos5433_dsi_driver_data = {
.max_freq = 1500,
.wait_for_reset = 0,
.num_bits_resol = 12,
+ .pll_p_offset = 13,
.reg_values = exynos5433_reg_values,
};
@@ -428,6 +432,7 @@ static const struct samsung_dsim_driver_data
exynos5422_dsi_driver_data = {
.max_freq = 1500,
.wait_for_reset = 1,
.num_bits_resol = 12,
+ .pll_p_offset = 13,
.reg_values = exynos5422_reg_values,
};
@@ -559,7 +564,8 @@ static unsigned long samsung_dsim_set_pll(struct
samsung_dsim *dsi,
writel(driver_data->reg_values[PLL_TIMER],
dsi->reg_base + driver_data->plltmr_reg);
- reg = DSIM_PLL_EN | DSIM_PLL_P(p) | DSIM_PLL_M(m) | DSIM_PLL_S(s);
+ reg = DSIM_PLL_EN | DSIM_PLL_P(p, driver_data->pll_p_offset) |
+ DSIM_PLL_M(m) | DSIM_PLL_S(s);
if (driver_data->has_freqband) {
static const unsigned long freq_bands[] = {
diff --git a/include/drm/bridge/samsung-dsim.h
b/include/drm/bridge/samsung-dsim.h
index 0c5a905f3de7..df3d030daec6 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -53,6 +53,7 @@ struct samsung_dsim_driver_data {
unsigned int max_freq;
unsigned int wait_for_reset;
unsigned int num_bits_resol;
+ unsigned int pll_p_offset;
const unsigned int *reg_values;
};
--
2.25.1
[PATCH v9 11/18] drm: bridge: samsung-dsim: Add atomic_check
Look like an explicit fixing up of mode_flags is required for DSIM IP
present in i.MX8M Mini/Nano SoCs.
At least the LCDIF + DSIM needs active low sync polarities in order
to correlate the correct sync flags of the surrounding components in
the chain to make sure the whole pipeline can work properly.
On the other hand the i.MX 8M Mini Applications Processor Reference Manual,
Rev. 3, 11/2020 says.
"13.6.3.5.2 RGB interface
Vsync, Hsync, and VDEN are active high signals."
i.MX 8M Mini Applications Processor Reference Manual Rev. 3, 11/2020
3.6.3.5.2 RGB interface
i.MX 8M Nano Applications Processor Reference Manual Rev. 2, 07/2022
13.6.2.7.2 RGB interface
both claim "Vsync, Hsync, and VDEN are active high signals.", the
LCDIF must generate inverted HS/VS/DE signals, i.e. active LOW.
No clear evidence about whether it can be documentation issues or
something, so added a comment FIXME for this and updated the active low
sync polarities using SAMSUNG_DSIM_TYPE_IMX8MM hw_type.
v9:
* none
v8:
* update the comments about sync signals polarities
* added clear commit message by including i.MX8M Nano details
v7:
* fix the hw_type checking logic
v6:
* none
v5:
* rebase based new bridge changes [mszyprow]
* remove DSIM_QUIRK_FIXUP_SYNC_POL
* add hw_type check for sync polarities change.
v4:
* none
v3:
* add DSIM_QUIRK_FIXUP_SYNC_POL to handle mode_flasg fixup
v2:
* none
v1:
* fix mode flags in atomic_check instead of mode_fixup
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index ec3ab679afd9..c79f7dc49e17 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1342,6 +1342,32 @@ static void samsung_dsim_atomic_post_disable(struct
drm_bridge *bridge,
pm_runtime_put_sync(dsi->dev);
}
+static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
+struct drm_bridge_state *bridge_state,
+struct drm_crtc_state *crtc_state,
+struct drm_connector_state *conn_state)
+{
+ struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+ struct drm_display_mode *adjusted_mode = _state->adjusted_mode;
+
+ /*
+* The i.MX8M Mini/Nano glue logic between LCDIF and DSIM
+* inverts HS/VS/DE sync signals polarity, therefore, while
+* i.MX 8M Mini Applications Processor Reference Manual Rev. 3, 11/2020
+* 13.6.3.5.2 RGB interface
+* i.MX 8M Nano Applications Processor Reference Manual Rev. 2, 07/2022
+* 13.6.2.7.2 RGB interface
+* both claim "Vsync, Hsync, and VDEN are active high signals.", the
+* LCDIF must generate inverted HS/VS/DE signals, i.e. active LOW.
+*/
+ if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM) {
+ adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC |
DRM_MODE_FLAG_NVSYNC);
+ adjusted_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC |
DRM_MODE_FLAG_PVSYNC);
+ }
+
+ return 0;
+}
+
static void samsung_dsim_mode_set(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
const struct drm_display_mode *adjusted_mode)
@@ -1364,6 +1390,7 @@ static const struct drm_bridge_funcs
samsung_dsim_bridge_funcs = {
.atomic_duplicate_state =
drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state =
drm_atomic_helper_bridge_destroy_state,
.atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_check = samsung_dsim_atomic_check,
.atomic_pre_enable = samsung_dsim_atomic_pre_enable,
.atomic_enable = samsung_dsim_atomic_enable,
.atomic_disable = samsung_dsim_atomic_disable,
--
2.25.1
[PATCH v9 10/18] drm: bridge: samsung-dsim: Init exynos host for first DSI transfer
The existing drm panels and bridges in Exynos required host
initialization during the first DSI command transfer even though
the initialization was done before.
This host reinitialization is handled via DSIM_STATE_REINITIALIZED
flag and triggers from host transfer.
Do this exclusively for Exynos.
Initial logic is derived from Marek Szyprowski changes.
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
Changes from v9:
- derived from v8
- added comments
drivers/gpu/drm/bridge/samsung-dsim.c | 15 ++-
include/drm/bridge/samsung-dsim.h | 5 +++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 2e15d753fdd0..ec3ab679afd9 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1254,6 +1254,19 @@ static int samsung_dsim_init(struct samsung_dsim *dsi,
unsigned int flag)
{
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
+ /*
+* FIXME:
+* The existing drm panels and bridges in Exynos required host
+* initialization during the first DSI command transfer even though
+* the initialization was done before.
+*
+* This host reinitialization is handled via DSIM_STATE_REINITIALIZED
+* flag and triggers from host transfer. Do this exclusively for Exynos.
+*/
+ if ((dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM) &&
+ dsi->state & DSIM_STATE_REINITIALIZED)
+ return 0;
+
if (dsi->state & flag)
return 0;
@@ -1467,7 +1480,7 @@ static ssize_t samsung_dsim_host_transfer(struct
mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
- ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+ ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
if (ret)
return ret;
diff --git a/include/drm/bridge/samsung-dsim.h
b/include/drm/bridge/samsung-dsim.h
index b8132bf8e36f..0c5a905f3de7 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -17,8 +17,9 @@ struct samsung_dsim;
#define DSIM_STATE_ENABLED BIT(0)
#define DSIM_STATE_INITIALIZED BIT(1)
-#define DSIM_STATE_CMD_LPM BIT(2)
-#define DSIM_STATE_VIDOUT_AVAILABLEBIT(3)
+#define DSIM_STATE_REINITIALIZED BIT(2)
+#define DSIM_STATE_CMD_LPM BIT(3)
+#define DSIM_STATE_VIDOUT_AVAILABLEBIT(4)
enum samsung_dsim_type {
SAMSUNG_DSIM_TYPE_EXYNOS3250,
--
2.25.1
[PATCH v9 09/18] drm: bridge: samsung-dsim: Add host init in pre_enable
Host transfer() in DSI master will invoke only when the DSI commands
are sent from DSI devices like DSI Panel or DSI bridges and this host
transfer wouldn't invoke for I2C-based-DSI bridge drivers.
Handling DSI host initialization in transfer calls misses the controller
setup for I2C configured DSI bridges.
This patch adds the DSI initialization from transfer to bridge pre_enable
as the bridge pre_enable API is invoked by core as it is common across
all classes of DSI device drivers.
v9:
* added the patch again
v3:
* none
v2:
* check initialized state in samsung_dsim_init
v1:
* keep DSI init in host transfer
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 9adab5d372cc..2e15d753fdd0 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1250,10 +1250,13 @@ static void samsung_dsim_disable_irq(struct
samsung_dsim *dsi)
disable_irq(dsi->irq);
}
-static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int flag)
{
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
+ if (dsi->state & flag)
+ return 0;
+
samsung_dsim_reset(dsi);
samsung_dsim_enable_irq(dsi);
@@ -1266,6 +1269,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi)
samsung_dsim_set_phy_ctrl(dsi);
samsung_dsim_init_link(dsi);
+ dsi->state |= flag;
+
return 0;
}
@@ -1285,6 +1290,10 @@ static void samsung_dsim_atomic_pre_enable(struct
drm_bridge *bridge,
}
dsi->state |= DSIM_STATE_ENABLED;
+
+ ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+ if (ret)
+ return;
}
static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
@@ -1458,12 +1467,9 @@ static ssize_t samsung_dsim_host_transfer(struct
mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
- if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
- ret = samsung_dsim_init(dsi);
- if (ret)
- return ret;
- dsi->state |= DSIM_STATE_INITIALIZED;
- }
+ ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+ if (ret)
+ return ret;
ret = mipi_dsi_create_packet(, msg);
if (ret < 0)
--
2.25.1
[PATCH v9 08/18] drm: bridge: samsung-dsim: Mark PHY as optional
In i.MX8M Mini/Nano SoC the DSI Phy requires a MIPI DPHY bit
to reset in order to activate the PHY and that can be done via
upstream i.MX8M blk-ctrl driver.
So, mark the phy get as optional.
v9, v8, v7, v6, v5, v4, v3, v2:
* none
v1:
* new patch
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index a4379c2ccb77..9adab5d372cc 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1584,7 +1584,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
if (IS_ERR(dsi->reg_base))
return PTR_ERR(dsi->reg_base);
- dsi->phy = devm_phy_get(dev, "dsim");
+ dsi->phy = devm_phy_optional_get(dev, "dsim");
if (IS_ERR(dsi->phy)) {
dev_info(dev, "failed to get dsim phy\n");
return PTR_ERR(dsi->phy);
--
2.25.1
[PATCH v9 07/18] drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices
The child devices in MIPI DSI can be binding with OF-graph
and also via child nodes.
The OF-graph interface represents the child devices via
remote and associated endpoint numbers like
dsi {
compatible = "fsl,imx8mm-mipi-dsim";
ports {
port@0 {
reg = <0>;
dsi_in_lcdif: endpoint@0 {
reg = <0>;
remote-endpoint = <_out_dsi>;
};
};
port@1 {
reg = <1>;
dsi_out_bridge: endpoint {
remote-endpoint = <_in_dsi>;
};
};
};
The child node interface represents the child devices via
conventional child nodes on given DSI parent like
dsi {
compatible = "samsung,exynos5433-mipi-dsi";
ports {
port@0 {
reg = <0>;
dsi_to_mic: endpoint {
remote-endpoint = <_to_dsi>;
};
};
};
panel@0 {
reg = <0>;
};
};
As Samsung DSIM bridge is common DSI IP across all Exynos DSI
and NXP i.MX8M host controllers, this patch adds support to
lookup the child devices whether its bindings on the associated
host represent OF-graph or child node interfaces.
v9, v8, v7, v6, v5, v4, v3:
* none
v2:
* new patch
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 38 +--
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index baad09b2daeb..a4379c2ccb77 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1356,18 +1356,52 @@ static int samsung_dsim_host_attach(struct
mipi_dsi_host *host,
struct samsung_dsim *dsi = host_to_dsi(host);
const struct samsung_dsim_plat_data *pdata = dsi->plat_data;
struct device *dev = dsi->dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *remote;
struct drm_panel *panel;
int ret;
- panel = of_drm_find_panel(device->dev.of_node);
+ /**
+* Devices can also be child nodes when we also control that device
+* through the upstream device (ie, MIPI-DCS for a MIPI-DSI device).
+*
+* Lookup for a child node of the given parent that isn't either port
+* or ports.
+*/
+ for_each_available_child_of_node(np, remote) {
+ if (of_node_name_eq(remote, "port") ||
+ of_node_name_eq(remote, "ports"))
+ continue;
+
+ goto of_find_panel_or_bridge;
+ }
+
+ /*
+* of_graph_get_remote_node() produces a noisy error message if port
+* node isn't found and the absence of the port is a legit case here,
+* so at first we silently check whether graph presents in the
+* device-tree node.
+*/
+ if (!of_graph_is_present(np))
+ return -ENODEV;
+
+ remote = of_graph_get_remote_node(np, 1, 0);
+
+of_find_panel_or_bridge:
+ if (!remote)
+ return -ENODEV;
+
+ panel = of_drm_find_panel(remote);
if (!IS_ERR(panel)) {
dsi->out_bridge = devm_drm_panel_bridge_add(dev, panel);
} else {
- dsi->out_bridge = of_drm_find_bridge(device->dev.of_node);
+ dsi->out_bridge = of_drm_find_bridge(remote);
if (!dsi->out_bridge)
dsi->out_bridge = ERR_PTR(-EINVAL);
}
+ of_node_put(remote);
+
if (IS_ERR(dsi->out_bridge)) {
ret = PTR_ERR(dsi->out_bridge);
DRM_DEV_ERROR(dev, "failed to find the bridge: %d\n", ret);
--
2.25.1
[PATCH v9 06/18] drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge
Samsung MIPI DSIM controller is common DSI IP that can be used in various
SoCs like Exynos, i.MX8M Mini/Nano.
In order to access this DSI controller between various platform SoCs,
the ideal way to incorporate this in the drm stack is via the drm bridge
driver.
This patch is trying to differentiate platform-specific and bridge driver
code by maintaining exynos platform glue code in exynos_drm_dsi.c driver
and common bridge driver code in samsung-dsim.c providing that the new
platform-specific glue should be supported in the bridge driver, unlike
exynos platform drm drivers.
- Add samsung_dsim_plat_data for keeping platform-specific attributes like
host_ops, irq_ops, and hw_type.
- Initialize the plat_data hooks for exynos platform in exynos_drm_dsi.c.
- samsung_dsim_probe is the common probe call across exynos_drm_dsi.c and
samsung-dsim.c.
- plat_data hooks like host_ops and irq_ops are invoked during the
respective bridge call chains.
v9:
* drop the bridge attach fix for Exynos
v8:
* update the commit message head
v7:
* fix the drm bridge attach chain for exynos drm dsi driver
* fix the hw_type checking logic
v6:
* handle previous bridge pointer for exynos dsi
v5:
* [mszyprow] reworked driver initialization using the same approach as in
drivers/gpu/drm/{exynos/exynos_dp.c, bridge/analogix/analogix_dp_core.c},
removed weak functions, moved exynos_dsi_driver back to exynos_drm_dsi.c
and restored integration with exynos_drm custom initialization.
* updated the commit message [Jagan]
v4:
* include Inki Dae in MAINTAINERS
* remove dsi_driver probe in exynos_drm_drv to support multi-arch build
v3:
* restore gpio related fixes
* restore proper bridge chain
* rework initialization issue
* fix header includes in proper way
v2:
* fixed exynos dsi driver conversion (Marek Szyprowski)
* updated commit message
* updated MAINTAINERS file
v1:
* don't maintain component_ops in bridge driver
* don't maintain platform glue code in bridge driver
* add platform-specific glue code and make a common bridge
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
MAINTAINERS |9 +
drivers/gpu/drm/bridge/Kconfig | 12 +
drivers/gpu/drm/bridge/Makefile |1 +
drivers/gpu/drm/bridge/samsung-dsim.c | 1703 ++
drivers/gpu/drm/exynos/Kconfig |1 +
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1784 ++-
include/drm/bridge/samsung-dsim.h | 113 ++
7 files changed, 1952 insertions(+), 1671 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c
create mode 100644 include/drm/bridge/samsung-dsim.h
diff --git a/MAINTAINERS b/MAINTAINERS
index f3edff6b1cad..9a37cce05062 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6675,6 +6675,15 @@ T: git git://anongit.freedesktop.org/drm/drm-misc
F: Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml
F: drivers/gpu/drm/panel/panel-samsung-db7430.c
+DRM DRIVER FOR SAMSUNG MIPI DSIM BRIDGE
+M: Jagan Teki
+M: Marek Szyprowski
+M: Inki Dae
S: Maintained
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 57946d80b02d..8e85dac9f53e 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -231,6 +231,18 @@ config DRM_PARADE_PS8640
The PS8640 is a high-performance and low-power
MIPI DSI to eDP converter
+config DRM_SAMSUNG_DSIM
+ tristate "Samsung MIPI DSIM bridge driver"
+ depends on COMMON_CLK
+ depends on OF && HAS_IOMEM
+ select DRM_KMS_HELPER
+ select DRM_MIPI_DSI
+ select DRM_PANEL_BRIDGE
+ help
+ The Samsung MIPI DSIM bridge controller driver.
+ This MIPI DSIM bridge can be found it on Exynos SoCs and
+ NXP's i.MX8M Mini/Nano.
+
config DRM_SIL_SII8620
tristate "Silicon Image SII8620 HDMI/MHL bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 1884803c6860..dae843723991 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) +=
megachips-stdp-ge-b850v
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
+obj-$(CONFIG_DRM_SAMSUNG_DSIM) += samsung-dsim.o
obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
obj-$(CONFIG_DRM_SII902X) += sii902x.o
obj-$(CONFIG_DRM_SII9234) += sii9234.o
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
new file mode 100644
index ..baad09b2daeb
--- /dev/null
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -0,0 +1,1703 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Samsung MIPI DSIM bridge driver.
+ *
+ * Copyright (C) 2021 Amarula Solutions(India)
+ * Copyright (c) 2014 Samsung Electronics Co.,
[PATCH v9 05/18] drm: exynos: dsi: Properly name HSA/HBP/HFP/HSE bits
HSA/HBP/HFP/HSE mode bits in Processor Reference Manuals specify a naming conversion as 'disable mode bit' due to its bit definition, 0 = Enable and 1 = Disable. For HSE bit, the i.MX 8M Mini/Nano/Plus Applications Processor Reference Manual named this bit as 'HseDisableMode' but the bit definition is quite opposite like 0 = Disables transfer 1 = Enables transfer which clearly states that HSE is not a disable bit. HSE is named as per the manual even though it is not a disable bit however the driver logic for handling HSE is based on the MIPI_DSI_MODE_VIDEO_HSE flag itself. Cc: Nicolas Boichat Signed-off-by: Jagan Teki --- Changes for v9: - new patch drivers/gpu/drm/exynos/exynos_drm_dsi.c | 33 +++-- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 50a2a9ca88a9..b64bb6006b7d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -75,10 +75,27 @@ #define DSIM_MAIN_PIX_FORMAT_RGB565(0x4 << 12) #define DSIM_SUB_VC(((x) & 0x3) << 16) #define DSIM_MAIN_VC (((x) & 0x3) << 18) -#define DSIM_HSA_MODE (1 << 20) -#define DSIM_HBP_MODE (1 << 21) -#define DSIM_HFP_MODE (1 << 22) -#define DSIM_HSE_MODE (1 << 23) +#define DSIM_HSA_DISABLE_MODE (1 << 20) +#define DSIM_HBP_DISABLE_MODE (1 << 21) +#define DSIM_HFP_DISABLE_MODE (1 << 22) +/* + * The i.MX 8M Mini Applications Processor Reference Manual, + * Rev. 3, 11/2020 Page 4091 + * The i.MX 8M Nano Applications Processor Reference Manual, + * Rev. 2, 07/2022 Page 3058 + * The i.MX 8M Plus Applications Processor Reference Manual, + * Rev. 1, 06/2021 Page 5436 + * named this bit as 'HseDisableMode' but the bit definition + * is quite opposite like + * 0 = Disables transfer + * 1 = Enables transfer + * which clearly states that HSE is not a disable bit. + * + * This bit is named as per the manual even though it is not + * a disable bit however the driver logic for handling HSE + * is based on the MIPI_DSI_MODE_VIDEO_HSE flag itself. + */ +#define DSIM_HSE_DISABLE_MODE (1 << 23) #define DSIM_AUTO_MODE (1 << 24) #define DSIM_VIDEO_MODE(1 << 25) #define DSIM_BURST_MODE(1 << 26) @@ -804,13 +821,13 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi) if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_AUTO_VERT) reg |= DSIM_AUTO_MODE; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_HSE) - reg |= DSIM_HSE_MODE; + reg |= DSIM_HSE_DISABLE_MODE; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP) - reg |= DSIM_HFP_MODE; + reg |= DSIM_HFP_DISABLE_MODE; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP) - reg |= DSIM_HBP_MODE; + reg |= DSIM_HBP_DISABLE_MODE; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA) - reg |= DSIM_HSA_MODE; + reg |= DSIM_HSA_DISABLE_MODE; } if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET) -- 2.25.1
[PATCH v9 04/18] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags
HFP/HBP/HSA/EOT_PACKET modes in Exynos DSI host specifies
0 = Enable and 1 = Disable.
The logic for checking these mode flags was correct before
the MIPI_DSI*_NO_* mode flag conversion.
Fix the MIPI_DSI*_NO_* mode flags handling.
Fixes: <0f3b68b66a6d> ("drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling
features")
Reviewed-by: Nicolas Boichat
Reported-by: Sébastien Szymanski
Signed-off-by: Jagan Teki
---
Changes for v9:
- none
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index e5b1540c4ae4..50a2a9ca88a9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -805,15 +805,15 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi)
reg |= DSIM_AUTO_MODE;
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_HSE)
reg |= DSIM_HSE_MODE;
- if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP))
+ if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP)
reg |= DSIM_HFP_MODE;
- if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP))
+ if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP)
reg |= DSIM_HBP_MODE;
- if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA))
+ if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA)
reg |= DSIM_HSA_MODE;
}
- if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
+ if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)
reg |= DSIM_EOT_DISABLE;
switch (dsi->format) {
--
2.25.1
[PATCH v9 03/18] drm: exynos: dsi: Restore proper bridge chain order
From: Marek Szyprowski
Restore the proper bridge chain by finding the previous bridge
in the chain instead of passing NULL.
This establishes a proper bridge chain while attaching downstream
bridges.
v9, v4:
* none
v3:
* new patch
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index ec673223d6b7..e5b1540c4ae4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge,
{
struct exynos_dsi *dsi = bridge_to_dsi(bridge);
- return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags);
+ return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
+flags);
}
static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
@@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host
*host,
drm_bridge_add(>bridge);
- drm_bridge_attach(encoder, >bridge, NULL, 0);
+ drm_bridge_attach(encoder, >bridge,
+ list_first_entry_or_null(>bridge_chain,
+ struct drm_bridge,
+ chain_node), 0);
/*
* This is a temporary solution and should be made by more generic way.
--
2.25.1
[PATCH v9 02/18] drm/bridge: tc358764: Enable pre_enable_prev_first flag
From: Marek Szyprowski Enable the drm bridge pre_enable_prev_first flag so that the previous bridge pre_enable should be called first before the pre_enable for the tc358764 bridge is called. This makes sure that the previous bridge should be initialized properly before the tc358764 bridge is powered up. Signed-off-by: Marek Szyprowski Signed-off-by: Jagan Teki --- Changes for v9: - new patch drivers/gpu/drm/bridge/tc358764.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c index 53259c12d777..f85654f1b104 100644 --- a/drivers/gpu/drm/bridge/tc358764.c +++ b/drivers/gpu/drm/bridge/tc358764.c @@ -369,6 +369,7 @@ static int tc358764_probe(struct mipi_dsi_device *dsi) ctx->bridge.funcs = _bridge_funcs; ctx->bridge.of_node = dev->of_node; + ctx->bridge.pre_enable_prev_first = true; drm_bridge_add(>bridge); -- 2.25.1
[PATCH v9 01/18] drm: panel: Enable prepare_prev_first flag for samsung-s6e panels
Enable the drm panel prepare_prev_first flag so-that the previous
controller should be prepared first before the prepare for the
panel is called.
samsung-s6e3ha2, samsung-s6e63j0x03 and samsung-s6e8aa0 are the
effected samsung-s6e panels for this change.
This makes sure that the previous controller, likely to be a DSI
host controller should be initialized to LP-11 before the panel
is powered up.
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
Changes for v9:
- new patch
drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c| 1 +
drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 1 +
drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 1 +
3 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
index 5c621b15e84c..1355b2c27932 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
@@ -731,6 +731,7 @@ static int s6e3ha2_probe(struct mipi_dsi_device *dsi)
drm_panel_init(>panel, dev, _drm_funcs,
DRM_MODE_CONNECTOR_DSI);
+ ctx->panel.prepare_prev_first = true;
drm_panel_add(>panel);
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
index e06fd35de814..3223a9d06a50 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
@@ -462,6 +462,7 @@ static int s6e63j0x03_probe(struct mipi_dsi_device *dsi)
drm_panel_init(>panel, dev, _funcs,
DRM_MODE_CONNECTOR_DSI);
+ ctx->panel.prepare_prev_first = true;
ctx->bl_dev = backlight_device_register("s6e63j0x03", dev, ctx,
_bl_ops, NULL);
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
b/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
index 54213beafaf5..362eb10f10ce 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
@@ -1018,6 +1018,7 @@ static int s6e8aa0_probe(struct mipi_dsi_device *dsi)
drm_panel_init(>panel, dev, _drm_funcs,
DRM_MODE_CONNECTOR_DSI);
+ ctx->panel.prepare_prev_first = true;
drm_panel_add(>panel);
--
2.25.1
[PATCH v9 00/18] drm: bridge: Add Samsung MIPI DSIM bridge
This series supports common bridge support for Samsung MIPI DSIM which is used in Exynos and i.MX8MM SoC's. The final bridge supports both the Exynos and i.MX8M Mini/Nano/Plus. Changes for v9: - rebase on drm-misc-next - drop drm bridge attach fix for Exynos - added prepare_prev_first flag - added pre_enable_prev_first flag - fix bridge chain order for exynos - added fix for Exynos host init for first DSI transfer - added MEDIA_BUS_FMT_FIXED - return MEDIA_BUS_FMT_RGB888_1X24 output_fmt if supported output_fmt list is unsupported. - added MEDIA_BUS_FMT_YUYV10_1X20 - added MEDIA_BUS_FMT_YUYV12_1X24 Changes for v8: * fixed comment lines * fixed commit messages * fixed video mode bits * collect Marek Ack * fixed video mode bit names * update input formats logic * added imx8mplus support Changes for v7: * fix the drm bridge attach chain for exynos drm dsi driver * fix the hw_type checking logic Changes for v6: * handle previous bridge for exynos dsi while attaching bridge Changes for v5: * bridge changes to support multi-arch * updated and clear commit messages * add hw_type via plat data * removed unneeded quirk * rebased on linux-next t Changes for v4: * include Inki Dae in MAINTAINERS * remove dsi_driver probe in exynos_drm_drv to support multi-arch build * update init handling to ensure host init done on first cmd transfer Changes for v3: * fix the mult-arch build * fix dsi host init * updated commit messages Changes for v2: * fix bridge handling * fix dsi host init * correct the commit messages Tested in Engicam i.Core MX8M Mini SoM. Repo: https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v9 v8: https://lore.kernel.org/all/20221110183853.3678209-1-ja...@amarulasolutions.com/ Any inputs? Jagan. Jagan Teki (16): drm: panel: Enable prepare_prev_first flag for samsung-s6e panels drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags drm: exynos: dsi: Properly name HSA/HBP/HFP/HSE bits drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices drm: bridge: samsung-dsim: Mark PHY as optional drm: bridge: samsung-dsim: Add host init in pre_enable drm: bridge: samsung-dsim: Init exynos host for first DSI transfer drm: bridge: samsung-dsim: Add atomic_check drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts drm: bridge: samsung-dsim: Add input_bus_flags dt-bindings: display: exynos: dsim: Add NXP i.MX8M Mini/Nano support drm: bridge: samsung-dsim: Add i.MX8M Mini/Nano support dt-bindings: display: exynos: dsim: Add NXP i.MX8M Plus support drm: bridge: samsung-dsim: Add i.MX8M Plus support Marek Szyprowski (2): drm/bridge: tc358764: Enable pre_enable_prev_first flag drm: exynos: dsi: Restore proper bridge chain order .../bindings/display/exynos/exynos_dsim.txt |2 + MAINTAINERS |9 + drivers/gpu/drm/bridge/Kconfig| 12 + drivers/gpu/drm/bridge/Makefile |1 + drivers/gpu/drm/bridge/samsung-dsim.c | 1934 + drivers/gpu/drm/bridge/tc358764.c |1 + drivers/gpu/drm/exynos/Kconfig|1 + drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1769 +-- drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c |1 + .../gpu/drm/panel/panel-samsung-s6e63j0x03.c |1 + drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c |1 + include/drm/bridge/samsung-dsim.h | 116 + 12 files changed, 2195 insertions(+), 1653 deletions(-) create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c create mode 100644 include/drm/bridge/samsung-dsim.h -- 2.25.1
Re: [PATCH v11 2/5] dt-bindings: msm/dp: add data-lanes and link-frequencies property
On 09/12/2022 00:38, Kuogee Hsieh wrote: > > On 12/8/2022 3:33 PM, Dmitry Baryshkov wrote: >> On 09/12/2022 00:36, Kuogee Hsieh wrote: >>> Add both data-lanes and link-frequencies property into endpoint >>> >>> Changes in v7: >>> -- split yaml out of dtsi patch >>> -- link-frequencies from link rate to symbol rate >>> -- deprecation of old data-lanes property >>> >>> Changes in v8: >>> -- correct Bjorn mail address to kernel.org >>> >>> Changes in v10: >>> -- add menu item to data-lanes and link-frequecnis >>> >>> Changes in v11: >>> -- add endpoint property at port@1 >>> >>> Signed-off-by: Kuogee Hsieh ` >> >> Applying: dt-bindings: msm/dp: add data-lanes and link-frequencies >> property >> .git/rebase-apply/patch:47: trailing whitespace. >> >> .git/rebase-apply/patch:51: trailing whitespace. >> >> >> Also the dt_binding_check fails with an error for this schema. And >> after fixing the error in the schema I faced an example validation >> error. Did you check that the schema is correct and that the example >> validates against the schema? > > yes, but i run "make dt_binding_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/msm/dp-controller.yaml" > > at mu v5.15 branch since v5.15 branch is not correct branch to work on a kernel. Please do not send patches based on this. You must work on mainline, maintainer's next branch or linux-next. > > "make dt_binding_check" does not work at msm-next branch. Why would it not work there? I doubt that msm-next broke anything... Best regards, Krzysztof
Re: [PATCH v11 1/5] arm64: dts: qcom: add data-lanes and link-freuencies into dp_out endpoint
On 08/12/2022 23:36, Kuogee Hsieh wrote:
> Move data-lanes property from mdss_dp node to dp_out endpoint. Also
> add link-frequencies property into dp_out endpoint as well. The last
> frequency specified at link-frequencies will be the max link rate
> supported by DP.
>
> Changes in v5:
> -- revert changes at sc7180.dtsi and sc7280.dtsi
> -- add _out to sc7180-trogdor.dtsi and sc7280-herobrine.dtsi
>
> Changes in v6:
> -- add data-lanes and link-frequencies to yaml
>
> Changes in v7:
> -- change 16000 to 162000
> -- separate yaml to different patch
>
> Changes in v8:
> -- correct Bjorn mail address to kernel.org
>
> Changes in v9:
> -- use symbol rate (hz) for link-frequencies at dp_out at sc7180_trogdor.dtsi
>
> Signed-off-by: Kuogee Hsieh
> ---
> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 6 +-
> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 6 +-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index eae22e6..93b0cde 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -814,7 +814,11 @@ hp_i2c: {
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <_hot_plug_det>;
> - data-lanes = <0 1>;
> +};
> +
> +_out {
> +data-lanes = <0 1>;
Why adding two spaces? Just cut previous line and paste it, don't change it.
> +link-frequencies = /bits/ 64 <162000 27 54>;
> };
>
> _adc {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index c11e371..3c7a9d8 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -442,7 +442,11 @@ ap_i2c_tpm: {
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <_hot_plug_det>;
> - data-lanes = <0 1>;
> +};
> +
> +_out {
> + data-lanes = <0 1>;
Ditto
Best regards,
Krzysztof
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
On 12/9/22 14:38, Alexander Stein wrote: Am Freitag, 9. Dezember 2022, 13:43:02 CET schrieb Marek Vasut: On 12/9/22 13:21, Alexander Stein wrote: Hi Marek, Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut: On 12/9/22 10:36, Alexander Stein wrote: Hello Krzysztof, Hi, Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: On 09/12/2022 09:54, Alexander Stein wrote: Hello Krzysztof, thanks for the fast feedback. Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: On 09/12/2022 09:33, Alexander Stein wrote: It takes some time until the enable GPIO has settled when turning on. This delay is platform specific and may be caused by e.g. voltage shifts, capacitors etc. Signed-off-by: Alexander Stein --- .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 48a97bb3e2e0d..3f50d497cf8ac 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -32,6 +32,10 @@ properties: maxItems: 1 description: GPIO specifier for bridge_en pin (active high). + ti,enable-delay-us: +default: 1 +description: Enable time delay for enable-gpios Aren't you now mixing two separate delays? One for entire block on (I would assume mostly fixed delay) and one depending on regulators (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the second delays in your power supply? If so, the first one might be fixed and hard-coded in the driver? Apparently there are two different delays: reset time (t_reset) of 10ms as specified by datasheet. This is already ensured by a following delay after requesting enable_gpio as low and switching the GPIO to low in disable path. When enabling this GPIO it takes some time until it is valid on the chip, this is what this series is about. It's highly platform specific. Unfortunately this is completely unrelated to the vcc-supply regulator. This one has to be enabled before the enable GPIO can be enabled. So there is no regulator-ramp-delay. Your driver does one after another - regulator followed immediately by gpio - so this as well can be a delay from regulator (maybe not ramp but enable delay). The chip has two separate input pins: VCC -- power supply that's regulator EN -- reset line, that's GPIO Alexander is talking about EN line here. But this will introduce a section which must not be interrupted or delayed. This is impossible as the enable gpio is attached to an i2c expander in my case. Given the following time chart: vcc set EN enable GPIO PAD ||<-- t_raise -->| | | <-- t_vcc_gpio --> | | | <--t_enable_delay --> | t_raise is the time from changing the GPIO output at the expander until voltage on the EN (input) pad from the bridge has reached high voltage level. This is an electrical characteristic I can not change and have to take into account. t_vcc_gpio is the time from enabling supply voltage to enabling the bridge (removing from reset). Minimum t_vcc_gpio is something which can be addressed by the regulator and is no problem so far. But there is no upper bound to it. What exactly is your EN signal rise time (should be ns or so)? Can you look at that with a scope , maybe even with relation to the VCC regulator ? I checked EN rise time using a scope, it's ~110ms. I not an expert in hardware but on the mainboard there is some capacitor attached to this line, which increased the time, independent from the internal pull-up. This does seem like a hardware bug right there, can you double-check this with the hardware engineer ? Yep, checked with hardware engineer. An 470nF is attached, together with an open drain output and only the internal pull-up. So yes ~113ms rising time until 0.7 x VCC. I don't suppose you can have that capacitor reduced or better yet, some external pull up added, can you ?
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
Am Freitag, 9. Dezember 2022, 13:43:02 CET schrieb Marek Vasut: > On 12/9/22 13:21, Alexander Stein wrote: > > Hi Marek, > > > > Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut: > >> On 12/9/22 10:36, Alexander Stein wrote: > >>> Hello Krzysztof, > >> > >> Hi, > >> > >>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: > On 09/12/2022 09:54, Alexander Stein wrote: > > Hello Krzysztof, > > > > thanks for the fast feedback. > > > > Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: > >> On 09/12/2022 09:33, Alexander Stein wrote: > >>> It takes some time until the enable GPIO has settled when turning > >>> on. > >>> This delay is platform specific and may be caused by e.g. voltage > >>> shifts, capacitors etc. > >>> > >>> Signed-off-by: Alexander Stein > >>> --- > >>> > >>>.../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 > >>> > >>>1 file changed, 4 insertions(+) > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > >>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > >>> index 48a97bb3e2e0d..3f50d497cf8ac 100644 > >>> --- > >>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > >>> +++ > >>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > >>> > >>> @@ -32,6 +32,10 @@ properties: > >>>maxItems: 1 > >>>description: GPIO specifier for bridge_en pin (active high). > >>> > >>> + ti,enable-delay-us: > >>> +default: 1 > >>> +description: Enable time delay for enable-gpios > >> > >> Aren't you now mixing two separate delays? One for entire block on (I > >> would assume mostly fixed delay) and one depending on regulators > >> (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss > >> the > >> second delays in your power supply? If so, the first one might be > >> fixed > >> and hard-coded in the driver? > > > > Apparently there are two different delays: reset time (t_reset) of > > 10ms > > as > > specified by datasheet. This is already ensured by a following delay > > after > > requesting enable_gpio as low and switching the GPIO to low in disable > > path. > > > > When enabling this GPIO it takes some time until it is valid on the > > chip, > > this is what this series is about. It's highly platform specific. > > > > Unfortunately this is completely unrelated to the vcc-supply > > regulator. > > This one has to be enabled before the enable GPIO can be enabled. So > > there is no regulator-ramp-delay. > > Your driver does one after another - regulator followed immediately by > gpio - so this as well can be a delay from regulator (maybe not ramp > but > enable delay). > >> > >> The chip has two separate input pins: > >> > >> VCC -- power supply that's regulator > >> EN -- reset line, that's GPIO > >> > >> Alexander is talking about EN line here. > >> > >>> But this will introduce a section which must not be interrupted or > >>> delayed. > >>> This is impossible as the enable gpio is attached to an i2c expander in > >>> my > >>> case. > >>> > >>> Given the following time chart: > >>>vcc set EN > >>> > >>> enable GPIO PAD > >>> > >>> ||<-- t_raise -->| > >>> | > >>> | <-- t_vcc_gpio --> | | > >>> | <--t_enable_delay --> | > >>> > >>> t_raise is the time from changing the GPIO output at the expander until > >>> voltage on the EN (input) pad from the bridge has reached high voltage > >>> level. This is an electrical characteristic I can not change and have to > >>> take into account. > >>> t_vcc_gpio is the time from enabling supply voltage to enabling the > >>> bridge > >>> (removing from reset). Minimum t_vcc_gpio is something which can be > >>> addressed by the regulator and is no problem so far. But there is no > >>> upper bound to it. > >> > >> What exactly is your EN signal rise time (should be ns or so)? Can you > >> look at that with a scope , maybe even with relation to the VCC regulator > >> ? > > > > I checked EN rise time using a scope, it's ~110ms. I not an expert in > > hardware but on the mainboard there is some capacitor attached to this > > line, which increased the time, independent from the internal pull-up. > > This does seem like a hardware bug right there, can you double-check > this with the hardware engineer ? Yep, checked with hardware engineer. An 470nF is attached, together with an open drain output and only the internal pull-up. So yes ~113ms rising time until 0.7 x VCC. Best regards, Alexander > I would expect the capacitor to charge
Re: [PATCH linux-next] backlight: use sysfs_emit() to instead of scnprintf()
On Mon, Dec 05, 2022 at 03:56:47PM +0800, ye.xingc...@zte.com.cn wrote: > Subject: [PATCH linux-next] backlight: use sysfs_emit() to instead of > scnprintf() Isn't this a v2? (this isn't just a "nice to have"... I ended up delaying review for several days until I had time to look up where I had seen these changes before) > From: ye xingchen > > Follow the advice of the Documentation/filesystems/sysfs.rst and show() > should only use sysfs_emit() or sysfs_emit_at() when formatting the > value to be returned to user space. > > Signed-off-by: ye xingchen Reviewed-by: Daniel Thompson Daniel.
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
On 12/9/22 13:21, Alexander Stein wrote: Hi Marek, Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut: On 12/9/22 10:36, Alexander Stein wrote: Hello Krzysztof, Hi, Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: On 09/12/2022 09:54, Alexander Stein wrote: Hello Krzysztof, thanks for the fast feedback. Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: On 09/12/2022 09:33, Alexander Stein wrote: It takes some time until the enable GPIO has settled when turning on. This delay is platform specific and may be caused by e.g. voltage shifts, capacitors etc. Signed-off-by: Alexander Stein --- .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 48a97bb3e2e0d..3f50d497cf8ac 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -32,6 +32,10 @@ properties: maxItems: 1 description: GPIO specifier for bridge_en pin (active high). + ti,enable-delay-us: +default: 1 +description: Enable time delay for enable-gpios Aren't you now mixing two separate delays? One for entire block on (I would assume mostly fixed delay) and one depending on regulators (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the second delays in your power supply? If so, the first one might be fixed and hard-coded in the driver? Apparently there are two different delays: reset time (t_reset) of 10ms as specified by datasheet. This is already ensured by a following delay after requesting enable_gpio as low and switching the GPIO to low in disable path. When enabling this GPIO it takes some time until it is valid on the chip, this is what this series is about. It's highly platform specific. Unfortunately this is completely unrelated to the vcc-supply regulator. This one has to be enabled before the enable GPIO can be enabled. So there is no regulator-ramp-delay. Your driver does one after another - regulator followed immediately by gpio - so this as well can be a delay from regulator (maybe not ramp but enable delay). The chip has two separate input pins: VCC -- power supply that's regulator EN -- reset line, that's GPIO Alexander is talking about EN line here. But this will introduce a section which must not be interrupted or delayed. This is impossible as the enable gpio is attached to an i2c expander in my case. Given the following time chart: vcc set EN enable GPIO PAD ||<-- t_raise -->| | | <-- t_vcc_gpio --> | | | <--t_enable_delay --> | t_raise is the time from changing the GPIO output at the expander until voltage on the EN (input) pad from the bridge has reached high voltage level. This is an electrical characteristic I can not change and have to take into account. t_vcc_gpio is the time from enabling supply voltage to enabling the bridge (removing from reset). Minimum t_vcc_gpio is something which can be addressed by the regulator and is no problem so far. But there is no upper bound to it. What exactly is your EN signal rise time (should be ns or so)? Can you look at that with a scope , maybe even with relation to the VCC regulator ? I checked EN rise time using a scope, it's ~110ms. I not an expert in hardware but on the mainboard there is some capacitor attached to this line, which increased the time, independent from the internal pull-up. This does seem like a hardware bug right there, can you double-check this with the hardware engineer ? I would expect the capacitor to charge quickly when you flip the I2C expander output HIGH, unless the I2C expander output is open drain, at which point the transistor in the output is closed when the output is set to HIGH and the capacitor is charging over the DSI83 EN pullup , which might be slow.
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
Hi,
Am Freitag, 9. Dezember 2022, 13:10:00 CET schrieb Krzysztof Kozlowski:
> On 09/12/2022 13:02, Marek Vasut wrote:
> > On 12/9/22 10:36, Alexander Stein wrote:
> >> Hello Krzysztof,
> >
> > Hi,
> >
> >> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
> >>> On 09/12/2022 09:54, Alexander Stein wrote:
> Hello Krzysztof,
>
> thanks for the fast feedback.
>
> Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski:
> > On 09/12/2022 09:33, Alexander Stein wrote:
> >> It takes some time until the enable GPIO has settled when turning on.
> >> This delay is platform specific and may be caused by e.g. voltage
> >> shifts, capacitors etc.
> >>
> >> Signed-off-by: Alexander Stein
> >> ---
> >>
> >> .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4
> >>
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> >> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> >> index 48a97bb3e2e0d..3f50d497cf8ac 100644
> >> ---
> >> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> >> +++
> >> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> >>
> >> @@ -32,6 +32,10 @@ properties:
> >> maxItems: 1
> >> description: GPIO specifier for bridge_en pin (active high).
> >>
> >> + ti,enable-delay-us:
> >> +default: 1
> >> +description: Enable time delay for enable-gpios
> >
> > Aren't you now mixing two separate delays? One for entire block on (I
> > would assume mostly fixed delay) and one depending on regulators
> > (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss
> > the
> > second delays in your power supply? If so, the first one might be
> > fixed
> > and hard-coded in the driver?
>
> Apparently there are two different delays: reset time (t_reset) of 10ms
> as
> specified by datasheet. This is already ensured by a following delay
> after
> requesting enable_gpio as low and switching the GPIO to low in disable
> path.
>
> When enabling this GPIO it takes some time until it is valid on the
> chip,
> this is what this series is about. It's highly platform specific.
>
> Unfortunately this is completely unrelated to the vcc-supply regulator.
> This one has to be enabled before the enable GPIO can be enabled. So
> there is no regulator-ramp-delay.
> >>>
> >>> Your driver does one after another - regulator followed immediately by
> >>> gpio - so this as well can be a delay from regulator (maybe not ramp but
> >>> enable delay).
> >
> > The chip has two separate input pins:
> >
> > VCC -- power supply that's regulator
> > EN -- reset line, that's GPIO
> >
> > Alexander is talking about EN line here.
>
> I know, but mainline boards seems to be missing power supply, so that
> raises questions.
>
> Whether voltage on some input pin reaches specified level is hardly a
> problem of only this bridge. OTOH, datasheet describes another delay
> which is specific to the chip and which is fixed - t_en/ten/Ten.
The mainline board doesn't have a bridge node yet, as this issue has not been
solved. In my tree this looks like this:
dsi_lvds_bridge: bridge@2d {
compatible = "ti,sn65dsi83";
reg = <0x2d>;
enable-gpios = < 6 GPIO_ACTIVE_HIGH>;
vcc-supply = <_sn65dsi83_1v8>;
ti,enable-delay-us = <12>;
status = "disabled";
ports {
[...]
};
};
So there is a regulator providing VCC. Once stable enable-gpio is rising EN
pin. The rising time is what is board specific, but completely independent
from VCC supply. But as Krzysztof noticed this is independent from this bridge
driver and can occur everywhere where a GPIO switch/enable is involved.
I have an idea which I'll post on linux-gpio.
Regards,
Alexander
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
Hi Marek, Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut: > On 12/9/22 10:36, Alexander Stein wrote: > > Hello Krzysztof, > > Hi, > > > Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: > >> On 09/12/2022 09:54, Alexander Stein wrote: > >>> Hello Krzysztof, > >>> > >>> thanks for the fast feedback. > >>> > >>> Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: > On 09/12/2022 09:33, Alexander Stein wrote: > > It takes some time until the enable GPIO has settled when turning on. > > This delay is platform specific and may be caused by e.g. voltage > > shifts, capacitors etc. > > > > Signed-off-by: Alexander Stein > > --- > > > > .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 > > > > 1 file changed, 4 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > index 48a97bb3e2e0d..3f50d497cf8ac 100644 > > --- > > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > +++ > > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > > > @@ -32,6 +32,10 @@ properties: > > maxItems: 1 > > description: GPIO specifier for bridge_en pin (active high). > > > > + ti,enable-delay-us: > > +default: 1 > > +description: Enable time delay for enable-gpios > > Aren't you now mixing two separate delays? One for entire block on (I > would assume mostly fixed delay) and one depending on regulators > (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the > second delays in your power supply? If so, the first one might be fixed > and hard-coded in the driver? > >>> > >>> Apparently there are two different delays: reset time (t_reset) of 10ms > >>> as > >>> specified by datasheet. This is already ensured by a following delay > >>> after > >>> requesting enable_gpio as low and switching the GPIO to low in disable > >>> path. > >>> > >>> When enabling this GPIO it takes some time until it is valid on the > >>> chip, > >>> this is what this series is about. It's highly platform specific. > >>> > >>> Unfortunately this is completely unrelated to the vcc-supply regulator. > >>> This one has to be enabled before the enable GPIO can be enabled. So > >>> there is no regulator-ramp-delay. > >> > >> Your driver does one after another - regulator followed immediately by > >> gpio - so this as well can be a delay from regulator (maybe not ramp but > >> enable delay). > > The chip has two separate input pins: > > VCC -- power supply that's regulator > EN -- reset line, that's GPIO > > Alexander is talking about EN line here. > > > But this will introduce a section which must not be interrupted or > > delayed. > > This is impossible as the enable gpio is attached to an i2c expander in my > > case. > > > > Given the following time chart: > > vcc set EN > > > > enable GPIO PAD > > > >||<-- t_raise -->| > >| > >| <-- t_vcc_gpio --> | | > >| <--t_enable_delay --> | > > > > t_raise is the time from changing the GPIO output at the expander until > > voltage on the EN (input) pad from the bridge has reached high voltage > > level. This is an electrical characteristic I can not change and have to > > take into account. > > t_vcc_gpio is the time from enabling supply voltage to enabling the bridge > > (removing from reset). Minimum t_vcc_gpio is something which can be > > addressed by the regulator and is no problem so far. But there is no > > upper bound to it. > What exactly is your EN signal rise time (should be ns or so)? Can you > look at that with a scope , maybe even with relation to the VCC regulator ? I checked EN rise time using a scope, it's ~110ms. I not an expert in hardware but on the mainboard there is some capacitor attached to this line, which increased the time, independent from the internal pull-up. > The DSI84 EN pin already has a built-in pullup per DSI84 datasheet (see > Table 5-1. Pin Functions), so that should make the signal rise fast, > certainly not for seconds. Here it is >100ms, so the current waiting time is far too less. This results in errors regarding PLL lock failure. Best regards, Alexander
Re: [PATCH v11 2/5] dt-bindings: msm/dp: add data-lanes and link-frequencies property
On Thu, 08 Dec 2022 14:36:52 -0800, Kuogee Hsieh wrote:
> Add both data-lanes and link-frequencies property into endpoint
>
> Changes in v7:
> -- split yaml out of dtsi patch
> -- link-frequencies from link rate to symbol rate
> -- deprecation of old data-lanes property
>
> Changes in v8:
> -- correct Bjorn mail address to kernel.org
>
> Changes in v10:
> -- add menu item to data-lanes and link-frequecnis
>
> Changes in v11:
> -- add endpoint property at port@1
>
> Signed-off-by: Kuogee Hsieh `
> ---
> .../bindings/display/msm/dp-controller.yaml| 27
> ++
> 1 file changed, 27 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dp-controller.yaml:
properties:required: ['port@0', 'port@1'] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dp-controller.yaml:
properties: 'required' should not be valid under {'$ref':
'#/definitions/json-schema-prop-names'}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dp-controller.yaml:
ignoring, error in schema: properties: required
Documentation/devicetree/bindings/display/msm/dp-controller.example.dtb:0:0:
/example-0/displayport-controller@ae9: failed to match any schema with
compatible: ['qcom,sc7180-dp']
doc reference errors (make refcheckdocs):
See
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1670539015-11808-3-git-send-email-quic_khs...@quicinc.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Re: [syzbot] WARNING in drm_wait_one_vblank
Hello, syzbot has tested the proposed patch but the reproducer is still triggering an issue: WARNING in drm_wait_one_vblank platform vkms: vblank wait timed out on crtc 0 WARNING: CPU: 1 PID: 4329 at drivers/gpu/drm/drm_vblank.c:1269 drm_wait_one_vblank+0x2bc/0x500 drivers/gpu/drm/drm_vblank.c:1269 Modules linked in: CPU: 1 PID: 4329 Comm: syz-executor.5 Not tainted 6.1.0-rc8-syzkaller-00148-g0d1409e4ff08 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/26/2022 RIP: 0010:drm_wait_one_vblank+0x2bc/0x500 drivers/gpu/drm/drm_vblank.c:1269 Code: 85 f6 0f 84 a3 01 00 00 e8 a1 82 03 fd 4c 89 ef e8 19 34 1b 00 44 89 e1 4c 89 f2 48 c7 c7 80 67 5d 8a 48 89 c6 e8 1b 54 d1 04 <0f> 0b e9 87 fe ff ff e8 78 82 03 fd 31 ff 4c 89 ee e8 5e 7f 03 fd RSP: 0018:c90003887b40 EFLAGS: 00010282 RAX: RBX: 187a RCX: RDX: 888077e56080 RSI: 81615618 RDI: f52000710f5a RBP: 888146b6c000 R08: 0005 R09: R10: 8000 R11: R12: R13: 88801e146010 R14: 888146fb2dc0 R15: 888146ffe030 FS: 7fd446839700() GS:8880b9b0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 55d58fce0300 CR3: 66c1d000 CR4: 003506e0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: drm_fb_helper_ioctl+0x159/0x1a0 drivers/gpu/drm/drm_fb_helper.c:1259 do_fb_ioctl+0x1d5/0x6e0 drivers/video/fbdev/core/fbmem.c:1188 fb_ioctl+0xe7/0x150 drivers/video/fbdev/core/fbmem.c:1202 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:870 [inline] __se_sys_ioctl fs/ioctl.c:856 [inline] __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:856 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7fd445689409 Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 RSP: 002b:7fd446839168 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: 7fd44579bf80 RCX: 7fd445689409 RDX: RSI: 40044620 RDI: 0003 RBP: 7fd4468391d0 R08: R09: R10: R11: 0246 R12: 0001 R13: 7493021f R14: 7fd446839300 R15: 00022000 Tested on: commit: 0d1409e4 Merge tag 'drm-fixes-2022-12-09' of git://ano.. git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master console output: https://syzkaller.appspot.com/x/log.txt?x=10bf8cb788 kernel config: https://syzkaller.appspot.com/x/.config?x=f99d4932d068617a dashboard link: https://syzkaller.appspot.com/bug?extid=6f7fe2dbc479dca0ed17 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 Note: no patches were applied.
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
On 09/12/2022 13:02, Marek Vasut wrote: > On 12/9/22 10:36, Alexander Stein wrote: >> Hello Krzysztof, > > Hi, > >> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: >>> On 09/12/2022 09:54, Alexander Stein wrote: Hello Krzysztof, thanks for the fast feedback. Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: > On 09/12/2022 09:33, Alexander Stein wrote: >> It takes some time until the enable GPIO has settled when turning on. >> This delay is platform specific and may be caused by e.g. voltage >> shifts, capacitors etc. >> >> Signed-off-by: Alexander Stein >> --- >> >> .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> index 48a97bb3e2e0d..3f50d497cf8ac 100644 >> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> >> @@ -32,6 +32,10 @@ properties: >> maxItems: 1 >> description: GPIO specifier for bridge_en pin (active high). >> >> + ti,enable-delay-us: >> +default: 1 >> +description: Enable time delay for enable-gpios > > Aren't you now mixing two separate delays? One for entire block on (I > would assume mostly fixed delay) and one depending on regulators > (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the > second delays in your power supply? If so, the first one might be fixed > and hard-coded in the driver? Apparently there are two different delays: reset time (t_reset) of 10ms as specified by datasheet. This is already ensured by a following delay after requesting enable_gpio as low and switching the GPIO to low in disable path. When enabling this GPIO it takes some time until it is valid on the chip, this is what this series is about. It's highly platform specific. Unfortunately this is completely unrelated to the vcc-supply regulator. This one has to be enabled before the enable GPIO can be enabled. So there is no regulator-ramp-delay. >>> >>> Your driver does one after another - regulator followed immediately by >>> gpio - so this as well can be a delay from regulator (maybe not ramp but >>> enable delay). > > The chip has two separate input pins: > > VCC -- power supply that's regulator > EN -- reset line, that's GPIO > > Alexander is talking about EN line here. I know, but mainline boards seems to be missing power supply, so that raises questions. Whether voltage on some input pin reaches specified level is hardly a problem of only this bridge. OTOH, datasheet describes another delay which is specific to the chip and which is fixed - t_en/ten/Ten. Best regards, Krzysztof
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
On 12/9/22 10:36, Alexander Stein wrote: Hello Krzysztof, Hi, Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: On 09/12/2022 09:54, Alexander Stein wrote: Hello Krzysztof, thanks for the fast feedback. Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: On 09/12/2022 09:33, Alexander Stein wrote: It takes some time until the enable GPIO has settled when turning on. This delay is platform specific and may be caused by e.g. voltage shifts, capacitors etc. Signed-off-by: Alexander Stein --- .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 48a97bb3e2e0d..3f50d497cf8ac 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -32,6 +32,10 @@ properties: maxItems: 1 description: GPIO specifier for bridge_en pin (active high). + ti,enable-delay-us: +default: 1 +description: Enable time delay for enable-gpios Aren't you now mixing two separate delays? One for entire block on (I would assume mostly fixed delay) and one depending on regulators (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the second delays in your power supply? If so, the first one might be fixed and hard-coded in the driver? Apparently there are two different delays: reset time (t_reset) of 10ms as specified by datasheet. This is already ensured by a following delay after requesting enable_gpio as low and switching the GPIO to low in disable path. When enabling this GPIO it takes some time until it is valid on the chip, this is what this series is about. It's highly platform specific. Unfortunately this is completely unrelated to the vcc-supply regulator. This one has to be enabled before the enable GPIO can be enabled. So there is no regulator-ramp-delay. Your driver does one after another - regulator followed immediately by gpio - so this as well can be a delay from regulator (maybe not ramp but enable delay). The chip has two separate input pins: VCC -- power supply that's regulator EN -- reset line, that's GPIO Alexander is talking about EN line here. But this will introduce a section which must not be interrupted or delayed. This is impossible as the enable gpio is attached to an i2c expander in my case. Given the following time chart: vcc set EN enable GPIO PAD || | ||<-- t_raise -->| | <-- t_vcc_gpio --> | | | <--t_enable_delay --> | t_raise is the time from changing the GPIO output at the expander until voltage on the EN (input) pad from the bridge has reached high voltage level. This is an electrical characteristic I can not change and have to take into account. t_vcc_gpio is the time from enabling supply voltage to enabling the bridge (removing from reset). Minimum t_vcc_gpio is something which can be addressed by the regulator and is no problem so far. But there is no upper bound to it. What exactly is your EN signal rise time (should be ns or so)? Can you look at that with a scope , maybe even with relation to the VCC regulator ? The DSI84 EN pin already has a built-in pullup per DSI84 datasheet (see Table 5-1. Pin Functions), so that should make the signal rise fast, certainly not for seconds. [...]
Re: [PATCH v5 01/10] drm: bridge: cadence: convert mailbox functions to macro functions
Hi Sandor,
Am Montag, dem 28.11.2022 um 15:36 +0800 schrieb Sandor Yu:
> Mailbox access functions could be share to other mhdp driver and
> HDP-TX HDMI/DP PHY drivers, move those functions to head file
> include/drm/bridge/cdns-mhdp-mailbox.h and convert them to
> macro functions.
>
This does not look right. If those functions need to be called from
other units and implemented in the header, they should simply be static
inline functions rather than macros. Those macros obfuscate the code a
lot.
However, I don't believe those should be called by other units
directly. From what I can see this is mostly used by the PHY drivers,
to interact with the mailbox and read/write PHY registers. However,
there is no locking between the bridge and the core driver in any form,
so they could corrupt the mailbox state. The PHY drivers have mailbox
mutexes, but as far as I can see they aren't used and even if they were
they would not guard against concurrent access to the mailbox between
the bridge and the PHY driver.
I think the right thing to do here would be to have a regmap
implementation in the bridge driver, which uses the mailbox functions
to implement the register access. Then this regmap could be passed down
from the bridge driver to the PHY, which should simply be a child node
of the bridge in DT.
Regards,
Lucas
> Signed-off-by: Sandor Yu
> ---
> .../drm/bridge/cadence/cdns-mhdp8546-core.c | 197 +-
> .../drm/bridge/cadence/cdns-mhdp8546-core.h | 1 -
> include/drm/bridge/cdns-mhdp-mailbox.h| 240 ++
> 3 files changed, 242 insertions(+), 196 deletions(-)
> create mode 100644 include/drm/bridge/cdns-mhdp-mailbox.h
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 31442a922502..b77b0ddcc9b3 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -36,6 +36,7 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> @@ -55,200 +56,6 @@
> #include "cdns-mhdp8546-hdcp.h"
> #include "cdns-mhdp8546-j721e.h"
>
> -static int cdns_mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
> -{
> - int ret, empty;
> -
> - WARN_ON(!mutex_is_locked(>mbox_mutex));
> -
> - ret = readx_poll_timeout(readl, mhdp->regs + CDNS_MAILBOX_EMPTY,
> - empty, !empty, MAILBOX_RETRY_US,
> - MAILBOX_TIMEOUT_US);
> - if (ret < 0)
> - return ret;
> -
> - return readl(mhdp->regs + CDNS_MAILBOX_RX_DATA) & 0xff;
> -}
> -
> -static int cdns_mhdp_mailbox_write(struct cdns_mhdp_device *mhdp, u8 val)
> -{
> - int ret, full;
> -
> - WARN_ON(!mutex_is_locked(>mbox_mutex));
> -
> - ret = readx_poll_timeout(readl, mhdp->regs + CDNS_MAILBOX_FULL,
> - full, !full, MAILBOX_RETRY_US,
> - MAILBOX_TIMEOUT_US);
> - if (ret < 0)
> - return ret;
> -
> - writel(val, mhdp->regs + CDNS_MAILBOX_TX_DATA);
> -
> - return 0;
> -}
> -
> -static int cdns_mhdp_mailbox_recv_header(struct cdns_mhdp_device *mhdp,
> - u8 module_id, u8 opcode,
> - u16 req_size)
> -{
> - u32 mbox_size, i;
> - u8 header[4];
> - int ret;
> -
> - /* read the header of the message */
> - for (i = 0; i < sizeof(header); i++) {
> - ret = cdns_mhdp_mailbox_read(mhdp);
> - if (ret < 0)
> - return ret;
> -
> - header[i] = ret;
> - }
> -
> - mbox_size = get_unaligned_be16(header + 2);
> -
> - if (opcode != header[0] || module_id != header[1] ||
> - req_size != mbox_size) {
> - /*
> - * If the message in mailbox is not what we want, we need to
> - * clear the mailbox by reading its contents.
> - */
> - for (i = 0; i < mbox_size; i++)
> - if (cdns_mhdp_mailbox_read(mhdp) < 0)
> - break;
> -
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> -static int cdns_mhdp_mailbox_recv_data(struct cdns_mhdp_device *mhdp,
> -u8 *buff, u16 buff_size)
> -{
> - u32 i;
> - int ret;
> -
> - for (i = 0; i < buff_size; i++) {
> - ret = cdns_mhdp_mailbox_read(mhdp);
> - if (ret < 0)
> - return ret;
> -
> - buff[i] = ret;
> - }
> -
> - return 0;
> -}
> -
> -static int cdns_mhdp_mailbox_send(struct cdns_mhdp_device *mhdp, u8
> module_id,
> - u8 opcode, u16 size, u8 *message)
> -{
> - u8 header[4];
> - int ret, i;
> -
> - header[0] = opcode;
> - header[1] = module_id;
> - put_unaligned_be16(size, header + 2);
> -
> - for (i = 0; i < sizeof(header); i++) {
>
Re: [PATCH v5 11/12] arm64: dts: qcom: sc8280xp-crd: Enable EDP
On Thu, 8 Dec 2022 at 00:00, Bjorn Andersson wrote:
>
> From: Bjorn Andersson
>
> The SC8280XP CRD has a EDP display on MDSS0 DP3, enable relevant nodes
> and link it together with the backlight control.
>
> Signed-off-by: Bjorn Andersson
> Signed-off-by: Bjorn Andersson
> ---
>
> Changes since v4:
> - None
>
> arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 72 ++-
> 1 file changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index f09810e3d956..a7d2384cbbe8 100644
[skipped]
> @@ -230,6 +246,54 @@ vreg_l9d: ldo9 {
> };
> };
>
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
> +_dp3 {
> + compatible = "qcom,sc8280xp-edp";
> + status = "okay";
> +
> + data-lanes = <0 1 2 3>;
I hope to land Kuogee patches that move data-lanes to the endpoint
node, where they belong. Do we have any good way to proceed here?
Or would it be easier to land this patch as is and then, maybe next
cycle, move the property?
> +
> + aux-bus {
> + panel {
> + compatible = "edp-panel";
> + power-supply = <_edp_3p3>;
> +
> + backlight = <>;
> +
> + ports {
> + port {
> + edp_panel_in: endpoint {
> + remote-endpoint =
> <_dp3_out>;
> + };
> + };
> + };
> + };
> + };
> +
> + ports {
> + port@1 {
> + reg = <1>;
> + mdss0_dp3_out: endpoint {
> + remote-endpoint = <_panel_in>;
> + };
> + };
> + };
> +};
> +
> +_dp3_phy {
> + status = "okay";
> +
> + vdda-phy-supply = <_l6b>;
> + vdda-pll-supply = <_l3b>;
> +};
> +
> {
> perst-gpios = < 143 GPIO_ACTIVE_LOW>;
> wake-gpios = < 145 GPIO_ACTIVE_LOW>;
> @@ -496,6 +560,12 @@ hastings_reg_en: hastings-reg-en-state {
> {
> gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
>
> + edp_reg_en: edp-reg-en-state {
> + pins = "gpio25";
> + function = "gpio";
> + output-enable;
> + };
> +
> kybd_default: kybd-default-state {
> disable-pins {
> pins = "gpio102";
> --
> 2.37.3
>
--
With best wishes
Dmitry
Re: [PATCH v3 2/2] gpu/drm/panel: Add Sony TD4353 JDI panel driver
On 14.11.2022 15:55, Konrad Dybcio wrote:
>
>
> On 14/10/2022 18:36, Konrad Dybcio wrote:
>>
>>
>> On 30.09.2022 20:08, Konrad Dybcio wrote:
>>> Add support for the Sony TD4353 JDI 2160x1080 display panel used in
>>> some Sony Xperia XZ2 and XZ2 Compact smartphones. Due to the specifics
>>> of smartphone manufacturing, it is impossible to retrieve a better name
>>> for this panel.
>>>
>>> This revision adds support for the default 60 Hz configuration, however
>>> there could possibly be some room for expansion, as the display panels
>>> used on Sony devices have historically been capable of >2x refresh rate
>>> overclocking.
>>>
>>> Signed-off-by: Konrad Dybcio
>>> ---
>> Gentle bump
>>
>> Konrad
> Yet another one.
>
> Konrad
Hello? Anybody there? It's been quite a while..
Konrad
>>> Changes since v2:
>>> - "GPL v2" -> "GPL"
>>> - add missing S-o-b (how embarassing)
>>> - move { after sony_td4353_assert_reset_gpios() to a new line
>>>
>>> drivers/gpu/drm/panel/Kconfig | 10 +
>>> drivers/gpu/drm/panel/Makefile | 1 +
>>> drivers/gpu/drm/panel/panel-sony-td4353-jdi.c | 329 ++
>>> 3 files changed, 340 insertions(+)
>>> create mode 100644 drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
>>>
>>> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
>>> index a582ddd583c2..6ef1b48169b5 100644
>>> --- a/drivers/gpu/drm/panel/Kconfig
>>> +++ b/drivers/gpu/drm/panel/Kconfig
>>> @@ -637,6 +637,16 @@ config DRM_PANEL_SONY_ACX565AKM
>>> Say Y here if you want to enable support for the Sony ACX565AKM
>>> 800x600 3.5" panel (found on the Nokia N900).
>>> +config DRM_PANEL_SONY_TD4353_JDI
>>> + tristate "Sony TD4353 JDI panel"
>>> + depends on GPIOLIB && OF
>>> + depends on DRM_MIPI_DSI
>>> + depends on BACKLIGHT_CLASS_DEVICE
>>> + help
>>> + Say Y here if you want to enable support for the Sony Tama
>>> + TD4353 JDI command mode panel as found on some Sony Xperia
>>> + XZ2 and XZ2 Compact smartphones.
>>> +
>>> config DRM_PANEL_SONY_TULIP_TRULY_NT35521
>>> tristate "Sony Tulip Truly NT35521 panel"
>>> depends on GPIOLIB && OF
>>> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
>>> index 8e71aa7581b8..8ef27bc86f94 100644
>>> --- a/drivers/gpu/drm/panel/Makefile
>>> +++ b/drivers/gpu/drm/panel/Makefile
>>> @@ -64,6 +64,7 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) +=
>>> panel-sitronix-st7701.o
>>> obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7703) += panel-sitronix-st7703.o
>>> obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
>>> obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
>>> +obj-$(CONFIG_DRM_PANEL_SONY_TD4353_JDI) += panel-sony-td4353-jdi.o
>>> obj-$(CONFIG_DRM_PANEL_SONY_TULIP_TRULY_NT35521) +=
>>> panel-sony-tulip-truly-nt35521.o
>>> obj-$(CONFIG_DRM_PANEL_TDO_TL070WSH30) += panel-tdo-tl070wsh30.o
>>> obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
>>> diff --git a/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
>>> b/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
>>> new file mode 100644
>>> index ..11db62992b8b
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/panel/panel-sony-td4353-jdi.c
>>> @@ -0,0 +1,329 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2022 Konrad Dybcio
>>> + *
>>> + * Generated with linux-mdss-dsi-panel-driver-generator with a
>>> + * substantial amount of manual adjustments.
>>> + *
>>> + * SONY Downstream kernel calls this one:
>>> + * - "JDI ID3" for Akari (XZ2)
>>> + * - "JDI ID4" for Apollo (XZ2 Compact)
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#include
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +enum {
>>> + TYPE_TAMA_60HZ,
>>> + /*
>>> + * Leaving room for expansion - SONY very often uses
>>> + * *truly reliably* overclockable panels on their flagships!
>>> + */
>>> +};
>>> +
>>> +struct sony_td4353_jdi {
>>> + struct drm_panel panel;
>>> + struct mipi_dsi_device *dsi;
>>> + struct regulator_bulk_data supplies[3];
>>> + struct gpio_desc *panel_reset_gpio;
>>> + struct gpio_desc *touch_reset_gpio;
>>> + bool prepared;
>>> + int type;
>>> +};
>>> +
>>> +static inline struct sony_td4353_jdi *to_sony_td4353_jdi(struct drm_panel
>>> *panel)
>>> +{
>>> + return container_of(panel, struct sony_td4353_jdi, panel);
>>> +}
>>> +
>>> +static int sony_td4353_jdi_on(struct sony_td4353_jdi *ctx)
>>> +{
>>> + struct mipi_dsi_device *dsi = ctx->dsi;
>>> + struct device *dev = >dev;
>>> + int ret;
>>> +
>>> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
>>> +
>>> + ret = mipi_dsi_dcs_set_column_address(dsi, 0x, 0x0437);
>>> + if (ret < 0) {
>>> + dev_err(dev, "Failed to set column address: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + ret =
Re: [PATCH v5 10/12] arm64: dts: qcom: sc8280xp: Define some of the display blocks
On Wed, Dec 07, 2022 at 02:00:10PM -0800, Bjorn Andersson wrote:
> From: Bjorn Andersson
>
> Define the display clock controllers, the MDSS instances, the DP phys
> and connect these together.
>
> Signed-off-by: Bjorn Andersson
> Signed-off-by: Bjorn Andersson
> ---
>
> Changes since v4:
> - None
>
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 838 +
> 1 file changed, 838 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 9f3132ac2857..c2f186495506 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> + mdss0: display-subsystem@ae0 {
> + compatible = "qcom,sc8280xp-mdss";
> + reg = <0 0x0ae0 0 0x1000>;
> + reg-names = "mdss";
> +
> + power-domains = < MDSS_GDSC>;
> +
> + clocks = < GCC_DISP_AHB_CLK>,
> + < DISP_CC_MDSS_AHB_CLK>,
> + < DISP_CC_MDSS_MDP_CLK>;
> + clock-names = "iface",
> + "ahb",
> + "core";
> +
> + resets = < DISP_CC_MDSS_CORE_BCR>;
> +
> + interrupts = ;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + interconnects = <_noc MASTER_MDP0 0 _virt
> SLAVE_EBI1 0>,
> + <_noc MASTER_MDP1 0 _virt
> SLAVE_EBI1 0>;
> + interconnect-names = "mdp0-mem", "mdp1-mem";
> +
> + iommus = <_smmu 0x1000 0x402>;
> +
> + status = "disabled";
Please move status last.
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + mdss0_mdp: display-controller@ae01000 {
[...]
> + mdss1: display-subsystem@2200 {
> + compatible = "qcom,sc8280xp-mdss";
> + reg = <0 0x2200 0 0x1000>;
> + reg-names = "mdss";
> +
> + power-domains = < MDSS_GDSC>;
> +
> + clocks = < GCC_DISP_AHB_CLK>,
> + < DISP_CC_MDSS_AHB_CLK>,
> + < DISP_CC_MDSS_MDP_CLK>;
> + clock-names = "iface",
> + "ahb",
> + "core";
> +
> + resets = < DISP_CC_MDSS_CORE_BCR>;
> +
> + interrupts = ;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + interconnects = <_noc MASTER_MDP_CORE1_0 0
> _virt SLAVE_EBI1 0>,
> + <_noc MASTER_MDP_CORE1_1 0
> _virt SLAVE_EBI1 0>;
> + interconnect-names = "mdp0-mem", "mdp1-mem";
> +
> + iommus = <_smmu 0x1800 0x402>;
> +
> + status = "disabled";
Same here.
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + mdss1_mdp: display-controller@22001000 {
Johan
Re: [PATCH v5 12/12] arm64: dts: qcom: sa8295-adp: Enable DP instances
On Wed, Dec 07, 2022 at 02:00:12PM -0800, Bjorn Andersson wrote:
> From: Bjorn Andersson
>
> The SA8295P ADP has, among other interfaces, six MiniDP connectors which
> are connected to MDSS0 DP2 and DP3, and MDSS1 DP0 through DP3.
>
> Enable Display Clock controllers, MDSS instanced, MDPs, DP controllers,
> DP PHYs and link them all together.
>
> Signed-off-by: Bjorn Andersson
> Signed-off-by: Bjorn Andersson
> ---
>
> Changes since v4:
> - None
>
> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 243 ++-
> 1 file changed, 241 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> index 6c29d7d757e0..d55c8c5304cc 100644
> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +_dp2 {
> + status = "okay";
Please move 'status' last.
> +
> + data-lanes = <0 1 2 3>;
> +
> + ports {
> + port@1 {
> + reg = <1>;
> + mdss0_dp2_phy_out: endpoint {
> + remote-endpoint = <_connector_in>;
> + };
> + };
> + };
> +};
> +
> +_dp2_phy {
> + status = "okay";
Same here.
> +
> + vdda-phy-supply = <_l8g>;
> + vdda-pll-supply = <_l3g>;
> +};
> +
> +_dp3 {
> + status = "okay";
And here.
> +
> + data-lanes = <0 1 2 3>;
> +
> + ports {
> + port@1 {
> + reg = <1>;
> + mdss0_dp3_phy_out: endpoint {
> + remote-endpoint = <_connector_in>;
> + };
> + };
> + };
> +};
> +
> +_dp3_phy {
> + status = "okay";
And here.
> +
> + vdda-phy-supply = <_l8g>;
> + vdda-pll-supply = <_l3g>;
> +};
> +
> + {
> + status = "okay";
> +};
> +
> +_dp0 {
> + status = "okay";
And here.
> +
> + data-lanes = <0 1 2 3>;
> +
> + ports {
> + port@1 {
> + reg = <1>;
> + mdss1_dp0_phy_out: endpoint {
> + remote-endpoint = <_connector_in>;
> + };
> + };
> + };
> +};
> +
> +_dp0_phy {
> + status = "okay";
Ditto.
> +
> + vdda-phy-supply = <_l11g>;
> + vdda-pll-supply = <_l3g>;
> +};
> +
> +_dp1 {
> + status = "okay";
Ditto.
> +
> + data-lanes = <0 1 2 3>;
> +
> + ports {
> + port@1 {
> + reg = <1>;
> + mdss1_dp1_phy_out: endpoint {
> + remote-endpoint = <_connector_in>;
> + };
> + };
> + };
> +};
> +
> +_dp1_phy {
> + status = "okay";
Ditto.
> +
> + vdda-phy-supply = <_l11g>;
> + vdda-pll-supply = <_l3g>;
> +};
> +
> +_dp2 {
> + status = "okay";
Ditto.
> +
> + data-lanes = <0 1 2 3>;
> +
> + ports {
> + port@1 {
> + reg = <1>;
> + mdss1_dp2_phy_out: endpoint {
> + remote-endpoint = <_connector_in>;
> + };
> + };
> + };
> +};
> +
> +_dp2_phy {
> + status = "okay";
Ditto.
> +
> + vdda-phy-supply = <_l11g>;
> + vdda-pll-supply = <_l3g>;
> +};
> +
> +_dp3 {
> + status = "okay";
Ditto.
> +
> + data-lanes = <0 1 2 3>;
> +
> + ports {
> + port@1 {
> + reg = <1>;
> + mdss1_dp3_phy_out: endpoint {
> + remote-endpoint = <_connector_in>;
> + };
> + };
> + };
> +};
> +
> +_dp3_phy {
> + status = "okay";
Ditto.
> +
> + vdda-phy-supply = <_l11g>;
> + vdda-pll-supply = <_l3g>;
> +};
> +
> {
> perst-gpios = < 143 GPIO_ACTIVE_LOW>;
> wake-gpios = < 145 GPIO_ACTIVE_LOW>;
Johan
Re: [PATCH v5 11/12] arm64: dts: qcom: sc8280xp-crd: Enable EDP
On Wed, Dec 07, 2022 at 02:00:11PM -0800, Bjorn Andersson wrote:
> From: Bjorn Andersson
>
> The SC8280XP CRD has a EDP display on MDSS0 DP3, enable relevant nodes
> and link it together with the backlight control.
>
> Signed-off-by: Bjorn Andersson
> Signed-off-by: Bjorn Andersson
> ---
>
> Changes since v4:
> - None
>
> arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 72 ++-
> 1 file changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index f09810e3d956..a7d2384cbbe8 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> @@ -20,7 +20,7 @@ aliases {
> serial0 = _uart17;
> };
>
> - backlight {
> + backlight: backlight {
> compatible = "pwm-backlight";
> pwms = <_lpg 3 100>;
> enable-gpios = <_1_gpios 8 GPIO_ACTIVE_HIGH>;
> @@ -34,6 +34,22 @@ chosen {
> stdout-path = "serial0:115200n8";
> };
>
> + vreg_edp_3p3: regulator-edp-3p3 {
> + compatible = "regulator-fixed";
> +
> + regulator-name = "VREG_EDP_3P3";
Please use the net name from the schematics here (i.e. "VCC3LCD").
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> +
> + gpio = < 25 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_reg_en>;
> +
> + regulator-boot-on;
> + };
> +
> vreg_edp_bl: regulator-edp-bl {
> compatible = "regulator-fixed";
>
> @@ -230,6 +246,54 @@ vreg_l9d: ldo9 {
> };
> };
>
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
> +_dp3 {
> + compatible = "qcom,sc8280xp-edp";
> + status = "okay";
Please move the status property last (i.e. after data-lanes).
> +
> + data-lanes = <0 1 2 3>;
> +
> + aux-bus {
> + panel {
> + compatible = "edp-panel";
> + power-supply = <_edp_3p3>;
> +
> + backlight = <>;
> +
> + ports {
> + port {
> + edp_panel_in: endpoint {
> + remote-endpoint =
> <_dp3_out>;
> + };
> + };
> + };
> + };
> + };
> +
> + ports {
> + port@1 {
> + reg = <1>;
> + mdss0_dp3_out: endpoint {
> + remote-endpoint = <_panel_in>;
> + };
> + };
> + };
> +};
> +
> +_dp3_phy {
> + status = "okay";
Same here.
> +
> + vdda-phy-supply = <_l6b>;
> + vdda-pll-supply = <_l3b>;
> +};
> +
> {
> perst-gpios = < 143 GPIO_ACTIVE_LOW>;
> wake-gpios = < 145 GPIO_ACTIVE_LOW>;
> @@ -496,6 +560,12 @@ hastings_reg_en: hastings-reg-en-state {
> {
> gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
>
> + edp_reg_en: edp-reg-en-state {
> + pins = "gpio25";
> + function = "gpio";
> + output-enable;
'output-enable' is not valid for tlmm and causes the settings to be
rejected:
sc8280xp-tlmm f10.pinctrl: pin_config_group_set op failed for group
25
reg-fixed-voltage regulator-edp-3p3: Error applying setting, reverse
things back
> + };
> +
> kybd_default: kybd-default-state {
> disable-pins {
> pins = "gpio102";
Johan
Re: Try to address the DMA-buf coherency problem
Am 09.12.22 um 09:26 schrieb Tomasz Figa: [SNIP] Although I think the most common case on mainstream Linux today is properly allocating for device X (e.g. V4L2 video decoder or DRM-based GPU) and hoping that other devices would accept the buffers just fine, which isn't a given on most platforms (although often it's just about pixel format, width/height/stride alignment, tiling, etc. rather than the memory itself). That's why ChromiumOS has minigbm and Android has gralloc that act as the central point of knowledge on buffer allocation. Yeah, completely agree. The crux is really that we need to improve those user space allocators by providing more information from the kernel. 2. We don't properly communicate allocation requirements to userspace. E.g. even if you allocate from DMA-Heaps userspace can currently only guess if normal, CMA or even device specific memory is needed. DMA-buf heaps actually make it even more difficult for the userspace, because now it needs to pick the right heap. With allocation built into the specific UAPI (like V4L2), it's at least possible to allocate for one specific device without having any knowledge about allocation constraints in the userspace. Yes, same what Daniel said as well. We need to provide some more hints which allocator to use from the kernel. So if a device driver uses cached system memory on an architecture which devices which can't access it the right approach is clearly to reject the access. I'd like to accent the fact that "requires cache maintenance" != "can't access". Well that depends. As said above the exporter exports the buffer as it was allocated. If that means the the exporter provides a piece of memory which requires CPU cache snooping to access correctly then the best thing we can do is to prevent an importer which can't do this from attaching. Could you elaborate more about this case? Does it exist in practice? Do I assume correctly that it's about sharing a buffer between one DMA engine that is cache-coherent and another that is non-coherent, where the first one ends up having its accesses always go through some kind of a cache (CPU cache, L2/L3/... cache, etc.)? Yes, exactly that. What happens in this particular use case is that one device driver wrote to it's internal buffer with the CPU (so some cache lines where dirty) and then a device which couldn't deal with that tried to access it. If so, shouldn't that driver surround its CPU accesses with begin/end_cpu_access() in the first place? The problem is that the roles are reversed. The callbacks let the exporter knows that it needs to flush the caches when the importer is done accessing the buffer with the CPU. But here the exporter is the one accessing the buffer with the CPU and the importer then accesses stale data because it doesn't snoop the caches. What we could do is to force all exporters to use begin/end_cpu_access() even on it's own buffers and look at all the importers when the access is completed. But that would increase the complexity of the handling in the exporter. In other words we would then have code in the exporters which is only written for handling the constrains of the importers. This has a wide variety of consequences, especially that this functionality of the exporter can't be tested without a proper importer. I was also thinking about reversing the role of exporter and importer in the kernel, but came to the conclusion that doing this under the hood without userspace knowing about it is probably not going to work either. The case that I was suggesting was of a hardware block that actually sits behind the CPU cache and thus dirties it on writes, not the driver doing that. (I haven't personally encountered such a system, though.) Never heard of anything like that either, but who knows. We could say that all device drivers must always look at the coherency of the devices which want to access their buffers. But that would horrible complicate things for maintaining the drivers because then drivers would need to take into account requirements from other drivers while allocating their internal buffers. I think it's partially why we have the allocation part of the DMA mapping API, but currently it's only handling requirements of one device. And we don't have any information from the userspace what other devices the buffer would be used with... Exactly that, yes. Actually, do we even have such information in the userspace today? Let's say I do a video call in a web browser on a typical Linux system. I have a V4L2 camera, VAAPI video encoder and X11 display. The V4L2 camera fills in buffers with video frames and both encoder and display consume them. Do we have a central place which would know that a buffer needs to be allocated that works with the producer and all consumers? Both X11 and Wayland have protocols which can be used to display a certain DMA-buf handle, their feedback packages contain information how ideal your
Re: [PATCH] drm/bridge: anx7625: keep last configure timing
On Fri, Dec 9, 2022 at 11:05 AM Xin Ji wrote:
>
> Sometimes kernel may resume back quickly after suspend,
> and DRM not call .mode_set() to re-config
> display timing before calling .atomic_enable(), bridge
> driver with this patch to keep last configure timing.
>
> Signed-off-by: Xin Ji
Acked-by: Hsin-Yi Wang
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index b0ff1ecb80a5..eb9116503b63 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1403,7 +1403,6 @@ static void anx7625_stop_dp_work(struct anx7625_data
> *ctx)
> {
> ctx->hpd_status = 0;
> ctx->hpd_high_cnt = 0;
> - ctx->display_timing_valid = 0;
> }
>
> static void anx7625_start_dp_work(struct anx7625_data *ctx)
> --
> 2.25.1
>
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
On 09/12/2022 10:50, Krzysztof Kozlowski wrote: > On 09/12/2022 10:36, Alexander Stein wrote: >> Hello Krzysztof, >> >> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: >>> On 09/12/2022 09:54, Alexander Stein wrote: Hello Krzysztof, thanks for the fast feedback. Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: > On 09/12/2022 09:33, Alexander Stein wrote: >> It takes some time until the enable GPIO has settled when turning on. >> This delay is platform specific and may be caused by e.g. voltage >> shifts, capacitors etc. >> >> Signed-off-by: Alexander Stein >> --- >> >> .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> index 48a97bb3e2e0d..3f50d497cf8ac 100644 >> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> >> @@ -32,6 +32,10 @@ properties: >> maxItems: 1 >> description: GPIO specifier for bridge_en pin (active high). >> >> + ti,enable-delay-us: >> +default: 1 >> +description: Enable time delay for enable-gpios > > Aren't you now mixing two separate delays? One for entire block on (I > would assume mostly fixed delay) and one depending on regulators > (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the > second delays in your power supply? If so, the first one might be fixed > and hard-coded in the driver? Apparently there are two different delays: reset time (t_reset) of 10ms as specified by datasheet. This is already ensured by a following delay after requesting enable_gpio as low and switching the GPIO to low in disable path. When enabling this GPIO it takes some time until it is valid on the chip, this is what this series is about. It's highly platform specific. Unfortunately this is completely unrelated to the vcc-supply regulator. This one has to be enabled before the enable GPIO can be enabled. So there is no regulator-ramp-delay. >>> >>> Your driver does one after another - regulator followed immediately by >>> gpio - so this as well can be a delay from regulator (maybe not ramp but >>> enable delay). >> >> But this will introduce a section which must not be interrupted or delayed. >> This is impossible as the enable gpio is attached to an i2c expander in my >> case. >> >> Given the following time chart: >> >> vcc set EN >> enable GPIO PAD >> || | >> ||<-- t_raise -->| >> | <-- t_vcc_gpio --> | | >> | <--t_enable_delay --> | >> >> t_raise is the time from changing the GPIO output at the expander until >> voltage on the EN (input) pad from the bridge has reached high voltage level. >> This is an electrical characteristic I can not change and have to take into >> account. >> t_vcc_gpio is the time from enabling supply voltage to enabling the bridge >> (removing from reset). Minimum t_vcc_gpio is something which can be >> addressed >> by the regulator and is no problem so far. But there is no upper bound to it. >> >> If I understand you correctly, you want to specify t_enable_delay in a >> regulator property. This only works if you can upper bound t_vcc_gpio which >> is >> not possible due to e.g. scheduling and i2c bus contention. >> >> IMHO that's why there needs to be an configurable delay in the bridge driver. > > What I am saying that you might be here mixing two separate delays: > regulator enable and/or ramp delay (which more or less matches your > t_vcc_gpio) and t_raise. I don't know about which board we talk, but the > mainline users of this bridge do not have even regulator supply, > therefore its enable time might be not factored. > > Why this all raising questions? Because only your t_vcc_gpio should be > board dependent, right? Your bridge has fixed internal delays - from > datasheet: ten, tdis and treset. Nothing in your device is board > specific, thus I assume any enable delay is coming from power supply. > > Probably experiment to prove it would be to keep power supply enabled > always and check on the scope of EN pin. > > Anyway, even if this is variable delay on your EN input pin, it is still > input to the device and based on your time-diagram it is not a property > of the device. Property of the device could be: > > EN-pad goes high <--> output pins stable > > which is either: > 1. ten already described in datasheet, > 2. not the case here. Ah, I forgot, otherwise this is a generic property
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
On 09/12/2022 10:36, Alexander Stein wrote: > Hello Krzysztof, > > Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: >> On 09/12/2022 09:54, Alexander Stein wrote: >>> Hello Krzysztof, >>> >>> thanks for the fast feedback. >>> >>> Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: On 09/12/2022 09:33, Alexander Stein wrote: > It takes some time until the enable GPIO has settled when turning on. > This delay is platform specific and may be caused by e.g. voltage > shifts, capacitors etc. > > Signed-off-by: Alexander Stein > --- > > .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 > 1 file changed, 4 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > index 48a97bb3e2e0d..3f50d497cf8ac 100644 > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > @@ -32,6 +32,10 @@ properties: > maxItems: 1 > description: GPIO specifier for bridge_en pin (active high). > > + ti,enable-delay-us: > +default: 1 > +description: Enable time delay for enable-gpios Aren't you now mixing two separate delays? One for entire block on (I would assume mostly fixed delay) and one depending on regulators (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the second delays in your power supply? If so, the first one might be fixed and hard-coded in the driver? >>> >>> Apparently there are two different delays: reset time (t_reset) of 10ms as >>> specified by datasheet. This is already ensured by a following delay after >>> requesting enable_gpio as low and switching the GPIO to low in disable >>> path. >>> >>> When enabling this GPIO it takes some time until it is valid on the chip, >>> this is what this series is about. It's highly platform specific. >>> >>> Unfortunately this is completely unrelated to the vcc-supply regulator. >>> This one has to be enabled before the enable GPIO can be enabled. So >>> there is no regulator-ramp-delay. >> >> Your driver does one after another - regulator followed immediately by >> gpio - so this as well can be a delay from regulator (maybe not ramp but >> enable delay). > > But this will introduce a section which must not be interrupted or delayed. > This is impossible as the enable gpio is attached to an i2c expander in my > case. > > Given the following time chart: > > vcc set EN > enable GPIO PAD > || | > ||<-- t_raise -->| > | <-- t_vcc_gpio --> | | > | <--t_enable_delay --> | > > t_raise is the time from changing the GPIO output at the expander until > voltage on the EN (input) pad from the bridge has reached high voltage level. > This is an electrical characteristic I can not change and have to take into > account. > t_vcc_gpio is the time from enabling supply voltage to enabling the bridge > (removing from reset). Minimum t_vcc_gpio is something which can be addressed > by the regulator and is no problem so far. But there is no upper bound to it. > > If I understand you correctly, you want to specify t_enable_delay in a > regulator property. This only works if you can upper bound t_vcc_gpio which > is > not possible due to e.g. scheduling and i2c bus contention. > > IMHO that's why there needs to be an configurable delay in the bridge driver. What I am saying that you might be here mixing two separate delays: regulator enable and/or ramp delay (which more or less matches your t_vcc_gpio) and t_raise. I don't know about which board we talk, but the mainline users of this bridge do not have even regulator supply, therefore its enable time might be not factored. Why this all raising questions? Because only your t_vcc_gpio should be board dependent, right? Your bridge has fixed internal delays - from datasheet: ten, tdis and treset. Nothing in your device is board specific, thus I assume any enable delay is coming from power supply. Probably experiment to prove it would be to keep power supply enabled always and check on the scope of EN pin. Anyway, even if this is variable delay on your EN input pin, it is still input to the device and based on your time-diagram it is not a property of the device. Property of the device could be: EN-pad goes high <--> output pins stable which is either: 1. ten already described in datasheet, 2. not the case here. Best regards, Krzysztof
Re: [PATCH] drm/ast: Fix no display at WayLand after power-off
Hi
Am 09.12.22 um 10:11 schrieb Jammy Huang:
With WayLand, there is error log when display waken up from power-off:
gnome-shell: Failed to post KMS update: CRTC property (GAMMA_LUT) not found
gnome-shell: Page flip discarded: CRTC property (GAMMA_LUT) not found
To fix the issue, enable GAMMA_LUT property on CRTC.
This code has long been replaced by commit ce7fcf700386 ("drm/ast: Add
Atomic gamma lut support for aspeed"), which adds proper color
management to ast.
Please only submit patches for the latest driver in drm-misc-next.
Best regards
Thomas
Signed-off-by: Jammy Huang
---
drivers/gpu/drm/ast/ast_drv.h | 2 ++
drivers/gpu/drm/ast/ast_mode.c | 5 +++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 2e44b971c3a6..fd9af1cf0563 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -86,6 +86,8 @@ enum ast_tx_chip {
#define AST_DRAM_4Gx16 7
#define AST_DRAM_8Gx16 8
+#define MAX_COLOR_LUT_ENTRIES 256
+
/*
* Cursor plane
*/
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 1bc0220e6783..87bd9697bb44 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -74,7 +74,7 @@ static void ast_crtc_load_lut(struct ast_private *ast, struct
drm_crtc *crtc)
g = r + crtc->gamma_size;
b = g + crtc->gamma_size;
- for (i = 0; i < 256; i++)
+ for (i = 0; i < MAX_COLOR_LUT_ENTRIES; i++)
ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
}
@@ -1323,7 +1323,8 @@ static int ast_crtc_init(struct drm_device *dev)
if (ret)
return ret;
- drm_mode_crtc_set_gamma_size(crtc, 256);
+ drm_crtc_enable_color_mgmt(crtc, 0, false, MAX_COLOR_LUT_ENTRIES);
+ drm_mode_crtc_set_gamma_size(crtc, MAX_COLOR_LUT_ENTRIES);
drm_crtc_helper_add(crtc, _crtc_helper_funcs);
return 0;
base-commit: 8ed710da2873c2aeb3bb805864a699affaf1d03b
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
Hello Krzysztof, Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski: > On 09/12/2022 09:54, Alexander Stein wrote: > > Hello Krzysztof, > > > > thanks for the fast feedback. > > > > Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: > >> On 09/12/2022 09:33, Alexander Stein wrote: > >>> It takes some time until the enable GPIO has settled when turning on. > >>> This delay is platform specific and may be caused by e.g. voltage > >>> shifts, capacitors etc. > >>> > >>> Signed-off-by: Alexander Stein > >>> --- > >>> > >>> .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 > >>> 1 file changed, 4 insertions(+) > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > >>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > >>> index 48a97bb3e2e0d..3f50d497cf8ac 100644 > >>> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > >>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > >>> > >>> @@ -32,6 +32,10 @@ properties: > >>> maxItems: 1 > >>> description: GPIO specifier for bridge_en pin (active high). > >>> > >>> + ti,enable-delay-us: > >>> +default: 1 > >>> +description: Enable time delay for enable-gpios > >> > >> Aren't you now mixing two separate delays? One for entire block on (I > >> would assume mostly fixed delay) and one depending on regulators > >> (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the > >> second delays in your power supply? If so, the first one might be fixed > >> and hard-coded in the driver? > > > > Apparently there are two different delays: reset time (t_reset) of 10ms as > > specified by datasheet. This is already ensured by a following delay after > > requesting enable_gpio as low and switching the GPIO to low in disable > > path. > > > > When enabling this GPIO it takes some time until it is valid on the chip, > > this is what this series is about. It's highly platform specific. > > > > Unfortunately this is completely unrelated to the vcc-supply regulator. > > This one has to be enabled before the enable GPIO can be enabled. So > > there is no regulator-ramp-delay. > > Your driver does one after another - regulator followed immediately by > gpio - so this as well can be a delay from regulator (maybe not ramp but > enable delay). But this will introduce a section which must not be interrupted or delayed. This is impossible as the enable gpio is attached to an i2c expander in my case. Given the following time chart: vcc set EN enable GPIO PAD || | ||<-- t_raise -->| | <-- t_vcc_gpio --> | | | <--t_enable_delay --> | t_raise is the time from changing the GPIO output at the expander until voltage on the EN (input) pad from the bridge has reached high voltage level. This is an electrical characteristic I can not change and have to take into account. t_vcc_gpio is the time from enabling supply voltage to enabling the bridge (removing from reset). Minimum t_vcc_gpio is something which can be addressed by the regulator and is no problem so far. But there is no upper bound to it. If I understand you correctly, you want to specify t_enable_delay in a regulator property. This only works if you can upper bound t_vcc_gpio which is not possible due to e.g. scheduling and i2c bus contention. IMHO that's why there needs to be an configurable delay in the bridge driver. Best regards, Alexander
Re: Try to address the DMA-buf coherency problem
On Fri, 9 Dec 2022 17:26:06 +0900 Tomasz Figa wrote: > On Mon, Dec 5, 2022 at 5:29 PM Christian König > wrote: > > > > Hi Tomasz, > > > > Am 05.12.22 um 07:41 schrieb Tomasz Figa: > > > [SNIP] > > >> In other words explicit ownership transfer is not something we would > > >> want as requirement in the framework, cause otherwise we break tons of > > >> use cases which require concurrent access to the underlying buffer. > > >> > > >> When a device driver needs explicit ownership transfer it's perfectly > > >> possible to implement this using the dma_fence objects mentioned above. > > >> E.g. drivers can already look at who is accessing a buffer currently and > > >> can even grab explicit ownership of it by adding their own dma_fence > > >> objects. > > >> > > >> The only exception is CPU based access, e.g. when something is written > > >> with the CPU a cache flush might be necessary and when something is read > > >> with the CPU a cache invalidation might be necessary. > > >> > > > Okay, that's much clearer now, thanks for clarifying this. So we > > > should be covered for the cache maintenance needs originating from CPU > > > accesses already, +/- the broken cases which don't call the begin/end > > > CPU access routines that I mentioned above. > > > > > > Similarly, for any ownership transfer between different DMA engines, > > > we should be covered either by the userspace explicitly flushing the > > > hardware pipeline or attaching a DMA-buf fence to the buffer. > > > > > > But then, what's left to be solved? :) (Besides the cases of missing > > > begin/end CPU access calls.) > > > > Well there are multiple problems here: > > > > 1. A lot of userspace applications/frameworks assume that it can > > allocate the buffer anywhere and it just works. > > > > This isn't true at all, we have tons of cases where device can only > > access their special memory for certain use cases. > > Just look at scanout for displaying on dGPU, neither AMD nor NVidia > > supports system memory here. Similar cases exists for audio/video codecs > > where intermediate memory is only accessible by certain devices because > > of content protection. > > Ack. > > Although I think the most common case on mainstream Linux today is > properly allocating for device X (e.g. V4L2 video decoder or DRM-based > GPU) and hoping that other devices would accept the buffers just fine, > which isn't a given on most platforms (although often it's just about > pixel format, width/height/stride alignment, tiling, etc. rather than > the memory itself). That's why ChromiumOS has minigbm and Android has > gralloc that act as the central point of knowledge on buffer > allocation. Hi, as an anecdote, when I was improving Mutter's cross-DRM-device handling (for DisplayLink uses) a few years ago, I implemented several different approaches of where to allocate, to try until going for the slowest but guaranteed to work case of copying every update into and out of sysram. It seems there are two different approaches in general for allocation and sharing: 1. Try different things until it works or you run out of options pro: - no need for a single software component to know everything about every device in the system con: - who bothers with fallbacks, if the first try works on my system for my use case I test with? I.e. cost of code in users. - trial-and-error can be very laborious (allocate, share with all devices, populate, test) - the search space might be huge 2. Have a central component that knows what to do pro: - It might work on the first attempt, so no fallbacks in users. - It might be optimal. con: - You need a software component that knows everything about every single combination of hardware in existence, multiplied by use cases. Neither seems good, which brings us back to https://github.com/cubanismo/allocator . > > 2. We don't properly communicate allocation requirements to userspace. > > > > E.g. even if you allocate from DMA-Heaps userspace can currently only > > guess if normal, CMA or even device specific memory is needed. > > DMA-buf heaps actually make it even more difficult for the userspace, > because now it needs to pick the right heap. With allocation built > into the specific UAPI (like V4L2), it's at least possible to allocate > for one specific device without having any knowledge about allocation > constraints in the userspace. > > > > > 3. We seem to lack some essential parts of those restrictions in the > > documentation. > > > > Ack. > > > So if a device driver uses cached system memory on an architecture > > which > > devices which can't access it the right approach is clearly to reject > > the access. > > >>> I'd like to accent the fact that "requires cache maintenance" != "can't > > >>> access". > > >> Well that depends. As said above the exporter exports the buffer as it > > >> was allocated. > > >> > > >> If that means the the exporter provides a piece of
[PATCH] drm/ast: Fix no display at WayLand after power-off
With WayLand, there is error log when display waken up from power-off:
gnome-shell: Failed to post KMS update: CRTC property (GAMMA_LUT) not found
gnome-shell: Page flip discarded: CRTC property (GAMMA_LUT) not found
To fix the issue, enable GAMMA_LUT property on CRTC.
Signed-off-by: Jammy Huang
---
drivers/gpu/drm/ast/ast_drv.h | 2 ++
drivers/gpu/drm/ast/ast_mode.c | 5 +++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 2e44b971c3a6..fd9af1cf0563 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -86,6 +86,8 @@ enum ast_tx_chip {
#define AST_DRAM_4Gx16 7
#define AST_DRAM_8Gx16 8
+#define MAX_COLOR_LUT_ENTRIES 256
+
/*
* Cursor plane
*/
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 1bc0220e6783..87bd9697bb44 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -74,7 +74,7 @@ static void ast_crtc_load_lut(struct ast_private *ast, struct
drm_crtc *crtc)
g = r + crtc->gamma_size;
b = g + crtc->gamma_size;
- for (i = 0; i < 256; i++)
+ for (i = 0; i < MAX_COLOR_LUT_ENTRIES; i++)
ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
}
@@ -1323,7 +1323,8 @@ static int ast_crtc_init(struct drm_device *dev)
if (ret)
return ret;
- drm_mode_crtc_set_gamma_size(crtc, 256);
+ drm_crtc_enable_color_mgmt(crtc, 0, false, MAX_COLOR_LUT_ENTRIES);
+ drm_mode_crtc_set_gamma_size(crtc, MAX_COLOR_LUT_ENTRIES);
drm_crtc_helper_add(crtc, _crtc_helper_funcs);
return 0;
base-commit: 8ed710da2873c2aeb3bb805864a699affaf1d03b
--
2.25.1
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
On 09/12/2022 09:54, Alexander Stein wrote: > Hello Krzysztof, > > thanks for the fast feedback. > > Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: >> On 09/12/2022 09:33, Alexander Stein wrote: >>> It takes some time until the enable GPIO has settled when turning on. >>> This delay is platform specific and may be caused by e.g. voltage >>> shifts, capacitors etc. >>> >>> Signed-off-by: Alexander Stein >>> --- >>> >>> .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 >>> 1 file changed, 4 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >>> index 48a97bb3e2e0d..3f50d497cf8ac 100644 >>> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >>> >>> @@ -32,6 +32,10 @@ properties: >>> maxItems: 1 >>> description: GPIO specifier for bridge_en pin (active high). >>> >>> + ti,enable-delay-us: >>> +default: 1 >>> +description: Enable time delay for enable-gpios >> >> Aren't you now mixing two separate delays? One for entire block on (I >> would assume mostly fixed delay) and one depending on regulators >> (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the >> second delays in your power supply? If so, the first one might be fixed >> and hard-coded in the driver? > > Apparently there are two different delays: reset time (t_reset) of 10ms as > specified by datasheet. This is already ensured by a following delay after > requesting enable_gpio as low and switching the GPIO to low in disable path. > > When enabling this GPIO it takes some time until it is valid on the chip, > this > is what this series is about. It's highly platform specific. > > Unfortunately this is completely unrelated to the vcc-supply regulator. This > one has to be enabled before the enable GPIO can be enabled. So there is no > regulator-ramp-delay. Your driver does one after another - regulator followed immediately by gpio - so this as well can be a delay from regulator (maybe not ramp but enable delay). Best regards, Krzysztof
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
Hello Krzysztof, thanks for the fast feedback. Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof Kozlowski: > On 09/12/2022 09:33, Alexander Stein wrote: > > It takes some time until the enable GPIO has settled when turning on. > > This delay is platform specific and may be caused by e.g. voltage > > shifts, capacitors etc. > > > > Signed-off-by: Alexander Stein > > --- > > > > .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > index 48a97bb3e2e0d..3f50d497cf8ac 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > > > @@ -32,6 +32,10 @@ properties: > > maxItems: 1 > > description: GPIO specifier for bridge_en pin (active high). > > > > + ti,enable-delay-us: > > +default: 1 > > +description: Enable time delay for enable-gpios > > Aren't you now mixing two separate delays? One for entire block on (I > would assume mostly fixed delay) and one depending on regulators > (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the > second delays in your power supply? If so, the first one might be fixed > and hard-coded in the driver? Apparently there are two different delays: reset time (t_reset) of 10ms as specified by datasheet. This is already ensured by a following delay after requesting enable_gpio as low and switching the GPIO to low in disable path. When enabling this GPIO it takes some time until it is valid on the chip, this is what this series is about. It's highly platform specific. Unfortunately this is completely unrelated to the vcc-supply regulator. This one has to be enabled before the enable GPIO can be enabled. So there is no regulator-ramp-delay. Best regards, Alexander
Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
On 09/12/2022 09:33, Alexander Stein wrote: > It takes some time until the enable GPIO has settled when turning on. > This delay is platform specific and may be caused by e.g. voltage > shifts, capacitors etc. > > Signed-off-by: Alexander Stein > --- > .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 > 1 file changed, 4 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > index 48a97bb3e2e0d..3f50d497cf8ac 100644 > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > @@ -32,6 +32,10 @@ properties: > maxItems: 1 > description: GPIO specifier for bridge_en pin (active high). > > + ti,enable-delay-us: > +default: 1 > +description: Enable time delay for enable-gpios Aren't you now mixing two separate delays? One for entire block on (I would assume mostly fixed delay) and one depending on regulators (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss the second delays in your power supply? If so, the first one might be fixed and hard-coded in the driver? Best regards, Krzysztof
Re: [PATCH v2 2/4] dt-bindings: display/msm: add SoC-specific compats to qcom,mdp5.yaml
On 08/12/2022 01:54, Dmitry Baryshkov wrote: > Add platform-specific compatible entries to the qcom,mdp5.yaml to allow > distinguishing between various platforms. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/qcom,mdp5.yaml | 19 ++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml > index cbcbe8b47e9b..a7a97a4c46b4 100644 > --- a/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml > +++ b/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml > @@ -16,7 +16,24 @@ maintainers: > > properties: >compatible: > -const: qcom,mdp5 > +oneOf: > + - const: qcom,mdp5 > +deprecated: true > + - items: > + - enum: > +# msm8998 should either use old single-string compat or new There is no msm8998 in the list, so what is "new". I propose to use the new, bu tyou need to add it below. > +# qcom,msm8998-dpu > + - qcom,apq8084-mdp5 > + - qcom,msm8916-mdp5 > + - qcom,msm8917-mdp5 > + - qcom,msm8953-mdp5 > + - qcom,msm8974-mdp5 > + - qcom,msm8976-mdp5 > + - qcom,msm8994-mdp5 > + - qcom,msm8996-mdp5 > + - qcom,sdm630-mdp5 > + - qcom,sdm660-mdp5 > + - const: qcom,mdp5 > >reg: > maxItems: 1 Best regards, Krzysztof
Re: [PATCH v2 1/4] dt-bindings: display/msm: convert MDP5 schema to YAML format
On 08/12/2022 01:54, Dmitry Baryshkov wrote: > Convert the mdp5.txt into the yaml format. Changes to the existing (txt) > schema: > - MSM8996 has additional "iommu" clock, define it separately > - Add new properties used on some of platforms: >- interconnects, interconnect-names >- iommus >- power-domains >- operating-points-v2, opp-table > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
[PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property
It takes some time until the enable GPIO has settled when turning on. This delay is platform specific and may be caused by e.g. voltage shifts, capacitors etc. Signed-off-by: Alexander Stein --- .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 48a97bb3e2e0d..3f50d497cf8ac 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -32,6 +32,10 @@ properties: maxItems: 1 description: GPIO specifier for bridge_en pin (active high). + ti,enable-delay-us: +default: 1 +description: Enable time delay for enable-gpios + vcc-supply: description: A 1.8V power supply (see regulator/regulator.yaml). -- 2.34.1
[PATCH 2/2] drm: bridge: ti-sn65dsi83: Add enable delay support
It takes some time until the enable GPIO has settled when turning on.
This delay is platform specific and may be caused by e.g. voltage shifts,
capacitors etc.
Fall back to current default if not specified in DT.
Signed-off-by: Alexander Stein
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 047c14ddbbf11..6510ee384315e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -145,6 +145,7 @@ struct sn65dsi83 {
struct drm_bridge *panel_bridge;
struct gpio_desc*enable_gpio;
struct regulator*vcc;
+ u32 enable_delay;
boollvds_dual_link;
boollvds_dual_link_even_odd_swap;
};
@@ -346,7 +347,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge
*bridge,
/* Deassert reset */
gpiod_set_value_cansleep(ctx->enable_gpio, 1);
- usleep_range(1, 11000);
+ fsleep(ctx->enable_delay);
/* Get the LVDS format from the bridge state. */
bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
@@ -603,6 +604,10 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum
sn65dsi83_model model)
return dev_err_probe(dev, PTR_ERR(ctx->vcc),
"Failed to get supply 'vcc'\n");
+ if (of_property_read_u32(dev->of_node, "ti,enable-delay-us",
+>enable_delay))
+ ctx->enable_delay = 1;
+
return 0;
}
--
2.34.1
[PATCH 0/2] TI SN65DSI83 GPIO enable delay support
Hi all, this small series adds a new optional property for specifying a platform spefic enable delay. The LVDS Bridge requires at least a reset time of 10ms, but this is just the low pulse width. The actual rising time is a different matter and is highly platform specific. My platform has a rising time of ~110ms until the SN signal reaches VCC x 0.7 voltage level. Thus make this time platform configurable. Alexander Stein (2): dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property drm: bridge: ti-sn65dsi83: Add enable delay support .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 7 ++- 2 files changed, 10 insertions(+), 1 deletion(-) -- 2.34.1
Re: Try to address the DMA-buf coherency problem
On Mon, Dec 5, 2022 at 5:29 PM Christian König wrote: > > Hi Tomasz, > > Am 05.12.22 um 07:41 schrieb Tomasz Figa: > > [SNIP] > >> In other words explicit ownership transfer is not something we would > >> want as requirement in the framework, cause otherwise we break tons of > >> use cases which require concurrent access to the underlying buffer. > >> > >> When a device driver needs explicit ownership transfer it's perfectly > >> possible to implement this using the dma_fence objects mentioned above. > >> E.g. drivers can already look at who is accessing a buffer currently and > >> can even grab explicit ownership of it by adding their own dma_fence > >> objects. > >> > >> The only exception is CPU based access, e.g. when something is written > >> with the CPU a cache flush might be necessary and when something is read > >> with the CPU a cache invalidation might be necessary. > >> > > Okay, that's much clearer now, thanks for clarifying this. So we > > should be covered for the cache maintenance needs originating from CPU > > accesses already, +/- the broken cases which don't call the begin/end > > CPU access routines that I mentioned above. > > > > Similarly, for any ownership transfer between different DMA engines, > > we should be covered either by the userspace explicitly flushing the > > hardware pipeline or attaching a DMA-buf fence to the buffer. > > > > But then, what's left to be solved? :) (Besides the cases of missing > > begin/end CPU access calls.) > > Well there are multiple problems here: > > 1. A lot of userspace applications/frameworks assume that it can > allocate the buffer anywhere and it just works. > > This isn't true at all, we have tons of cases where device can only > access their special memory for certain use cases. > Just look at scanout for displaying on dGPU, neither AMD nor NVidia > supports system memory here. Similar cases exists for audio/video codecs > where intermediate memory is only accessible by certain devices because > of content protection. Ack. Although I think the most common case on mainstream Linux today is properly allocating for device X (e.g. V4L2 video decoder or DRM-based GPU) and hoping that other devices would accept the buffers just fine, which isn't a given on most platforms (although often it's just about pixel format, width/height/stride alignment, tiling, etc. rather than the memory itself). That's why ChromiumOS has minigbm and Android has gralloc that act as the central point of knowledge on buffer allocation. > > 2. We don't properly communicate allocation requirements to userspace. > > E.g. even if you allocate from DMA-Heaps userspace can currently only > guess if normal, CMA or even device specific memory is needed. DMA-buf heaps actually make it even more difficult for the userspace, because now it needs to pick the right heap. With allocation built into the specific UAPI (like V4L2), it's at least possible to allocate for one specific device without having any knowledge about allocation constraints in the userspace. > > 3. We seem to lack some essential parts of those restrictions in the > documentation. > Ack. > So if a device driver uses cached system memory on an architecture which > devices which can't access it the right approach is clearly to reject > the access. > >>> I'd like to accent the fact that "requires cache maintenance" != "can't > >>> access". > >> Well that depends. As said above the exporter exports the buffer as it > >> was allocated. > >> > >> If that means the the exporter provides a piece of memory which requires > >> CPU cache snooping to access correctly then the best thing we can do is > >> to prevent an importer which can't do this from attaching. > > Could you elaborate more about this case? Does it exist in practice? > > Do I assume correctly that it's about sharing a buffer between one DMA > > engine that is cache-coherent and another that is non-coherent, where > > the first one ends up having its accesses always go through some kind > > of a cache (CPU cache, L2/L3/... cache, etc.)? > > Yes, exactly that. What happens in this particular use case is that one > device driver wrote to it's internal buffer with the CPU (so some cache > lines where dirty) and then a device which couldn't deal with that tried > to access it. If so, shouldn't that driver surround its CPU accesses with begin/end_cpu_access() in the first place? The case that I was suggesting was of a hardware block that actually sits behind the CPU cache and thus dirties it on writes, not the driver doing that. (I haven't personally encountered such a system, though.) > > We could say that all device drivers must always look at the coherency > of the devices which want to access their buffers. But that would > horrible complicate things for maintaining the drivers because then > drivers would need to take into account requirements from other drivers > while allocating their internal buffers. I think it's partially why we
[PATCH] [next] drm/radeon: Replace 1-element arrays with flexible-array members
One-element arrays are deprecated, and we are replacing them with
flexible array members instead. So, replace one-element array with
flexible-array member in structs _ATOM_DISPLAY_OBJECT_PATH,
_ATOM_DISPLAY_OBJECT_PATH_TABLE, _ATOM_OBJECT_TABLE, GOP_VBIOS_CONTENT
_ATOM_GPIO_VOLTAGE_OBJECT_V3 and refactor the rest of the code accordingly.
It's worth mentioning that doing a build before/after this patch
results in no binary output differences.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/239
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Signed-off-by: Paulo Miguel Almeida
---
Notes for the maintainer:
- These are all fake-flexible arrays with references in source code for
the radeon driver. Given the way they are used, no change to *.c files
were required.
---
drivers/gpu/drm/radeon/atombios.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios.h
b/drivers/gpu/drm/radeon/atombios.h
index 235e59b547a1..8a6621f1e82c 100644
--- a/drivers/gpu/drm/radeon/atombios.h
+++ b/drivers/gpu/drm/radeon/atombios.h
@@ -4020,7 +4020,7 @@ typedef struct _ATOM_DISPLAY_OBJECT_PATH
USHORTusSize;//the size of
ATOM_DISPLAY_OBJECT_PATH
USHORTusConnObjectId;//Connector Object
ID
USHORTusGPUObjectId; //GPU ID
- USHORTusGraphicObjIds[1]; //1st Encoder Obj
source from GPU to last Graphic Obj destinate to connector.
+ USHORTusGraphicObjIds[]; //1st Encoder Obj
source from GPU to last Graphic Obj destinate to connector.
}ATOM_DISPLAY_OBJECT_PATH;
typedef struct _ATOM_DISPLAY_EXTERNAL_OBJECT_PATH
@@ -4037,7 +4037,7 @@ typedef struct _ATOM_DISPLAY_OBJECT_PATH_TABLE
UCHAR ucNumOfDispPath;
UCHAR ucVersion;
UCHAR ucPadding[2];
- ATOM_DISPLAY_OBJECT_PATHasDispPath[1];
+ ATOM_DISPLAY_OBJECT_PATHasDispPath[];
}ATOM_DISPLAY_OBJECT_PATH_TABLE;
@@ -4053,7 +4053,7 @@ typedef struct _ATOM_OBJECT_TABLE
//Above 4 object table
{
UCHAR ucNumberOfObjects;
UCHAR ucPadding[3];
- ATOM_OBJECT asObjects[1];
+ ATOM_OBJECT asObjects[];
}ATOM_OBJECT_TABLE;
typedef struct _ATOM_SRC_DST_TABLE_FOR_ONE_OBJECT
//usSrcDstTableOffset pointing to this structure
@@ -4615,7 +4615,7 @@ typedef struct _ATOM_GPIO_VOLTAGE_OBJECT_V3
UCHARucPhaseDelay;// phase delay in unit of micro second
UCHARucReserved;
ULONGulGpioMaskVal; // GPIO Mask value
- VOLTAGE_LUT_ENTRY_V2 asVolGpioLut[1];
+ VOLTAGE_LUT_ENTRY_V2 asVolGpioLut[];
}ATOM_GPIO_VOLTAGE_OBJECT_V3;
typedef struct _ATOM_LEAKAGE_VOLTAGE_OBJECT_V3
@@ -7964,7 +7964,7 @@ typedef struct {
typedef struct {
VFCT_IMAGE_HEADERVbiosHeader;
- UCHARVbiosContent[1];
+ UCHARVbiosContent[];
}GOP_VBIOS_CONTENT;
typedef struct {
--
2.38.1
