RE: [PATCH 1/1] drm/amdkfd: Add IPC API
[AMD Official Use Only - Internal Distribution Only]
Hi, Felix,
amdgpu_gem_prime_export has different define in the old driver. I added
some comment in the below codes.
Best Regards
Dennis Li
-Original Message-
From: amd-gfx On Behalf Of Felix
Kuehling
Sent: Tuesday, July 14, 2020 11:15 AM
To: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: Deucher, Alexander ; daniel.vet...@ffwll.ch;
airl...@gmail.com
Subject: [PATCH 1/1] drm/amdkfd: Add IPC API
This allows exporting and importing buffers. The API generates handles that can
be used with the HIP IPC API, i.e. big numbers rather than file descriptors.
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 5 +
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 56 +++-
drivers/gpu/drm/amd/amdkfd/Makefile | 3 +-
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 74 ++---
drivers/gpu/drm/amd/amdkfd/kfd_ipc.c | 263 ++
drivers/gpu/drm/amd/amdkfd/kfd_ipc.h | 55
drivers/gpu/drm/amd/amdkfd/kfd_module.c | 5 +
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 5 +
include/uapi/linux/kfd_ioctl.h| 62 -
9 files changed, 488 insertions(+), 40 deletions(-) create mode 100644
drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 3f2b695cf19e..0f8dc4c4f924 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -49,6 +49,7 @@ struct kfd_bo_va_list { struct kgd_mem {
struct mutex lock;
struct amdgpu_bo *bo;
+ struct kfd_ipc_obj *ipc_obj;
struct list_head bo_va_list;
/* protected by amdkfd_process_info.lock */
struct ttm_validate_buffer validate_list; @@ -240,9 +241,13 @@ int
amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
struct dma_buf *dmabuf,
+ struct kfd_ipc_obj *ipc_obj,
uint64_t va, void *vm,
struct kgd_mem **mem, uint64_t *size,
uint64_t *mmap_offset);
+int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
+ struct kgd_mem *mem,
+ struct kfd_ipc_obj **ipc_obj);
void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
void amdgpu_amdkfd_unreserve_memory_limit(struct amdgpu_bo *bo); diff --git
a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index c408936e8f98..cd5f23c0c2ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
#include "amdgpu_vm.h"
#include "amdgpu_amdkfd.h"
#include "amdgpu_dma_buf.h"
+#include "kfd_ipc.h"
#include
/* BO flag to indicate a KFD userptr BO */ @@ -1353,6 +1354,9 @@ int
amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
*size = 0;
}
+ /* Unreference the ipc_obj if applicable */
+ kfd_ipc_obj_put(>ipc_obj);
+
/* Free the BO*/
drm_gem_object_put_unlocked(>bo->tbo.base);
mutex_destroy(>lock);
@@ -1656,6 +1660,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev
*kgd,
int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
struct dma_buf *dma_buf,
+ struct kfd_ipc_obj *ipc_obj,
uint64_t va, void *vm,
struct kgd_mem **mem, uint64_t *size,
uint64_t *mmap_offset)
@@ -1692,15 +1697,18 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev
*kgd,
INIT_LIST_HEAD(&(*mem)->bo_va_list);
mutex_init(&(*mem)->lock);
-
- (*mem)->alloc_flags =
- ((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
- KFD_IOC_ALLOC_MEM_FLAGS_VRAM : KFD_IOC_ALLOC_MEM_FLAGS_GTT)
- | KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
- | KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
+ if (bo->kfd_bo)
+ (*mem)->alloc_flags = bo->kfd_bo->alloc_flags;
+ else
+ (*mem)->alloc_flags =
+ ((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
+ KFD_IOC_ALLOC_MEM_FLAGS_VRAM :
KFD_IOC_ALLOC_MEM_FLAGS_GTT)
+ | KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
+ | KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
drm_gem_object_get(>tbo.base);
(*mem)->bo = bo;
+ (*mem)->ipc_obj = ipc_obj;
(*mem)->va = va;
(*mem)->domain = (bo->preferred_domains &
Re: [PATCH 1/1] drm/amdkfd: Add IPC API
On Tue, 14 Jul 2020 at 14:09, Felix Kuehling wrote: > > Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie: > > On Tue, 14 Jul 2020 at 13:14, Felix Kuehling wrote: > >> This allows exporting and importing buffers. The API generates handles > >> that can be used with the HIP IPC API, i.e. big numbers rather than > >> file descriptors. > > First up why? I get the how. > > The "why" is compatibility with HIP code ported from CUDA. The > equivalent CUDA IPC API works with handles that can be communicated > through e.g. a pipe or shared memory. You can't do that with file > descriptors. Okay that sort of useful information should definitely be in the patch description. > > https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539 > > https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9 > > > > >> + * @share_handle is a 128 bit random number generated with > >> + * @get_random_bytes. This number should be very hard to guess. > >> + * Knowledge of the @share_handle implies authorization to access > >> + * the shared memory. User mode should treat it like a secret key. > >> + * It can be used to import this BO in a different process context > >> + * for IPC buffer sharing. The handle will be valid as long as the > >> + * underlying BO exists. If the same BO is exported multiple times, > > Do we have any examples of any APIs in the kernel that operate like > > this? That don't at least layer some sort of file permissions and > > access control on top? > > SystemV shared memory APIs (shmget, shmat) work similarly. There are > some permissions that can be specified by the exporter in shmget. > However, the handles are just numbers and much easier to guess (they are > 32-bit integers). The most restrictive permissions would allow only the > exporting UID to attach to the shared memory segment. > > I think DRM flink works similarly as well, with a global name IDR used > for looking up GEM objects using global object names. flink is why I asked, because flink was a mistake and not one I'd care to make again. shm is horrible also, but at least has some permissions on what users can attack it. > > The reason fd's are good is that combined with unix sockets, you can't > > sniff it, you can't ptrace a process and find it, you can't write it > > out in a coredump and have someone access it later. > > Arguably ptrace and core dumps give you access to all the memory > contents already. So you don't need the shared memory handle to access > memory in that case. core dumps might not dump this memory though, but yeah ptrace would likely already mean you have access. > > Maybe someone who knows security can ack merging this sort of uAPI > > design, I'm not confident in what it's doing is in any ways a good > > idea. I might have to ask some people to take a closer look. > > Please do. We have tried to make this API as secure as possible within > the constraints of the user mode API we needed to implement. I'll see if I hear back, but also if danvet has any input like I suppose it's UUID based buffer access, so maybe 128-bit is enough and you have enough entropy not to create anything insanely predictable. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] drm/amdkfd: Add IPC API
Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie: > On Tue, 14 Jul 2020 at 13:14, Felix Kuehling wrote: >> This allows exporting and importing buffers. The API generates handles >> that can be used with the HIP IPC API, i.e. big numbers rather than >> file descriptors. > First up why? I get the how. The "why" is compatibility with HIP code ported from CUDA. The equivalent CUDA IPC API works with handles that can be communicated through e.g. a pipe or shared memory. You can't do that with file descriptors. https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539 https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9 > >> + * @share_handle is a 128 bit random number generated with >> + * @get_random_bytes. This number should be very hard to guess. >> + * Knowledge of the @share_handle implies authorization to access >> + * the shared memory. User mode should treat it like a secret key. >> + * It can be used to import this BO in a different process context >> + * for IPC buffer sharing. The handle will be valid as long as the >> + * underlying BO exists. If the same BO is exported multiple times, > Do we have any examples of any APIs in the kernel that operate like > this? That don't at least layer some sort of file permissions and > access control on top? SystemV shared memory APIs (shmget, shmat) work similarly. There are some permissions that can be specified by the exporter in shmget. However, the handles are just numbers and much easier to guess (they are 32-bit integers). The most restrictive permissions would allow only the exporting UID to attach to the shared memory segment. I think DRM flink works similarly as well, with a global name IDR used for looking up GEM objects using global object names. > > The reason fd's are good is that combined with unix sockets, you can't > sniff it, you can't ptrace a process and find it, you can't write it > out in a coredump and have someone access it later. Arguably ptrace and core dumps give you access to all the memory contents already. So you don't need the shared memory handle to access memory in that case. > > To me this isn't secure design, it's obscure design, now I get to find > you've likely shipped this in "downstream" ROCm already, and have > customers running it. > > Maybe someone who knows security can ack merging this sort of uAPI > design, I'm not confident in what it's doing is in any ways a good > idea. I might have to ask some people to take a closer look. Please do. We have tried to make this API as secure as possible within the constraints of the user mode API we needed to implement. Thanks, Felix > > Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/5] drm/i915: Add enable/disable flip done and flip done handler
Hi Karthik,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next
tegra-drm/drm/tegra/for-next v5.8-rc5 next-20200713]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Karthik-B-S/Asynchronous-flip-implementation-for-i915/20200714-095304
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/i915_irq.c:700:5: warning: no previous prototype for
>> 'g4x_get_flip_counter' [-Wmissing-prototypes]
700 | u32 g4x_get_flip_counter(struct drm_crtc *crtc)
| ^~~~
vim +/g4x_get_flip_counter +700 drivers/gpu/drm/i915/i915_irq.c
699
> 700 u32 g4x_get_flip_counter(struct drm_crtc *crtc)
701 {
702 struct drm_i915_private *dev_priv = to_i915(crtc->dev);
703 enum pipe pipe = to_intel_crtc(crtc)->pipe;
704
705 return I915_READ(PIPE_FLIPCOUNT_G4X(pipe));
706 }
707
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] drm/amdkfd: Add IPC API
On Tue, 14 Jul 2020 at 13:14, Felix Kuehling wrote: > > This allows exporting and importing buffers. The API generates handles > that can be used with the HIP IPC API, i.e. big numbers rather than > file descriptors. First up why? I get the how. > + * @share_handle is a 128 bit random number generated with > + * @get_random_bytes. This number should be very hard to guess. > + * Knowledge of the @share_handle implies authorization to access > + * the shared memory. User mode should treat it like a secret key. > + * It can be used to import this BO in a different process context > + * for IPC buffer sharing. The handle will be valid as long as the > + * underlying BO exists. If the same BO is exported multiple times, Do we have any examples of any APIs in the kernel that operate like this? That don't at least layer some sort of file permissions and access control on top? The reason fd's are good is that combined with unix sockets, you can't sniff it, you can't ptrace a process and find it, you can't write it out in a coredump and have someone access it later. To me this isn't secure design, it's obscure design, now I get to find you've likely shipped this in "downstream" ROCm already, and have customers running it. Maybe someone who knows security can ack merging this sort of uAPI design, I'm not confident in what it's doing is in any ways a good idea. I might have to ask some people to take a closer look. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/1] Upstreaming the KFD IPC API
This API is used by MPI/UCX for efficiently sharing VRAM between MPI ranks on the same node. It has been part of the ROCm DKMS branch for a long time. This code is refactored to be less invasive for upstreaming. As a result struct kfd_bo and the associated interval tree is not needed upstream. The introduction of this API upstream bumps the KFD API version to 1.4. The corresponding Thunk change that goes with the proposed upstreaming of this API can be found here: https://github.com/RadeonOpenCompute/ROCT-Thunk-Interface/commit/682922bd405caaab64887e8e702b7d996ba9e2a8 Felix Kuehling (1): drm/amdkfd: Add IPC API drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 5 + .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 56 +++- drivers/gpu/drm/amd/amdkfd/Makefile | 3 +- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 74 ++--- drivers/gpu/drm/amd/amdkfd/kfd_ipc.c | 263 ++ drivers/gpu/drm/amd/amdkfd/kfd_ipc.h | 55 drivers/gpu/drm/amd/amdkfd/kfd_module.c | 5 + drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 5 + include/uapi/linux/kfd_ioctl.h| 62 - 9 files changed, 488 insertions(+), 40 deletions(-) create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/1] drm/amdkfd: Add IPC API
This allows exporting and importing buffers. The API generates handles
that can be used with the HIP IPC API, i.e. big numbers rather than
file descriptors.
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 5 +
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 56 +++-
drivers/gpu/drm/amd/amdkfd/Makefile | 3 +-
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 74 ++---
drivers/gpu/drm/amd/amdkfd/kfd_ipc.c | 263 ++
drivers/gpu/drm/amd/amdkfd/kfd_ipc.h | 55
drivers/gpu/drm/amd/amdkfd/kfd_module.c | 5 +
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 5 +
include/uapi/linux/kfd_ioctl.h| 62 -
9 files changed, 488 insertions(+), 40 deletions(-)
create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 3f2b695cf19e..0f8dc4c4f924 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -49,6 +49,7 @@ struct kfd_bo_va_list {
struct kgd_mem {
struct mutex lock;
struct amdgpu_bo *bo;
+ struct kfd_ipc_obj *ipc_obj;
struct list_head bo_va_list;
/* protected by amdkfd_process_info.lock */
struct ttm_validate_buffer validate_list;
@@ -240,9 +241,13 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev
*kgd,
int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
struct dma_buf *dmabuf,
+ struct kfd_ipc_obj *ipc_obj,
uint64_t va, void *vm,
struct kgd_mem **mem, uint64_t *size,
uint64_t *mmap_offset);
+int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
+ struct kgd_mem *mem,
+ struct kfd_ipc_obj **ipc_obj);
void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
void amdgpu_amdkfd_unreserve_memory_limit(struct amdgpu_bo *bo);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index c408936e8f98..cd5f23c0c2ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
#include "amdgpu_vm.h"
#include "amdgpu_amdkfd.h"
#include "amdgpu_dma_buf.h"
+#include "kfd_ipc.h"
#include
/* BO flag to indicate a KFD userptr BO */
@@ -1353,6 +1354,9 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
*size = 0;
}
+ /* Unreference the ipc_obj if applicable */
+ kfd_ipc_obj_put(>ipc_obj);
+
/* Free the BO*/
drm_gem_object_put_unlocked(>bo->tbo.base);
mutex_destroy(>lock);
@@ -1656,6 +1660,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev
*kgd,
int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
struct dma_buf *dma_buf,
+ struct kfd_ipc_obj *ipc_obj,
uint64_t va, void *vm,
struct kgd_mem **mem, uint64_t *size,
uint64_t *mmap_offset)
@@ -1692,15 +1697,18 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev
*kgd,
INIT_LIST_HEAD(&(*mem)->bo_va_list);
mutex_init(&(*mem)->lock);
-
- (*mem)->alloc_flags =
- ((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
- KFD_IOC_ALLOC_MEM_FLAGS_VRAM : KFD_IOC_ALLOC_MEM_FLAGS_GTT)
- | KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
- | KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
+ if (bo->kfd_bo)
+ (*mem)->alloc_flags = bo->kfd_bo->alloc_flags;
+ else
+ (*mem)->alloc_flags =
+ ((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
+ KFD_IOC_ALLOC_MEM_FLAGS_VRAM :
KFD_IOC_ALLOC_MEM_FLAGS_GTT)
+ | KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
+ | KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
drm_gem_object_get(>tbo.base);
(*mem)->bo = bo;
+ (*mem)->ipc_obj = ipc_obj;
(*mem)->va = va;
(*mem)->domain = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT;
@@ -1713,6 +1721,42 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev
*kgd,
return 0;
}
+int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
+ struct kgd_mem *mem,
+ struct kfd_ipc_obj **ipc_obj)
+{
+ struct amdgpu_device *adev = NULL;
+ struct dma_buf *dmabuf;
+ int r = 0;
+
+ if
linux-next: manual merge of the drm-intel tree with the drm tree
Hi all,
Today's linux-next merge of the drm-intel tree got a conflict in:
drivers/gpu/drm/drm_probe_helper.c
between commit:
12c683e12cd8 ("drm: bridge: Pass drm_display_info to drm_bridge_funcs
.mode_valid()")
from the drm tree and commit:
1c26b8e09004 ("drm/probe_helper: Add
drm_connector_helper_funcs.mode_valid_ctx")
from the drm-intel tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/gpu/drm/drm_probe_helper.c
index 09e872e61315,601a4f25bb47..
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@@ -114,10 -116,8 +116,10 @@@ drm_mode_validate_pipeline(struct drm_d
}
bridge = drm_bridge_chain_get_first_bridge(encoder);
- ret = drm_bridge_chain_mode_valid(bridge,
- >display_info,
- mode);
- if (ret != MODE_OK) {
- *status = drm_bridge_chain_mode_valid(bridge, mode);
++ *status = drm_bridge_chain_mode_valid(bridge,
++>display_info,
++mode);
+ if (*status != MODE_OK) {
/* There is also no point in continuing for crtc check
* here. */
continue;
pgpZzNWWEfzFt.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2.1 1/8] dt-bindings: media: renesas, fcp: Convert binding to YAML
On Wed, 01 Jul 2020 09:05:25 +0300, Laurent Pinchart wrote: > Convert the Renesas R-Car FCP text binding to YAML. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Geert Uytterhoeven > Reviewed-by: Niklas Söderlund > --- > Changes since v2: > > - Refer to the correct device in the comment above the example > > Changes since v1: > > - Simplify comments on compatible strings > - Update MAINTAINERS > --- > .../devicetree/bindings/media/renesas,fcp.txt | 34 --- > .../bindings/media/renesas,fcp.yaml | 56 +++ > MAINTAINERS | 2 +- > 3 files changed, 57 insertions(+), 35 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/media/renesas,fcp.txt > create mode 100644 Documentation/devicetree/bindings/media/renesas,fcp.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 02/10] dt-bindings: display: Add ingenic,ipu.yaml
On Tue, 30 Jun 2020 01:52:02 +0200, Paul Cercueil wrote: > Add documentation of the Device Tree bindings for the Image Processing > Unit (IPU) found in most Ingenic SoCs. > > Signed-off-by: Paul Cercueil > --- > > Notes: > v2: Add missing 'const' in items list > > .../bindings/display/ingenic,ipu.yaml | 65 +++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/ingenic,ipu.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: fix update of display surface when resolution changes
From: Roland Scheidegger
The assignment of metadata overwrote the new display resolution values,
hence we'd miss the size actually changed and wouldn't redefine the
surface. This would then lead to command buffer error when trying to
update the screen target (due to the size mismatch), and result in a
VM with black screen.
Fixes: 504901dbb0b5 ("drm/vmwgfx: Refactor surface_define to use
vmw_surface_metadata")
Reviewed-by: Charmaine Lee
Signed-off-by: Roland Scheidegger
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 9ffa9c75a5da..16b385629688 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1069,10 +1069,6 @@ vmw_stdu_primary_plane_prepare_fb(struct drm_plane
*plane,
if (new_content_type != SAME_AS_DISPLAY) {
struct vmw_surface_metadata metadata = {0};
- metadata.base_size.width = hdisplay;
- metadata.base_size.height = vdisplay;
- metadata.base_size.depth = 1;
-
/*
* If content buffer is a buffer object, then we have to
* construct surface info
@@ -1104,6 +1100,10 @@ vmw_stdu_primary_plane_prepare_fb(struct drm_plane
*plane,
metadata = new_vfbs->surface->metadata;
}
+ metadata.base_size.width = hdisplay;
+ metadata.base_size.height = vdisplay;
+ metadata.base_size.depth = 1;
+
if (vps->surf) {
struct drm_vmw_size cur_base_size =
vps->surf->metadata.base_size;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 3/5] drm/i915: Add checks specific to async flips
Support added only for async flips on primary plane.
If flip is requested on any other plane, reject it.
Make sure there is no change in fbc, offset and framebuffer modifiers
when async flip is requested.
If any of these are modified, reject async flip.
v2: -Replace DRM_ERROR (Paulo)
-Add check for changes in OFFSET, FBC, RC(Paulo)
v3: -Removed TODO as benchmarking tests have been run now.
v4: -Added more state checks for async flip (Ville)
-Moved intel_atomic_check_async to the end of intel_atomic_check
as the plane checks needs to pass before this. (Ville)
-Removed crtc_state->enable_fbc check. (Ville)
-Set the I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP flag for async
flip case as scanline counter is not reliable here.
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_display.c | 103 +++
1 file changed, 103 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index aa0eebeca7ff..fe06db9cf38e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14825,6 +14825,101 @@ static bool intel_cpu_transcoders_need_modeset(struct
intel_atomic_state *state,
return false;
}
+static int intel_atomic_check_async(struct intel_atomic_state *state)
+{
+ struct intel_crtc_state *old_crtc_state, *new_crtc_state;
+ struct intel_plane_state *new_plane_state, *old_plane_state;
+ struct intel_crtc *crtc;
+ struct intel_plane *intel_plane;
+ int i, n_planes = 0;
+
+ for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+ new_crtc_state, i) {
+ if (needs_modeset(new_crtc_state)) {
+ DRM_DEBUG_KMS("Modeset Requried. Async flip not
supported\n");
+ return -EINVAL;
+ }
+
+ if (!new_crtc_state->uapi.active) {
+ DRM_DEBUG_KMS("CRTC inactive\n");
+ return -EINVAL;
+ }
+ if (old_crtc_state->active_planes !=
new_crtc_state->active_planes) {
+ DRM_DEBUG_KMS("Active planes cannot be changed during
async flip\n");
+ return -EINVAL;
+ }
+ }
+
+ for_each_oldnew_intel_plane_in_state(state, intel_plane,
old_plane_state,
+new_plane_state, i) {
+ /*TODO: Async flip is only supported through the page flip IOCTL
+* as of now. So support currently added for primary plane only.
+* Support for other planes should be added when async flip is
+* enabled in the atomic IOCTL path.
+*/
+ if (intel_plane->id != PLANE_PRIMARY)
+ return -EINVAL;
+
+ if ((old_plane_state->color_plane[0].x !=
+new_plane_state->color_plane[0].x) ||
+ (old_plane_state->color_plane[0].y !=
+new_plane_state->color_plane[0].y)) {
+ DRM_DEBUG_KMS("Offsets cannot be changed in async
flip\n");
+ return -EINVAL;
+ }
+
+ if (old_plane_state->uapi.fb->modifier !=
+ new_plane_state->uapi.fb->modifier) {
+ DRM_DEBUG_KMS("Framebuffer modifiers cannot be changed
in async flip\n");
+ return -EINVAL;
+ }
+
+ if (old_plane_state->uapi.fb->format !=
+ new_plane_state->uapi.fb->format) {
+ DRM_DEBUG_KMS("Framebuffer format cannot be changed in
async flip\n");
+ return -EINVAL;
+ }
+
+ if (intel_wm_need_update(old_plane_state, new_plane_state)) {
+ DRM_DEBUG_KMS("WM update not allowed in async flip\n");
+ return -EINVAL;
+ }
+
+ if (old_plane_state->uapi.alpha != new_plane_state->uapi.alpha)
{
+ DRM_DEBUG_KMS("Alpha value cannot be changed in async
flip\n");
+ return -EINVAL;
+ }
+
+ if (old_plane_state->uapi.pixel_blend_mode !=
new_plane_state->uapi.pixel_blend_mode) {
+ DRM_DEBUG_KMS("Pixel blend mode cannot be changed in
async flip\n");
+ return -EINVAL;
+ }
+
+ if (old_plane_state->uapi.color_encoding !=
new_plane_state->uapi.color_encoding) {
+ DRM_DEBUG_KMS("Color encoding cannot be changed in
async flip\n");
+ return -EINVAL;
+ }
+
+ if (old_plane_state->uapi.color_range !=
new_plane_state->uapi.color_range) {
+ DRM_DEBUG_KMS("Color range cannot be
[PATCH v4 1/5] drm/i915: Add enable/disable flip done and flip done handler
Add enable/disable flip done functions and the flip done handler
function which handles the flip done interrupt.
Enable the flip done interrupt in IER.
Enable flip done function is called before writing the
surface address register as the write to this register triggers
the flip done interrupt
Flip done handler is used to send the page flip event as soon as the
surface address is written as per the requirement of async flips.
The interrupt is disabled after the event is sent.
v2: -Change function name from icl_* to skl_* (Paulo)
-Move flip handler to this patch (Paulo)
-Remove vblank_put() (Paulo)
-Enable flip done interrupt for gen9+ only (Paulo)
-Enable flip done interrupt in power_well_post_enable hook (Paulo)
-Removed the event check in flip done handler to handle async
flips without pageflip events.
v3: -Move skl_disable_flip_done out of interrupt handler (Paulo)
-Make the pending vblank event NULL in the begining of
flip_done_handler to remove sporadic WARN_ON that is seen.
v4: -Calculate timestamps using flip done time stamp and current
timestamp for async flips (Ville)
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_display.c | 10 +++
drivers/gpu/drm/i915/i915_irq.c | 83 ++--
drivers/gpu/drm/i915/i915_irq.h | 2 +
drivers/gpu/drm/i915/i915_reg.h | 4 +-
4 files changed, 91 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 729ec6e0d43a..d2c04770dd6a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15552,6 +15552,13 @@ static void intel_atomic_commit_tail(struct
intel_atomic_state *state)
intel_dbuf_pre_plane_update(state);
+ for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+ if (new_crtc_state->uapi.async_flip) {
+ skl_enable_flip_done(>base);
+ break;
+ }
+ }
+
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
dev_priv->display.commit_modeset_enables(state);
@@ -15573,6 +15580,9 @@ static void intel_atomic_commit_tail(struct
intel_atomic_state *state)
drm_atomic_helper_wait_for_flip_done(dev, >base);
for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+ if (new_crtc_state->uapi.async_flip)
+ skl_disable_flip_done(>base);
+
if (new_crtc_state->hw.active &&
!needs_modeset(new_crtc_state) &&
!new_crtc_state->preload_luts &&
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 562b43ed077f..9812a8051c5e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -697,14 +697,24 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc)
return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xff;
}
+u32 g4x_get_flip_counter(struct drm_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+ enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+ return I915_READ(PIPE_FLIPCOUNT_G4X(pipe));
+}
+
u32 g4x_get_vblank_counter(struct drm_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc->dev);
enum pipe pipe = to_intel_crtc(crtc)->pipe;
+ if (crtc->state->async_flip)
+ return g4x_get_flip_counter(crtc);
+
return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
}
-
/*
* On certain encoders on certain platforms, pipe
* scanline register will not work to get the scanline,
@@ -737,17 +747,24 @@ static u32
__intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
* pipe frame time stamp. The time stamp value
* is sampled at every start of vertical blank.
*/
- scan_prev_time = intel_de_read_fw(dev_priv,
- PIPE_FRMTMSTMP(crtc->pipe));
-
+ if (!crtc->config->uapi.async_flip)
+ scan_prev_time = intel_de_read_fw(dev_priv,
+
PIPE_FRMTMSTMP(crtc->pipe));
+ else
+ scan_prev_time = intel_de_read_fw(dev_priv,
+
PIPE_FLIPTMSTMP(crtc->pipe));
/*
* The TIMESTAMP_CTR register has the current
* time stamp value.
*/
scan_curr_time = intel_de_read_fw(dev_priv, IVB_TIMESTAMP_CTR);
- scan_post_time = intel_de_read_fw(dev_priv,
- PIPE_FRMTMSTMP(crtc->pipe));
+ if (!crtc->config->uapi.async_flip)
+ scan_post_time =
[PATCH v4 5/5] drm/i915: Enable async flips in i915
Enable asynchronous flips in i915 for gen9+ platforms. v2: -Async flip enablement should be a stand alone patch (Paulo) v3: -Move the patch to the end of the serires (Paulo) v4: -Rebased. Signed-off-by: Karthik B S Signed-off-by: Vandita Kulkarni --- drivers/gpu/drm/i915/display/intel_display.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index fe06db9cf38e..c9abba98ad22 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -17886,6 +17886,9 @@ static void intel_mode_config_init(struct drm_i915_private *i915) mode_config->funcs = _mode_funcs; + if (INTEL_GEN(i915) >= 9) + mode_config->async_page_flip = true; + /* * Maximum framebuffer dimensions, chosen to match * the maximum render engine surface size on gen4+. -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 2/5] drm/i915: Add support for async flips in I915
Set the Async Address Update Enable bit in plane ctl
when async flip is requested.
v2: -Move the Async flip enablement to individual patch (Paulo)
v3: -Rebased.
v4: -Add separate plane hook for async flip case (Ville)
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_display.c | 6 +
drivers/gpu/drm/i915/display/intel_sprite.c | 25
drivers/gpu/drm/i915/i915_reg.h | 1 +
3 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index d2c04770dd6a..aa0eebeca7ff 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4765,6 +4765,12 @@ u32 skl_plane_ctl(const struct intel_crtc_state
*crtc_state,
const struct drm_intel_sprite_colorkey *key = _state->ckey;
u32 plane_ctl;
+ /* During Async flip, no other updates are allowed */
+ if (crtc_state->uapi.async_flip) {
+ plane_ctl |= PLANE_CTL_ASYNC_FLIP;
+ return plane_ctl;
+ }
+
plane_ctl = PLANE_CTL_ENABLE;
if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
b/drivers/gpu/drm/i915/display/intel_sprite.c
index d03860fef2d7..7cffdb48e6df 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -603,6 +603,24 @@ icl_program_input_csc(struct intel_plane *plane,
PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2), 0x0);
}
+static void
+skl_program_async_surface_address(struct drm_i915_private *dev_priv,
+ const struct intel_plane_state *plane_state,
+ enum pipe pipe, enum plane_id plane_id,
+ u32 surf_addr)
+{
+ unsigned long irqflags;
+ u32 plane_ctl = plane_state->ctl;
+
+ spin_lock_irqsave(_priv->uncore.lock, irqflags);
+
+ intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
+ intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
+ intel_plane_ggtt_offset(plane_state) + surf_addr);
+
+ spin_unlock_irqrestore(_priv->uncore.lock, irqflags);
+}
+
static void
skl_program_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
@@ -631,6 +649,13 @@ skl_program_plane(struct intel_plane *plane,
u32 keymsk, keymax;
u32 plane_ctl = plane_state->ctl;
+ /* During Async flip, no other updates are allowed */
+ if (crtc_state->uapi.async_flip) {
+ skl_program_async_surface_address(dev_priv, plane_state,
+ pipe, plane_id, surf_addr);
+ return;
+ }
+
plane_ctl |= skl_plane_ctl_crtc(crtc_state);
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3c4480172e84..879aaeaf093a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6916,6 +6916,7 @@ enum {
#define PLANE_CTL_TILED_X(1 << 10)
#define PLANE_CTL_TILED_Y(4 << 10)
#define PLANE_CTL_TILED_YF (5 << 10)
+#define PLANE_CTL_ASYNC_FLIP (1 << 9)
#define PLANE_CTL_FLIP_HORIZONTAL(1 << 8)
#define PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE (1 << 4) /* TGL+ */
#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 4/5] drm/i915: Do not call drm_crtc_arm_vblank_event in async flips
Since the flip done event will be sent in the flip_done_handler,
no need to add the event to the list and delay it for later.
v2: -Moved the async check above vblank_get as it
was causing issues for PSR.
v3: -No need to wait for vblank to pass, as this wait was causing a
16ms delay once every few flips.
v4: -Rebased.
Signed-off-by: Karthik B S
Signed-off-by: Vandita Kulkarni
---
drivers/gpu/drm/i915/display/intel_sprite.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
b/drivers/gpu/drm/i915/display/intel_sprite.c
index 7cffdb48e6df..2f1bc8bde516 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -93,6 +93,9 @@ void intel_pipe_update_start(const struct intel_crtc_state
*new_crtc_state)
DEFINE_WAIT(wait);
u32 psr_status;
+ if (new_crtc_state->uapi.async_flip)
+ goto irq_disable;
+
vblank_start = adjusted_mode->crtc_vblank_start;
if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
vblank_start = DIV_ROUND_UP(vblank_start, 2);
@@ -206,7 +209,7 @@ void intel_pipe_update_end(struct intel_crtc_state
*new_crtc_state)
* Would be slightly nice to just grab the vblank count and arm the
* event outside of the critical section - the spinlock might spin for a
* while ... */
- if (new_crtc_state->uapi.event) {
+ if (new_crtc_state->uapi.event && !new_crtc_state->uapi.async_flip) {
drm_WARN_ON(_priv->drm,
drm_crtc_vblank_get(>base) != 0);
@@ -220,6 +223,9 @@ void intel_pipe_update_end(struct intel_crtc_state
*new_crtc_state)
local_irq_enable();
+ if (new_crtc_state->uapi.async_flip)
+ return;
+
if (intel_vgpu_active(dev_priv))
return;
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/exynos: gem: Fix sparse warning
20. 7. 14. 오전 1:03에 Sam Ravnborg 이(가) 쓴 글:
> On Mon, Jul 13, 2020 at 09:07:08AM +0200, Marek Szyprowski wrote:
>> kvaddr element of the exynos_gem object points to a memory buffer, thus
>> it should not have a __iomem annotation. Then, to avoid a warning or
>> casting on assignment to fbi structure, the screen_buffer element of the
>> union should be used instead of the screen_base.
>>
>> Reported-by: kernel test robot
>> Suggested-by: Sam Ravnborg
>> Signed-off-by: Marek Szyprowski
> Reviewed-by: Sam Ravnborg
>
> I expect one of the exynos maintianers (Inki?) to pick it up.
Picked it up.
Thanks,
Inki Dae
>
> Sam
>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +-
>> drivers/gpu/drm/exynos/exynos_drm_gem.h | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> index 56a2b47e1af7..5147f5929be7 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> @@ -92,7 +92,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper
>> *helper,
>> offset = fbi->var.xoffset * fb->format->cpp[0];
>> offset += fbi->var.yoffset * fb->pitches[0];
>>
>> -fbi->screen_base = exynos_gem->kvaddr + offset;
>> +fbi->screen_buffer = exynos_gem->kvaddr + offset;
>> fbi->screen_size = size;
>> fbi->fix.smem_len = size;
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> index 7445748288da..74e926abeff0 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> @@ -40,7 +40,7 @@ struct exynos_drm_gem {
>> unsigned intflags;
>> unsigned long size;
>> void*cookie;
>> -void __iomem*kvaddr;
>> +void*kvaddr;
>> dma_addr_t dma_addr;
>> unsigned long dma_attrs;
>> struct sg_table *sgt;
>> --
>> 2.17.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://protect2.fireeye.com/url?k=33cc4690-6e52dd7a-33cdcddf-0cc47a6cba04-3234389cf6ac8e89=1=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 0/5] Asynchronous flip implementation for i915
Without async flip support in the kernel, fullscreen apps where game resolution is equal to the screen resolution, must perform an extra blit per frame prior to flipping. Asynchronous page flips will also boost the FPS of Mesa benchmarks. v2: -Few patches have been squashed and patches have been shuffled as per the reviews on the previous version. v3: -Few patches have been squashed and patches have been shuffled as per the reviews on the previous version. v4: -Made changes to fix the sequence and time stamp issue as per the comments received on the previous version. -Timestamps are calculated using the flip done time stamp and current timestamp. Here I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP flag is used for timestamp calculations. -Event is sent from the interrupt handler immediately using this updated timestamps and sequence. -Added more state checks as async flip should only allow change in plane surface address and nothing else should be allowed to change. -Added a separate plane hook for async flip. -Need to find a way to reject fbc enabling if it comes as part of this flip as bspec states that changes to FBC are not allowed. Karthik B S (5): drm/i915: Add enable/disable flip done and flip done handler drm/i915: Add support for async flips in I915 drm/i915: Add checks specific to async flips drm/i915: Do not call drm_crtc_arm_vblank_event in async flips drm/i915: Enable async flips in i915 drivers/gpu/drm/i915/display/intel_display.c | 122 +++ drivers/gpu/drm/i915/display/intel_sprite.c | 33 - drivers/gpu/drm/i915/i915_irq.c | 83 +++-- drivers/gpu/drm/i915/i915_irq.h | 2 + drivers/gpu/drm/i915/i915_reg.h | 5 +- 5 files changed, 236 insertions(+), 9 deletions(-) -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vmwgfx: Fix two list_for_each loop exit tests
Am 26.06.20 um 12:39 schrieb Dan Carpenter:
> These if statements are supposed to be true if we ended the
> list_for_each_entry() loops without hitting a break statement but they
> don't work.
>
> In the first loop, we increment "i" after the "if (i == unit)" condition
> so we don't necessarily know that "i" is not equal to unit at the end of
> the loop.
So, if I understand this right, this would only really be a problem if
there's no list entries at all, right? That is i == unit == 0.
Not sure if that can actually happen, but in any case the fix looks correct.
>
> In the second loop we exit when mode is not pointing to a valid
> drm_display_mode struct so it doesn't make sense to check "mode->type".
Looks good to me too, condition order seems fine to me as well, though I
wouldn't particularly care.
Applied to vmwgfx-next as well, thanks.
Roland
>
> Fixes: a278724aa23c ("drm/vmwgfx: Implement fbdev on kms v2")
> Signed-off-by: Dan Carpenter
> ---
> I reversed the second condition as well, just because I was copy and
> pasting the exit condition. Plus I always feel like error handling is
> better than success handling. If anyone feel strongly, then I can send
> a v2.
>
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 3c97654b5a43..44168a7d7b44 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2576,7 +2576,7 @@ int vmw_kms_fbdev_init_data(struct vmw_private
> *dev_priv,
> ++i;
> }
>
> - if (i != unit) {
> + if (>head == _priv->dev->mode_config.connector_list) {
> DRM_ERROR("Could not find initial display unit.\n");
> ret = -EINVAL;
> goto out_unlock;
> @@ -2600,13 +2600,13 @@ int vmw_kms_fbdev_init_data(struct vmw_private
> *dev_priv,
> break;
> }
>
> - if (mode->type & DRM_MODE_TYPE_PREFERRED)
> - *p_mode = mode;
> - else {
> + if (>head == >modes) {
> WARN_ONCE(true, "Could not find initial preferred mode.\n");
> *p_mode = list_first_entry(>modes,
> struct drm_display_mode,
> head);
> + } else {
> + *p_mode = mode;
> }
>
> out_unlock:
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vmwgfx: Use correct vmw_legacy_display_unit pointer
Sorry for the delay, finally got time to look at this, seems all correct
to me, thanks. Applied to our vmvgfx-next tree. (I do wonder how this
somehow was supposed to work for all this time...)
Roland
Am 26.06.20 um 12:34 schrieb Dan Carpenter:
> The "entry" pointer is an offset from the list head and it doesn't
> point to a valid vmw_legacy_display_unit struct. Presumably the
> intent was to point to the last entry.
>
> Also the "i++" wasn't used so I have removed that as well.
>
> Fixes: d7e1958dbe4a ("drm/vmwgfx: Support older hardware.")
> Signed-off-by: Dan Carpenter
> ---
> From static analysis. Not tested. This bug celebrated its tenth
> birthday last month. :)
>
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> index 16dafff5cab1..009f1742bed5 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> @@ -81,7 +81,7 @@ static int vmw_ldu_commit_list(struct vmw_private *dev_priv)
> struct vmw_legacy_display_unit *entry;
> struct drm_framebuffer *fb = NULL;
> struct drm_crtc *crtc = NULL;
> - int i = 0;
> + int i;
>
> /* If there is no display topology the host just assumes
>* that the guest will set the same layout as the host.
> @@ -92,12 +92,11 @@ static int vmw_ldu_commit_list(struct vmw_private
> *dev_priv)
> crtc = >base.crtc;
> w = max(w, crtc->x + crtc->mode.hdisplay);
> h = max(h, crtc->y + crtc->mode.vdisplay);
> - i++;
> }
>
> if (crtc == NULL)
> return 0;
> - fb = entry->base.crtc.primary->state->fb;
> + fb = crtc->primary->state->fb;
>
> return vmw_kms_write_svga(dev_priv, w, h, fb->pitches[0],
> fb->format->cpp[0] * 8,
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v1] drm/msm/dpu: add support for clk and bw scaling for display
On Thu, Jun 18, 2020 at 07:38:41PM +0530, Kalyan Thota wrote: > This change adds support to scale src clk and bandwidth as > per composition requirements. > > Interconnect registration for bw has been moved to mdp > device node from mdss to facilitate the scaling. > > Changes in v1: > - Address armv7 compilation issues with the patch (Rob) > > Signed-off-by: Kalyan Thota It seems this is an evolution of this series: https://patchwork.kernel.org/project/linux-arm-msm/list/?series=265351 Are the DT bits of the series still valid? If so please include them in the series, otherwise please add DT patches to allow folks to test and review, and get them landed in Bjorn's tree after the driver changes have landed. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check
20. 6. 19. 오후 7:36에 Marek Szyprowski 이(가) 쓴 글:
> Use common helper for checking the contiguity of the imported dma-buf.
>
> Signed-off-by: Marek Szyprowski
Acked-by : Inki Dae
Thanks,
Inki Dae
> ---
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 23 +++
> 1 file changed, 3 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index efa476858db5..1716a023bca0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -431,27 +431,10 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device
> *dev,
> {
> struct exynos_drm_gem *exynos_gem;
>
> - if (sgt->nents < 1)
> + /* check if the entries in the sg_table are contiguous */
> + if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size) {
> + DRM_ERROR("buffer chunks must be mapped contiguously");
> return ERR_PTR(-EINVAL);
> -
> - /*
> - * Check if the provided buffer has been mapped as contiguous
> - * into DMA address space.
> - */
> - if (sgt->nents > 1) {
> - dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> - struct scatterlist *s;
> - unsigned int i;
> -
> - for_each_sg(sgt->sgl, s, sgt->nents, i) {
> - if (!sg_dma_len(s))
> - break;
> - if (sg_dma_address(s) != next_addr) {
> - DRM_ERROR("buffer chunks must be mapped
> contiguously");
> - return ERR_PTR(-EINVAL);
> - }
> - next_addr = sg_dma_address(s) + sg_dma_len(s);
> - }
> }
>
> exynos_gem = exynos_drm_gem_init(dev, attach->dmabuf->size);
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues
20. 6. 19. 오후 7:36에 Marek Szyprowski 이(가) 쓴 글:
> The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
> returns the number of the created entries in the DMA address space.
> However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
> dma_unmap_sg must be called with the original number of the entries
> passed to the dma_map_sg().
>
> struct sg_table is a common structure used for describing a non-contiguous
> memory buffer, used commonly in the DRM and graphics subsystems. It
> consists of a scatterlist with memory pages and DMA addresses (sgl entry),
> as well as the number of scatterlist entries: CPU pages (orig_nents entry)
> and DMA mapped pages (nents entry).
>
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling DMA-mapping functions with a wrong number of entries or
> ignoring the number of mapped entries returned by the dma_map_sg()
> function.
>
> To avoid such issues, lets use a common dma-mapping wrappers operating
> directly on the struct sg_table objects and use scatterlist page
> iterators where possible. This, almost always, hides references to the
> nents and orig_nents entries, making the code robust, easier to follow
> and copy/paste safe.
>
> Signed-off-by: Marek Szyprowski
Acked-by : Inki Dae
Thanks,
Inki Dae
> ---
> drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index fcee33a43aca..7014a8cd971a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -395,8 +395,8 @@ static void g2d_userptr_put_dma_addr(struct g2d_data *g2d,
> return;
>
> out:
> - dma_unmap_sg(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt->sgl,
> - g2d_userptr->sgt->nents, DMA_BIDIRECTIONAL);
> + dma_unmap_sgtable(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt,
> + DMA_BIDIRECTIONAL, 0);
>
> pages = frame_vector_pages(g2d_userptr->vec);
> if (!IS_ERR(pages)) {
> @@ -511,10 +511,10 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct
> g2d_data *g2d,
>
> g2d_userptr->sgt = sgt;
>
> - if (!dma_map_sg(to_dma_dev(g2d->drm_dev), sgt->sgl, sgt->nents,
> - DMA_BIDIRECTIONAL)) {
> + ret = dma_map_sgtable(to_dma_dev(g2d->drm_dev), sgt,
> + DMA_BIDIRECTIONAL, 0);
> + if (ret) {
> DRM_DEV_ERROR(g2d->dev, "failed to map sgt with dma region.\n");
> - ret = -ENOMEM;
> goto err_sg_free_table;
> }
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] dt-bindings: media: renesas, vsp1: Convert binding to YAML
On Sun, 21 Jun 2020 03:47:33 +0300, Laurent Pinchart wrote: > Convert the Renesas R-Car VSP1 text binding to YAML. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Geert Uytterhoeven > --- > Changes since v1: > > - Simplify comments on compatible strings > - Update MAINTAINERS > --- > .../bindings/media/renesas,vsp1.txt | 30 --- > .../bindings/media/renesas,vsp1.yaml | 83 +++ > MAINTAINERS | 2 +- > 3 files changed, 84 insertions(+), 31 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/media/renesas,vsp1.txt > create mode 100644 Documentation/devicetree/bindings/media/renesas,vsp1.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 8/8] dt-bindings: media: renesas, vsp1: Add power-domains and resets
On Sun, 21 Jun 2020 03:47:34 +0300, Laurent Pinchart wrote: > The power-domains and resets properties are used in all DT sources in > the kernel but are absent from the bindings. Document them and make them > mandatory. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Geert Uytterhoeven > --- > .../devicetree/bindings/media/renesas,vsp1.yaml| 14 ++ > 1 file changed, 14 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 6/8] dt-bindings: media: renesas,fdp1: Add resets property
On Sun, 21 Jun 2020 03:47:32 +0300, Laurent Pinchart wrote: > The resets property is used in DT sources in the kernel tree. Document > it and make it mandatory. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Geert Uytterhoeven > --- > Changes since v1: > > - Fix typo in commit message > --- > Documentation/devicetree/bindings/media/renesas,fdp1.yaml | 5 + > 1 file changed, 5 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/8] dt-bindings: media: renesas, fdp1: Make power-domains mandatory
On Sun, 21 Jun 2020 03:47:31 +0300, Laurent Pinchart wrote: > All DT source files in the kernel tree specify the power-domains > property. Make it mandatory. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Geert Uytterhoeven > --- > Changes since v1: > > - Fix typo in comment message > --- > Documentation/devicetree/bindings/media/renesas,fdp1.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/8] dt-bindings: media: renesas, fdp1: Convert binding to YAML
On Sun, 21 Jun 2020 03:47:30 +0300, Laurent Pinchart wrote: > Convert the Renesas R-Car FDP1 text binding to YAML. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Geert Uytterhoeven > --- > Changes since v1: > > - Update MAINTAINERS > --- > .../bindings/media/renesas,fdp1.txt | 37 --- > .../bindings/media/renesas,fdp1.yaml | 63 +++ > MAINTAINERS | 2 +- > 3 files changed, 64 insertions(+), 38 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/media/renesas,fdp1.txt > create mode 100644 Documentation/devicetree/bindings/media/renesas,fdp1.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/8] dt-bindings: media: renesas, fcp: Make power-domains mandatory
On Sun, 21 Jun 2020 03:47:28 +0300, Laurent Pinchart wrote: > All DT source files in the kernel tree specify the power-domains > property. Make it mandatory. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Geert Uytterhoeven > --- > Changes since v1: > > - Fix typo in commit message > --- > Documentation/devicetree/bindings/media/renesas,fcp.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/8] dt-bindings: media: renesas, fcp: Add resets and iommus properties
On Sun, 21 Jun 2020 03:47:29 +0300, Laurent Pinchart wrote: > The resets and iommus properties are used in DT sources in the kernel > tree. Document them, and make resets mandatory. The iommus property is > optional as not all platforms wire the FCP to a functional IOMMU. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Geert Uytterhoeven > --- > Documentation/devicetree/bindings/media/renesas,fcp.yaml | 9 + > 1 file changed, 9 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] drm/msm: Avoid possible infinite probe deferral and speed booting
Hi, On Mon, Jul 13, 2020 at 1:25 PM Rob Herring wrote: > > On Mon, Jul 13, 2020 at 9:08 AM Doug Anderson wrote: > > > > Hi, > > > > On Mon, Jul 13, 2020 at 7:11 AM Rob Herring wrote: > > > > > > On Fri, Jul 10, 2020 at 5:02 PM Douglas Anderson > > > wrote: > > > > > > > > I found that if I ever had a little mistake in my kernel config, > > > > or device tree, or graphics driver that my system would sit in a loop > > > > at bootup trying again and again and again. An example log was: > > > > > > Why do we care about optimizing the error case? > > > > It actually results in a _fully_ infinite loop. That is: if anything > > small causes a component of DRM to fail to probe then the whole system > > doesn't boot because it just loops trying to probe over and over > > again. The messages I put in the commit message are printed over and > > over and over again. > > Sounds like a bug as that's not what should happen. > > If you defer during boot (initcalls), then you'll be on the deferred > list until late_initcall and everything is retried. After > late_initcall, only devices getting added should trigger probing. But > maybe the adding and then removing a device is causing a re-trigger. Right, I'm nearly certain that the adding and then removing is causing a re-trigger. I believe the loop would happen for any case where we have a probe function that: 1. Adds devices. 2. After adding devices it decides that it needs to defer. 3. Removes the devices it added. 4. Return -EPROBE_DEFER from its probe function. Specifically from what I know about how -EPROBE_DEFER works I'm not sure how it wouldn't cause an infinite loop in that case. Perhaps the missing part of my explanation, though, is why it never gets out of this infinite loop. In my case I purposely made the bridge chip "ti-sn65dsi86.c" return an error (-EINVAL) in its probe every time. Obviously I wasn't going to get a display up like this, but I just wanted to not loop forever at bootup. I tracked down exactly why we get an - EPROBE_DEFER over and over in this case. You can see it in msm_dsi_host_register(). If some components haven't shown up when that function runs it will _always_ return -EPROBE_DEFER. In my case, since I caused the bridge to fail to probe, those components will _never_ show up. That means that msm_dsi_host_register() will _always_ return -EPROBE_DEFER. I haven't dug through all the DRM code enough, but it doesn't necessarily seem like the wrong behavior. If the bridge driver or a panel was a module then (presumably) they could show up later and so it should be OK for it to defer, right? So with all that, it doesn't really feel like this is a bug so much as it's an unsupported use case. The current deferral logic simply can't handle the case we're throwing at it. You cannot return -EPROBE_DEFER if your probe function adds devices each time through the probe function. Assuming all the above makes sense, that means we're stuck with: a) This patch series, which makes us not add devices. b) Some other patch series which rearchitects the MSM graphics stack to not return -EPROBE_DEFER in this case. c) Smarten up the deferral system to somehow detect this loop. I'm really not sure how to do this. You'd have to somehow know that you keep adding the same devices over and over again and they didn't get us out of the deferral loop last time and so you should eventually give up. > > > > msm ae0.mdss: bound ae01000.mdp (ops 0xffe596e951f8) > > > > msm_dsi ae94000.dsi: ae94000.dsi supply gdsc not found, using dummy > > > > regulator > > > > msm_dsi_manager_register: failed to register mipi dsi host for DSI 0 > > > > [drm:ti_sn_bridge_probe] *ERROR* could not find any panel node > > > > ... > > > > > > > > I finally tracked it down where this was happening: > > > > - msm_pdev_probe() is called. > > > > - msm_pdev_probe() registers drivers. Registering drivers kicks > > > > off processing of probe deferrals. > > > > - component_master_add_with_match() could return -EPROBE_DEFER. > > > > making msm_pdev_probe() return -EPROBE_DEFER. > > > > - When msm_pdev_probe() returned the processing of probe deferrals > > > > happens. > > > > - Loop back to the start. > > > > > > > > It looks like we can fix this by marking "mdss" as a "simple-bus". > > > > I have no idea if people consider this the right thing to do or a > > > > hack. Hopefully it's the right thing to do. :-) > > > > > > It's a simple test. Do the child devices have any dependency on the > > > parent to probe and/or function? If so, not a simple-bus. > > > > Great! You can see in the earlier patch in the series that the very > > first thing that happens when the parent device probes is that it > > calls devm_of_platform_populate(). That means no dependencies, right? > > It should. But then I reviewed the MDSS binding today and it looks > like the MDSS is the interrupt parent for at least some child devices? Hrm. How does
Re: [PATCH v4 3/3] drm/msm: handle for EPROBE_DEFER for of_icc_get
On Mon, Jul 13, 2020 at 06:53:42PM -0400, Jonathan Marek wrote:
> Check for errors instead of silently not using icc if the msm driver
> probes before the interconnect driver.
>
> Allow ENODATA for ocmem path, as it is optional and this error
> is returned when "gfx-mem" path is provided but not "ocmem".
>
> Because msm_gpu_cleanup assumes msm_gpu_init has been called, the icc path
> init needs to be after msm_gpu_init for the error path to work.
A possible future improvement would be to move the ocmem check to the target
specific code for 3xx and 4xx where you could be a bit more demanding that the
ocmem path actually exist.
Reviewed-by: Jordan Crouse
> Signed-off-by: Jonathan Marek
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 65 +++--
> 1 file changed, 38 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index ad64d4b7e8d7..3f1ecc1de965 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -895,7 +895,7 @@ static int adreno_get_legacy_pwrlevels(struct device *dev)
> return 0;
> }
>
> -static int adreno_get_pwrlevels(struct device *dev,
> +static void adreno_get_pwrlevels(struct device *dev,
> struct msm_gpu *gpu)
> {
> unsigned long freq = ULONG_MAX;
> @@ -930,24 +930,6 @@ static int adreno_get_pwrlevels(struct device *dev,
> }
>
> DBG("fast_rate=%u, slow_rate=2700", gpu->fast_rate);
> -
> - /* Check for an interconnect path for the bus */
> - gpu->icc_path = of_icc_get(dev, "gfx-mem");
> - if (!gpu->icc_path) {
> - /*
> - * Keep compatbility with device trees that don't have an
> - * interconnect-names property.
> - */
> - gpu->icc_path = of_icc_get(dev, NULL);
> - }
> - if (IS_ERR(gpu->icc_path))
> - gpu->icc_path = NULL;
> -
> - gpu->ocmem_icc_path = of_icc_get(dev, "ocmem");
> - if (IS_ERR(gpu->ocmem_icc_path))
> - gpu->ocmem_icc_path = NULL;
> -
> - return 0;
> }
>
> int adreno_gpu_ocmem_init(struct device *dev, struct adreno_gpu *adreno_gpu,
> @@ -993,9 +975,11 @@ int adreno_gpu_init(struct drm_device *drm, struct
> platform_device *pdev,
> struct adreno_gpu *adreno_gpu,
> const struct adreno_gpu_funcs *funcs, int nr_rings)
> {
> - struct adreno_platform_config *config = pdev->dev.platform_data;
> + struct device *dev = >dev;
> + struct adreno_platform_config *config = dev->platform_data;
> struct msm_gpu_config adreno_gpu_config = { 0 };
> struct msm_gpu *gpu = _gpu->base;
> + int ret;
>
> adreno_gpu->funcs = funcs;
> adreno_gpu->info = adreno_info(config->rev);
> @@ -1007,15 +991,42 @@ int adreno_gpu_init(struct drm_device *drm, struct
> platform_device *pdev,
>
> adreno_gpu_config.nr_rings = nr_rings;
>
> - adreno_get_pwrlevels(>dev, gpu);
> + adreno_get_pwrlevels(dev, gpu);
>
> - pm_runtime_set_autosuspend_delay(>dev,
> + pm_runtime_set_autosuspend_delay(dev,
> adreno_gpu->info->inactive_period);
> - pm_runtime_use_autosuspend(>dev);
> - pm_runtime_enable(>dev);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_enable(dev);
>
> - return msm_gpu_init(drm, pdev, _gpu->base, >base,
> + ret = msm_gpu_init(drm, pdev, _gpu->base, >base,
> adreno_gpu->info->name, _gpu_config);
> + if (ret)
> + return ret;
> +
> + /* Check for an interconnect path for the bus */
> + gpu->icc_path = of_icc_get(dev, "gfx-mem");
> + if (!gpu->icc_path) {
> + /*
> + * Keep compatbility with device trees that don't have an
> + * interconnect-names property.
> + */
> + gpu->icc_path = of_icc_get(dev, NULL);
> + }
> + if (IS_ERR(gpu->icc_path)) {
> + ret = PTR_ERR(gpu->icc_path);
> + gpu->icc_path = NULL;
> + return ret;
> + }
> +
> + gpu->ocmem_icc_path = of_icc_get(dev, "ocmem");
> + if (IS_ERR(gpu->ocmem_icc_path)) {
> + ret = PTR_ERR(gpu->ocmem_icc_path);
> + gpu->ocmem_icc_path = NULL;
> + /* allow -ENODATA, ocmem icc is optional */
> + if (ret != -ENODATA)
> + return ret;
> + }
> + return 0;
> }
>
> void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
> @@ -1029,8 +1040,8 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
>
> pm_runtime_disable(>gpu_pdev->dev);
>
> + msm_gpu_cleanup(_gpu->base);
> +
> icc_put(gpu->icc_path);
> icc_put(gpu->ocmem_icc_path);
> -
> - msm_gpu_cleanup(_gpu->base);
> }
> --
> 2.26.1
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [Freedreno] [PATCH v4 2/3] drm/msm: reset devfreq freq_table/max_state before devfreq_add_device
On Mon, Jul 13, 2020 at 06:53:41PM -0400, Jonathan Marek wrote: > These never get set back to 0 when probing fails, so an attempt to probe > again results in broken behavior. Fix the problem by setting thse to zero > before they are used. Reviewed-by: Jordan Crouse > Signed-off-by: Jonathan Marek > --- > drivers/gpu/drm/msm/msm_gpu.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c > index a22d30622306..aa9775ab52f0 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.c > +++ b/drivers/gpu/drm/msm/msm_gpu.c > @@ -93,7 +93,11 @@ static void msm_devfreq_init(struct msm_gpu *gpu) > /* >* Don't set the freq_table or max_state and let devfreq build the table >* from OPP > + * After a deferred probe, these may have be left to non-zero values, > + * so set them back to zero before creating the devfreq device >*/ > + msm_devfreq_profile.freq_table = NULL; > + msm_devfreq_profile.max_state = 0; > > gpu->devfreq.devfreq = devm_devfreq_add_device(>pdev->dev, > _devfreq_profile, DEVFREQ_GOV_SIMPLE_ONDEMAND, > -- > 2.26.1 > > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/3] drm/msm: fix unbalanced pm_runtime_enable in adreno_gpu_{init,cleanup}
On Mon, Jul 13, 2020 at 06:53:40PM -0400, Jonathan Marek wrote:
> adreno_gpu_init calls pm_runtime_enable, so adreno_gpu_cleanup needs to
> call pm_runtime_disable.
Reviewed-by: Jordan Crouse
> Signed-off-by: Jonathan Marek
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 89673c7ed473..ad64d4b7e8d7 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -1021,11 +1021,14 @@ int adreno_gpu_init(struct drm_device *drm, struct
> platform_device *pdev,
> void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
> {
> struct msm_gpu *gpu = _gpu->base;
> + struct msm_drm_private *priv = gpu->dev->dev_private;
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++)
> release_firmware(adreno_gpu->fw[i]);
>
> + pm_runtime_disable(>gpu_pdev->dev);
> +
> icc_put(gpu->icc_path);
> icc_put(gpu->ocmem_icc_path);
>
> --
> 2.26.1
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] drm/msm: Avoid possible infinite probe deferral and speed booting
On Mon, Jul 13, 2020 at 1:25 PM Rob Herring wrote: > > On Mon, Jul 13, 2020 at 9:08 AM Doug Anderson wrote: > > > > Hi, > > > > On Mon, Jul 13, 2020 at 7:11 AM Rob Herring wrote: > > > > > > On Fri, Jul 10, 2020 at 5:02 PM Douglas Anderson > > > wrote: > > > > > > > > I found that if I ever had a little mistake in my kernel config, > > > > or device tree, or graphics driver that my system would sit in a loop > > > > at bootup trying again and again and again. An example log was: > > > > > > Why do we care about optimizing the error case? > > > > It actually results in a _fully_ infinite loop. That is: if anything > > small causes a component of DRM to fail to probe then the whole system > > doesn't boot because it just loops trying to probe over and over > > again. The messages I put in the commit message are printed over and > > over and over again. > > Sounds like a bug as that's not what should happen. > > If you defer during boot (initcalls), then you'll be on the deferred > list until late_initcall and everything is retried. After > late_initcall, only devices getting added should trigger probing. But > maybe the adding and then removing a device is causing a re-trigger. > > > > > msm ae0.mdss: bound ae01000.mdp (ops 0xffe596e951f8) > > > > msm_dsi ae94000.dsi: ae94000.dsi supply gdsc not found, using dummy > > > > regulator > > > > msm_dsi_manager_register: failed to register mipi dsi host for DSI 0 > > > > [drm:ti_sn_bridge_probe] *ERROR* could not find any panel node > > > > ... > > > > > > > > I finally tracked it down where this was happening: > > > > - msm_pdev_probe() is called. > > > > - msm_pdev_probe() registers drivers. Registering drivers kicks > > > > off processing of probe deferrals. > > > > - component_master_add_with_match() could return -EPROBE_DEFER. > > > > making msm_pdev_probe() return -EPROBE_DEFER. > > > > - When msm_pdev_probe() returned the processing of probe deferrals > > > > happens. > > > > - Loop back to the start. > > > > > > > > It looks like we can fix this by marking "mdss" as a "simple-bus". > > > > I have no idea if people consider this the right thing to do or a > > > > hack. Hopefully it's the right thing to do. :-) > > > > > > It's a simple test. Do the child devices have any dependency on the > > > parent to probe and/or function? If so, not a simple-bus. > > > > Great! You can see in the earlier patch in the series that the very > > first thing that happens when the parent device probes is that it > > calls devm_of_platform_populate(). That means no dependencies, right? > > It should. But then I reviewed the MDSS binding today and it looks > like the MDSS is the interrupt parent for at least some child devices? > yes, that is correct BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/25] dma-fence: basic lockdep annotations
On Tue, 14 Jul 2020 at 02:39, Christian König wrote: > > Am 13.07.20 um 18:26 schrieb Daniel Vetter: > > Hi Christian, > > > > On Wed, Jul 08, 2020 at 04:57:21PM +0200, Christian König wrote: > >> Could we merge this controlled by a separate config option? > >> > >> This way we could have the checks upstream without having to fix all the > >> stuff before we do this? > > Discussions died out a bit, do you consider this a blocker for the first > > two patches, or good for an ack on these? > > Yes, I think the first two can be merged without causing any pain. Feel > free to add my ab on them. > > And the third one can go in immediately as well. Acked-by: Dave Airlie for the first 2 + indefinite explains. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] drm/msm: Avoid possible infinite probe deferral and speed booting
On Mon, Jul 13, 2020 at 9:08 AM Doug Anderson wrote: > > Hi, > > On Mon, Jul 13, 2020 at 7:11 AM Rob Herring wrote: > > > > On Fri, Jul 10, 2020 at 5:02 PM Douglas Anderson > > wrote: > > > > > > I found that if I ever had a little mistake in my kernel config, > > > or device tree, or graphics driver that my system would sit in a loop > > > at bootup trying again and again and again. An example log was: > > > > Why do we care about optimizing the error case? > > It actually results in a _fully_ infinite loop. That is: if anything > small causes a component of DRM to fail to probe then the whole system > doesn't boot because it just loops trying to probe over and over > again. The messages I put in the commit message are printed over and > over and over again. Sounds like a bug as that's not what should happen. If you defer during boot (initcalls), then you'll be on the deferred list until late_initcall and everything is retried. After late_initcall, only devices getting added should trigger probing. But maybe the adding and then removing a device is causing a re-trigger. > > > msm ae0.mdss: bound ae01000.mdp (ops 0xffe596e951f8) > > > msm_dsi ae94000.dsi: ae94000.dsi supply gdsc not found, using dummy > > > regulator > > > msm_dsi_manager_register: failed to register mipi dsi host for DSI 0 > > > [drm:ti_sn_bridge_probe] *ERROR* could not find any panel node > > > ... > > > > > > I finally tracked it down where this was happening: > > > - msm_pdev_probe() is called. > > > - msm_pdev_probe() registers drivers. Registering drivers kicks > > > off processing of probe deferrals. > > > - component_master_add_with_match() could return -EPROBE_DEFER. > > > making msm_pdev_probe() return -EPROBE_DEFER. > > > - When msm_pdev_probe() returned the processing of probe deferrals > > > happens. > > > - Loop back to the start. > > > > > > It looks like we can fix this by marking "mdss" as a "simple-bus". > > > I have no idea if people consider this the right thing to do or a > > > hack. Hopefully it's the right thing to do. :-) > > > > It's a simple test. Do the child devices have any dependency on the > > parent to probe and/or function? If so, not a simple-bus. > > Great! You can see in the earlier patch in the series that the very > first thing that happens when the parent device probes is that it > calls devm_of_platform_populate(). That means no dependencies, right? It should. But then I reviewed the MDSS binding today and it looks like the MDSS is the interrupt parent for at least some child devices? > So that means it's fine/correct to add "simple-bus" here? > > > > > Once I do this I notice that my boot gets marginally faster (you > > > don't need to probe the sub devices over and over) and also if I > > > > Can you quantify that? > > I'd say < 100 us. I can try to quantify more if needed, but it wasn't > the point of this patch. > > > > Have you run with devlinks enabled. You need a command line option to > > enable. That too should reduce deferred probes. > > Ah, good idea! I will try it. However, even with devlinks, if there > is any chance of deferred probes then we need a fix like this. The > point of the patch isn't about speeding things up but about avoiding > an infinite loop at bootup due to a small bug. I think a deferred probe would only happen if there's a dependency we don't track (but we're tracking about everything that's common). But if there's some error, I'm not sure what would happen. Seems like a good test case. :) > > > have a problem it doesn't loop forever (on my system it still > > > gets upset about some stuck clocks in that case, but at least I > > > can boot up). > > > > Deferred probe only runs when a device is added, so it's not like it > > is continually running. > > If you don't mind looking at the code patch, see: > > https://lore.kernel.org/r/20200710160131.4.I358ea82de218ea5f4406572ade23f5e121297555@changeid/ > > Specifically you can see that each time we try to probe we were > calling of_platform_populate(). That appeared to be enough to trigger > things. Like I said, sounds like a bug. Even if 'simple-bus' is the appropriate thing to do here, it should be fixed or at least understood. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/drm_fb_helper: fix fbdev with sparc64
On Mon, Jul 13, 2020 at 8:39 PM Sam Ravnborg wrote:
>
> On Mon, Jul 13, 2020 at 06:21:59PM +0200, Daniel Vetter wrote:
> > On Fri, Jul 10, 2020 at 08:28:16AM +0200, Thomas Zimmermann wrote:
> > > Hi
> > >
> > > Am 09.07.20 um 21:30 schrieb Sam Ravnborg:
> > > > Mark reported that sparc64 would panic while booting using qemu.
> > > > Mark bisected this to a patch that introduced generic fbdev emulation to
> > > > the bochs DRM driver.
> > > > Mark pointed out that a similar bug was fixed before where
> > > > the sys helpers was replaced by cfb helpers.
> > > >
> > > > The culprint here is that the framebuffer reside in IO memory which
> > > > requires SPARC ASI_PHYS (physical) loads and stores.
> > > >
> > > > The current bohcs DRM driver uses a shadow buffer.
> > > > So all copying to the framebuffer happens in
> > > > drm_fb_helper_dirty_blit_real().
> > > >
> > > > The fix is to replace the memcpy with memcpy_toio() from io.h.
> > > >
> > > > memcpy_toio() uses writeb() where the original fbdev code
> > > > used sbus_memcpy_toio(). The latter uses sbus_writeb().
> > > >
> > > > The difference between writeb() and sbus_memcpy_toio() is
> > > > that writeb() writes bytes in little-endian, where sbus_writeb() writes
> > > > bytes in big-endian. As endian does not matter for byte writes they are
> > > > the same. So we can safely use memcpy_toio() here.
> > > >
> > > > For many architectures memcpy_toio() is a simple memcpy().
> > > > One sideeffect that is unknow is if this has any impact on other
> > > > architectures.
> > > > So far the analysis tells that this change is OK for other arch's.
> > > > but testing would be good.
> > > >
> > > > Signed-off-by: Sam Ravnborg
> > > > Reported-by: Mark Cave-Ayland
> > > > Tested-by: Mark Cave-Ayland
> > > > Cc: Mark Cave-Ayland
> > > > Cc: Thomas Zimmermann
> > > > Cc: Gerd Hoffmann
> > > > Cc: "David S. Miller"
> > > > Cc: sparcli...@vger.kernel.org
> > >
> > > So this actually is a problem in practice. Do you know how userspace
> > > handles this?
> > >
> > > For this patch
> > >
> > > Acked-by: Thomas Zimmermann
> > >
> > > but I'd like to have someone with more architecture expertise ack this
> > > as well.
> > >
> > > Best regards
> > > Thomas
> > >
> > > > ---
> > > > drivers/gpu/drm/drm_fb_helper.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > > b/drivers/gpu/drm/drm_fb_helper.c
> > > > index 5609e164805f..4d05b0ab1592 100644
> > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > @@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> > > > drm_fb_helper *fb_helper,
> > > > unsigned int y;
> > > >
> > > > for (y = clip->y1; y < clip->y2; y++) {
> > > > - memcpy(dst, src, len);
> > > > + memcpy_toio(dst, src, len);
> >
> > I don't think we can do this unconditionally, there's fbdev-helper drivers
> > using shmem helpers, and for shmem memcpy_toio is wrong. We need a switch
> > to fix this properly I think.
> >
> > What Dave Airlie mentioned is just about memcpy_toio vs the sparc bus
> > version, for which we don't have any drivers really. But I do think we
> > need to differentiate between memcpy and memcpy_tio. That's what this
> > entire annoying _cfb_ vs _sys_ business is all about, and also what gem
> > vram helpers have to deal with.
>
> I did some more digging - taking notes see where this gets us.
>
> fbdev have a fb_memcpy_tofb macro used in fb_write() that is architecture
> dependent:
> __aarch64__ memcpy_toio
> __alpha__ memcpy_toio
> __arm__ memcpy_toio
> __hppa__memcpy_toio
> __i386__memcpy_toio
> __powerpc__ memcpy_toio
> __sh__ memcpy_toio
> __sparc__ sbus_memcpy_toio
> __x86_64__ memcpy_toio
>
> Others memcpy
>
> If we then take a closer look at memcpy_toio it is defined like this:
> alpha __raw (optimized based on size / alignment)
> arm optimised memcpy - same as memcpy
> arm64 __raw (optimized based on size / alignment)
> hexagon memcpy
> ia64writeb which is little endian so the same as memcpy
> m68kmemcpy
> mipsmemcpy
> parisc __raw (optimized based on size/alignment)
> s390s390 copy operations
> sh direct copies (looks like __raw copies)
> sparc writeb
> sparc64 sparc64 copies
> x86_64 x86_64 optimies copies (movs assembler)
>
> Others memcpy
Aside from the sbus_memcpy_toio I don't think any of this matters.
fbdev is simply older than memcpy_toio I think, on modern
architectures you should always use memcpy_toio if it's an __mmio
pointer, and normal memcpy for
Re: [PATCH] drm/drm_fb_helper: fix fbdev with sparc64
On Mon, Jul 13, 2020 at 06:21:59PM +0200, Daniel Vetter wrote:
> On Fri, Jul 10, 2020 at 08:28:16AM +0200, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 09.07.20 um 21:30 schrieb Sam Ravnborg:
> > > Mark reported that sparc64 would panic while booting using qemu.
> > > Mark bisected this to a patch that introduced generic fbdev emulation to
> > > the bochs DRM driver.
> > > Mark pointed out that a similar bug was fixed before where
> > > the sys helpers was replaced by cfb helpers.
> > >
> > > The culprint here is that the framebuffer reside in IO memory which
> > > requires SPARC ASI_PHYS (physical) loads and stores.
> > >
> > > The current bohcs DRM driver uses a shadow buffer.
> > > So all copying to the framebuffer happens in
> > > drm_fb_helper_dirty_blit_real().
> > >
> > > The fix is to replace the memcpy with memcpy_toio() from io.h.
> > >
> > > memcpy_toio() uses writeb() where the original fbdev code
> > > used sbus_memcpy_toio(). The latter uses sbus_writeb().
> > >
> > > The difference between writeb() and sbus_memcpy_toio() is
> > > that writeb() writes bytes in little-endian, where sbus_writeb() writes
> > > bytes in big-endian. As endian does not matter for byte writes they are
> > > the same. So we can safely use memcpy_toio() here.
> > >
> > > For many architectures memcpy_toio() is a simple memcpy().
> > > One sideeffect that is unknow is if this has any impact on other
> > > architectures.
> > > So far the analysis tells that this change is OK for other arch's.
> > > but testing would be good.
> > >
> > > Signed-off-by: Sam Ravnborg
> > > Reported-by: Mark Cave-Ayland
> > > Tested-by: Mark Cave-Ayland
> > > Cc: Mark Cave-Ayland
> > > Cc: Thomas Zimmermann
> > > Cc: Gerd Hoffmann
> > > Cc: "David S. Miller"
> > > Cc: sparcli...@vger.kernel.org
> >
> > So this actually is a problem in practice. Do you know how userspace
> > handles this?
> >
> > For this patch
> >
> > Acked-by: Thomas Zimmermann
> >
> > but I'd like to have someone with more architecture expertise ack this
> > as well.
> >
> > Best regards
> > Thomas
> >
> > > ---
> > > drivers/gpu/drm/drm_fb_helper.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > b/drivers/gpu/drm/drm_fb_helper.c
> > > index 5609e164805f..4d05b0ab1592 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> > > drm_fb_helper *fb_helper,
> > > unsigned int y;
> > >
> > > for (y = clip->y1; y < clip->y2; y++) {
> > > - memcpy(dst, src, len);
> > > + memcpy_toio(dst, src, len);
>
> I don't think we can do this unconditionally, there's fbdev-helper drivers
> using shmem helpers, and for shmem memcpy_toio is wrong. We need a switch
> to fix this properly I think.
>
> What Dave Airlie mentioned is just about memcpy_toio vs the sparc bus
> version, for which we don't have any drivers really. But I do think we
> need to differentiate between memcpy and memcpy_tio. That's what this
> entire annoying _cfb_ vs _sys_ business is all about, and also what gem
> vram helpers have to deal with.
I did some more digging - taking notes see where this gets us.
fbdev have a fb_memcpy_tofb macro used in fb_write() that is architecture
dependent:
__aarch64__ memcpy_toio
__alpha__ memcpy_toio
__arm__ memcpy_toio
__hppa__memcpy_toio
__i386__memcpy_toio
__powerpc__ memcpy_toio
__sh__ memcpy_toio
__sparc__ sbus_memcpy_toio
__x86_64__ memcpy_toio
Others memcpy
If we then take a closer look at memcpy_toio it is defined like this:
alpha __raw (optimized based on size / alignment)
arm optimised memcpy - same as memcpy
arm64 __raw (optimized based on size / alignment)
hexagon memcpy
ia64writeb which is little endian so the same as memcpy
m68kmemcpy
mipsmemcpy
parisc __raw (optimized based on size/alignment)
s390s390 copy operations
sh direct copies (looks like __raw copies)
sparc writeb
sparc64 sparc64 copies
x86_64 x86_64 optimies copies (movs assembler)
Others memcpy
As already analyzed sbus_memcpy_toio and memcpy_toio for sparc64 is the
same. So from the above we can see that fbdev code always uses
memcpy_toio or something equivalent when copying to framebuffer.
The conclusions so far:
- Copying to a framebuffer is correct to use memcpy_toio
- fbdev could have the macro fb_memcpy_tofb replaced by a direct call to
memcpy_toio - I think the fb_memcpy_tofb macro predates the common
memcpy_toio macro which explains why this was not used
This
Re: [RFC PATCH] interconnect: qcom: add functions to query addr/cmds for a path
On Mon, Jul 13, 2020 at 06:24:26PM +0300, Georgi Djakov wrote: > On 7/1/20 07:25, Jonathan Marek wrote: > > The a6xx GMU can vote for ddr and cnoc bandwidth, but it needs to be able > > to query the interconnect driver for bcm addresses and commands. > > It's not very clear to me how the GMU firmware would be dealing with this? > Does > anyone have an idea whether the GMU makes any bandwidth decisions? Or is it > just > a static configuration and it just enables/disables a TCS? The GMU can perform a direct vote to the hardware. For now it is a static configuration with pre-determined bandwidths generated from the OPP table. > I think that we can query the address from the cmd-db, but we have to know the > bcm names and the path. All the BCM/TCS information looks to be very low-level > and implementation specific, so exposing it through an API is not very good, > but hard-coding all this information is not good either. Exactly my concern. The BCM information in particular is going to end up being extremely target specific. Jordan > Thanks, > Georgi > > > > > I'm not sure what is the best way to go about implementing this, this is > > what I came up with. > > > > I included a quick example of how this can be used by the a6xx driver to > > fill out the GMU bw_table (two ddr bandwidth levels in this example, note > > this would be using the frequency table in dts and not hardcoded values). > > > > Signed-off-by: Jonathan Marek > > --- > > drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 20 --- > > drivers/interconnect/qcom/icc-rpmh.c | 50 +++ > > include/soc/qcom/icc.h| 11 ++ > > 3 files changed, 68 insertions(+), 13 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [v1] drm/msm/dpu: add support for clk and bw scaling for display
On Mon, Jul 13, 2020 at 8:59 AM wrote:
>
> On 2020-07-10 22:38, Rob Clark wrote:
> > On Thu, Jun 18, 2020 at 7:09 AM Kalyan Thota
> > wrote:
> >>
> >> This change adds support to scale src clk and bandwidth as
> >> per composition requirements.
> >>
> >> Interconnect registration for bw has been moved to mdp
> >> device node from mdss to facilitate the scaling.
> >>
> >> Changes in v1:
> >> - Address armv7 compilation issues with the patch (Rob)
> >>
> >> Signed-off-by: Kalyan Thota
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 109
> >> +
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +-
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 37 -
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h| 4 +
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 9 +-
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 84
> >> +++
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 +
> >> 8 files changed, 233 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> >> index 7c230f7..e52bc44 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> >> @@ -29,6 +29,74 @@ enum dpu_perf_mode {
> >> DPU_PERF_MODE_MAX
> >> };
> >>
> >> +/**
> >> + * @_dpu_core_perf_calc_bw() - to calculate BW per crtc
> >> + * @kms - pointer to the dpu_kms
> >> + * @crtc - pointer to a crtc
> >> + * Return: returns aggregated BW for all planes in crtc.
> >> + */
> >> +static u64 _dpu_core_perf_calc_bw(struct dpu_kms *kms,
> >> + struct drm_crtc *crtc)
> >> +{
> >> + struct drm_plane *plane;
> >> + struct dpu_plane_state *pstate;
> >> + u64 crtc_plane_bw = 0;
> >> + u32 bw_factor;
> >> +
> >> + drm_atomic_crtc_for_each_plane(plane, crtc) {
> >> + pstate = to_dpu_plane_state(plane->state);
> >> + if (!pstate)
> >> + continue;
> >> +
> >> + crtc_plane_bw += pstate->plane_fetch_bw;
> >> + }
> >> +
> >> + bw_factor = kms->catalog->perf.bw_inefficiency_factor;
> >> + if (bw_factor) {
> >> + crtc_plane_bw *= bw_factor;
> >> + do_div(crtc_plane_bw, 100);
> >> + }
> >> +
> >> + return crtc_plane_bw;
> >> +}
> >> +
> >> +/**
> >> + * _dpu_core_perf_calc_clk() - to calculate clock per crtc
> >> + * @kms - pointer to the dpu_kms
> >> + * @crtc - pointer to a crtc
> >> + * @state - pointer to a crtc state
> >> + * Return: returns max clk for all planes in crtc.
> >> + */
> >> +static u64 _dpu_core_perf_calc_clk(struct dpu_kms *kms,
> >> + struct drm_crtc *crtc, struct drm_crtc_state *state)
> >> +{
> >> + struct drm_plane *plane;
> >> + struct dpu_plane_state *pstate;
> >> + struct drm_display_mode *mode;
> >> + u64 crtc_clk;
> >> + u32 clk_factor;
> >> +
> >> + mode = >adjusted_mode;
> >> +
> >> + crtc_clk = mode->vtotal * mode->hdisplay *
> >> drm_mode_vrefresh(mode);
> >> +
> >> + drm_atomic_crtc_for_each_plane(plane, crtc) {
> >> + pstate = to_dpu_plane_state(plane->state);
> >> + if (!pstate)
> >> + continue;
> >> +
> >> + crtc_clk = max(pstate->plane_clk, crtc_clk);
> >> + }
> >> +
> >> + clk_factor = kms->catalog->perf.clk_inefficiency_factor;
> >> + if (clk_factor) {
> >> + crtc_clk *= clk_factor;
> >> + do_div(crtc_clk, 100);
> >> + }
> >> +
> >> + return crtc_clk;
> >> +}
> >> +
> >> static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
> >> {
> >> struct msm_drm_private *priv;
> >> @@ -51,12 +119,7 @@ static void _dpu_core_perf_calc_crtc(struct
> >> dpu_kms *kms,
> >> dpu_cstate = to_dpu_crtc_state(state);
> >> memset(perf, 0, sizeof(struct dpu_core_perf_params));
> >>
> >> - if (!dpu_cstate->bw_control) {
> >> - perf->bw_ctl = kms->catalog->perf.max_bw_high *
> >> - 1000ULL;
> >> - perf->max_per_pipe_ib = perf->bw_ctl;
> >> - perf->core_clk_rate = kms->perf.max_core_clk_rate;
> >> - } else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM)
> >> {
> >> + if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
> >> perf->bw_ctl = 0;
> >> perf->max_per_pipe_ib = 0;
> >> perf->core_clk_rate = 0;
> >> @@ -64,6 +127,10 @@ static void _dpu_core_perf_calc_crtc(struct
> >> dpu_kms *kms,
> >> perf->bw_ctl = kms->perf.fix_core_ab_vote;
> >> perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote;
> >> perf->core_clk_rate =
[PATCH v4 2/2] drm/i915/mst: filter out the display mode exceed sink's capability
From: Lee Shawn C
So far, max dot clock rate for MST mode rely on physcial
bandwidth limitation. It would caused compatibility issue
if source display resolution exceed MST hub output ability.
For example, source DUT had DP 1.2 output capability.
And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
monitor connected. Source would retrieve EDID from external
and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps
because it did not surpass DP physical bandwidth limitation.
Do modeset to 4k@60fps, source output display data but MST
docking can't output HDMI properly due to this resolution
already over HDMI 1.4 spec.
Refer to commit ("drm/dp_mst: Use full_pbn
instead of available_pbn for bandwidth checks").
Source driver should refer to full_pbn to evaluate sink
output capability. And filter out the resolution surpass
sink output limitation.
Changes since v1:
* Using mgr->base.lock to protect full_pbn.
Changes since v2:
* Add ctx lock.
Changes since v3:
* s/intel_dp_mst_mode_clock_exceed_pbn_bandwidth/
intel_dp_mst_mode_clock_exceeds_pbn_bw/
* Use the new drm_connector_helper_funcs.mode_valid_ctx to properly pipe
down the drm_modeset_acquire_ctx that the probe helpers are using, so
we can safely grab >base.lock without deadlocking
Changes since v4:
* Move drm_dp_calc_pbn_mode(mode->clock, bpp, false) > port->full_pbn
check
* Fix the bpp we use in drm_dp_calc_pbn_mode()
* Drop leftover (!mgr) check
* Don't check for if full_pbn is unset. To be clear - it _can_ be unset,
but if it is then it's certainly a bug in DRM or a non-compliant sink
as full_pbn should always be populated by the time we call
->mode_valid_ctx.
We should workaround non-compliant sinks with full_pbn=0, but that
should happen in the DP MST helpers so we can estimate the full_pbn
value as best we can.
Tested-by: Imre Deak
Reviewed-by: Imre Deak
Cc: Manasi Navare
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Cooper Chiou
Co-developed-by: Lyude Paul
Signed-off-by: Lee Shawn C
squash! drm/i915/mst: filter out the display mode exceed sink's capability
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 55 ++---
1 file changed, 38 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index bdc19b04b2c10..a2d91a4997001 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -639,39 +639,60 @@ static int intel_dp_mst_get_modes(struct drm_connector
*connector)
return intel_dp_mst_get_ddc_modes(connector);
}
-static enum drm_mode_status
-intel_dp_mst_mode_valid(struct drm_connector *connector,
- struct drm_display_mode *mode)
+static int
+intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
+ struct drm_display_mode *mode,
+ struct drm_modeset_acquire_ctx *ctx,
+ enum drm_mode_status *status)
{
struct drm_i915_private *dev_priv = to_i915(connector->dev);
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_connector->mst_port;
+ struct drm_dp_mst_topology_mgr *mgr = _dp->mst_mgr;
+ struct drm_dp_mst_port *port = intel_connector->port;
+ const int min_bpp = 18;
int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
int max_rate, mode_rate, max_lanes, max_link_clock;
+ int ret;
- if (drm_connector_is_unregistered(connector))
- return MODE_ERROR;
+ if (drm_connector_is_unregistered(connector)) {
+ *status = MODE_ERROR;
+ return 0;
+ }
- if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
- return MODE_NO_DBLESCAN;
+ if (mode->flags & DRM_MODE_FLAG_DBLSCAN) {
+ *status = MODE_NO_DBLESCAN;
+ return 0;
+ }
max_link_clock = intel_dp_max_link_rate(intel_dp);
max_lanes = intel_dp_max_lane_count(intel_dp);
max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
- mode_rate = intel_dp_link_required(mode->clock, 18);
+ mode_rate = intel_dp_link_required(mode->clock, min_bpp);
- /* TODO - validate mode against available PBN for link */
- if (mode->clock < 1)
- return MODE_CLOCK_LOW;
+ ret = drm_modeset_lock(>base.lock, ctx);
+ if (ret)
+ return ret;
- if (mode->flags & DRM_MODE_FLAG_DBLCLK)
- return MODE_H_ILLEGAL;
+ if (mode_rate > max_rate || mode->clock > max_dotclk ||
+ drm_dp_calc_pbn_mode(mode->clock, min_bpp, false) > port->full_pbn)
{
+ *status = MODE_CLOCK_HIGH;
+ return 0;
+ }
+
+ if (mode->clock < 1) {
+ *status = MODE_CLOCK_LOW;
+ return 0;
+ }
- if (mode_rate >
[PATCH v4 1/2] drm/probe_helper: Add drm_connector_helper_funcs.mode_valid_ctx
This is just an atomic version of mode_valid, which is intended to be
used for situations where a driver might need to check the atomic state
of objects other than the connector itself. One such example is with
MST, where the maximum possible bandwidth on a connector can change
dynamically irregardless of the display configuration.
Changes since v1:
* Use new drm logging functions
* Make some corrections in the mode_valid_ctx kdoc
* Return error codes or 0 from ->mode_valid_ctx() on fail, and store the
drm_mode_status in an additional function parameter
Changes since v2:
* Don't accidentally assign ret to mode->status on success, or we'll
squash legitimate mode validation results
* Don't forget to assign MODE_OK to status in drm_connector_mode_valid()
if we have no callbacks
* Drop leftover hunk in drm_modes.h around enum drm_mode_status
Changes since v3:
* s/return ret/return 0/ in drm_mode_validate_pipeline()
* Minor cleanup in drm_connector_mode_valid()
Tested-by: Imre Deak
Reviewed-by: Imre Deak
Cc: Lee Shawn C
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_crtc_helper_internal.h | 7 +-
drivers/gpu/drm/drm_probe_helper.c | 101 ++---
include/drm/drm_modeset_helper_vtables.h | 42 +
3 files changed, 113 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h
b/drivers/gpu/drm/drm_crtc_helper_internal.h
index f0a66ef47e5ad..25ce42e799952 100644
--- a/drivers/gpu/drm/drm_crtc_helper_internal.h
+++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
@@ -73,8 +73,11 @@ enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc
*crtc,
const struct drm_display_mode *mode);
enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
const struct drm_display_mode
*mode);
-enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
- struct drm_display_mode *mode);
+int
+drm_connector_mode_valid(struct drm_connector *connector,
+struct drm_display_mode *mode,
+struct drm_modeset_acquire_ctx *ctx,
+enum drm_mode_status *status);
struct drm_encoder *
drm_connector_get_single_encoder(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index e0ed58d291ed9..d6017726cc2a0 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -86,17 +86,19 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
return MODE_OK;
}
-static enum drm_mode_status
+static int
drm_mode_validate_pipeline(struct drm_display_mode *mode,
- struct drm_connector *connector)
+ struct drm_connector *connector,
+ struct drm_modeset_acquire_ctx *ctx,
+ enum drm_mode_status *status)
{
struct drm_device *dev = connector->dev;
- enum drm_mode_status ret = MODE_OK;
struct drm_encoder *encoder;
+ int ret;
/* Step 1: Validate against connector */
- ret = drm_connector_mode_valid(connector, mode);
- if (ret != MODE_OK)
+ ret = drm_connector_mode_valid(connector, mode, ctx, status);
+ if (ret || *status != MODE_OK)
return ret;
/* Step 2: Validate against encoders and crtcs */
@@ -104,8 +106,8 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
struct drm_bridge *bridge;
struct drm_crtc *crtc;
- ret = drm_encoder_mode_valid(encoder, mode);
- if (ret != MODE_OK) {
+ *status = drm_encoder_mode_valid(encoder, mode);
+ if (*status != MODE_OK) {
/* No point in continuing for crtc check as this encoder
* will not accept the mode anyway. If all encoders
* reject the mode then, at exit, ret will not be
@@ -114,10 +116,10 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
}
bridge = drm_bridge_chain_get_first_bridge(encoder);
- ret = drm_bridge_chain_mode_valid(bridge,
- >display_info,
- mode);
- if (ret != MODE_OK) {
+ *status = drm_bridge_chain_mode_valid(bridge,
+ >display_info,
+ mode);
+ if (*status != MODE_OK) {
/* There is also no point in continuing for crtc check
* here. */
continue;
@@ -127,17 +129,17 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
if
[PATCH v4 0/2] drm/probe_helper, i915: Validate MST modes against PBN limits
Something we've been missing for a while with drivers that support MST is being able to prune modes that can't be set due to bandwidth limitations. So, let's go ahead and add that. This also adds a new hook that was needed, mode_valid_ctx, so that we can grab additional locks as needed when validating modes. Lee Shawn C (1): drm/i915/mst: filter out the display mode exceed sink's capability Lyude Paul (1): drm/probe_helper: Add drm_connector_helper_funcs.mode_valid_ctx drivers/gpu/drm/drm_crtc_helper_internal.h | 7 +- drivers/gpu/drm/drm_probe_helper.c | 101 +--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 55 +++ include/drm/drm_modeset_helper_vtables.h| 42 4 files changed, 151 insertions(+), 54 deletions(-) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/25] dma-fence: basic lockdep annotations
Am 13.07.20 um 18:26 schrieb Daniel Vetter: Hi Christian, On Wed, Jul 08, 2020 at 04:57:21PM +0200, Christian König wrote: Could we merge this controlled by a separate config option? This way we could have the checks upstream without having to fix all the stuff before we do this? Discussions died out a bit, do you consider this a blocker for the first two patches, or good for an ack on these? Yes, I think the first two can be merged without causing any pain. Feel free to add my ab on them. And the third one can go in immediately as well. Thanks, Christian. Like I said I don't plan to merge patches where I know it causes a lockdep splat with a driver still. At least for now. Thanks, Daniel Thanks, Christian. Am 07.07.20 um 22:12 schrieb Daniel Vetter: Design is similar to the lockdep annotations for workers, but with some twists: - We use a read-lock for the execution/worker/completion side, so that this explicit annotation can be more liberally sprinkled around. With read locks lockdep isn't going to complain if the read-side isn't nested the same way under all circumstances, so ABBA deadlocks are ok. Which they are, since this is an annotation only. - We're using non-recursive lockdep read lock mode, since in recursive read lock mode lockdep does not catch read side hazards. And we _very_ much want read side hazards to be caught. For full details of this limitation see commit e91498589746065e3ae95d9a00b068e525eec34f Author: Peter Zijlstra Date: Wed Aug 23 13:13:11 2017 +0200 locking/lockdep/selftests: Add mixed read-write ABBA tests - To allow nesting of the read-side explicit annotations we explicitly keep track of the nesting. lock_is_held() allows us to do that. - The wait-side annotation is a write lock, and entirely done within dma_fence_wait() for everyone by default. - To be able to freely annotate helper functions I want to make it ok to call dma_fence_begin/end_signalling from soft/hardirq context. First attempt was using the hardirq locking context for the write side in lockdep, but this forces all normal spinlocks nested within dma_fence_begin/end_signalling to be spinlocks. That bollocks. The approach now is to simple check in_atomic(), and for these cases entirely rely on the might_sleep() check in dma_fence_wait(). That will catch any wrong nesting against spinlocks from soft/hardirq contexts. The idea here is that every code path that's critical for eventually signalling a dma_fence should be annotated with dma_fence_begin/end_signalling. The annotation ideally starts right after a dma_fence is published (added to a dma_resv, exposed as a sync_file fd, attached to a drm_syncobj fd, or anything else that makes the dma_fence visible to other kernel threads), up to and including the dma_fence_wait(). Examples are irq handlers, the scheduler rt threads, the tail of execbuf (after the corresponding fences are visible), any workers that end up signalling dma_fences and really anything else. Not annotated should be code paths that only complete fences opportunistically as the gpu progresses, like e.g. shrinker/eviction code. The main class of deadlocks this is supposed to catch are: Thread A: mutex_lock(A); mutex_unlock(A); dma_fence_signal(); Thread B: mutex_lock(A); dma_fence_wait(); mutex_unlock(A); Thread B is blocked on A signalling the fence, but A never gets around to that because it cannot acquire the lock A. Note that dma_fence_wait() is allowed to be nested within dma_fence_begin/end_signalling sections. To allow this to happen the read lock needs to be upgraded to a write lock, which means that any other lock is acquired between the dma_fence_begin_signalling() call and the call to dma_fence_wait(), and still held, this will result in an immediate lockdep complaint. The only other option would be to not annotate such calls, defeating the point. Therefore these annotations cannot be sprinkled over the code entirely mindless to avoid false positives. Originally I hope that the cross-release lockdep extensions would alleviate the need for explicit annotations: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2FArticles%2F709849%2Fdata=02%7C01%7Cchristian.koenig%40amd.com%7Ca3f4bf29ad9640f56a5308d82749770e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637302543770870283sdata=jSHWG%2FNEZ9NqgT4V2l62sEVjfMeH5a%2F4Bbh1SPrKf%2Fw%3Dreserved=0 But there's a few reasons why that's not an option: - It's not happening in upstream, since it got reverted due to too many false positives: commit e966eaeeb623f09975ef362c2866fae6f86844f9 Author: Ingo Molnar Date: Tue Dec 12 12:31:16 2017 +0100 locking/lockdep: Remove the cross-release locking checks This code (CONFIG_LOCKDEP_CROSSRELEASE=y and CONFIG_LOCKDEP_COMPLETIONS=y), while it
Re: [v5] dt-bindings: msm: disp: add yaml schemas for DPU and DSI bindings
On Fri, Jul 10, 2020 at 07:27:49PM +0530, Krishna Manikandan wrote:
> MSM Mobile Display Subsytem (MDSS) encapsulates sub-blocks
> like DPU display controller, DSI etc. Add YAML schema
> for the device tree bindings for the same.
>
> Signed-off-by: Krishna Manikandan
>
> Changes in v2:
> - Changed dpu to DPU (Sam Ravnborg)
> - Fixed indentation issues (Sam Ravnborg)
> - Added empty line between different properties (Sam Ravnborg)
> - Replaced reference txt files with their corresponding
> yaml files (Sam Ravnborg)
> - Modified the file to use "|" only when it is
> necessary (Sam Ravnborg)
>
> Changes in v3:
> - Corrected the license used (Rob Herring)
> - Added maxItems for properties (Rob Herring)
> - Dropped generic descriptions (Rob Herring)
> - Added ranges property (Rob Herring)
> - Corrected the indendation (Rob Herring)
> - Added additionalProperties (Rob Herring)
> - Split dsi file into two, one for dsi controller
> and another one for dsi phy per target (Rob Herring)
> - Corrected description for pinctrl-names (Rob Herring)
> - Corrected the examples used in yaml file (Rob Herring)
> - Delete dsi.txt and dpu.txt (Rob Herring)
>
> Changes in v4:
> - Move schema up by one level (Rob Herring)
> - Add patternProperties for mdp node (Rob Herring)
> - Corrected description of some properties (Rob Herring)
>
> Changes in v5:
> - Correct the indentation (Rob Herring)
> - Remove unnecessary description from properties (Rob Herring)
> - Correct the number of interconnect entries (Rob Herring)
> - Add interconnect names for sc7180 (Rob Herring)
> - Add description for ports (Rob Herring)
> - Remove common properties (Rob Herring)
> - Add unevalutatedProperties (Rob Herring)
> - Reference existing dsi controller yaml in the common
> dsi controller file (Rob Herring)
> - Correct the description of clock names to include only the
> clocks that are required (Rob Herring)
> - Remove properties which are already covered under the common
> binding (Rob Herring)
> - Add dsi phy supply nodes which are required for sc7180 and
> sdm845 targets (Rob Herring)
> - Add type ref for syscon-sfpb (Rob Herring)
> ---
> .../bindings/display/dsi-controller.yaml | 4 +-
> .../bindings/display/msm/dpu-sc7180.yaml | 230 +++
> .../bindings/display/msm/dpu-sdm845.yaml | 210 ++
> .../devicetree/bindings/display/msm/dpu.txt| 141
> .../display/msm/dsi-common-controller.yaml | 178 +++
> .../display/msm/dsi-controller-sc7180.yaml | 115 ++
> .../display/msm/dsi-controller-sdm845.yaml | 115 ++
> .../bindings/display/msm/dsi-phy-sc7180.yaml | 79 +++
> .../bindings/display/msm/dsi-phy-sdm845.yaml | 81 +++
> .../devicetree/bindings/display/msm/dsi-phy.yaml | 79 +++
> .../devicetree/bindings/display/msm/dsi.txt| 246
> -
> 11 files changed, 1089 insertions(+), 389 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
> delete mode 100644 Documentation/devicetree/bindings/display/msm/dpu.txt
> create mode 100644
> Documentation/devicetree/bindings/display/msm/dsi-common-controller.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/msm/dsi-controller-sc7180.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/msm/dsi-controller-sdm845.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/msm/dsi-phy-sc7180.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/msm/dsi-phy-sdm845.yaml
> create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy.yaml
> delete mode 100644 Documentation/devicetree/bindings/display/msm/dsi.txt
>
> diff --git a/Documentation/devicetree/bindings/display/dsi-controller.yaml
> b/Documentation/devicetree/bindings/display/dsi-controller.yaml
> index fd986c3..85b71b1 100644
> --- a/Documentation/devicetree/bindings/display/dsi-controller.yaml
> +++ b/Documentation/devicetree/bindings/display/dsi-controller.yaml
> @@ -28,7 +28,7 @@ description: |
>
> properties:
>$nodename:
> -pattern: "^dsi-controller(@.*)?$"
> +pattern: "^dsi(@.*)?$"
>
>"#address-cells":
> const: 1
> @@ -76,7 +76,7 @@ patternProperties:
> examples:
>- |
> #include
> -dsi-controller@a0351000 {
> +dsi@a0351000 {
> reg = <0xa0351000 0x1000>;
> #address-cells = <1>;
> #size-cells = <0>;
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
>
Re: [PATCH 01/25] dma-fence: basic lockdep annotations
Hi Christian, On Wed, Jul 08, 2020 at 04:57:21PM +0200, Christian König wrote: > Could we merge this controlled by a separate config option? > > This way we could have the checks upstream without having to fix all the > stuff before we do this? Discussions died out a bit, do you consider this a blocker for the first two patches, or good for an ack on these? Like I said I don't plan to merge patches where I know it causes a lockdep splat with a driver still. At least for now. Thanks, Daniel > > Thanks, > Christian. > > Am 07.07.20 um 22:12 schrieb Daniel Vetter: > > Design is similar to the lockdep annotations for workers, but with > > some twists: > > > > - We use a read-lock for the execution/worker/completion side, so that > >this explicit annotation can be more liberally sprinkled around. > >With read locks lockdep isn't going to complain if the read-side > >isn't nested the same way under all circumstances, so ABBA deadlocks > >are ok. Which they are, since this is an annotation only. > > > > - We're using non-recursive lockdep read lock mode, since in recursive > >read lock mode lockdep does not catch read side hazards. And we > >_very_ much want read side hazards to be caught. For full details of > >this limitation see > > > >commit e91498589746065e3ae95d9a00b068e525eec34f > >Author: Peter Zijlstra > >Date: Wed Aug 23 13:13:11 2017 +0200 > > > >locking/lockdep/selftests: Add mixed read-write ABBA tests > > > > - To allow nesting of the read-side explicit annotations we explicitly > >keep track of the nesting. lock_is_held() allows us to do that. > > > > - The wait-side annotation is a write lock, and entirely done within > >dma_fence_wait() for everyone by default. > > > > - To be able to freely annotate helper functions I want to make it ok > >to call dma_fence_begin/end_signalling from soft/hardirq context. > >First attempt was using the hardirq locking context for the write > >side in lockdep, but this forces all normal spinlocks nested within > >dma_fence_begin/end_signalling to be spinlocks. That bollocks. > > > >The approach now is to simple check in_atomic(), and for these cases > >entirely rely on the might_sleep() check in dma_fence_wait(). That > >will catch any wrong nesting against spinlocks from soft/hardirq > >contexts. > > > > The idea here is that every code path that's critical for eventually > > signalling a dma_fence should be annotated with > > dma_fence_begin/end_signalling. The annotation ideally starts right > > after a dma_fence is published (added to a dma_resv, exposed as a > > sync_file fd, attached to a drm_syncobj fd, or anything else that > > makes the dma_fence visible to other kernel threads), up to and > > including the dma_fence_wait(). Examples are irq handlers, the > > scheduler rt threads, the tail of execbuf (after the corresponding > > fences are visible), any workers that end up signalling dma_fences and > > really anything else. Not annotated should be code paths that only > > complete fences opportunistically as the gpu progresses, like e.g. > > shrinker/eviction code. > > > > The main class of deadlocks this is supposed to catch are: > > > > Thread A: > > > > mutex_lock(A); > > mutex_unlock(A); > > > > dma_fence_signal(); > > > > Thread B: > > > > mutex_lock(A); > > dma_fence_wait(); > > mutex_unlock(A); > > > > Thread B is blocked on A signalling the fence, but A never gets around > > to that because it cannot acquire the lock A. > > > > Note that dma_fence_wait() is allowed to be nested within > > dma_fence_begin/end_signalling sections. To allow this to happen the > > read lock needs to be upgraded to a write lock, which means that any > > other lock is acquired between the dma_fence_begin_signalling() call and > > the call to dma_fence_wait(), and still held, this will result in an > > immediate lockdep complaint. The only other option would be to not > > annotate such calls, defeating the point. Therefore these annotations > > cannot be sprinkled over the code entirely mindless to avoid false > > positives. > > > > Originally I hope that the cross-release lockdep extensions would > > alleviate the need for explicit annotations: > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2FArticles%2F709849%2Fdata=02%7C01%7Cchristian.koenig%40amd.com%7Cff1a9dd17c544534eeb808d822b21ba2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637297495649621566sdata=pbDwf%2BAG1UZ5bLZeep7VeGVQMnlQhX0TKG1d6Ok8GfQ%3Dreserved=0 > > > > But there's a few reasons why that's not an option: > > > > - It's not happening in upstream, since it got reverted due to too > >many false positives: > > > > commit e966eaeeb623f09975ef362c2866fae6f86844f9 > > Author: Ingo Molnar > > Date: Tue Dec 12 12:31:16 2017 +0100 > > > > locking/lockdep: Remove the cross-release locking checks > > > >
Re: [PATCH] drm/drm_fb_helper: fix fbdev with sparc64
On Fri, Jul 10, 2020 at 08:28:16AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 09.07.20 um 21:30 schrieb Sam Ravnborg:
> > Mark reported that sparc64 would panic while booting using qemu.
> > Mark bisected this to a patch that introduced generic fbdev emulation to
> > the bochs DRM driver.
> > Mark pointed out that a similar bug was fixed before where
> > the sys helpers was replaced by cfb helpers.
> >
> > The culprint here is that the framebuffer reside in IO memory which
> > requires SPARC ASI_PHYS (physical) loads and stores.
> >
> > The current bohcs DRM driver uses a shadow buffer.
> > So all copying to the framebuffer happens in
> > drm_fb_helper_dirty_blit_real().
> >
> > The fix is to replace the memcpy with memcpy_toio() from io.h.
> >
> > memcpy_toio() uses writeb() where the original fbdev code
> > used sbus_memcpy_toio(). The latter uses sbus_writeb().
> >
> > The difference between writeb() and sbus_memcpy_toio() is
> > that writeb() writes bytes in little-endian, where sbus_writeb() writes
> > bytes in big-endian. As endian does not matter for byte writes they are
> > the same. So we can safely use memcpy_toio() here.
> >
> > For many architectures memcpy_toio() is a simple memcpy().
> > One sideeffect that is unknow is if this has any impact on other
> > architectures.
> > So far the analysis tells that this change is OK for other arch's.
> > but testing would be good.
> >
> > Signed-off-by: Sam Ravnborg
> > Reported-by: Mark Cave-Ayland
> > Tested-by: Mark Cave-Ayland
> > Cc: Mark Cave-Ayland
> > Cc: Thomas Zimmermann
> > Cc: Gerd Hoffmann
> > Cc: "David S. Miller"
> > Cc: sparcli...@vger.kernel.org
>
> So this actually is a problem in practice. Do you know how userspace
> handles this?
>
> For this patch
>
> Acked-by: Thomas Zimmermann
>
> but I'd like to have someone with more architecture expertise ack this
> as well.
>
> Best regards
> Thomas
>
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index 5609e164805f..4d05b0ab1592 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -399,7 +399,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> > drm_fb_helper *fb_helper,
> > unsigned int y;
> >
> > for (y = clip->y1; y < clip->y2; y++) {
> > - memcpy(dst, src, len);
> > + memcpy_toio(dst, src, len);
I don't think we can do this unconditionally, there's fbdev-helper drivers
using shmem helpers, and for shmem memcpy_toio is wrong. We need a switch
to fix this properly I think.
What Dave Airlie mentioned is just about memcpy_toio vs the sparc bus
version, for which we don't have any drivers really. But I do think we
need to differentiate between memcpy and memcpy_tio. That's what this
entire annoying _cfb_ vs _sys_ business is all about, and also what gem
vram helpers have to deal with.
-Daniel
> > src += fb->pitches[0];
> > dst += fb->pitches[0];
> > }
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: KASAN: use-after-free Read in drm_gem_object_release
Adding Thomas, who's the main author for vram helpers. -Daniel On Fri, Jul 10, 2020 at 1:53 PM Dan Carpenter wrote: > > On Fri, Jul 10, 2020 at 04:24:03PM +0800, butt3rflyh4ck wrote: > > I report a bug (in linux-5.8.0-rc4) found by syzkaller. > > > > kernel config: > > https://github.com/butterflyhack/syzkaller-fuzz/blob/master/v5.8.0-rc4.config > > > > I test the reproducer and crash too. > > > > In the drm_em_vram_t() function, ttm_bo_init() function call > ^ > This a typo. The function name is drm_gem_vram_init(). > > > ttm_bo_init_reserved(), > > the ttm_bo_init_reserved() function call ttm_bo_put(), it will free > > gbo->bo that is struct ttm_buffer_object. > > > > then, goto the err_drm_gem_object_release lable, > > drm_gem_object_release() function will free gbo->bo.base, so cause use > > after free. > > > > There is a third free in drm_gem_vram_create(). This is a triple free > bug. The correct place to free this is in drm_gem_vram_create() because > that's where it was allocated. > > This code is quite subtle so I'm not going to attempt to fix it because > I can't test it. > > regards, > dan carpenter > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vkms: add wait_for_vblanks in atomic_commit_tail
On Fri, Jul 10, 2020 at 02:05:33PM -0300, Melissa Wen wrote: > On 07/02, Daniel Vetter wrote: > > On Wed, Jul 01, 2020 at 03:31:34PM +, Sidong Yang wrote: > > > there is an error when igt test is run continuously. > > > vkms_atomic_commit_tail() > > > need to call drm_atomic_helper_wait_for_vblanks() for give up ownership of > > > vblank events. without this code, next atomic commit will not enable > > > vblank > > > and raise timeout error. > > > > > > Signed-off-by: Sidong Yang > > > --- > > > drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c > > > b/drivers/gpu/drm/vkms/vkms_drv.c > > > index 1e8b2169d834..10b9be67a068 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > @@ -93,6 +93,8 @@ static void vkms_atomic_commit_tail(struct > > > drm_atomic_state *old_state) > > > flush_work(_state->composer_work); > > > } > > > > > > + drm_atomic_helper_wait_for_vblanks(dev, old_state); > > > > Uh, we have a wait_for_flip_done right above, which should be doing > > exactly the same, but more precisely: Instead of just waiting for any > > vblank to happen, we wait for exactly the vblank corresponding to this > > atomic commit. So no races possible. If this is papering over some issue, > > then I think more debugging is needed. > > > > What exactly is going wrong here for you? > > Hi Daniel and Sidong, > > I noticed a similar issue when running the IGT test kms_cursor_crc. For > example, a subtest that passes on the first run (alpha-opaque) fails on > the second due to a kind of busy waiting in subtest preparation (the > subtest fails before actually running). > > In addition, in the same test, the dpms subtest started to fail since > the commit that change from wait_for_vblanks to wait_for_flip_done. By > reverting this commit, the dpms subtest passes again and the sequential > subtests return to normal. > > I am trying to figure out what's missing from using flip_done op on > vkms, since I am also interested in solving this problem and I > understand that the change for flip_done has been discussed in the past. > > Do you have any idea? Uh, not at all. This is indeed rather surprising ... What exactly is the failure mode when running a test the 2nd time? Full igt logs might give me an idea. But yeah this is kinda surprising. Also happy to chat on irc for debugging ideas, that might be faster (I'm danvet on #dri-devel on freenode). -Daniel > > Melissa > > > -Daniel > > > > > + > > > drm_atomic_helper_cleanup_planes(dev, old_state); > > > } > > > > > > -- > > > 2.17.1 > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/exynos: gem: Fix sparse warning
On Mon, Jul 13, 2020 at 09:07:08AM +0200, Marek Szyprowski wrote:
> kvaddr element of the exynos_gem object points to a memory buffer, thus
> it should not have a __iomem annotation. Then, to avoid a warning or
> casting on assignment to fbi structure, the screen_buffer element of the
> union should be used instead of the screen_base.
>
> Reported-by: kernel test robot
> Suggested-by: Sam Ravnborg
> Signed-off-by: Marek Szyprowski
Reviewed-by: Sam Ravnborg
I expect one of the exynos maintianers (Inki?) to pick it up.
Sam
> ---
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_gem.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 56a2b47e1af7..5147f5929be7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -92,7 +92,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper
> *helper,
> offset = fbi->var.xoffset * fb->format->cpp[0];
> offset += fbi->var.yoffset * fb->pitches[0];
>
> - fbi->screen_base = exynos_gem->kvaddr + offset;
> + fbi->screen_buffer = exynos_gem->kvaddr + offset;
> fbi->screen_size = size;
> fbi->fix.smem_len = size;
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> index 7445748288da..74e926abeff0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> @@ -40,7 +40,7 @@ struct exynos_drm_gem {
> unsigned intflags;
> unsigned long size;
> void*cookie;
> - void __iomem*kvaddr;
> + void*kvaddr;
> dma_addr_t dma_addr;
> unsigned long dma_attrs;
> struct sg_table *sgt;
> --
> 2.17.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: sil_sii8620: initialize return of sii8620_readb
Hi Tom.
On Sun, Jul 12, 2020 at 08:24:53AM -0700, t...@redhat.com wrote:
> From: Tom Rix
>
> clang static analysis flags this error
>
> sil-sii8620.c:184:2: warning: Undefined or garbage value
> returned to caller [core.uninitialized.UndefReturn]
> return ret;
> ^~
>
> sii8620_readb calls sii8620_read_buf.
> sii8620_read_buf can return without setting its output
> pararmeter 'ret'.
>
> So initialize ret.
>
> Fixes: ce6e153f414a ("drm/bridge: add Silicon Image SiI8620 driver")
>
> Signed-off-by: Tom Rix
Thnaks, applied to drm-misc-next as the fix is not urgent.
Sam
> ---
> drivers/gpu/drm/bridge/sil-sii8620.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 3540e4931383..da933d477e5f 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -178,7 +178,7 @@ static void sii8620_read_buf(struct sii8620 *ctx, u16
> addr, u8 *buf, int len)
>
> static u8 sii8620_readb(struct sii8620 *ctx, u16 addr)
> {
> - u8 ret;
> + u8 ret = 0;
>
> sii8620_read_buf(ctx, addr, , 1);
> return ret;
> --
> 2.18.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/20] Documentation: eliminate duplicated words
On Tue, 7 Jul 2020 11:03:54 -0700 Randy Dunlap wrote: > Documentation/admin-guide/mm/numaperf.rst |2 +- > Documentation/block/pr.rst|2 +- > Documentation/core-api/printk-basics.rst |2 +- > Documentation/dev-tools/kgdb.rst |2 +- > Documentation/fpga/dfl.rst|2 +- > Documentation/gpu/drm-uapi.rst|2 +- > Documentation/gpu/komeda-kms.rst |2 +- > Documentation/hid/intel-ish-hid.rst |2 +- > Documentation/i2c/upgrading-clients.rst |2 +- > Documentation/kbuild/kconfig-language.rst |2 +- > Documentation/leds/ledtrig-transient.rst |2 +- > Documentation/maintainer/maintainer-entry-profile.rst |2 +- > Documentation/mips/ingenic-tcu.rst|2 +- > Documentation/misc-devices/xilinx_sdfec.rst |2 +- > Documentation/powerpc/vas-api.rst |2 +- > Documentation/s390/vfio-ap.rst|2 +- > Documentation/scsi/advansys.rst |2 +- > Documentation/security/keys/trusted-encrypted.rst |2 +- > Documentation/virt/kvm/api.rst|2 +- > Documentation/vm/memory-model.rst |2 +- > 20 files changed, 20 insertions(+), 20 deletions(-) I've applied this set, minus #17 that was already picked up by Martin. Thanks, jon ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: sil_sii8620: initialize return of sii8620_readb
On 12.07.2020 17:24, t...@redhat.com wrote:
> From: Tom Rix
>
> clang static analysis flags this error
>
> sil-sii8620.c:184:2: warning: Undefined or garbage value
>returned to caller [core.uninitialized.UndefReturn]
> return ret;
> ^~
>
> sii8620_readb calls sii8620_read_buf.
> sii8620_read_buf can return without setting its output
> pararmeter 'ret'.
>
> So initialize ret.
>
> Fixes: ce6e153f414a ("drm/bridge: add Silicon Image SiI8620 driver")
>
> Signed-off-by: Tom Rix
> ---
> drivers/gpu/drm/bridge/sil-sii8620.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 3540e4931383..da933d477e5f 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -178,7 +178,7 @@ static void sii8620_read_buf(struct sii8620 *ctx, u16
> addr, u8 *buf, int len)
>
> static u8 sii8620_readb(struct sii8620 *ctx, u16 addr)
> {
> - u8 ret;
> + u8 ret = 0;
In theory it shouldn't cause any harm, but this protections makes things
simpler.
Reviewed-by: Andrzej Hajda
Regards
Andrzej
>
> sii8620_read_buf(ctx, addr, , 1);
> return ret;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] drm/msm: Avoid possible infinite probe deferral and speed booting
Hi, On Mon, Jul 13, 2020 at 7:11 AM Rob Herring wrote: > > On Fri, Jul 10, 2020 at 5:02 PM Douglas Anderson > wrote: > > > > I found that if I ever had a little mistake in my kernel config, > > or device tree, or graphics driver that my system would sit in a loop > > at bootup trying again and again and again. An example log was: > > Why do we care about optimizing the error case? It actually results in a _fully_ infinite loop. That is: if anything small causes a component of DRM to fail to probe then the whole system doesn't boot because it just loops trying to probe over and over again. The messages I put in the commit message are printed over and over and over again. > > msm ae0.mdss: bound ae01000.mdp (ops 0xffe596e951f8) > > msm_dsi ae94000.dsi: ae94000.dsi supply gdsc not found, using dummy > > regulator > > msm_dsi_manager_register: failed to register mipi dsi host for DSI 0 > > [drm:ti_sn_bridge_probe] *ERROR* could not find any panel node > > ... > > > > I finally tracked it down where this was happening: > > - msm_pdev_probe() is called. > > - msm_pdev_probe() registers drivers. Registering drivers kicks > > off processing of probe deferrals. > > - component_master_add_with_match() could return -EPROBE_DEFER. > > making msm_pdev_probe() return -EPROBE_DEFER. > > - When msm_pdev_probe() returned the processing of probe deferrals > > happens. > > - Loop back to the start. > > > > It looks like we can fix this by marking "mdss" as a "simple-bus". > > I have no idea if people consider this the right thing to do or a > > hack. Hopefully it's the right thing to do. :-) > > It's a simple test. Do the child devices have any dependency on the > parent to probe and/or function? If so, not a simple-bus. Great! You can see in the earlier patch in the series that the very first thing that happens when the parent device probes is that it calls devm_of_platform_populate(). That means no dependencies, right? So that means it's fine/correct to add "simple-bus" here? > > Once I do this I notice that my boot gets marginally faster (you > > don't need to probe the sub devices over and over) and also if I > > Can you quantify that? I'd say < 100 us. I can try to quantify more if needed, but it wasn't the point of this patch. > Have you run with devlinks enabled. You need a command line option to > enable. That too should reduce deferred probes. Ah, good idea! I will try it. However, even with devlinks, if there is any chance of deferred probes then we need a fix like this. The point of the patch isn't about speeding things up but about avoiding an infinite loop at bootup due to a small bug. > > have a problem it doesn't loop forever (on my system it still > > gets upset about some stuck clocks in that case, but at least I > > can boot up). > > Deferred probe only runs when a device is added, so it's not like it > is continually running. If you don't mind looking at the code patch, see: https://lore.kernel.org/r/20200710160131.4.I358ea82de218ea5f4406572ade23f5e121297555@changeid/ Specifically you can see that each time we try to probe we were calling of_platform_populate(). That appeared to be enough to trigger things. -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 1/4] driver core: add device probe log helper
On Mon, Jul 13, 2020 at 5:43 PM Andrzej Hajda wrote:
>
> During probe every time driver gets resource it should usually check for
> error printk some message if it is not -EPROBE_DEFER and return the error.
> This pattern is simple but requires adding few lines after any resource
> acquisition code, as a result it is often omitted or implemented only
> partially.
> dev_err_probe helps to replace such code sequences with simple call,
> so code:
> if (err != -EPROBE_DEFER)
> dev_err(dev, ...);
> return err;
> becomes:
> return dev_err_probe(dev, err, ...);
>
FWIW,
Reviewed-by: Andy Shevchenko
> Signed-off-by: Andrzej Hajda
> Reviewed-by: Rafael J. Wysocki
> Reviewed-by: Mark Brown
> ---
> drivers/base/core.c| 42 ++
> include/linux/device.h | 3 +++
> 2 files changed, 45 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 67d39a90b45c..3a827c82933f 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3953,6 +3953,48 @@ define_dev_printk_level(_dev_info, KERN_INFO);
>
> #endif
>
> +/**
> + * dev_err_probe - probe error check and log helper
> + * @dev: the pointer to the struct device
> + * @err: error value to test
> + * @fmt: printf-style format string
> + * @...: arguments as specified in the format string
> + *
> + * This helper implements common pattern present in probe functions for error
> + * checking: print debug or error message depending if the error value is
> + * -EPROBE_DEFER and propagate error upwards.
> + * It replaces code sequence:
> + * if (err != -EPROBE_DEFER)
> + * dev_err(dev, ...);
> + * else
> + * dev_dbg(dev, ...);
> + * return err;
> + * with
> + * return dev_err_probe(dev, err, ...);
> + *
> + * Returns @err.
> + *
> + */
> +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, fmt);
> + vaf.fmt = fmt;
> + vaf.va =
> + if (err != -EPROBE_DEFER)
I would rather see positive conditional (slightly better to parse when
read), but here I think it's more or less equal.
> + dev_err(dev, "error %d: %pV", err, );
> + else
> + dev_dbg(dev, "error %d: %pV", err, );
> + va_end(args);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(dev_err_probe);
> +
> static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> {
> return fwnode && !IS_ERR(fwnode->secondary);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 15460a5ac024..6b2272ae9af8 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -964,6 +964,9 @@ void device_link_remove(void *consumer, struct device
> *supplier);
> void device_links_supplier_sync_state_pause(void);
> void device_links_supplier_sync_state_resume(void);
>
> +extern __printf(3, 4)
> +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
> +
> /* Create alias, so I can be autoloaded. */
> #define MODULE_ALIAS_CHARDEV(major,minor) \
> MODULE_ALIAS("char-major-" __stringify(major) "-" __stringify(minor))
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #24 from Maurice Gale (mauricega...@gmail.com) --- Created attachment 290253 --> https://bugzilla.kernel.org/attachment.cgi?id=290253=edit New log with drm 116 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #23 from Maurice Gale (mauricega...@gmail.com) --- Hi! Here's my environment: Monitor: - dell p4317q (Quad display Monitor) - Aside from the above, I have also tested on 4 separate monitors. Connectors: - Display -> HDMI (Brand: Ivanky) - Display -> HDMI (Brand: Ivanky) - Display -> Mini Display (Brand: Ivanky) - Display -> Display (Brand: Ivanky) Desktop: - Dell Optiplex Xe3 - No laptops or special adaptors are used Link to connectors: https://www.amazon.com/gp/product/B076RMR48C/ref=ppx_yo_dt_b_asin_title_o06_s00?ie=UTF8=1 I have attached the new log with suggested boot options (116-drm-log.txt) Please let me know if there is anything else that you need. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/i915/selftest: fix an error return path where err is not being set
Quoting Colin King (2020-07-13 15:25:51)
> From: Colin Ian King
>
> There is an error condition where err is not being set and an uninitialized
> garbage value in err is being returned. Fix this by assigning err to an
> appropriate error return value before taking the error exit path.
>
> Addresses-Coverity: ("Uninitialized scalar value")
> Fixes: ed2690a9ca89 ("drm/i915/selftest: Check that GPR are restored across
> noa_wait")
> Signed-off-by: Colin Ian King
Thanks, pushed.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v5] dt-bindings: msm: disp: add yaml schemas for DPU and DSI bindings
On Fri, 10 Jul 2020 19:27:49 +0530, Krishna Manikandan wrote: > MSM Mobile Display Subsytem (MDSS) encapsulates sub-blocks > like DPU display controller, DSI etc. Add YAML schema > for the device tree bindings for the same. > > Signed-off-by: Krishna Manikandan > > Changes in v2: > - Changed dpu to DPU (Sam Ravnborg) > - Fixed indentation issues (Sam Ravnborg) > - Added empty line between different properties (Sam Ravnborg) > - Replaced reference txt files with their corresponding > yaml files (Sam Ravnborg) > - Modified the file to use "|" only when it is > necessary (Sam Ravnborg) > > Changes in v3: > - Corrected the license used (Rob Herring) > - Added maxItems for properties (Rob Herring) > - Dropped generic descriptions (Rob Herring) > - Added ranges property (Rob Herring) > - Corrected the indendation (Rob Herring) > - Added additionalProperties (Rob Herring) > - Split dsi file into two, one for dsi controller > and another one for dsi phy per target (Rob Herring) > - Corrected description for pinctrl-names (Rob Herring) > - Corrected the examples used in yaml file (Rob Herring) > - Delete dsi.txt and dpu.txt (Rob Herring) > > Changes in v4: > - Move schema up by one level (Rob Herring) > - Add patternProperties for mdp node (Rob Herring) > - Corrected description of some properties (Rob Herring) > > Changes in v5: > - Correct the indentation (Rob Herring) > - Remove unnecessary description from properties (Rob Herring) > - Correct the number of interconnect entries (Rob Herring) > - Add interconnect names for sc7180 (Rob Herring) > - Add description for ports (Rob Herring) > - Remove common properties (Rob Herring) > - Add unevalutatedProperties (Rob Herring) > - Reference existing dsi controller yaml in the common > dsi controller file (Rob Herring) > - Correct the description of clock names to include only the > clocks that are required (Rob Herring) > - Remove properties which are already covered under the common > binding (Rob Herring) > - Add dsi phy supply nodes which are required for sc7180 and > sdm845 targets (Rob Herring) > - Add type ref for syscon-sfpb (Rob Herring) > --- > .../bindings/display/dsi-controller.yaml | 4 +- > .../bindings/display/msm/dpu-sc7180.yaml | 230 +++ > .../bindings/display/msm/dpu-sdm845.yaml | 210 ++ > .../devicetree/bindings/display/msm/dpu.txt| 141 > .../display/msm/dsi-common-controller.yaml | 178 +++ > .../display/msm/dsi-controller-sc7180.yaml | 115 ++ > .../display/msm/dsi-controller-sdm845.yaml | 115 ++ > .../bindings/display/msm/dsi-phy-sc7180.yaml | 79 +++ > .../bindings/display/msm/dsi-phy-sdm845.yaml | 81 +++ > .../devicetree/bindings/display/msm/dsi-phy.yaml | 79 +++ > .../devicetree/bindings/display/msm/dsi.txt| 246 > - > 11 files changed, 1089 insertions(+), 389 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml > delete mode 100644 Documentation/devicetree/bindings/display/msm/dpu.txt > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-common-controller.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-controller-sc7180.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-controller-sdm845.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-phy-sc7180.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-phy-sdm845.yaml > create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy.yaml > delete mode 100644 Documentation/devicetree/bindings/display/msm/dsi.txt > My bot found errors running 'make dt_binding_check' on your patch: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/msm/dsi-controller-sc7180.example.dt.yaml: example-0: dsi@ae94000:reg:0: [0, 183058432, 0, 1024] is too long /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/msm/dpu-sdm845.example.dt.yaml: example-0: mdss@ae0:reg:0: [0, 182452224, 0, 4096] is too long /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/msm/dsi-phy-sc7180.example.dt.yaml: example-0: dsi-phy@ae94400:reg:0: [0, 183059456, 0, 512] is too long /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/msm/dsi-phy-sc7180.example.dt.yaml: example-0: dsi-phy@ae94400:reg:1: [0, 183059968, 0, 640] is too long
[PATCH v9 0/4] driver core: add probe error check helper
Hi All, Thanks for comments. Changes since v8: - fixed typo in function name, - removed cocci script (added by mistake) Changes since v7: - improved commit message - added R-Bs Changes since v6: - removed leftovers from old naming scheme in commit descritions, - added R-Bs. Changes since v5: - removed patch adding macro, dev_err_probe(dev, PTR_ERR(ptr), ...) should be used instead, - added dev_dbg logging in case of -EPROBE_DEFER, - renamed functions and vars according to comments, - extended docs, - cosmetics. Original message (with small adjustments): Recently I took some time to re-check error handling in drivers probe code, and I have noticed that number of incorrect resource acquisition error handling increased and there are no other propositions which can cure the situation. So I have decided to resend my old proposition of probe_err helper which should simplify resource acquisition error handling, it also extend it with adding defer probe reason to devices_deferred debugfs property, which should improve debugging experience for developers/testers. I have also added two patches showing usage and benefits of the helper. My dirty/ad-hoc cocci scripts shows that this helper can be used in at least 2700 places saving about 3500 lines of code. Regards Andrzej Andrzej Hajda (4): driver core: add device probe log helper driver core: add deferring probe reason to devices_deferred property drm/bridge/sii8620: fix resource acquisition error handling drm/bridge: lvds-codec: simplify error handling drivers/base/base.h | 3 ++ drivers/base/core.c | 46 drivers/base/dd.c| 23 +- drivers/gpu/drm/bridge/lvds-codec.c | 10 ++ drivers/gpu/drm/bridge/sil-sii8620.c | 21 ++--- include/linux/device.h | 3 ++ 6 files changed, 86 insertions(+), 20 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 3/4] drm/bridge/sii8620: fix resource acquisition error handling
In case of error during resource acquisition driver should print error
message only in case it is not deferred probe, using dev_err_probe helper
solves the issue. Moreover it records defer probe reason for debugging.
Signed-off-by: Andrzej Hajda
Reviewed-by: Neil Armstrong
---
drivers/gpu/drm/bridge/sil-sii8620.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
b/drivers/gpu/drm/bridge/sil-sii8620.c
index 92acd336aa89..389c1f029774 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -2299,10 +2299,9 @@ static int sii8620_probe(struct i2c_client *client,
INIT_LIST_HEAD(>mt_queue);
ctx->clk_xtal = devm_clk_get(dev, "xtal");
- if (IS_ERR(ctx->clk_xtal)) {
- dev_err(dev, "failed to get xtal clock from DT\n");
- return PTR_ERR(ctx->clk_xtal);
- }
+ if (IS_ERR(ctx->clk_xtal))
+ return dev_err_probe(dev, PTR_ERR(ctx->clk_xtal),
+"failed to get xtal clock from DT\n");
if (!client->irq) {
dev_err(dev, "no irq provided\n");
@@ -2313,16 +2312,14 @@ static int sii8620_probe(struct i2c_client *client,
sii8620_irq_thread,
IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
"sii8620", ctx);
- if (ret < 0) {
- dev_err(dev, "failed to install IRQ handler\n");
- return ret;
- }
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+"failed to install IRQ handler\n");
ctx->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
- if (IS_ERR(ctx->gpio_reset)) {
- dev_err(dev, "failed to get reset gpio from DT\n");
- return PTR_ERR(ctx->gpio_reset);
- }
+ if (IS_ERR(ctx->gpio_reset))
+ return dev_err_probe(dev, PTR_ERR(ctx->gpio_reset),
+"failed to get reset gpio from DT\n");
ctx->supplies[0].supply = "cvcc10";
ctx->supplies[1].supply = "iovcc18";
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 1/4] driver core: add device probe log helper
During probe every time driver gets resource it should usually check for
error printk some message if it is not -EPROBE_DEFER and return the error.
This pattern is simple but requires adding few lines after any resource
acquisition code, as a result it is often omitted or implemented only
partially.
dev_err_probe helps to replace such code sequences with simple call,
so code:
if (err != -EPROBE_DEFER)
dev_err(dev, ...);
return err;
becomes:
return dev_err_probe(dev, err, ...);
Signed-off-by: Andrzej Hajda
Reviewed-by: Rafael J. Wysocki
Reviewed-by: Mark Brown
---
drivers/base/core.c| 42 ++
include/linux/device.h | 3 +++
2 files changed, 45 insertions(+)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 67d39a90b45c..3a827c82933f 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3953,6 +3953,48 @@ define_dev_printk_level(_dev_info, KERN_INFO);
#endif
+/**
+ * dev_err_probe - probe error check and log helper
+ * @dev: the pointer to the struct device
+ * @err: error value to test
+ * @fmt: printf-style format string
+ * @...: arguments as specified in the format string
+ *
+ * This helper implements common pattern present in probe functions for error
+ * checking: print debug or error message depending if the error value is
+ * -EPROBE_DEFER and propagate error upwards.
+ * It replaces code sequence:
+ * if (err != -EPROBE_DEFER)
+ * dev_err(dev, ...);
+ * else
+ * dev_dbg(dev, ...);
+ * return err;
+ * with
+ * return dev_err_probe(dev, err, ...);
+ *
+ * Returns @err.
+ *
+ */
+int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ va_start(args, fmt);
+ vaf.fmt = fmt;
+ vaf.va =
+
+ if (err != -EPROBE_DEFER)
+ dev_err(dev, "error %d: %pV", err, );
+ else
+ dev_dbg(dev, "error %d: %pV", err, );
+
+ va_end(args);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(dev_err_probe);
+
static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
{
return fwnode && !IS_ERR(fwnode->secondary);
diff --git a/include/linux/device.h b/include/linux/device.h
index 15460a5ac024..6b2272ae9af8 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -964,6 +964,9 @@ void device_link_remove(void *consumer, struct device
*supplier);
void device_links_supplier_sync_state_pause(void);
void device_links_supplier_sync_state_resume(void);
+extern __printf(3, 4)
+int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
+
/* Create alias, so I can be autoloaded. */
#define MODULE_ALIAS_CHARDEV(major,minor) \
MODULE_ALIAS("char-major-" __stringify(major) "-" __stringify(minor))
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 4/4] drm/bridge: lvds-codec: simplify error handling
Using dev_err_probe code has following advantages:
- shorter code,
- recorded defer probe reason for debugging,
- uniform error code logging.
Signed-off-by: Andrzej Hajda
Reviewed-by: Neil Armstrong
---
drivers/gpu/drm/bridge/lvds-codec.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/lvds-codec.c
b/drivers/gpu/drm/bridge/lvds-codec.c
index 24fb1befdfa2..f19d9f7a5db2 100644
--- a/drivers/gpu/drm/bridge/lvds-codec.c
+++ b/drivers/gpu/drm/bridge/lvds-codec.c
@@ -71,13 +71,9 @@ static int lvds_codec_probe(struct platform_device *pdev)
lvds_codec->connector_type = (uintptr_t)of_device_get_match_data(dev);
lvds_codec->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
GPIOD_OUT_HIGH);
- if (IS_ERR(lvds_codec->powerdown_gpio)) {
- int err = PTR_ERR(lvds_codec->powerdown_gpio);
-
- if (err != -EPROBE_DEFER)
- dev_err(dev, "powerdown GPIO failure: %d\n", err);
- return err;
- }
+ if (IS_ERR(lvds_codec->powerdown_gpio))
+ return dev_err_probe(dev, PTR_ERR(lvds_codec->powerdown_gpio),
+"powerdown GPIO failure\n");
/* Locate the panel DT node. */
panel_node = of_graph_get_remote_node(dev->of_node, 1, 0);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 2/4] driver core: add deferring probe reason to devices_deferred property
/sys/kernel/debug/devices_deferred property contains list of deferred devices.
This list does not contain reason why the driver deferred probe, the patch
improves it.
The natural place to set the reason is dev_err_probe function introduced
recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of
printk the message will be attached to a deferred device and printed when user
reads devices_deferred property.
Signed-off-by: Andrzej Hajda
Reviewed-by: Mark Brown
Reviewed-by: Javier Martinez Canillas
Reviewed-by: Andy Shevchenko
Reviewed-by: Rafael J. Wysocki
---
v9:
- fixed typo in function name
v8:
- improved commit message
---
drivers/base/base.h | 3 +++
drivers/base/core.c | 8 ++--
drivers/base/dd.c | 23 ++-
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 95c22c0f9036..c3562adf4789 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -93,6 +93,7 @@ struct device_private {
struct klist_node knode_class;
struct list_head deferred_probe;
struct device_driver *async_driver;
+ char *deferred_probe_reason;
struct device *device;
u8 dead:1;
};
@@ -134,6 +135,8 @@ extern void device_release_driver_internal(struct device
*dev,
extern void driver_detach(struct device_driver *drv);
extern int driver_probe_device(struct device_driver *drv, struct device *dev);
extern void driver_deferred_probe_del(struct device *dev);
+extern void device_set_deferred_probe_reason(const struct device *dev,
+struct va_format *vaf);
static inline int driver_match_device(struct device_driver *drv,
struct device *dev)
{
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3a827c82933f..d04d19458795 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3963,6 +3963,8 @@ define_dev_printk_level(_dev_info, KERN_INFO);
* This helper implements common pattern present in probe functions for error
* checking: print debug or error message depending if the error value is
* -EPROBE_DEFER and propagate error upwards.
+ * In case of -EPROBE_DEFER it sets also defer probe reason, which can be
+ * checked later by reading devices_deferred debugfs attribute.
* It replaces code sequence:
* if (err != -EPROBE_DEFER)
* dev_err(dev, ...);
@@ -3984,10 +3986,12 @@ int dev_err_probe(const struct device *dev, int err,
const char *fmt, ...)
vaf.fmt = fmt;
vaf.va =
- if (err != -EPROBE_DEFER)
+ if (err != -EPROBE_DEFER) {
dev_err(dev, "error %d: %pV", err, );
- else
+ } else {
+ device_set_deferred_probe_reason(dev, );
dev_dbg(dev, "error %d: %pV", err, );
+ }
va_end(args);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 9a1d940342ac..7555b31bdfdc 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "base.h"
#include "power/power.h"
@@ -136,6 +137,8 @@ void driver_deferred_probe_del(struct device *dev)
if (!list_empty(>p->deferred_probe)) {
dev_dbg(dev, "Removed from deferred list\n");
list_del_init(>p->deferred_probe);
+ kfree(dev->p->deferred_probe_reason);
+ dev->p->deferred_probe_reason = NULL;
}
mutex_unlock(_probe_mutex);
}
@@ -211,6 +214,23 @@ void device_unblock_probing(void)
driver_deferred_probe_trigger();
}
+/**
+ * device_set_deferred_probe_reason() - Set defer probe reason message for
device
+ * @dev: the pointer to the struct device
+ * @vaf: the pointer to va_format structure with message
+ */
+void device_set_deferred_probe_reason(const struct device *dev, struct
va_format *vaf)
+{
+ const char *drv = dev_driver_string(dev);
+
+ mutex_lock(_probe_mutex);
+
+ kfree(dev->p->deferred_probe_reason);
+ dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s: %pV", drv,
vaf);
+
+ mutex_unlock(_probe_mutex);
+}
+
/*
* deferred_devs_show() - Show the devices in the deferred probe pending list.
*/
@@ -221,7 +241,8 @@ static int deferred_devs_show(struct seq_file *s, void
*data)
mutex_lock(_probe_mutex);
list_for_each_entry(curr, _probe_pending_list, deferred_probe)
- seq_printf(s, "%s\n", dev_name(curr->device));
+ seq_printf(s, "%s\t%s", dev_name(curr->device),
+ curr->device->p->deferred_probe_reason ?: "\n");
mutex_unlock(_probe_mutex);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/i915/selftest: fix an error return path where err is not being set
From: Colin Ian King
There is an error condition where err is not being set and an uninitialized
garbage value in err is being returned. Fix this by assigning err to an
appropriate error return value before taking the error exit path.
Addresses-Coverity: ("Uninitialized scalar value")
Fixes: ed2690a9ca89 ("drm/i915/selftest: Check that GPR are restored across
noa_wait")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/i915/selftests/i915_perf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/selftests/i915_perf.c
b/drivers/gpu/drm/i915/selftests/i915_perf.c
index deb6dec1b5ab..7aa73bb03381 100644
--- a/drivers/gpu/drm/i915/selftests/i915_perf.c
+++ b/drivers/gpu/drm/i915/selftests/i915_perf.c
@@ -329,6 +329,7 @@ static int live_noa_gpr(void *arg)
cs = intel_ring_begin(rq, 2 * 32 + 2);
if (IS_ERR(cs)) {
i915_request_add(rq);
+ err = PTR_ERR(cs);
goto out_rq;
}
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [v1] drm/msm/dpu: enumerate second cursor pipe for external interface
On Mon, Jul 13, 2020 at 3:18 AM wrote:
>
> On 2020-07-10 22:19, Rob Clark wrote:
> > On Thu, Jun 25, 2020 at 5:46 AM Kalyan Thota
> > wrote:
> >>
> >> Setup an RGB HW pipe as cursor which can be used on
> >> secondary interface.
> >>
> >> For SC7180 2 HW pipes are enumerated as cursors
> >> 1 - primary interface
> >> 2 - secondary interface
> >>
> >> Signed-off-by: Kalyan Thota
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 ++--
> >> 1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> index 8f2357d..23061fd 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> @@ -117,10 +117,10 @@
> >> .reg_off = 0x2AC, .bit_off = 0},
> >> .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> >> .reg_off = 0x2AC, .bit_off = 8},
> >> - .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> >> - .reg_off = 0x2B4, .bit_off = 8},
> >> .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> >> - .reg_off = 0x2BC, .bit_off = 8},
> >> + .reg_off = 0x2B4, .bit_off = 8},
> >> + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> >> + .reg_off = 0x2C4, .bit_off = 8},
> >
> > It looks like you shifted the register offset here from 0x2bc to
> > 0x2c4, was that intentional?
> >
> > BR,
> > -R
> Yes Rob, the offset was wrong which i corrected in this patch.
Thanks for confirming. In the future, it would have been useful to
mention that in the commit msg.
BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] drm/msm: Avoid possible infinite probe deferral and speed booting
On Fri, Jul 10, 2020 at 5:02 PM Douglas Anderson wrote: > > I found that if I ever had a little mistake in my kernel config, > or device tree, or graphics driver that my system would sit in a loop > at bootup trying again and again and again. An example log was: Why do we care about optimizing the error case? > msm ae0.mdss: bound ae01000.mdp (ops 0xffe596e951f8) > msm_dsi ae94000.dsi: ae94000.dsi supply gdsc not found, using dummy > regulator > msm_dsi_manager_register: failed to register mipi dsi host for DSI 0 > [drm:ti_sn_bridge_probe] *ERROR* could not find any panel node > ... > > I finally tracked it down where this was happening: > - msm_pdev_probe() is called. > - msm_pdev_probe() registers drivers. Registering drivers kicks > off processing of probe deferrals. > - component_master_add_with_match() could return -EPROBE_DEFER. > making msm_pdev_probe() return -EPROBE_DEFER. > - When msm_pdev_probe() returned the processing of probe deferrals > happens. > - Loop back to the start. > > It looks like we can fix this by marking "mdss" as a "simple-bus". > I have no idea if people consider this the right thing to do or a > hack. Hopefully it's the right thing to do. :-) It's a simple test. Do the child devices have any dependency on the parent to probe and/or function? If so, not a simple-bus. > Once I do this I notice that my boot gets marginally faster (you > don't need to probe the sub devices over and over) and also if I Can you quantify that? Have you run with devlinks enabled. You need a command line option to enable. That too should reduce deferred probes. > have a problem it doesn't loop forever (on my system it still > gets upset about some stuck clocks in that case, but at least I > can boot up). Deferred probe only runs when a device is added, so it's not like it is continually running. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vboxvideo: Replace HTTP links with HTTPS ones
Hi,
On 7/13/20 2:49 PM, Alexander A. Klimov wrote:
Rationale:
Reduces attack surface on kernel devs opening the links for MITM
as HTTPS traffic is much harder to manipulate.
Deterministic algorithm:
For each file:
If not .svg:
For each line:
If doesn't contain `\bxmlns\b`:
For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`:
If both the HTTP and HTTPS versions
return 200 OK and serve the same content:
Replace HTTP with HTTPS.
Signed-off-by: Alexander A. Klimov
The "new" https link works for me and I see no reason why
not to do this, other then that some weird site might still
only do http, so:
Reviewed-by: Hans de Goede
(oh on second reading I see that the script already checks
that the new link works, ah well)
Regards,
Hans
---
Continuing my work started at 93431e0607e5.
See also: git log --oneline '--author=Alexander A. Klimov
' v5.7..master
(Actually letting a shell for loop submit all this stuff for me.)
If there are any URLs to be removed completely or at least not just
HTTPSified:
Just clearly say so and I'll *undo my change*.
See also: https://lkml.org/lkml/2020/6/27/64
If there are any valid, but yet not changed URLs:
See: https://lkml.org/lkml/2020/6/26/837
If you apply the patch, please let me know.
Sorry again to all maintainers who complained about subject lines.
Now I realized that you want an actually perfect prefixes,
not just subsystem ones.
I tried my best...
And yes, *I could* (at least half-)automate it.
Impossible is nothing! :)
drivers/gpu/drm/vboxvideo/hgsmi_defs.h | 2 +-
drivers/gpu/drm/vboxvideo/vbox_hgsmi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vboxvideo/hgsmi_defs.h
b/drivers/gpu/drm/vboxvideo/hgsmi_defs.h
index 6c8df1cdb087..3cb52f2b2274 100644
--- a/drivers/gpu/drm/vboxvideo/hgsmi_defs.h
+++ b/drivers/gpu/drm/vboxvideo/hgsmi_defs.h
@@ -58,7 +58,7 @@ struct hgsmi_buffer_tail {
/* Reserved, must be initialized to 0. */
u32 reserved;
/*
-* One-at-a-Time Hash: http://www.burtleburtle.net/bob/hash/doobs.html
+* One-at-a-Time Hash: https://www.burtleburtle.net/bob/hash/doobs.html
* Over the header, offset and for first 4 bytes of the tail.
*/
u32 checksum;
diff --git a/drivers/gpu/drm/vboxvideo/vbox_hgsmi.c
b/drivers/gpu/drm/vboxvideo/vbox_hgsmi.c
index 94b60654a012..a9ca4d0c3eca 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_hgsmi.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_hgsmi.c
@@ -8,7 +8,7 @@
#include "vboxvideo_vbe.h"
#include "hgsmi_defs.h"
-/* One-at-a-Time Hash from http://www.burtleburtle.net/bob/hash/doobs.html */
+/* One-at-a-Time Hash from https://www.burtleburtle.net/bob/hash/doobs.html */
static u32 hgsmi_hash_process(u32 hash, const u8 *data, int size)
{
while (size--) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/6] Add support for GPU DDR BW scaling
This series adds support for GPU DDR bandwidth scaling and is based on the bindings from Georgi [1]. This is mostly a rebase of Sharat's patches [2] on the tip of msm-next branch. Changes from v4: - Squashed a patch to another one to fix Jonathan's comment - Add back the pm_runtime_get_if_in_use() check Changes from v3: - Rebased on top of Jonathan's patch which adds support for changing gpu freq through hfi on newer targets - As suggested by Rob, left the icc_path intact for pre-a6xx GPUs [1] https://kernel.googlesource.com/pub/scm/linux/kernel/git/vireshk/pm/+log/opp/linux-next/ [2] https://patchwork.freedesktop.org/series/75291/ Sharat Masetty (6): dt-bindings: drm/msm/gpu: Document gpu opp table drm: msm: a6xx: send opp instead of a frequency drm: msm: a6xx: use dev_pm_opp_set_bw to scale DDR arm64: dts: qcom: SDM845: Enable GPU DDR bw scaling arm64: dts: qcom: sc7180: Add interconnects property for GPU arm64: dts: qcom: sc7180: Add opp-peak-kBps to GPU opp .../devicetree/bindings/display/msm/gpu.txt| 28 ++ arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 ++ drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 108 - drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +- drivers/gpu/drm/msm/msm_gpu.c | 3 +- drivers/gpu/drm/msm/msm_gpu.h | 3 +- 7 files changed, 112 insertions(+), 50 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 5/6] arm64: dts: qcom: sc7180: Add interconnects property for GPU
From: Sharat Masetty
This patch adds the interconnects property to the GPU node. This enables
the GPU->DDR path bandwidth voting.
Signed-off-by: Sharat Masetty
Signed-off-by: Akhil P Oommen
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 31b9217..a567297 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1470,6 +1470,8 @@
operating-points-v2 = <_opp_table>;
qcom,gmu = <>;
+ interconnects = <_noc MASTER_GFX3D _virt
SLAVE_EBI1>;
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 4/6] arm64: dts: qcom: SDM845: Enable GPU DDR bw scaling
From: Sharat Masetty
This patch adds the interconnects property for the gpu node and the
opp-peak-kBps property to the opps of the gpu opp table. This should
help enable DDR bandwidth scaling dynamically and proportionally to the
GPU frequency.
Signed-off-by: Sharat Masetty
Signed-off-by: Akhil P Oommen
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 8eb5a31..5e9561a 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3515,42 +3515,51 @@
qcom,gmu = <>;
+ interconnects = <_noc MASTER_GFX3D _noc
SLAVE_EBI1>;
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
opp-71000 {
opp-hz = /bits/ 64 <71000>;
opp-level =
;
+ opp-peak-kBps = <7216000>;
};
opp-67500 {
opp-hz = /bits/ 64 <67500>;
opp-level =
;
+ opp-peak-kBps = <7216000>;
};
opp-59600 {
opp-hz = /bits/ 64 <59600>;
opp-level =
;
+ opp-peak-kBps = <622>;
};
opp-52000 {
opp-hz = /bits/ 64 <52000>;
opp-level = ;
+ opp-peak-kBps = <622>;
};
opp-41400 {
opp-hz = /bits/ 64 <41400>;
opp-level =
;
+ opp-peak-kBps = <4068000>;
};
opp-34200 {
opp-hz = /bits/ 64 <34200>;
opp-level = ;
+ opp-peak-kBps = <2724000>;
};
opp-25700 {
opp-hz = /bits/ 64 <25700>;
opp-level =
;
+ opp-peak-kBps = <1648000>;
};
};
};
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 6/6] arm64: dts: qcom: sc7180: Add opp-peak-kBps to GPU opp
From: Sharat Masetty
Add opp-peak-kBps bindings to the GPU opp table, listing the peak
GPU -> DDR bandwidth requirement for each opp level. This will be
used to scale the DDR bandwidth along with the GPU frequency dynamically.
Signed-off-by: Sharat Masetty
Reviewed-by: Matthias Kaehlcke
Signed-off-by: Akhil P Oommen
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index a567297..8567e9e 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1478,36 +1478,43 @@
opp-8 {
opp-hz = /bits/ 64 <8>;
opp-level =
;
+ opp-peak-kBps = <8532000>;
};
opp-65000 {
opp-hz = /bits/ 64 <65000>;
opp-level =
;
+ opp-peak-kBps = <7216000>;
};
opp-56500 {
opp-hz = /bits/ 64 <56500>;
opp-level = ;
+ opp-peak-kBps = <5412000>;
};
opp-43000 {
opp-hz = /bits/ 64 <43000>;
opp-level =
;
+ opp-peak-kBps = <5412000>;
};
opp-35500 {
opp-hz = /bits/ 64 <35500>;
opp-level = ;
+ opp-peak-kBps = <3072000>;
};
opp-26700 {
opp-hz = /bits/ 64 <26700>;
opp-level =
;
+ opp-peak-kBps = <3072000>;
};
opp-18000 {
opp-hz = /bits/ 64 <18000>;
opp-level =
;
+ opp-peak-kBps = <1804000>;
};
};
};
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 3/6] drm: msm: a6xx: use dev_pm_opp_set_bw to scale DDR
From: Sharat Masetty
This patches replaces the previously used static DDR vote and uses
dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling
GPU frequency. Also since the icc path voting is handled completely
in the opp driver, remove the icc_path handle and its usage in the
drm driver.
Signed-off-by: Sharat Masetty
Signed-off-by: Akhil P Oommen
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 856db46..a6f43ff 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -133,7 +133,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
dev_pm_opp *opp)
if (!gmu->legacy) {
a6xx_hfi_set_freq(gmu, perf_index);
- icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
+ dev_pm_opp_set_bw(>pdev->dev, opp);
pm_runtime_put(gmu->dev);
return;
}
@@ -157,11 +157,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
dev_pm_opp *opp)
if (ret)
dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret);
- /*
-* Eventually we will want to scale the path vote with the frequency but
-* for now leave it at max so that the performance is nominal.
-*/
- icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
+ dev_pm_opp_set_bw(>pdev->dev, opp);
pm_runtime_put(gmu->dev);
}
@@ -849,6 +845,19 @@ static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu,
struct a6xx_gmu *gmu)
dev_pm_opp_put(gpu_opp);
}
+static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu *gmu)
+{
+ struct dev_pm_opp *gpu_opp;
+ unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index];
+
+ gpu_opp = dev_pm_opp_find_freq_exact(>pdev->dev, gpu_freq, true);
+ if (IS_ERR_OR_NULL(gpu_opp))
+ return;
+
+ dev_pm_opp_set_bw(>pdev->dev, gpu_opp);
+ dev_pm_opp_put(gpu_opp);
+}
+
int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
{
struct adreno_gpu *adreno_gpu = _gpu->base;
@@ -873,7 +882,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
}
/* Set the bus quota to a reasonable value for boot */
- icc_set_bw(gpu->icc_path, 0, MBps_to_icc(3072));
+ a6xx_gmu_set_initial_bw(gpu, gmu);
/* Enable the GMU interrupt */
gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_CLR, ~0);
@@ -1049,7 +1058,7 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
a6xx_gmu_shutdown(gmu);
/* Remove the bus vote */
- icc_set_bw(gpu->icc_path, 0, 0);
+ dev_pm_opp_set_bw(>pdev->dev, NULL);
/*
* Make sure the GX domain is off before turning off the GMU (CX)
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/6] dt-bindings: drm/msm/gpu: Document gpu opp table
From: Sharat Masetty
Update documentation to list the gpu opp table bindings including the
newly added "opp-peak-kBps" needed for GPU-DDR bandwidth scaling.
Signed-off-by: Sharat Masetty
Acked-by: Rob Herring
Signed-off-by: Akhil P Oommen
---
.../devicetree/bindings/display/msm/gpu.txt| 28 ++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt
b/Documentation/devicetree/bindings/display/msm/gpu.txt
index fd779cd..1af0ff1 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -112,6 +112,34 @@ Example a6xx (with GMU):
interconnects = <_hlos MASTER_GFX3D _hlos SLAVE_EBI1>;
interconnect-names = "gfx-mem";
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-43000 {
+ opp-hz = /bits/ 64 <43000>;
+ opp-level = ;
+ opp-peak-kBps = <5412000>;
+ };
+
+ opp-35500 {
+ opp-hz = /bits/ 64 <35500>;
+ opp-level = ;
+ opp-peak-kBps = <3072000>;
+ };
+
+ opp-26700 {
+ opp-hz = /bits/ 64 <26700>;
+ opp-level = ;
+ opp-peak-kBps = <3072000>;
+ };
+
+ opp-18000 {
+ opp-hz = /bits/ 64 <18000>;
+ opp-level = ;
+ opp-peak-kBps = <1804000>;
+ };
+ };
+
qcom,gmu = <>;
zap-shader {
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/6] drm: msm: a6xx: send opp instead of a frequency
From: Sharat Masetty
This patch changes the plumbing to send the devfreq recommended opp rather
than the frequency. Also consolidate and rearrange the code in a6xx to set
the GPU frequency and the icc vote in preparation for the upcoming
changes for GPU->DDR scaling votes.
Signed-off-by: Sharat Masetty
Signed-off-by: Akhil P Oommen
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 89 +++
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +-
drivers/gpu/drm/msm/msm_gpu.c | 3 +-
drivers/gpu/drm/msm/msm_gpu.h | 3 +-
4 files changed, 52 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 21e77d6..856db46 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -103,17 +103,45 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu)
A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF));
}
-static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
+void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
{
- struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
- struct adreno_gpu *adreno_gpu = _gpu->base;
- struct msm_gpu *gpu = _gpu->base;
- int ret;
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ struct a6xx_gmu *gmu = _gpu->gmu;
+ u32 perf_index;
+ unsigned long gpu_freq;
+ int ret = 0;
+
+ gpu_freq = dev_pm_opp_get_freq(opp);
+
+ if (gpu_freq == gmu->freq)
+ return;
+
+ for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
+ if (gpu_freq == gmu->gpu_freqs[perf_index])
+ break;
+
+ gmu->current_perf_index = perf_index;
+ gmu->freq = gmu->gpu_freqs[perf_index];
+
+ /*
+* This can get called from devfreq while the hardware is idle. Don't
+* bring up the power if it isn't already active
+*/
+ if (pm_runtime_get_if_in_use(gmu->dev) == 0)
+ return;
+
+ if (!gmu->legacy) {
+ a6xx_hfi_set_freq(gmu, perf_index);
+ icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
+ pm_runtime_put(gmu->dev);
+ return;
+ }
gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0);
gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING,
- ((3 & 0xf) << 28) | index);
+ ((3 & 0xf) << 28) | perf_index);
/*
* Send an invalid index as a vote for the bus bandwidth and let the
@@ -134,37 +162,6 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int
index)
* for now leave it at max so that the performance is nominal.
*/
icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
-}
-
-void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
-{
- struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
- struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
- struct a6xx_gmu *gmu = _gpu->gmu;
- u32 perf_index = 0;
-
- if (freq == gmu->freq)
- return;
-
- for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
- if (freq == gmu->gpu_freqs[perf_index])
- break;
-
- gmu->current_perf_index = perf_index;
- gmu->freq = gmu->gpu_freqs[perf_index];
-
- /*
-* This can get called from devfreq while the hardware is idle. Don't
-* bring up the power if it isn't already active
-*/
- if (pm_runtime_get_if_in_use(gmu->dev) == 0)
- return;
-
- if (gmu->legacy)
- __a6xx_gmu_set_freq(gmu, perf_index);
- else
- a6xx_hfi_set_freq(gmu, perf_index);
-
pm_runtime_put(gmu->dev);
}
@@ -839,6 +836,19 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
a6xx_gmu_rpmh_off(gmu);
}
+static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu
*gmu)
+{
+ struct dev_pm_opp *gpu_opp;
+ unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index];
+
+ gpu_opp = dev_pm_opp_find_freq_exact(>pdev->dev, gpu_freq, true);
+ if (IS_ERR_OR_NULL(gpu_opp))
+ return;
+
+ a6xx_gmu_set_freq(gpu, gpu_opp);
+ dev_pm_opp_put(gpu_opp);
+}
+
int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
{
struct adreno_gpu *adreno_gpu = _gpu->base;
@@ -898,10 +908,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
enable_irq(gmu->hfi_irq);
/* Set the GPU to the current freq */
- if (gmu->legacy)
- __a6xx_gmu_set_freq(gmu, gmu->current_perf_index);
- else
- a6xx_hfi_set_freq(gmu, gmu->current_perf_index);
+ a6xx_gmu_set_initial_freq(gpu, gmu);
/*
* "enable" the GX power domain which won't actually do anything but it
Re: [PATCH] drm: omapdrm: Replace HTTP links with HTTPS ones
Hi Alexander, Thank you for the patch. On Mon, Jul 13, 2020 at 02:28:59PM +0200, Alexander A. Klimov wrote: > Rationale: > Reduces attack surface on kernel devs opening the links for MITM > as HTTPS traffic is much harder to manipulate. > > Deterministic algorithm: > For each file: > If not .svg: > For each line: > If doesn't contain `\bxmlns\b`: > For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`: > If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`: > If both the HTTP and HTTPS versions > return 200 OK and serve the same content: > Replace HTTP with HTTPS. > > Signed-off-by: Alexander A. Klimov > --- > Continuing my work started at 93431e0607e5. > See also: git log --oneline '--author=Alexander A. Klimov > ' v5.7..master > (Actually letting a shell for loop submit all this stuff for me.) > > If there are any URLs to be removed completely or at least not just > HTTPSified: > Just clearly say so and I'll *undo my change*. > See also: https://lkml.org/lkml/2020/6/27/64 > > If there are any valid, but yet not changed URLs: > See: https://lkml.org/lkml/2020/6/26/837 > > If you apply the patch, please let me know. > > Sorry again to all maintainers who complained about subject lines. > Now I realized that you want an actually perfect prefixes, > not just subsystem ones. > I tried my best... You've done good here :-) > And yes, *I could* (at least half-)automate it. > Impossible is nothing! :) > > > drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 2 +- > drivers/gpu/drm/omapdrm/dss/Kconfig | 4 ++-- > drivers/gpu/drm/omapdrm/dss/base.c | 2 +- > drivers/gpu/drm/omapdrm/dss/dispc.h | 2 +- > drivers/gpu/drm/omapdrm/dss/dispc_coefs.c | 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi.h | 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi4.c | 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi4_core.c| 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi4_core.h| 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi5.c | 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi5_core.c| 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi5_core.h| 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi_phy.c | 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi_pll.c | 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi_wp.c | 2 +- > drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 2 +- > drivers/gpu/drm/omapdrm/dss/omapdss.h | 2 +- > drivers/gpu/drm/omapdrm/dss/output.c| 2 +- > drivers/gpu/drm/omapdrm/dss/pll.c | 2 +- > drivers/gpu/drm/omapdrm/dss/video-pll.c | 2 +- > drivers/gpu/drm/omapdrm/omap_connector.c| 2 +- > drivers/gpu/drm/omapdrm/omap_crtc.c | 2 +- > drivers/gpu/drm/omapdrm/omap_debugfs.c | 2 +- > drivers/gpu/drm/omapdrm/omap_dmm_priv.h | 2 +- > drivers/gpu/drm/omapdrm/omap_dmm_tiler.c| 2 +- > drivers/gpu/drm/omapdrm/omap_dmm_tiler.h| 2 +- > drivers/gpu/drm/omapdrm/omap_drv.c | 2 +- > drivers/gpu/drm/omapdrm/omap_drv.h | 2 +- > drivers/gpu/drm/omapdrm/omap_encoder.c | 2 +- > drivers/gpu/drm/omapdrm/omap_fb.c | 2 +- > drivers/gpu/drm/omapdrm/omap_fbdev.c| 2 +- > drivers/gpu/drm/omapdrm/omap_gem.c | 2 +- > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 2 +- > drivers/gpu/drm/omapdrm/omap_irq.c | 2 +- > drivers/gpu/drm/omapdrm/omap_plane.c| 2 +- > drivers/gpu/drm/omapdrm/tcm-sita.c | 2 +- > 37 files changed, 38 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > index 3484b5d4a91c..ec394746cd2d 100644 > --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > @@ -2,7 +2,7 @@ > /* > * Generic DSI Command Mode panel driver > * > - * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/ > + * Copyright (C) 2013 Texas Instruments Incorporated - https://www.ti.com/ I wonder if we shouldn't drop the URL instead. I'll let Tomi reply. > * Author: Tomi Valkeinen > */ > > diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig > b/drivers/gpu/drm/omapdrm/dss/Kconfig > index 2658c521b702..e11b258a2294 100644 > --- a/drivers/gpu/drm/omapdrm/dss/Kconfig > +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig > @@ -80,7 +80,7 @@ config OMAP5_DSS_HDMI > select OMAP2_DSS_HDMI_COMMON > help > HDMI Interface for OMAP5 and similar cores. This adds the High > - Definition Multimedia Interface. See http://www.hdmi.org/ for HDMI > + Definition Multimedia Interface. See https://www.hdmi.org/ for HDMI > specification. Same here, the URL has little value given that specs are not public, and it's fairly evident
[PATCH v2 07/24] scsi: aacraid: commsup: Fix a bunch of function header issues
Some parameters not documented. Others misspelled.
Also, functions must follow directly after the header that documents them.
Fixes the following W=1 kernel build warning(s):
drivers/scsi/aacraid/commsup.c:223: warning: Function parameter or member
'scmd' not described in 'aac_fib_alloc_tag'
drivers/scsi/aacraid/commsup.c:421: warning: Function parameter or member
'qid' not described in 'aac_queue_get'
drivers/scsi/aacraid/commsup.c:421: warning: Function parameter or member
'hw_fib' not described in 'aac_queue_get'
drivers/scsi/aacraid/commsup.c:421: warning: Excess function parameter
'priority' description in 'aac_queue_get'
drivers/scsi/aacraid/commsup.c:421: warning: Excess function parameter 'fib'
description in 'aac_queue_get'
drivers/scsi/aacraid/commsup.c:943: warning: Function parameter or member
'fibptr' not described in 'aac_fib_complete'
drivers/scsi/aacraid/commsup.c:943: warning: Excess function parameter 'fib'
description in 'aac_fib_complete'
drivers/scsi/aacraid/commsup.c:1061: warning: Excess function parameter 'dev'
description in 'AIF_SNIFF_TIMEOUT'
drivers/scsi/aacraid/commsup.c:1061: warning: Excess function parameter
'fibptr' description in 'AIF_SNIFF_TIMEOUT'
drivers/scsi/aacraid/commsup.c:2428: warning: Function parameter or member
'data' not described in 'aac_command_thread'
drivers/scsi/aacraid/commsup.c:2428: warning: Excess function parameter 'dev'
description in 'aac_command_thread'
Cc: Adaptec OEM Raid Solutions
Cc: Sumit Semwal
Cc: "Christian König"
Cc: "PMC-Sierra, Inc"
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Lee Jones
---
drivers/scsi/aacraid/commsup.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 8ee4e1abe568d..adbdc3b7c7a70 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -214,6 +214,7 @@ int aac_fib_setup(struct aac_dev * dev)
/**
* aac_fib_alloc_tag-allocate a fib using tags
* @dev: Adapter to allocate the fib for
+ * @scmd: SCSI command
*
* Allocate a fib from the adapter fib pool using tags
* from the blk layer.
@@ -405,8 +406,8 @@ static int aac_get_entry (struct aac_dev * dev, u32 qid,
struct aac_entry **entr
* aac_queue_get - get the next free QE
* @dev: Adapter
* @index: Returned index
- * @priority: Priority of fib
- * @fib: Fib to associate with the queue entry
+ * @qid: Queue number
+ * @hw_fib: Fib to associate with the queue entry
* @wait: Wait if queue full
* @fibptr: Driver fib object to go with fib
* @nonotify: Don't notify the adapter
@@ -934,7 +935,7 @@ int aac_fib_adapter_complete(struct fib *fibptr, unsigned
short size)
/**
* aac_fib_complete- fib completion handler
- * @fib: FIB to complete
+ * @fibptr: FIB to complete
*
* Will do all necessary work to complete a FIB.
*/
@@ -1049,6 +1050,7 @@ static void aac_handle_aif_bu(struct aac_dev *dev, struct
aac_aifcmd *aifcmd)
}
}
+#define AIF_SNIFF_TIMEOUT (500*HZ)
/**
* aac_handle_aif - Handle a message from the firmware
* @dev: Which adapter this fib is from
@@ -1057,8 +1059,6 @@ static void aac_handle_aif_bu(struct aac_dev *dev, struct
aac_aifcmd *aifcmd)
* This routine handles a driver notify fib from the adapter and
* dispatches it to the appropriate routine for handling.
*/
-
-#define AIF_SNIFF_TIMEOUT (500*HZ)
static void aac_handle_aif(struct aac_dev * dev, struct fib * fibptr)
{
struct hw_fib * hw_fib = fibptr->hw_fib_va;
@@ -2416,7 +2416,7 @@ static int aac_send_hosttime(struct aac_dev *dev, struct
timespec64 *now)
/**
* aac_command_thread - command processing thread
- * @dev: Adapter to monitor
+ * @data: Adapter to monitor
*
* Waits on the commandready event in it's queue. When the event gets set
* it will pull FIBs off it's queue. It will continue to pull FIBs off
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/exynos: gem: Fix sparse warning
kvaddr element of the exynos_gem object points to a memory buffer, thus
it should not have a __iomem annotation. Then, to avoid a warning or
casting on assignment to fbi structure, the screen_buffer element of the
union should be used instead of the screen_base.
Reported-by: kernel test robot
Suggested-by: Sam Ravnborg
Signed-off-by: Marek Szyprowski
---
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_gem.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 56a2b47e1af7..5147f5929be7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -92,7 +92,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper
*helper,
offset = fbi->var.xoffset * fb->format->cpp[0];
offset += fbi->var.yoffset * fb->pitches[0];
- fbi->screen_base = exynos_gem->kvaddr + offset;
+ fbi->screen_buffer = exynos_gem->kvaddr + offset;
fbi->screen_size = size;
fbi->fix.smem_len = size;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h
b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 7445748288da..74e926abeff0 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -40,7 +40,7 @@ struct exynos_drm_gem {
unsigned intflags;
unsigned long size;
void*cookie;
- void __iomem*kvaddr;
+ void*kvaddr;
dma_addr_t dma_addr;
unsigned long dma_attrs;
struct sg_table *sgt;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/lima: Expose job_hang_limit module parameter
Applied to drm-misc-next:
https://cgit.freedesktop.org/drm/drm-misc/
Sorry for the late response.
Regards,
Qiang
On Tue, Jul 7, 2020 at 12:17 AM Andrey Lebedev wrote:
>
> Hello guys,
>
> What is the status of this patch? Was this committed to any branch? Is
> it pending for merge to the mainline? Do I have to do anything in order
> to make it mergeable?
>
> On 6/19/20 10:58 AM, Andrey Lebedev wrote:
> > From: Andrey Lebedev
> >
> > Some pp or gp jobs can be successfully repeated even after they time outs.
> > Introduce lima module parameter to specify number of times a job can hang
> > before being dropped.
> >
> > Signed-off-by: Andrey Lebedev
> > ---
> >
> > Now all types are correct (uint).
> >
> > drivers/gpu/drm/lima/lima_drv.c | 4
> > drivers/gpu/drm/lima/lima_drv.h | 1 +
> > drivers/gpu/drm/lima/lima_sched.c | 5 +++--
> > 3 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/lima/lima_drv.c
> > b/drivers/gpu/drm/lima/lima_drv.c
> > index a831565af813..ab460121fd52 100644
> > --- a/drivers/gpu/drm/lima/lima_drv.c
> > +++ b/drivers/gpu/drm/lima/lima_drv.c
> > @@ -19,6 +19,7 @@
> > int lima_sched_timeout_ms;
> > uint lima_heap_init_nr_pages = 8;
> > uint lima_max_error_tasks;
> > +uint lima_job_hang_limit;
> >
> > MODULE_PARM_DESC(sched_timeout_ms, "task run timeout in ms");
> > module_param_named(sched_timeout_ms, lima_sched_timeout_ms, int, 0444);
> > @@ -29,6 +30,9 @@ module_param_named(heap_init_nr_pages,
> > lima_heap_init_nr_pages, uint, 0444);
> > MODULE_PARM_DESC(max_error_tasks, "max number of error tasks to save");
> > module_param_named(max_error_tasks, lima_max_error_tasks, uint, 0644);
> >
> > +MODULE_PARM_DESC(job_hang_limit, "number of times to allow a job to hang
> > before dropping it (default 0)");
> > +module_param_named(job_hang_limit, lima_job_hang_limit, uint, 0444);
> > +
> > static int lima_ioctl_get_param(struct drm_device *dev, void *data,
> > struct drm_file *file)
> > {
> > struct drm_lima_get_param *args = data;
> > diff --git a/drivers/gpu/drm/lima/lima_drv.h
> > b/drivers/gpu/drm/lima/lima_drv.h
> > index fdbd4077c768..c738d288547b 100644
> > --- a/drivers/gpu/drm/lima/lima_drv.h
> > +++ b/drivers/gpu/drm/lima/lima_drv.h
> > @@ -11,6 +11,7 @@
> > extern int lima_sched_timeout_ms;
> > extern uint lima_heap_init_nr_pages;
> > extern uint lima_max_error_tasks;
> > +extern uint lima_job_hang_limit;
> >
> > struct lima_vm;
> > struct lima_bo;
> > diff --git a/drivers/gpu/drm/lima/lima_sched.c
> > b/drivers/gpu/drm/lima/lima_sched.c
> > index e6cefda00279..1602985dfa04 100644
> > --- a/drivers/gpu/drm/lima/lima_sched.c
> > +++ b/drivers/gpu/drm/lima/lima_sched.c
> > @@ -503,8 +503,9 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe,
> > const char *name)
> >
> > INIT_WORK(>recover_work, lima_sched_recover_work);
> >
> > - return drm_sched_init(>base, _sched_ops, 1, 0,
> > - msecs_to_jiffies(timeout), name);
> > + return drm_sched_init(>base, _sched_ops, 1,
> > + lima_job_hang_limit, msecs_to_jiffies(timeout),
> > + name);
> > }
> >
> > void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)
> >
>
> --
> Andrey Lebedev aka -.- . -.. -.. . .-.
> Software engineer
> Homepage: http://lebedev.lt/
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: gem: Fix sparse warning
Hi Sam,
On 10.07.2020 20:30, Sam Ravnborg wrote:
> Hi Marek.
>
> On Tue, Jul 07, 2020 at 01:08:59PM +0200, Marek Szyprowski wrote:
>> Add a proper cast on the exynos_gem->kvaddr assignment to avoid a sparse
>> warning.
>>
>> Reported-by: kernel test robot
>> Fixes: 9940d9d93406 ("drm/exynos: gem: Get rid of the internal 'pages'
>> array")
>> Signed-off-by: Marek Szyprowski
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index efa476858db5..65d11784f29f 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -59,7 +59,7 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem
>> *exynos_gem, bool kvmap)
>> }
>>
>> if (kvmap)
>> -exynos_gem->kvaddr = exynos_gem->cookie;
>> +exynos_gem->kvaddr = (void __iomem *)exynos_gem->cookie;
> >From a brif look at the code the correct fix looks to me to drop the
> __iomem annotation of kvaddr.
> And then no cast is needed.
>
> Care to take a look at this?
Right, besides dropping iomem annotation, I will change fbi->screen_base
assignment to fbi->screen_buffer. This was probably the main reason of
this iomem annotation.
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
