Re: [PATCH v1 4/6] dma-buf: Acquire wait-wound context on attachment
On 7/15/22 09:50, Christian König wrote: > Am 15.07.22 um 02:52 schrieb Dmitry Osipenko: >> Intel i915 GPU driver uses wait-wound mutex to lock multiple GEMs on the >> attachment to the i915 dma-buf. In order to let all drivers utilize >> shared >> wait-wound context during attachment in a general way, make dma-buf >> core to >> acquire the ww context internally for the attachment operation and update >> i915 driver to use the importer's ww context instead of the internal one. >> >> From now on all dma-buf exporters shall use the importer's ww context >> for >> the attachment operation. >> >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/dma-buf/dma-buf.c | 8 +- >> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- >> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- >> drivers/gpu/drm/i915/gem/i915_gem_object.h | 6 ++--- >> drivers/gpu/drm/i915/i915_gem_evict.c | 2 +- >> drivers/gpu/drm/i915/i915_gem_ww.c | 26 +++ >> drivers/gpu/drm/i915/i915_gem_ww.h | 15 +-- >> 7 files changed, 47 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c >> index 0ee588276534..37545ecb845a 100644 >> --- a/drivers/dma-buf/dma-buf.c >> +++ b/drivers/dma-buf/dma-buf.c >> @@ -807,6 +807,8 @@ static struct sg_table * __map_dma_buf(struct >> dma_buf_attachment *attach, >> * Optionally this calls &dma_buf_ops.attach to allow >> device-specific attach >> * functionality. >> * >> + * Exporters shall use ww_ctx acquired by this function. >> + * >> * Returns: >> * >> * A pointer to newly created &dma_buf_attachment on success, or a >> negative >> @@ -822,6 +824,7 @@ dma_buf_dynamic_attach_unlocked(struct dma_buf >> *dmabuf, struct device *dev, >> void *importer_priv) >> { >> struct dma_buf_attachment *attach; >> + struct ww_acquire_ctx ww_ctx; >> int ret; >> if (WARN_ON(!dmabuf || !dev)) >> @@ -841,7 +844,8 @@ dma_buf_dynamic_attach_unlocked(struct dma_buf >> *dmabuf, struct device *dev, >> attach->importer_ops = importer_ops; >> attach->importer_priv = importer_priv; >> - dma_resv_lock(dmabuf->resv, NULL); >> + ww_acquire_init(&ww_ctx, &reservation_ww_class); >> + dma_resv_lock(dmabuf->resv, &ww_ctx); > > That won't work like this. The core property of a WW context is that you > need to unwind all the locks and re-quire them with the contended one > first. > > When you statically lock the imported one here you can't do that any more. You're right. I felt that something is missing here, but couldn't notice. I'll think more about this and enable CONFIG_DEBUG_WW_MUTEX_SLOWPATH. Thank you! -- Best regards, Dmitry
Re: [PATCH v5 4/9] drm: selftest: convert drm_format selftest to KUnit
On 7/15/22 02:03, Daniel Latypov wrote: > On Thu, Jul 14, 2022 at 4:51 PM Guenter Roeck wrote: >> >> On Fri, Jul 08, 2022 at 05:30:47PM -0300, Maíra Canal wrote: >>> Considering the current adoption of the KUnit framework, convert the >>> DRM format selftest to the KUnit API. >>> >>> Tested-by: David Gow >>> Acked-by: Daniel Latypov >>> Reviewed-by: Javier Martinez Canillas >>> Signed-off-by: Maíra Canal >> >> This patch results in: >> >> Building powerpc:allmodconfig ... failed >> -- >> Error log: >> drivers/gpu/drm/tests/drm_format_test.c: In function >> 'igt_check_drm_format_min_pitch': >> drivers/gpu/drm/tests/drm_format_test.c:271:1: error: the frame size of 3712 >> bytes is larger than 2048 bytes >> >> presumably due to function nesting. > > This can happen when there's a lot of KUNIT_EXPECT_* calls in a single > function. > See [1] for some related context. > There were a number of patches that went into 5.18 ([2] and others) to > try and mitigate this, but it's not always enough. > > Ideally the compiler would see that the stack-local variables used in > these macros don't need to stick around, but it doesn't always > happen... Thanks Daniel for the explanation. > One workaround would be to split up the test case functions into smaller > chunks. > Maíra, Could you please look at splitting in smaller chunks to mitigate this issue ? -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v1 1/6] dma-buf: Add _unlocked postfix to function names
Am 15.07.22 um 02:52 schrieb Dmitry Osipenko: Add _unlocked postfix to the dma-buf API function names in a preparation to move all non-dynamic dma-buf users over to the dynamic locking specification. This patch only renames API functions, preparing drivers to the common locking convention. Later on we will make the "unlocked" functions to take the reservation lock. Suggested-by: Christian König Signed-off-by: Dmitry Osipenko --- drivers/dma-buf/dma-buf.c | 76 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +- drivers/gpu/drm/armada/armada_gem.c | 14 ++-- drivers/gpu/drm/drm_gem_cma_helper.c | 6 +- drivers/gpu/drm/drm_gem_shmem_helper.c| 6 +- drivers/gpu/drm/drm_prime.c | 12 +-- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 6 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c| 12 +-- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 20 ++--- drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 8 +- drivers/gpu/drm/tegra/gem.c | 27 +++ drivers/infiniband/core/umem_dmabuf.c | 11 +-- .../common/videobuf2/videobuf2-dma-contig.c | 15 ++-- .../media/common/videobuf2/videobuf2-dma-sg.c | 12 +-- .../common/videobuf2/videobuf2-vmalloc.c | 6 +- .../platform/nvidia/tegra-vde/dmabuf-cache.c | 12 +-- drivers/misc/fastrpc.c| 12 +-- drivers/xen/gntdev-dmabuf.c | 14 ++-- include/linux/dma-buf.h | 34 + 21 files changed, 161 insertions(+), 152 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 44574fbe7482..d16237a6ffaa 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -795,7 +795,7 @@ static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, } /** - * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list + * dma_buf_dynamic_attach_unlocked - Add the device to dma_buf's attachments list * @dmabuf: [in]buffer to attach device to. * @dev: [in]device to be attached. * @importer_ops: [in]importer operations for the attachment @@ -817,9 +817,9 @@ static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, * indicated with the error code -EBUSY. */ struct dma_buf_attachment * -dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, - const struct dma_buf_attach_ops *importer_ops, - void *importer_priv) +dma_buf_dynamic_attach_unlocked(struct dma_buf *dmabuf, struct device *dev, + const struct dma_buf_attach_ops *importer_ops, + void *importer_priv) { struct dma_buf_attachment *attach; int ret; @@ -892,25 +892,25 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, if (dma_buf_is_dynamic(attach->dmabuf)) dma_resv_unlock(attach->dmabuf->resv); - dma_buf_detach(dmabuf, attach); + dma_buf_detach_unlocked(dmabuf, attach); return ERR_PTR(ret); } -EXPORT_SYMBOL_NS_GPL(dma_buf_dynamic_attach, DMA_BUF); +EXPORT_SYMBOL_NS_GPL(dma_buf_dynamic_attach_unlocked, DMA_BUF); /** - * dma_buf_attach - Wrapper for dma_buf_dynamic_attach + * dma_buf_attach_unlocked - Wrapper for dma_buf_dynamic_attach * @dmabuf: [in]buffer to attach device to. * @dev: [in]device to be attached. * - * Wrapper to call dma_buf_dynamic_attach() for drivers which still use a static - * mapping. + * Wrapper to call dma_buf_dynamic_attach_unlocked() for drivers which still + * use a static mapping. */ -struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, - struct device *dev) +struct dma_buf_attachment *dma_buf_attach_unlocked(struct dma_buf *dmabuf, + struct device *dev) { - return dma_buf_dynamic_attach(dmabuf, dev, NULL, NULL); + return dma_buf_dynamic_attach_unlocked(dmabuf, dev, NULL, NULL); } -EXPORT_SYMBOL_NS_GPL(dma_buf_attach, DMA_BUF); +EXPORT_SYMBOL_NS_GPL(dma_buf_attach_unlocked, DMA_BUF); static void __unmap_dma_buf(struct dma_buf_attachment *attach, struct sg_table *sg_table, @@ -923,7 +923,7 @@ static void __unmap_dma_buf(struct dma_buf_attachment *attach, } /** - * dma_buf_detach - Remove the given attachment from dmabuf's attachments list + * dma_buf_detach_unlocked - Remove the given attachment from dmabuf's attachments list * @dmabuf: [in]buffer to detach from. * @attach: [in]attachment to be detached; is free'd after this call. * @@ -931,7 +931,8 @@ static void __unmap_dma_buf(struct dma_buf_attachment *attach, * * Optionally thi
Re: [PATCH v1 2/6] drm/gem: Take reservation lock for vmap/vunmap operations
Am 15.07.22 um 02:52 schrieb Dmitry Osipenko: The new common dma-buf locking convention will require buffer importers to hold the reservation lock around mapping operations. Make DRM GEM core to take the lock around the vmapping operations and update QXL and i915 drivers to use the locked functions for the case where DRM core now holds the lock. This patch prepares DRM core and drivers to transition to the common dma-buf locking convention where vmapping of exported GEMs will be done under the held reservation lock. Oh ^^ That looks like a bug fix to me! At least drm_gem_ttm_vmap() and drm_gem_ttm_vunmap() already expected that they are called with the reservation lock held. Otherwise you could mess up internal structures in the TTM buffer object while vmapping it. I will take a deeper look at this. Regards, Christian. Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/drm_client.c | 4 +-- drivers/gpu/drm/drm_gem.c| 28 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 ++--- drivers/gpu/drm/drm_prime.c | 4 +-- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 17 ++-- drivers/gpu/drm/qxl/qxl_prime.c | 4 +-- include/drm/drm_gem.h| 3 +++ 8 files changed, 50 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 2b230b4d6942..fbcb1e995384 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -323,7 +323,7 @@ drm_client_buffer_vmap(struct drm_client_buffer *buffer, * fd_install step out of the driver backend hooks, to make that * final step optional for internal users. */ - ret = drm_gem_vmap(buffer->gem, map); + ret = drm_gem_vmap_unlocked(buffer->gem, map); if (ret) return ret; @@ -345,7 +345,7 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) { struct iosys_map *map = &buffer->map; - drm_gem_vunmap(buffer->gem, map); + drm_gem_vunmap_unlocked(buffer->gem, map); } EXPORT_SYMBOL(drm_client_buffer_vunmap); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index eb0c2d041f13..9769c33cad99 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1155,6 +1155,8 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent, int drm_gem_pin(struct drm_gem_object *obj) { + dma_resv_assert_held(obj->resv); + if (obj->funcs->pin) return obj->funcs->pin(obj); else @@ -1163,6 +1165,8 @@ int drm_gem_pin(struct drm_gem_object *obj) void drm_gem_unpin(struct drm_gem_object *obj) { + dma_resv_assert_held(obj->resv); + if (obj->funcs->unpin) obj->funcs->unpin(obj); } @@ -1171,6 +1175,8 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map) { int ret; + dma_resv_assert_held(obj->resv); + if (!obj->funcs->vmap) return -EOPNOTSUPP; @@ -1186,6 +1192,8 @@ EXPORT_SYMBOL(drm_gem_vmap); void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) { + dma_resv_assert_held(obj->resv); + if (iosys_map_is_null(map)) return; @@ -1197,6 +1205,26 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) } EXPORT_SYMBOL(drm_gem_vunmap); +int drm_gem_vmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map) +{ + int ret; + + dma_resv_lock(obj->resv, NULL); + ret = drm_gem_vmap(obj, map); + dma_resv_unlock(obj->resv); + + return ret; +} +EXPORT_SYMBOL(drm_gem_vmap_unlocked); + +void drm_gem_vunmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map) +{ + dma_resv_lock(obj->resv, NULL); + drm_gem_vunmap(obj, map); + dma_resv_unlock(obj->resv); +} +EXPORT_SYMBOL(drm_gem_vunmap_unlocked); + /** * drm_gem_lock_reservations - Sets up the ww context and acquires * the lock on an array of GEM objects. diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 880a4975507f..e35e224e6303 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -354,7 +354,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map, ret = -EINVAL; goto err_drm_gem_vunmap; } - ret = drm_gem_vmap(obj, &map[i]); + ret = drm_gem_vmap_unlocked(obj, &map[i]); if (ret) goto err_drm_gem_vunmap; } @@ -376,7 +376,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map, obj = drm_gem_fb_get_obj(fb, i); if (!obj) continue; -
Re: [PATCH -next] drm/amdgpu: double free error and freeing uninitialized null pointer
On 7/14/2022 9:13 PM, André Almeida wrote: Às 12:06 de 14/07/22, Sebin Sebastian escreveu: On Tue, Jul 12, 2022 at 12:14:27PM -0300, André Almeida wrote: Hi Sebin, Às 10:29 de 10/07/22, Sebin Sebastian escreveu: Fix two coverity warning's double free and and an uninitialized pointer read. Both tmp and new are pointing at same address and both are freed which leads to double free. Freeing tmp in the condition after new is assigned with new address fixes the double free issue. new is not initialized to null which also leads to a free on an uninitialized pointer. Coverity issue: 1518665 (uninitialized pointer read) 1518679 (double free) What are those numbers? These numbers are the issue ID's for the errors that are being reported by the coverity static analyzer tool. I see, but I don't know which tool was used, so those seem like random number to me. I would just remove this part of your commit message, but if you want to keep it, you need to at least mention what's the tool. new variable is not needed to initialize. The only condition double free happens is: tmp = new; if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) { ret = -EINVAL; goto error_free; *// if it hits this* }/ / and can be avoided like: error_free: - kfree(tmp); + if (tmp != new) + kfree(tmp); kfree(new); return ret; } Regards, S.Amarnath Signed-off-by: Sebin Sebastian --- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c index f3b3c688e4e7..d82fe0e1b06b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c @@ -1660,7 +1660,7 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f, { struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private; char reg_offset[11]; - uint32_t *new, *tmp = NULL; + uint32_t *new = NULL, *tmp = NULL; int ret, i = 0, len = 0; do { @@ -1692,17 +1692,19 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f, goto error_free; } If the `if (!new) {` above this line is true, will be tmp freed? Yes, It doesn't seem to free tmp here. Should I free tmp immediately after the do while loop and remove `kfree(tmp)` from the `if (ret)` block? Thanks for pointing out the errors. If you free immediately after the while loop, then you would risk a use after free here: swap(adev->reset_dump_reg_list, tmp); So this isn't the solution either. ret = down_write_killable(&adev->reset_domain->sem); - if (ret) + if (ret) { + kfree(tmp); goto error_free; + } swap(adev->reset_dump_reg_list, tmp); swap(adev->reset_dump_reg_value, new); adev->num_regs = i; up_write(&adev->reset_domain->sem); + kfree(tmp); ret = size; error_free: - kfree(tmp); kfree(new); return ret; }
[PATCH v1 1/1] kfd: fixed memleak about release topology
memleak will happend that reload module due to ignore kfree when release topology Signed-off-by: ZhiJie.zhang --- drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c index 8d50d207cf66..8b86f56bd50c 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c @@ -872,6 +872,7 @@ static void kfd_topology_release_sysfs(void) } kobject_del(sys_props.kobj_topology); kobject_put(sys_props.kobj_topology); + kfree(sys_props.kobj_topology); sys_props.kobj_topology = NULL; } } -- 2.34.1
[PATCH RFC] drm/i915/gt: Retry RING_HEAD reset until it sticks
From: Chris Wilson On Haswell, in particular, we see an issue where resets fails because the engine resumes from an incorrect RING_HEAD. Since the RING_HEAD doesn't point to the remaining requests to re-run, but may instead point into the uninitialised portion of the ring, the GPU may be then fed invalid instructions from a privileged context, oft pushing the GPU into an unrecoverable hang. If at first the write doesn't succeed, try, try again. References: https://gitlab.freedesktop.org/drm/intel/-/issues/5432 Testcase: igt/i915_selftest/hangcheck Signed-off-by: Chris Wilson Cc: Andrzej Hajda Cc: Mika Kuoppala Signed-off-by: Mauro Carvalho Chehab --- .../gpu/drm/i915/gt/intel_ring_submission.c | 44 +-- drivers/gpu/drm/i915/i915_utils.h | 10 + 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c index d5d6f1fadcae..cc53feb1f8ed 100644 --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c @@ -190,6 +190,7 @@ static bool stop_ring(struct intel_engine_cs *engine) static int xcs_resume(struct intel_engine_cs *engine) { struct intel_ring *ring = engine->legacy.ring; + ktime_t kt; ENGINE_TRACE(engine, "ring:{HEAD:%04x, TAIL:%04x}\n", ring->head, ring->tail); @@ -228,9 +229,20 @@ static int xcs_resume(struct intel_engine_cs *engine) set_pp_dir(engine); /* First wake the ring up to an empty/idle ring */ - ENGINE_WRITE_FW(engine, RING_HEAD, ring->head); + until_timeout_ns(kt, 2 * NSEC_PER_MSEC) { + ENGINE_WRITE_FW(engine, RING_HEAD, ring->head); + if (ENGINE_READ_FW(engine, RING_HEAD) == ring->head) + break; + } + ENGINE_WRITE_FW(engine, RING_TAIL, ring->head); - ENGINE_POSTING_READ(engine, RING_TAIL); + if (ENGINE_READ_FW(engine, RING_HEAD) != ENGINE_READ_FW(engine, RING_TAIL)) { + ENGINE_TRACE(engine, "failed to reset empty ring: [%x, %x]: %x\n", +ENGINE_READ_FW(engine, RING_HEAD), +ENGINE_READ_FW(engine, RING_TAIL), +ring->head); + goto err; + } ENGINE_WRITE_FW(engine, RING_CTL, RING_CTL_SIZE(ring->size) | RING_VALID); @@ -239,12 +251,16 @@ static int xcs_resume(struct intel_engine_cs *engine) if (__intel_wait_for_register_fw(engine->uncore, RING_CTL(engine->mmio_base), RING_VALID, RING_VALID, -5000, 0, NULL)) +5000, 0, NULL)) { + ENGINE_TRACE(engine, "failed to restart\n"); goto err; + } - if (GRAPHICS_VER(engine->i915) > 2) + if (GRAPHICS_VER(engine->i915) > 2) { ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING)); + ENGINE_POSTING_READ(engine, RING_MI_MODE); + } /* Now awake, let it get started */ if (ring->tail != ring->head) { @@ -257,16 +273,16 @@ static int xcs_resume(struct intel_engine_cs *engine) return 0; err: - drm_err(&engine->i915->drm, - "%s initialization failed; " - "ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n", - engine->name, - ENGINE_READ(engine, RING_CTL), - ENGINE_READ(engine, RING_CTL) & RING_VALID, - ENGINE_READ(engine, RING_HEAD), ring->head, - ENGINE_READ(engine, RING_TAIL), ring->tail, - ENGINE_READ(engine, RING_START), - i915_ggtt_offset(ring->vma)); + ENGINE_TRACE(engine, +"initialization failed; " +"ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n", +ENGINE_READ(engine, RING_CTL), +ENGINE_READ(engine, RING_CTL) & RING_VALID, +ENGINE_READ(engine, RING_HEAD), ring->head, +ENGINE_READ(engine, RING_TAIL), ring->tail, +ENGINE_READ(engine, RING_START), +i915_ggtt_offset(ring->vma)); + GEM_TRACE_DUMP(); return -EIO; } diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index c10d68cdc3ca..717fb6b9cc15 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -256,6 +256,16 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms) } } +/** + * until_timeout_ns - Keep retrying (busy spin) until the duration has passed + * @end: temporary var to be used to track the spent ti
Re: [RESEND] media: mediatek: vcodec: Add to support VP9 inner racing mode
Il 15/07/22 08:49, Mingjia Zhang ha scritto: In order to reduce decoder latency, enable VP9 inner racing mode. Send lat trans buffer information to core when trigger lat to work, need not to wait until lat decode done. Signed-off-by: mingjia zhang --- CTS/GTS test pass CTS/GTS passing is a good indication but, please, test with GStreamer (and show the output, as well!). Thanks, Angelo --- .../vcodec/vdec/vdec_vp9_req_lat_if.c | 64 --- 1 file changed, 40 insertions(+), 24 deletions(-)
Re: [PATCH v2 0/2] drm: A couple of fixes for drm_copy_field() helper function
On 7/5/22 12:02, Javier Martinez Canillas wrote: > Hello, > > Peter Robinson reported me a kernel bug in one of his aarch64 test boards > and even though I was not able to reproduce it, I think that figured out > what the problem was. It seems the cause is that a DRM driver doesn't set > some of the struct drm fields copied to userspace via DRM_IOCTL_VERSION. > > Even though this is a driver bug, we can make drm_copy_field() more robust > and warn about it instead of leading to an attempt to copy a NULL pointer. > > While looking at this, I also found that a variable in drm_copy_field() is > not using the correct type. So I included that change in the patch-set too. > Pushed these to drm-misc (drm-misc-next). Thanks! -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v5 00/13] Canaan devicetree fixes
On Tue, 05 Jul 2022 14:52:01 PDT (-0700), m...@conchuod.ie wrote: From: Conor Dooley Hey all, This series should rid us of dtbs_check errors for the RISC-V Canaan k210 based boards. To make keeping it that way a little easier, I changed the Canaan devicetree Makefile so that it would build all of the devicetrees in the directory if SOC_CANAAN. I *DO NOT* have any Canaan hardware so I have not tested any of this in action. Since I sent v1, I tried to buy some since it's cheap - but could out of the limited stockists none seemed to want to deliver to Ireland :( I based the series on next-20220617. Thanks, Conor. Changes since v4: - add Rob's tags from v3 - sram: rephrase the binding description - ASoC: dropped the applied binding Changes since v3: - dts: drop the bogus "regs" property pointed out by Niklas - dma/timer: add Serge's reviews (and expand on the dma interrupt description) - dts: add Niklas' T-b where I felt it was suitable. lmk if you think it applies more broadly - spi: drop the applied spi dt-binding change. Thanks Mark. Changes since v2: - i2s: added clocks maxItems - dma: unconditionally extended the interrupts & dropped canaan compatible - timer: as per Sergey, split the timer dts nodes in 2 & drop the binding patch - ili9341: add a canaan specific compatible to the binding and dts Changes since v1: - I added a new dt node & compatible for the SRAM memory controller due Damien's wish to preserve the inter-op with U-Boot. - The dw-apb-ssi binding now uses the default rx/tx widths - A new patch fixes bus {ranges,reg} warnings - Rearranged the patches in a slightly more logical order Conor Dooley (13): dt-bindings: display: convert ilitek,ili9341.txt to dt-schema dt-bindings: display: ili9341: document canaan kd233's lcd dt-bindings: dma: dw-axi-dmac: extend the number of interrupts dt-bindings: memory-controllers: add canaan k210 sram controller riscv: dts: canaan: fix the k210's memory node riscv: dts: canaan: fix the k210's timer nodes riscv: dts: canaan: fix mmc node names riscv: dts: canaan: fix kd233 display spi frequency riscv: dts: canaan: use custom compatible for k210 i2s riscv: dts: canaan: remove spi-max-frequency from controllers riscv: dts: canaan: fix bus {ranges,reg} warnings riscv: dts: canaan: add specific compatible for kd233's LCD riscv: dts: canaan: build all devicetress if SOC_CANAAN .../bindings/display/ilitek,ili9341.txt | 27 --- .../display/panel/ilitek,ili9341.yaml | 49 + .../bindings/dma/snps,dw-axi-dmac.yaml| 7 +- .../memory-controllers/canaan,k210-sram.yaml | 52 + arch/riscv/boot/dts/canaan/Makefile | 10 ++- arch/riscv/boot/dts/canaan/canaan_kd233.dts | 6 +- arch/riscv/boot/dts/canaan/k210.dtsi | 73 +-- .../riscv/boot/dts/canaan/sipeed_maix_bit.dts | 2 +- .../boot/dts/canaan/sipeed_maix_dock.dts | 2 +- arch/riscv/boot/dts/canaan/sipeed_maix_go.dts | 2 +- .../boot/dts/canaan/sipeed_maixduino.dts | 2 +- 11 files changed, 159 insertions(+), 73 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml I'm trying to sort out how to merge this one. I'm not opposed to taking it through the RISC-V tree as Rob's reviewed/acked the bindings, but just figured I'd say something before putting anything on for-next to try and minimize confusion. Unless I'm missing something it's just patch 3 that's been taken so far, via Vinod's tree. I've dropped that one and put the rest on palmer/riscv-canaan_dt_schema, if that looks good then I'll take it into riscv/for-next when this loops back to the top of my queue. Thanks!
Re: [PATCH 00/10] video: fbdev: atari: Miscellaneous fixes and cleanups
Hi Geert, somehow this series slipped into my spam folder ... only saw it now. Am 12.07.2022 um 03:50 schrieb Geert Uytterhoeven: Hi all, This patch series contains miscellaneous fixes and cleanups for the Atari frame buffer device driver, which were identified while working on the Atari DRM driver. Most of them have been tested on ARAnyM, and should be safe to apply, except perhaps for the last one, which is marked RFC. Thanks for your comments! Geert Uytterhoeven (10): video: fbdev: atari: Simplify atafb_pan_display() video: fbdev: atari: Remove bogus FB_VMODE_YWRAP flags video: fbdev: atari: Fix inverse handling video: fbdev: atari: Fix ext_setcolreg() video: fbdev: atari: Remove unneeded casts from void * video: fbdev: atari: Remove unneeded casts to void * video: fbdev: atari: Fix TT High video mode vertical refresh video: fbdev: atari: Fix VGA modes video: fbdev: atari: Remove unused definitions and variables [RFC] video: fbdev: atari: Remove backward bug-compatibility Documentation/m68k/kernel-options.rst | 4 +- drivers/video/fbdev/atafb.c | 101 +++--- 2 files changed, 29 insertions(+), 76 deletions(-) Looks good to me. (I'll still try to test it on hardware this weekend.) I'd suggest the last one be applied as well - if the regression can only be triggered by a X server resolution switch, I doubt it'll matter in practice. Cheers, Michael
Re: [Intel-gfx] [PATCH 1/1] drm/i915/guc: Update to GuC version 70.1.1 #forregzbot
[TLDR: I'm adding this regression report to the list of tracked regressions; all text from me you find below is based on a few templates paragraphs you might have encountered already already in similar form.] Hi, this is your Linux kernel regression tracker. On 15.07.22 01:08, Dave Airlie wrote: > On Fri, 15 Apr 2022 at 10:15, Matt Roper wrote: >> >> On Tue, Apr 12, 2022 at 03:59:55PM -0700, john.c.harri...@intel.com wrote: >>> From: John Harrison >>> >>> The latest GuC firmware drops the context descriptor pool in favour of >>> passing all creation data in the create H2G. It also greatly simplifies >>> the work queue and removes the process descriptor used for multi-LRC >>> submission. So, remove all mention of LRC and process descriptors and >>> update the registration code accordingly. >>> >>> Unfortunately, the new API also removes the ability to set default >>> values for the scheduling policies at context registration time. >>> Instead, a follow up H2G must be sent. The individual scheduling >>> policy update H2G commands are also dropped in favour of a single KLV >>> based H2G. So, change the update wrappers accordingly and call this >>> during context registration.. >>> >>> Of course, this second H2G per registration might fail due to being >>> backed up. The registration code has a complicated state machine to >>> cope with the actual registration call failing. However, if that works >>> then there is no support for unwinding if a further call should fail. >>> Unwinding would require sending a H2G to de-register - but that can't >>> be done because the CTB is already backed up. >>> >>> So instead, add a new flag to say whether the context has a pending >>> policy update. This is set if the policy H2G fails at registration >>> time. The submission code checks for this flag and retries the policy >>> update if set. If that call fails, the submission path early exists >>> with a retry error. This is something that is already supported for >>> other reasons. >>> >>> Signed-off-by: John Harrison >>> Reviewed-by: Daniele Ceraolo Spurio >> >> Applied to drm-intel-gt-next. Thanks for the patch and review. >> > > (cc'ing Linus and danvet, as a headsup, there is also a phoronix > article where this was discovered). > > Okay WTF. > > This is in no way acceptable. This needs to be fixed in 5.19-rc ASAP. > > Once hardware is released and we remove the gate flag by default, you > cannot just bump firmware versions blindly. > > The kernel needs to retain compatibility with all released firmwares > since a device was declared supported. > > This needs to be reverted, and then 70 should be introduced with a > fallback to 69 versions. > > Very disappointing, I expect this to get dealt with v.quickly. To be sure below issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression tracking bot: #regzbot ^introduced 2584b3549f4c4081 #regzbot title #regzbot ignore-activity This isn't a regression? This issue or a fix for it are already discussed somewhere else? It was fixed already? You want to clarify when the regression started to happen? Or point out I got the title or something else totally wrong? Then just reply -- ideally with also telling regzbot about it, as explained here: https://linux-regtracking.leemhuis.info/tracked-regression/ Reminder for developers: When fixing the issue, add 'Link:' tags pointing to the report (the mail this one replies to), as explained for in the Linux kernel's documentation; above webpage explains why this is important for tracked regressions. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) P.S.: As the Linux kernel's regression tracker I deal with a lot of reports and sometimes miss something important when writing mails like this. If that's the case here, don't hesitate to tell me in a public reply, it's in everyone's interest to set the public record straight.
Re: [PATCH] drm: Fix EDID firmware load on resume
Hi, Sorry, my email client removed every tab. I will send the v2 in a new thread. Thanks. Matthieu On Thu, Jul 14 2022 at 11:23:10 AM -0300, André Almeida wrote: Hi Matthieu, Thanks for your patch. Às 11:58 de 06/07/22, Matthieu CHARETTE escreveu: Loading an EDID using drm.edid_firmware parameter makes resume to fail after firmware cache is being cleaned. This is because edid_load() use a temporary device to request the firmware. This cause the EDID firmware not to be cached from suspend. And, requesting the EDID firmware return an error during resume. So the request_firmware() call should use a permanent device for each connector. Also, we should cache the EDID even if no monitor is connected, in case it's plugged while suspended. Signed-off-by: Matthieu CHARETTE --- drivers/gpu/drm/drm_connector.c | 9 drivers/gpu/drm/drm_edid_load.c | 81 - include/drm/drm_connector.h | 12 + include/drm/drm_edid.h | 3 ++ 4 files changed, 94 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 1c48d162c77e..e8819ebf1c4b 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -31,6 +31,7 @@ #include #include +#include #include #include "drm_crtc_internal.h" @@ -289,6 +290,9 @@ int drm_connector_init(struct drm_device *dev, drm_connector_get_cmdline_mode(connector); + connector->edid_load_pdev = NULL; + drm_cache_edid_firmware(connector); + /* We should add connectors at the end to avoid upsetting the connector * index too much. */ @@ -473,6 +477,11 @@ void drm_connector_cleanup(struct drm_connector *connector) connector->tile_group = NULL; } + if (connector->edid_load_pdev) { + platform_device_unregister(connector->edid_load_pdev); + connector->edid_load_pdev = NULL; + } + The indentation of your patch is wrong in different places, like in this if here. It should be like + if (connector->edid_load_pdev) { + platform_device_unregister(connector->edid_load_pdev); + connector->edid_load_pdev = NULL; + } ./scripts/checkpatch.pl can help you detect those issues for your v2 Thanks, André
[PATCH] drm: Fix EDID firmware load on resume
Loading an EDID using drm.edid_firmware parameter makes resume to fail after firmware cache is being cleaned. This is because edid_load() use a temporary device to request the firmware. This cause the EDID firmware not to be cached from suspend. And, requesting the EDID firmware return an error during resume. So the request_firmware() call should use a permanent device for each connector. Also, we should cache the EDID even if no monitor is connected, in case it's plugged while suspended. Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2061 Signed-off-by: Matthieu CHARETTE --- drivers/gpu/drm/drm_connector.c | 9 drivers/gpu/drm/drm_edid_load.c | 81 - include/drm/drm_connector.h | 12 + include/drm/drm_edid.h | 3 ++ 4 files changed, 94 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 1c48d162c77e..e8819ebf1c4b 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -31,6 +31,7 @@ #include #include +#include #include #include "drm_crtc_internal.h" @@ -289,6 +290,9 @@ int drm_connector_init(struct drm_device *dev, drm_connector_get_cmdline_mode(connector); + connector->edid_load_pdev = NULL; + drm_cache_edid_firmware(connector); + /* We should add connectors at the end to avoid upsetting the connector * index too much. */ @@ -473,6 +477,11 @@ void drm_connector_cleanup(struct drm_connector *connector) connector->tile_group = NULL; } + if (connector->edid_load_pdev) { + platform_device_unregister(connector->edid_load_pdev); + connector->edid_load_pdev = NULL; + } + list_for_each_entry_safe(mode, t, &connector->probed_modes, head) drm_mode_remove(connector, mode); diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 37d8ba3ddb46..5a82be9917ec 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -167,6 +167,19 @@ static int edid_size(const u8 *edid, int data_size) return (edid[0x7e] + 1) * EDID_LENGTH; } +static struct platform_device *edid_pdev(const char *connector_name) +{ + struct platform_device *pdev = platform_device_register_simple(connector_name, -1, NULL, 0); + + if (IS_ERR(pdev)) { + DRM_ERROR("Failed to register EDID firmware platform device " + "for connector \"%s\"\n", connector_name); + return ERR_CAST(pdev); + } + + return pdev; +} + static void *edid_load(struct drm_connector *connector, const char *name, const char *connector_name) { @@ -182,18 +195,17 @@ static void *edid_load(struct drm_connector *connector, const char *name, fwdata = generic_edid[builtin]; fwsize = sizeof(generic_edid[builtin]); } else { - struct platform_device *pdev; + struct platform_device *pdev = connector->edid_load_pdev; int err; - pdev = platform_device_register_simple(connector_name, -1, NULL, 0); - if (IS_ERR(pdev)) { - DRM_ERROR("Failed to register EDID firmware platform device " - "for connector \"%s\"\n", connector_name); - return ERR_CAST(pdev); + if (WARN_ON(!pdev)) { + pdev = edid_pdev(connector_name); + if (IS_ERR(pdev)) + return ERR_CAST(pdev); + connector->edid_load_pdev = pdev; } err = request_firmware(&fw, name, &pdev->dev); - platform_device_unregister(pdev); if (err) { DRM_ERROR("Requesting EDID firmware \"%s\" failed (err=%d)\n", name, err); @@ -263,11 +275,9 @@ static void *edid_load(struct drm_connector *connector, const char *name, return edid; } -struct edid *drm_load_edid_firmware(struct drm_connector *connector) +static char *edid_name(const char *connector_name) { - const char *connector_name = connector->name; char *edidname, *last, *colon, *fwstr, *edidstr, *fallback = NULL; - struct edid *edid; if (edid_firmware[0] == '\0') return ERR_PTR(-ENOENT); @@ -310,8 +320,57 @@ struct edid *drm_load_edid_firmware(struct drm_connector *connector) if (*last == '\n') *last = '\0'; - edid = edid_load(connector, edidname, connector_name); + edidname = kstrdup(edidname, GFP_KERNEL); + if (!edidname) { + kfree(fwstr); + return ERR_PTR(-ENOMEM); + } + kfree(fwstr); + return edidname; +} + +void drm_cache_edid_firmware(struct drm_connector *connector) +{
Re: [PATCH v1 1/6] dma-buf: Add _unlocked postfix to function names
On 7/15/22 10:19, Christian König wrote: >> -struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment >> *attach, >> - enum dma_data_direction direction) >> +struct sg_table * >> +dma_buf_map_attachment_unlocked(struct dma_buf_attachment *attach, >> + enum dma_data_direction direction) > > The locking state of mapping and unmapping operations depend on if the > attachment is dynamic or not. > > So this here is not a good idea at all since it suggests that the > function is always called without holding the lock. I had the same thought while was working on this patch and initially was thinking about adding an "unlocked" alias to dma_buf_map_attachment(). In the end I decided that it will create even more confusion and it's simpler just to rename this func here since there are only two drivers using the dynamic mapping. Do you have suggestions how to improve it? -- Best regards, Dmitry
Re: [PATCH] drm: correct comments
Hi, On Fri, Jul 15, 2022 at 05:22:56PM +0800, Liu Zixian wrote: > On failure, these functions return errno, not NULL. They don't return an errno but an error pointer Maxime
Re: [Intel-gfx] [PATCH RFC] drm/i915/gt: Retry RING_HEAD reset until it sticks
On 15.07.2022 10:26, Mauro Carvalho Chehab wrote: From: Chris Wilson On Haswell, in particular, we see an issue where resets fails because the engine resumes from an incorrect RING_HEAD. Since the RING_HEAD doesn't point to the remaining requests to re-run, but may instead point into the uninitialised portion of the ring, the GPU may be then fed invalid instructions from a privileged context, oft pushing the GPU into an unrecoverable hang. If at first the write doesn't succeed, try, try again. References: https://gitlab.freedesktop.org/drm/intel/-/issues/5432 Testcase: igt/i915_selftest/hangcheck Signed-off-by: Chris Wilson Cc: Andrzej Hajda Cc: Mika Kuoppala Signed-off-by: Mauro Carvalho Chehab That is pity hw does not provide reliable way of reset. Reviewed-by: Andrzej Hajda Regards Andrzej --- .../gpu/drm/i915/gt/intel_ring_submission.c | 44 +-- drivers/gpu/drm/i915/i915_utils.h | 10 + 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c index d5d6f1fadcae..cc53feb1f8ed 100644 --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c @@ -190,6 +190,7 @@ static bool stop_ring(struct intel_engine_cs *engine) static int xcs_resume(struct intel_engine_cs *engine) { struct intel_ring *ring = engine->legacy.ring; + ktime_t kt; ENGINE_TRACE(engine, "ring:{HEAD:%04x, TAIL:%04x}\n", ring->head, ring->tail); @@ -228,9 +229,20 @@ static int xcs_resume(struct intel_engine_cs *engine) set_pp_dir(engine); /* First wake the ring up to an empty/idle ring */ - ENGINE_WRITE_FW(engine, RING_HEAD, ring->head); + until_timeout_ns(kt, 2 * NSEC_PER_MSEC) { + ENGINE_WRITE_FW(engine, RING_HEAD, ring->head); + if (ENGINE_READ_FW(engine, RING_HEAD) == ring->head) + break; + } + ENGINE_WRITE_FW(engine, RING_TAIL, ring->head); - ENGINE_POSTING_READ(engine, RING_TAIL); + if (ENGINE_READ_FW(engine, RING_HEAD) != ENGINE_READ_FW(engine, RING_TAIL)) { + ENGINE_TRACE(engine, "failed to reset empty ring: [%x, %x]: %x\n", +ENGINE_READ_FW(engine, RING_HEAD), +ENGINE_READ_FW(engine, RING_TAIL), +ring->head); + goto err; + } ENGINE_WRITE_FW(engine, RING_CTL, RING_CTL_SIZE(ring->size) | RING_VALID); @@ -239,12 +251,16 @@ static int xcs_resume(struct intel_engine_cs *engine) if (__intel_wait_for_register_fw(engine->uncore, RING_CTL(engine->mmio_base), RING_VALID, RING_VALID, -5000, 0, NULL)) +5000, 0, NULL)) { + ENGINE_TRACE(engine, "failed to restart\n"); goto err; + } - if (GRAPHICS_VER(engine->i915) > 2) + if (GRAPHICS_VER(engine->i915) > 2) { ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING)); + ENGINE_POSTING_READ(engine, RING_MI_MODE); + } /* Now awake, let it get started */ if (ring->tail != ring->head) { @@ -257,16 +273,16 @@ static int xcs_resume(struct intel_engine_cs *engine) return 0; err: - drm_err(&engine->i915->drm, - "%s initialization failed; " - "ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n", - engine->name, - ENGINE_READ(engine, RING_CTL), - ENGINE_READ(engine, RING_CTL) & RING_VALID, - ENGINE_READ(engine, RING_HEAD), ring->head, - ENGINE_READ(engine, RING_TAIL), ring->tail, - ENGINE_READ(engine, RING_START), - i915_ggtt_offset(ring->vma)); + ENGINE_TRACE(engine, +"initialization failed; " +"ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n", +ENGINE_READ(engine, RING_CTL), +ENGINE_READ(engine, RING_CTL) & RING_VALID, +ENGINE_READ(engine, RING_HEAD), ring->head, +ENGINE_READ(engine, RING_TAIL), ring->tail, +ENGINE_READ(engine, RING_START), +i915_ggtt_offset(ring->vma)); + GEM_TRACE_DUMP(); return -EIO; } diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index c10d68cdc3ca..717fb6b9cc15 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -256,6 +256,16 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
linux-next: build failure after merge of the bitmap tree
Hi all, After merging the bitmap tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/gpu/drm/i915/gt/intel_sseu.c: In function 'intel_sseu_print_ss_info': drivers/gpu/drm/i915/gt/intel_sseu.c:868:52: error: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'long unsigned int' [-Werror=format=] 868 | seq_printf(m, " %s Geometry DSS: %u\n", type, | ~^ || |unsigned int | %lu 869 | bitmap_weight(sseu->geometry_subslice_mask.xehp, | || |long unsigned int 870 | XEHP_BITMAP_BITS(sseu->geometry_subslice_mask))); | ~~~ drivers/gpu/drm/i915/gt/intel_sseu.c:871:51: error: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'long unsigned int' [-Werror=format=] 871 | seq_printf(m, " %s Compute DSS: %u\n", type, | ~^ | | | unsigned int | %lu 872 | bitmap_weight(sseu->compute_subslice_mask.xehp, | ~~~ || |long unsigned int 873 | XEHP_BITMAP_BITS(sseu->compute_subslice_mask))); | ~~ cc1: all warnings being treated as errors In file included from include/linux/printk.h:573, from include/linux/kernel.h:29, from arch/x86/include/asm/percpu.h:27, from arch/x86/include/asm/nospec-branch.h:14, from arch/x86/include/asm/paravirt_types.h:40, from arch/x86/include/asm/ptrace.h:97, from arch/x86/include/asm/math_emu.h:5, from arch/x86/include/asm/processor.h:13, from arch/x86/include/asm/timex.h:5, from include/linux/timex.h:67, from include/linux/time32.h:13, from include/linux/time.h:60, from include/linux/stat.h:19, from include/linux/module.h:13, from drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c:9: drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c: In function 'rvu_npc_exact_alloc_mem_table_entry': drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c:454:27: error: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'long unsigned int' [-Werror=format=] 454 | dev_dbg(rvu->dev, "%s: No space in 4 way exact way, weight=%u\n", __func__, | ^~ include/linux/dynamic_debug.h:134:29: note: in definition of macro '__dynamic_func_call' 134 | func(&id, ##__VA_ARGS__); \ | ^~~ include/linux/dynamic_debug.h:166:9: note: in expansion of macro '_dynamic_func_call' 166 | _dynamic_func_call(fmt,__dynamic_dev_dbg, \ | ^~ include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg' 155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~ include/linux/dev_printk.h:155:30: note: in expansion of macro 'dev_fmt' 155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~ drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c:454:9: note: in expansion of macro 'dev_dbg' 454 | dev_dbg(rvu->dev, "%s: No space in 4 way exact way, weight=%u\n", __func__, | ^~~ drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c:454:69: note: format string is defined here 454 | dev_dbg(rvu->dev, "%s: No space in 4 way exact way, weight=%u\n", __func__, |~^ | | | unsigned int |%lu In file included from include/linux/device.h:15, fr
Re: [PATCH v1 1/6] dma-buf: Add _unlocked postfix to function names
Am 15.07.22 um 11:31 schrieb Dmitry Osipenko: On 7/15/22 10:19, Christian König wrote: -struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, - enum dma_data_direction direction) +struct sg_table * +dma_buf_map_attachment_unlocked(struct dma_buf_attachment *attach, + enum dma_data_direction direction) The locking state of mapping and unmapping operations depend on if the attachment is dynamic or not. So this here is not a good idea at all since it suggests that the function is always called without holding the lock. I had the same thought while was working on this patch and initially was thinking about adding an "unlocked" alias to dma_buf_map_attachment(). In the end I decided that it will create even more confusion and it's simpler just to rename this func here since there are only two drivers using the dynamic mapping. Do you have suggestions how to improve it? Just keep it as it is. The ultimative goal is to switch all drivers over to the dynamic mapping or at least over to assume that the map/unmap callbacks are called with the lock held. Regards, Christian.
Re: [PATCH 6/8] dt-bindings: backlight: Update Lee Jones' email address
On Thu, Jul 14, 2022 at 12:25:31PM +0100, Lee Jones wrote: > Going forward, I'll be using my kernel.org for upstream work. > > Cc: Daniel Thompson Acked-by: Daniel Thompson > Cc: Jingoo Han > Cc: Rob Herring > Cc: Krzysztof Kozlowski > Cc: dri-devel@lists.freedesktop.org > Cc: linux-l...@vger.kernel.org > Cc: devicet...@vger.kernel.org > Signed-off-by: Lee Jones > Signed-off-by: Lee Jones Daniel.
[PATCH 1/2] drm/ttm: fix locking in vmap/vunmap TTM GEM helpers
I've stumbled over this while reviewing patches for DMA-buf and it looks like we completely messed the locking up here. In general most TTM function should only be called while holding the appropriate BO resv lock. Without this we could break the internal buffer object state here. Only compile tested! Signed-off-by: Christian König Fixes: 43676605f890 drm/ttm: Add vmap/vunmap to TTM and TTM GEM helpers --- drivers/gpu/drm/drm_gem_ttm_helper.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c index d5962a34c01d..e5fc875990c4 100644 --- a/drivers/gpu/drm/drm_gem_ttm_helper.c +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c @@ -64,8 +64,13 @@ int drm_gem_ttm_vmap(struct drm_gem_object *gem, struct iosys_map *map) { struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem); + int ret; + + dma_resv_lock(gem->resv, NULL); + ret = ttm_bo_vmap(bo, map); + dma_resv_unlock(gem->resv); - return ttm_bo_vmap(bo, map); + return ret; } EXPORT_SYMBOL(drm_gem_ttm_vmap); @@ -82,7 +87,9 @@ void drm_gem_ttm_vunmap(struct drm_gem_object *gem, { struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem); + dma_resv_lock(gem->resv, NULL); ttm_bo_vunmap(bo, map); + dma_resv_unlock(gem->resv); } EXPORT_SYMBOL(drm_gem_ttm_vunmap); -- 2.25.1
[PATCH 2/2] drm/ttm: add dma_resv_assert_held() calls to vmap/vunmap
Let's make sure nobody is calling those functions without holding the appropriate locks. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo_util.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 1530982338e9..497ee1fdbad7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -402,6 +402,8 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo, struct iosys_map *map) struct ttm_resource *mem = bo->resource; int ret; + dma_resv_assert_held(bo->base.resv); + ret = ttm_mem_io_reserve(bo->bdev, mem); if (ret) return ret; @@ -460,6 +462,8 @@ void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct iosys_map *map) { struct ttm_resource *mem = bo->resource; + dma_resv_assert_held(bo->base.resv); + if (iosys_map_is_null(map)) return; -- 2.25.1
[PATCH v5 00/13] Add MediaTek MT6370 PMIC support
From: ChiaEn Wu This patch series add MediaTek MT6370 PMIC support. The MT6370 is a highly-integrated smart power management IC, which includes a single cell Li-Ion/Li-Polymer switching battery charger, a USB Type-C & Power Delivery (PD) controller, dual Flash LED current sources, a RGB LED driver, a backlight WLED driver, a display bias driver and a general LDO for portable devices. In this series of patches, we based on Andy Shevchenko's mfd patch used to adjust the Makefile order. (https://lore.kernel.org/all/20220616182524.7956-2-andriy.shevche...@linux.intel.com/) Among with this, we took some changes for MT6370 and refined the MT6370 device tree files to comply with DT specifications. "[PATCH v5 06/13] dt-bindings: mfd: Add MediaTek MT6370" depends on previous DT binding patches, so before applying this patch, please apply other DT patches first. Thanks! Thank you, ChiaEn Wu --- Changes in v5: - In patch 07/13: - Add the comma in the last REGMAP_IRQ_REG_LINE(), DEFINE_RES_IRQ_NAMED() and MFD_CELL_RES() - Add the prefix in the first parameter of all mfd_cell - Move enum and struct mt6370_info to mt6370.h - Remove struct device *dev in struct mt6370_info - Revise the description of Kconfig help text - Revise MODULE_DESCRIPTION() - In patch 08/13: - Add comma for the last index of mt6370_reg_init. - Use dev_err_probe to decrease LOC. - Use 'dev' variable to make probe function more clean. - Refine kconfig text. - Remove both 'else' in set_vbus callback. - Remove comma for of_device_id if the assigned member is only one. - In patch 09/13: - Replace using snprintf() with sysfs_emit() in mt6370_adc_read_label() - Remove macro ADC_CONV_TIME_US - Revise all variable ordering - Revise the description of Kconfig help text - Revise MODULE_DESCRIPTION() - In patch 10/13: - Replace unsigned int type of pwr_rdy with bool in mt6370_chg_set_online() - Remove redundant 'else' in mt6370_chg_field_get() - Revise 'if-else' in mt6370_chg_field_set() - Revise 'if' condition in mt6370_chg_enable_irq() - Revise all text 'otg' --> 'OTG' - Revise MT6370_MIVR_IBUS_TH_100_MA --> MT6370_MIVR_IBUS_TH_100_mA - Revise the description of Kconfig help text - In patch 12/13: - Refine descriptions. - Refine the macro name. - Refine the bracket and blanks. - In patch 13/13: - Add missed - Add struct device *dev in probe() to make code cleaning - Remove useless including header file , - Remove useless variable uasage in mt6370_init_backlight_properties() - Remove redundant checking enable_gpio in mt6370_bl_update_status() - Remove redundant parentheses in mt6370_bl_get_brightness() - Revise the description of Kconfig help text - Revise the calculation of hys_th_steps Changes in v4: - In patch 02/13: - Add minItems of "io-channel-names" - Replace text "Mediatek" with "MediaTek" - In patch 06/13: - Roll back all "$ref: " to v2 patch style (using "/schemas/...") - In patch 07/13: - Replace text "Mediatek" with "MediaTek" in Kconfig - Replace "first break and then return" with "return directly" in "mt6370_check_vendor_info()" - Add module name related description in Kconfig helptext - Add Copyright in the source code - Add header file "mt6370.h" for all "#define IRQ" - Adjust Makefile order of MT6370 - Refine "bank_idx" and "bank_addr" in "mt6375_regmap_read()" / "mt6375_regmap_write()" - Refine redundant "else if" in "mt6370_regmap_read()" - In patch 08/13: - Replace text "Mediatek" with "MediaTek" in Kconfig - Replace "first ret=regulator_(dis/en)able and then return" with "return directly" in "mt6370_tcpc_set_vbus()" - Replace header file with - Add Copyright in the source code - Add module name related description in Kconfig helptext - Remove header file - Refine all probe error by using dev_err_probe() - In patch 09/13: - Replace text "Mediatek" with "MediaTek" - Replace all "first dev_err() and then return" with "return dev_err_probe()" - Add Copyright in the source code - Add module name related description in Kconfig - Add unit suffix of macro "ADC_CONV_POLLING_TIME" - Add new macro "ADC_CONV_TIME_MS" - Adjust the position of include file - Adjust the postions between and - Fix some incorrect characters - In patch 10/13: - Replace text "Mediatek" with "MediaTek" in Kconfig and MODULE_DESCRIPTION() - Replace "mt6370_chg_val_to_reg" and "mt6370_chg_reg_to_val" with "linear_range" API - Replace "first bre
[PATCH v5 01/13] dt-bindings: usb: Add MediaTek MT6370 TCPC
From: ChiYuan Huang Add MediaTek MT6370 TCPC binding documentation. Signed-off-by: ChiYuan Huang Reviewed-by: Krzysztof Kozlowski --- .../bindings/usb/mediatek,mt6370-tcpc.yaml | 36 ++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/mediatek,mt6370-tcpc.yaml diff --git a/Documentation/devicetree/bindings/usb/mediatek,mt6370-tcpc.yaml b/Documentation/devicetree/bindings/usb/mediatek,mt6370-tcpc.yaml new file mode 100644 index 000..72f56cc --- /dev/null +++ b/Documentation/devicetree/bindings/usb/mediatek,mt6370-tcpc.yaml @@ -0,0 +1,36 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/usb/mediatek,mt6370-tcpc.yaml#"; +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; + +title: MediatTek MT6370 Type-C Port Switch and Power Delivery controller + +maintainers: + - ChiYuan Huang + +description: | + MediaTek MT6370 is a multi-functional device. + It integrates charger, ADC, flash, RGB indicators, + regulators (DSV/VIBLDO), and TypeC Port Switch with Power Delivery controller. + This document only describes MT6370 Type-C Port Switch and + Power Delivery controller. + +properties: + compatible: +enum: + - mediatek,mt6370-tcpc + + interrupts: +maxItems: 1 + + connector: +type: object +$ref: /schemas/connector/usb-connector.yaml# +unevaluatedProperties: false + +additionalProperties: false + +required: + - compatible + - interrupts -- 2.7.4
[PATCH v5 02/13] dt-bindings: power: supply: Add MediaTek MT6370 Charger
From: ChiaEn Wu Add MediaTek MT6370 Charger binding documentation. Signed-off-by: ChiaEn Wu Reviewed-by: Krzysztof Kozlowski --- .../power/supply/mediatek,mt6370-charger.yaml | 88 ++ 1 file changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml diff --git a/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml b/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml new file mode 100644 index 000..bd09a0a --- /dev/null +++ b/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/power/supply/mediatek,mt6370-charger.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek MT6370 Battery Charger + +maintainers: + - ChiaEn Wu + +description: | + This module is part of the MT6370 MFD device. + Provides Battery Charger, Boost for OTG devices and BC1.2 detection. + +properties: + compatible: +const: mediatek,mt6370-charger + + interrupts: +description: | + Specify what irqs are needed to be handled by MT6370 Charger driver. IRQ + "MT6370_IRQ_CHG_MIVR", "MT6370_IRQ_ATTACH" and "MT6370_IRQ_OVPCTRL_UVP_D" + are required. +items: + - description: BC1.2 done irq + - description: usb plug in irq + - description: mivr irq + + interrupt-names: +items: + - const: attach_i + - const: uvp_d_evt + - const: mivr + + io-channels: +description: | + Use ADC channel to read VBUS, IBUS, IBAT, etc., info. +minItems: 1 +items: + - description: | + VBUS voltage with lower accuracy (+-75mV) but higher measure + range (1~22V) + - description: | + VBUS voltage with higher accuracy (+-30mV) but lower measure + range (1~9.76V) + - description: the main system input voltage + - description: battery voltage + - description: battery temperature-sense input voltage + - description: IBUS current (required) + - description: battery current + - description: | + regulated output voltage to supply for the PWM low-side gate driver + and the bootstrap capacitor + - description: IC junction temperature + + io-channel-names: +minItems: 1 +items: + - const: vbusdiv5 + - const: vbusdiv2 + - const: vsys + - const: vbat + - const: ts_bat + - const: ibus + - const: ibat + - const: chg_vddp + - const: temp_jc + + usb-otg-vbus-regulator: +type: object +description: OTG boost regulator. +unevaluatedProperties: false +$ref: /schemas/regulator/regulator.yaml# + +properties: + enable-gpios: +maxItems: 1 + +required: + - compatible + - interrupts + - interrupt-names + - io-channels + +additionalProperties: false + +... -- 2.7.4
[PATCH v5 03/13] dt-bindings: leds: mt6370: Add MediaTek MT6370 current sink type LED indicator
From: ChiYuan Huang Add MediaTek MT6370 current sink type LED indicator binding documentation. Signed-off-by: ChiYuan Huang Reviewed-by: Krzysztof Kozlowski --- .../bindings/leds/mediatek,mt6370-indicator.yaml | 77 ++ 1 file changed, 77 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml diff --git a/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml new file mode 100644 index 000..45030f3 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/mediatek,mt6370-indicator.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LED driver for MT6370 PMIC from MediaTek Integrated. + +maintainers: + - Alice Chen + +description: | + This module is part of the MT6370 MFD device. + Add MT6370 LED driver include 4-channel RGB LED support Register/PWM/Breath Mode + +allOf: + - $ref: leds-class-multicolor.yaml# + +properties: + compatible: +const: mediatek,mt6370-indicator + + "#address-cells": +const: 1 + + "#size-cells": +const: 0 + +patternProperties: + "^multi-led@[0-3]$": +type: object + +properties: + reg: +enum: [0, 1, 2, 3] + + "#address-cells": +const: 1 + + "#size-cells": +const: 0 + +patternProperties: + "^led@[0-2]$": +type: object +$ref: common.yaml# +unevaluatedProperties: false + +required: + - reg + - color + +required: + - reg + - color + - "#address-cells" + - "#size-cells" + + "^led@[0-3]$": +type: object +$ref: common.yaml# +unevaluatedProperties: false + +properties: + reg: +enum: [0, 1, 2, 3] + +required: + - reg + - color + +required: + - compatible + - "#address-cells" + - "#size-cells" + +additionalProperties: false -- 2.7.4
[PATCH v5 04/13] dt-bindings: leds: Add MediaTek MT6370 flashlight
From: Alice Chen Add MediaTek MT6370 flashlight binding documentation. Signed-off-by: Alice Chen Reviewed-by: Krzysztof Kozlowski --- .../bindings/leds/mediatek,mt6370-flashlight.yaml | 41 ++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/mediatek,mt6370-flashlight.yaml diff --git a/Documentation/devicetree/bindings/leds/mediatek,mt6370-flashlight.yaml b/Documentation/devicetree/bindings/leds/mediatek,mt6370-flashlight.yaml new file mode 100644 index 000..e9d02ed --- /dev/null +++ b/Documentation/devicetree/bindings/leds/mediatek,mt6370-flashlight.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/mediatek,mt6370-flashlight.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Flash LED driver for MT6370 PMIC from MediaTek Integrated. + +maintainers: + - Alice Chen + +description: | + This module is part of the MT6370 MFD device. + Add MT6370 flash LED driver include 2-channel flash LED support Torch/Strobe Mode. + +properties: + compatible: +const: mediatek,mt6370-flashlight + + "#address-cells": +const: 1 + + "#size-cells": +const: 0 + +patternProperties: + "^led@[0-1]$": +type: object +$ref: common.yaml# +unevaluatedProperties: false + +properties: + reg: +enum: [0, 1] + +required: + - compatible + - "#address-cells" + - "#size-cells" + +additionalProperties: false -- 2.7.4
[PATCH v5 05/13] dt-bindings: backlight: Add MediaTek MT6370 backlight
From: ChiYuan Huang Add MT6370 backlight binding documentation. Signed-off-by: ChiYuan Huang Reviewed-by: Rob Herring --- .../leds/backlight/mediatek,mt6370-backlight.yaml | 92 ++ 1 file changed, 92 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml diff --git a/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml new file mode 100644 index 000..d674212 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml @@ -0,0 +1,92 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/backlight/mediatek,mt6370-backlight.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek MT6370 Backlight + +maintainers: + - ChiaEn Wu + +description: | + This module is part of the MT6370 MFD device. + The MT6370 Backlight WLED driver supports up to a 29V output voltage for + 4 channels of 8 series WLEDs. Each channel supports up to 30mA of current + capability with 2048 current steps (11 bits) in exponential or linear + mapping curves. + +allOf: + - $ref: common.yaml# + +properties: + compatible: +const: mediatek,mt6370-backlight + + default-brightness: +minimum: 0 +maximum: 2048 + + max-brightness: +minimum: 0 +maximum: 2048 + + enable-gpios: +description: External backlight 'enable' pin +maxItems: 1 + + mediatek,bled-pwm-enable: +description: | + Enable external PWM input for backlight dimming +type: boolean + + mediatek,bled-pwm-hys-enable: +description: | + Enable the backlight input-hysteresis for PWM mode +type: boolean + + mediatek,bled-pwm-hys-input-th-steps: +$ref: /schemas/types.yaml#/definitions/uint8 +enum: [1, 4, 16, 64] +description: | + The selection of the upper and lower bounds threshold of backlight + PWM resolution. If we choose selection 64, the variation of PWM + resolution needs more than 64 steps. + + mediatek,bled-ovp-shutdown: +description: | + Enable the backlight shutdown when OVP level triggered +type: boolean + + mediatek,bled-ovp-microvolt: +enum: [1700, 2100, 2500, 2900] +description: | + Backlight OVP level selection. + + mediatek,bled-ocp-shutdown: +description: | + Enable the backlight shutdown when OCP level triggerred. +type: boolean + + mediatek,bled-ocp-microamp: +enum: [90, 120, 150, 180] +description: | + Backlight OC level selection. + + mediatek,bled-channel-use: +$ref: /schemas/types.yaml#/definitions/uint8 +description: | + Backlight LED channel to be used. + Each bit mapping to: +- 0: CH4 +- 1: CH3 +- 2: CH2 +- 3: CH1 +minimum: 1 +maximum: 15 + +required: + - compatible + - mediatek,bled-channel-use + +additionalProperties: false -- 2.7.4
[PATCH v5 06/13] dt-bindings: mfd: Add MediaTek MT6370
From: ChiYuan Huang Add MediaTek MT6370 binding documentation. Signed-off-by: ChiYuan Huang Reviewed-by: Krzysztof Kozlowski --- .../devicetree/bindings/mfd/mediatek,mt6370.yaml | 280 + include/dt-bindings/iio/adc/mediatek,mt6370_adc.h | 18 ++ 2 files changed, 298 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml create mode 100644 include/dt-bindings/iio/adc/mediatek,mt6370_adc.h diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml new file mode 100644 index 000..410e2d4 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml @@ -0,0 +1,280 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/mediatek,mt6370.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek MT6370 SubPMIC + +maintainers: + - ChiYuan Huang + +description: | + MT6370 is a highly-integrated smart power management IC, which includes a + single cell Li-Ion/Li-Polymer switching battery charger, a USB Type-C & + Power Delivery (PD) controller, dual flash LED current sources, a RGB LED + driver, a backlight WLED driver, a display bias driver and a general LDO for + portable devices. + +properties: + compatible: +const: mediatek,mt6370 + + reg: +maxItems: 1 + + wakeup-source: true + + interrupts: +maxItems: 1 + + interrupt-controller: true + + "#interrupt-cells": +const: 1 + + adc: +type: object +description: | + Provides 9 channels for system monitoring, including VBUSDIV5 (lower + accuracy, higher measure range), VBUSDIV2 (higher accuracy, lower + measure range), VBAT, VSYS, CHG_VDDP, TS_BAT, IBUS, IBAT, and TEMP_JC. + +properties: + compatible: +const: mediatek,mt6370-adc + + "#io-channel-cells": +const: 1 + +required: + - compatible + - "#io-channel-cells" + + backlight: +type: object +$ref: /schemas/leds/backlight/mediatek,mt6370-backlight.yaml# + + charger: +type: object +$ref: /schemas/power/supply/mediatek,mt6370-charger.yaml# + + tcpc: +type: object +$ref: /schemas/usb/mediatek,mt6370-tcpc.yaml# + + indicator: +type: object +$ref: /schemas/leds/mediatek,mt6370-indicator.yaml# + + flashlight: +type: object +$ref: /schemas/leds/mediatek,mt6370-flashlight.yaml# + + regulators: +type: object +description: | + List all supported regulators, which support the control for DisplayBias + voltages and one general purpose LDO which commonly used to drive the + vibrator. + +patternProperties: + "^(dsvbst|vibldo)$": +$ref: /schemas/regulator/regulator.yaml# +type: object +unevaluatedProperties: false + + "^(dsvpos|dsvneg)$": +$ref: /schemas/regulator/regulator.yaml# +type: object +unevaluatedProperties: false + +properties: + enable-gpios: +maxItems: 1 + +required: + - compatible + - reg + - interrupts + - interrupt-controller + - "#interrupt-cells" + - regulators + - adc + - backlight + - indicator + - tcpc + - charger + - flashlight + +additionalProperties: false + +examples: + - | +#include +#include +#include +#include +i2c { + #address-cells = <1>; + #size-cells = <0>; + +pmic@34 { +compatible = "mediatek,mt6370"; +reg = <0x34>; +wakeup-source; +interrupts-extended = <&gpio26 3 IRQ_TYPE_LEVEL_LOW>; +interrupt-controller; +#interrupt-cells = <1>; + +mt6370_adc: adc { + compatible = "mediatek,mt6370-adc"; + #io-channel-cells = <1>; +}; + +backlight { + compatible = "mediatek,mt6370-backlight"; + mediatek,bled-channel-use = /bits/ 8 <15>; +}; + +charger { + compatible = "mediatek,mt6370-charger"; + interrupts = <48>, <68>, <6>; + interrupt-names = "attach_i", "uvp_d_evt", "mivr"; + io-channels = <&mt6370_adc MT6370_CHAN_IBUS>; + + mt6370_otg_vbus: usb-otg-vbus-regulator { +regulator-name = "mt6370-usb-otg-vbus"; +regulator-min-microvolt = <435>; +regulator-max-microvolt = <580>; +regulator-min-microamp = <50>; +regulator-max-microamp = <300>; + }; +}; + +indicator { + compatible = "mediatek,mt6370-indicator"; + #address-cells = <1>; + #size-cells = <0>; + + multi-led@0 { +reg = <0>; +function = LED_FUNCTION_INDICATOR; +color = ; +led-max-microamp = <24000>; +#address-cells = <1>; +#size-cells = <0>; +led@0 { + reg = <0>; + color = ; +}; +
[PATCH v5 07/13] mfd: mt6370: Add MediaTek MT6370 support
From: ChiYuan Huang This adds support for the MediaTek MT6370 SubPMIC. MediaTek MT6370 is a SubPMIC consisting of a single cell battery charger with ADC monitoring, RGB LEDs, dual channel flashlight, WLED backlight driver, display bias voltage supply, one general purpose LDO, and the USB Type-C & PD controller complies with the latest USB Type-C and PD standards. Signed-off-by: ChiYuan Huang --- v5 - Add the comma in the last REGMAP_IRQ_REG_LINE(), DEFINE_RES_IRQ_NAMED() and MFD_CELL_RES() - Add the prefix in the first parameter of all mfd_cell - Move enum and struct mt6370_info to mt6370.h - Remove struct device *dev in struct mt6370_info - Revise the description of Kconfig help text - Revise MODULE_DESCRIPTION() --- drivers/mfd/Kconfig | 16 +++ drivers/mfd/Makefile | 1 + drivers/mfd/mt6370.c | 281 +++ drivers/mfd/mt6370.h | 99 ++ 4 files changed, 397 insertions(+) create mode 100644 drivers/mfd/mt6370.c create mode 100644 drivers/mfd/mt6370.h diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 3b59456..a9bcae3 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -937,6 +937,22 @@ config MFD_MT6360 PMIC part includes 2-channel BUCKs and 2-channel LDOs LDO part includes 4-channel LDOs +config MFD_MT6370 + tristate "MediaTek MT6370 SubPMIC" + select MFD_CORE + select REGMAP_I2C + select REGMAP_IRQ + depends on I2C + help + Say Y here to enable MT6370 SubPMIC functional support. + It consists of a single cell battery charger with ADC monitoring, RGB + LEDs, dual channel flashlight, WLED backlight driver, display bias + voltage supply, one general purpose LDO, and the USB Type-C & PD + controller complies with the latest USB Type-C and PD standards. + + This driver can also be built as a module. If so, the module + will be called "mt6370". + config MFD_MT6397 tristate "MediaTek MT6397 PMIC Support" select MFD_CORE diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 8c69867..81dbed3 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -173,6 +173,7 @@ obj-$(CONFIG_MFD_MAX8998) += max8998.o max8998-irq.o obj-$(CONFIG_MFD_MP2629) += mp2629.o obj-$(CONFIG_MFD_MT6360) += mt6360-core.o +obj-$(CONFIG_MFD_MT6370) += mt6370.o mt6397-objs:= mt6397-core.o mt6397-irq.o mt6358-irq.o obj-$(CONFIG_MFD_MT6397) += mt6397.o diff --git a/drivers/mfd/mt6370.c b/drivers/mfd/mt6370.c new file mode 100644 index 000..070c1c7 --- /dev/null +++ b/drivers/mfd/mt6370.c @@ -0,0 +1,281 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Richtek Technology Corp. + * + * Author: ChiYuan Huang + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "mt6370.h" + +#define MT6370_REG_DEV_INFO0x100 +#define MT6370_REG_CHG_IRQ10x1C0 +#define MT6370_REG_CHG_MASK1 0x1E0 + +#define MT6370_VENID_MASK GENMASK(7, 4) + +#define MT6370_NUM_IRQREGS 16 +#define MT6370_USBC_I2CADDR0x4E +#define MT6370_REG_ADDRLEN 2 +#define MT6370_REG_MAXADDR 0x1FF + +#define MT6370_VENID_RT50810x8 +#define MT6370_VENID_RT5081A 0xA +#define MT6370_VENID_MT63700xE +#define MT6370_VENID_MT63710xF +#define MT6370_VENID_MT6372P 0x9 +#define MT6370_VENID_MT6372CP 0xB + +static const struct regmap_irq mt6370_irqs[] = { + REGMAP_IRQ_REG_LINE(MT6370_IRQ_DIRCHGON, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_TREG, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_AICR, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_MIVR, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_PWR_RDY, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_FL_CHG_VINOVP, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_VSYSUV, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_VSYSOV, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_VBATOV, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_VINOVPCHG, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_TS_BAT_COLD, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_TS_BAT_COOL, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_TS_BAT_WARM, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_TS_BAT_HOT, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_TS_STATC, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_FAULT, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_STATC, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_TMR, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_BATABS, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_ADPBAD, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_RVP, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_TSHUTDOWN, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_IINMEAS, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHG_ICCMEAS, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_CHGDET_DONE, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_WDTMR, 8), + REGMAP_IRQ_REG_LINE(MT6370_IRQ_SSFINISH,
[PATCH v5 08/13] usb: typec: tcpci_mt6370: Add MediaTek MT6370 tcpci driver
From: ChiYuan Huang The MT6370 is a highly-integrated smart power management IC, which includes a single cell Li-Ion/Li-Polymer switching battery charger, a USB Type-C & Power Delivery (PD) controller, dual Flash LED current sources, a RGB LED driver, a backlight WLED driver, a display bias driver and a general LDO for portable devices. This commit add support for the Type-C & Power Delivery controller in MediaTek MT6370 IC. Signed-off-by: ChiYuan Huang --- v5 - Add comma for the last index of mt6370_reg_init. - Use dev_err_probe to decrease LOC. - Use 'dev' variable to make probe function more clean. - Refine kconfig text. - Remove both 'else' in set_vbus callback. - Remove comma for of_device_id if the assigned member is only one. --- drivers/usb/typec/tcpm/Kconfig| 11 ++ drivers/usb/typec/tcpm/Makefile | 1 + drivers/usb/typec/tcpm/tcpci_mt6370.c | 207 ++ 3 files changed, 219 insertions(+) create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6370.c diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig index 073fd2e..e6b88ca 100644 --- a/drivers/usb/typec/tcpm/Kconfig +++ b/drivers/usb/typec/tcpm/Kconfig @@ -35,6 +35,17 @@ config TYPEC_MT6360 USB Type-C. It works with Type-C Port Controller Manager to provide USB PD and USB Type-C functionalities. +config TYPEC_TCPCI_MT6370 + tristate "MediaTek MT6370 Type-C driver" + depends on MFD_MT6370 + help + MediaTek MT6370 is a multi-functional IC that includes + USB Type-C. It works with Type-C Port Controller Manager + to provide USB PD and USB Type-C functionalities. + + This driver can also be built as a module. The module + will be called "tcpci_mt6370". + config TYPEC_TCPCI_MAXIM tristate "Maxim TCPCI based Type-C chip driver" help diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile index 7d499f3..906d9dc 100644 --- a/drivers/usb/typec/tcpm/Makefile +++ b/drivers/usb/typec/tcpm/Makefile @@ -6,4 +6,5 @@ typec_wcove-y := wcove.o obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o obj-$(CONFIG_TYPEC_MT6360) += tcpci_mt6360.o +obj-$(CONFIG_TYPEC_TCPCI_MT6370) += tcpci_mt6370.o obj-$(CONFIG_TYPEC_TCPCI_MAXIM)+= tcpci_maxim.o diff --git a/drivers/usb/typec/tcpm/tcpci_mt6370.c b/drivers/usb/typec/tcpm/tcpci_mt6370.c new file mode 100644 index 000..a08b0ba --- /dev/null +++ b/drivers/usb/typec/tcpm/tcpci_mt6370.c @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Richtek Technology Corp. + * + * Author: ChiYuan Huang + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "tcpci.h" + +#define MT6370_REG_SYSCTRL80x9B + +#define MT6370_AUTOIDLE_MASK BIT(3) + +#define MT6370_VENDOR_ID 0x29CF +#define MT6370_TCPC_DID_A 0x2170 + +struct mt6370_priv { + struct device *dev; + struct regulator *vbus; + struct tcpci *tcpci; + struct tcpci_data tcpci_data; + int irq; +}; + +static const struct reg_sequence mt6370_reg_init[] = { + REG_SEQ(0xA0, 0x1, 1000), + REG_SEQ(0x81, 0x38, 0), + REG_SEQ(0x82, 0x82, 0), + REG_SEQ(0xBA, 0xFC, 0), + REG_SEQ(0xBB, 0x50, 0), + REG_SEQ(0x9E, 0x8F, 0), + REG_SEQ(0xA1, 0x5, 0), + REG_SEQ(0xA2, 0x4, 0), + REG_SEQ(0xA3, 0x4A, 0), + REG_SEQ(0xA4, 0x01, 0), + REG_SEQ(0x95, 0x01, 0), + REG_SEQ(0x80, 0x71, 0), + REG_SEQ(0x9B, 0x3A, 1000), +}; + +static int mt6370_tcpc_init(struct tcpci *tcpci, struct tcpci_data *data) +{ + u16 did; + int ret; + + ret = regmap_register_patch(data->regmap, mt6370_reg_init, + ARRAY_SIZE(mt6370_reg_init)); + if (ret) + return ret; + + ret = regmap_raw_read(data->regmap, TCPC_BCD_DEV, &did, sizeof(u16)); + if (ret) + return ret; + + if (did == MT6370_TCPC_DID_A) + return regmap_write(data->regmap, TCPC_FAULT_CTRL, 0x80); + + return 0; +} + +static int mt6370_tcpc_set_vconn(struct tcpci *tcpci, struct tcpci_data *data, +bool enable) +{ + return regmap_update_bits(data->regmap, MT6370_REG_SYSCTRL8, + MT6370_AUTOIDLE_MASK, + !enable ? MT6370_AUTOIDLE_MASK : 0); +} + +static int mt6370_tcpc_set_vbus(struct tcpci *tcpci, struct tcpci_data *data, + bool source, bool sink) +{ + struct mt6370_priv *priv = container_of(data, struct mt6370_priv, + tcpci_data); + int ret; + + ret = regulator_is_enabled(priv->vbus); + if (ret < 0) + return
[PATCH v5 09/13] iio: adc: mt6370: Add MediaTek MT6370 support
From: ChiaEn Wu MediaTek MT6370 is a SubPMIC consisting of a single cell battery charger with ADC monitoring, RGB LEDs, dual channel flashlight, WLED backlight driver, display bias voltage supply, one general purpose LDO, and the USB Type-C & PD controller complies with the latest USB Type-C and PD standards. This adds support the MT6370 ADC driver for system monitoring, including charger current, voltage, and temperature. Signed-off-by: ChiaEn Wu --- v5 - Replace using snprintf() with sysfs_emit() in mt6370_adc_read_label() - Remove macro ADC_CONV_TIME_US - Revise all variable ordering - Revise the description of Kconfig help text - Revise MODULE_DESCRIPTION() --- drivers/iio/adc/Kconfig | 12 ++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/mt6370-adc.c | 273 +++ 3 files changed, 286 insertions(+) create mode 100644 drivers/iio/adc/mt6370-adc.c diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 48ace74..60bcc28 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -737,6 +737,18 @@ config MEDIATEK_MT6360_ADC is used in smartphones and tablets and supports a 11 channel general purpose ADC. +config MEDIATEK_MT6370_ADC + tristate "MediaTek MT6370 ADC driver" + depends on MFD_MT6370 + help + Say yes here to enable MediaTek MT6370 ADC support. + + This ADC driver provides 9 channels for system monitoring (charger + current, voltage, and temperature). + + This driver can also be built as a module. If so, the module + will be called "mt6370-adc". + config MEDIATEK_MT6577_AUXADC tristate "MediaTek AUXADC driver" depends on ARCH_MEDIATEK || COMPILE_TEST diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index 39d806f..0ce285c 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -68,6 +68,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o obj-$(CONFIG_MCP3422) += mcp3422.o obj-$(CONFIG_MCP3911) += mcp3911.o obj-$(CONFIG_MEDIATEK_MT6360_ADC) += mt6360-adc.o +obj-$(CONFIG_MEDIATEK_MT6370_ADC) += mt6370-adc.o obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o obj-$(CONFIG_MESON_SARADC) += meson_saradc.o diff --git a/drivers/iio/adc/mt6370-adc.c b/drivers/iio/adc/mt6370-adc.c new file mode 100644 index 000..51ef133 --- /dev/null +++ b/drivers/iio/adc/mt6370-adc.c @@ -0,0 +1,273 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Richtek Technology Corp. + * + * Author: ChiaEn Wu + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define MT6370_REG_CHG_CTRL3 0x113 /* AICR */ +#define MT6370_REG_CHG_CTRL7 0x117 /* ICHG */ +#define MT6370_REG_CHG_ADC 0x121 +#define MT6370_REG_ADC_DATA_H 0x14C + +#define MT6370_ADC_START_MASK BIT(0) +#define MT6370_ADC_IN_SEL_MASK GENMASK(7, 4) +#define MT6370_AICR_ICHG_MASK GENMASK(7, 2) + +#define MT6370_AICR_400MA 0x6 +#define MT6370_ICHG_500MA 0x4 +#define MT6370_ICHG_900MA 0x8 + +#define ADC_CONV_TIME_MS 35 +#define ADC_CONV_POLLING_TIME_US 1000 + +struct mt6370_adc_data { + struct device *dev; + struct regmap *regmap; + /* +* This mutex lock is for preventing the different ADC channels +* from being read at the same time. +*/ + struct mutex adc_lock; +}; + +static int mt6370_adc_read_channel(struct mt6370_adc_data *priv, int chan, + unsigned long addr, int *val) +{ + unsigned int reg_val; + __be16 be_val; + int ret; + + mutex_lock(&priv->adc_lock); + + reg_val = MT6370_ADC_START_MASK | + FIELD_PREP(MT6370_ADC_IN_SEL_MASK, addr); + ret = regmap_write(priv->regmap, MT6370_REG_CHG_ADC, reg_val); + if (ret) + goto adc_unlock; + + msleep(ADC_CONV_TIME_MS); + + ret = regmap_read_poll_timeout(priv->regmap, + MT6370_REG_CHG_ADC, reg_val, + !(reg_val & MT6370_ADC_START_MASK), + ADC_CONV_POLLING_TIME_US, + ADC_CONV_TIME_MS * 1000 * 3); + if (ret) { + dev_err(priv->dev, "Failed to read ADC register (%d)\n", ret); + goto adc_unlock; + } + + ret = regmap_raw_read(priv->regmap, MT6370_REG_ADC_DATA_H, + &be_val, sizeof(be_val)); + if (ret) + goto adc_unlock; + + *val = be16_to_cpu(be_val); + ret = IIO_VAL_INT; + +adc_unlock: + mutex_unlock(&priv->adc_lock); + + return ret; +} + +static int mt6370_adc_read_scale(struct mt6370_adc_data *priv, +int c
[PATCH v5 10/13] power: supply: mt6370: Add MediaTek MT6370 charger driver
From: ChiaEn Wu MediaTek MT6370 is a SubPMIC consisting of a single cell battery charger with ADC monitoring, RGB LEDs, dual channel flashlight, WLED backlight driver, display bias voltage supply, one general purpose LDO, and the USB Type-C & PD controller complies with the latest USB Type-C and PD standards. This adds MediaTek MT6370 Charger driver support. The charger module of MT6370 supports High-Accuracy Voltage/Current Regulation, Average Input Current Regulation, Battery Temperature Sensing, Over-Temperature Protection, DPDM Detection for BC1.2. Signed-off-by: ChiaEn Wu --- v5 - Replace unsigned int type of pwr_rdy with bool in mt6370_chg_set_online() - Remove redundant 'else' in mt6370_chg_field_get() - Revise 'if-else' in mt6370_chg_field_set() - Revise 'if' condition in mt6370_chg_enable_irq() - Revise all text 'otg' --> 'OTG' - Revise MT6370_MIVR_IBUS_TH_100_MA --> MT6370_MIVR_IBUS_TH_100_mA - Revise the description of Kconfig help text --- drivers/power/supply/Kconfig | 14 + drivers/power/supply/Makefile |1 + drivers/power/supply/mt6370-charger.c | 1062 + 3 files changed, 1077 insertions(+) create mode 100644 drivers/power/supply/mt6370-charger.c diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index 1aa8323..591deb8 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -619,6 +619,20 @@ config CHARGER_MT6360 Average Input Current Regulation, Battery Temperature Sensing, Over-Temperature Protection, DPDM Detection for BC1.2. +config CHARGER_MT6370 + tristate "MediaTek MT6370 Charger Driver" + depends on MFD_MT6370 + depends on REGULATOR + select LINEAR_RANGES + help + Say Y here to enable MT6370 Charger Part. + The device supports High-Accuracy Voltage/Current Regulation, + Average Input Current Regulation, Battery Temperature Sensing, + Over-Temperature Protection, DPDM Detection for BC1.2. + + This driver can also be built as a module. If so, the module + will be called "mt6370-charger". + config CHARGER_QCOM_SMBB tristate "Qualcomm Switch-Mode Battery Charger and Boost" depends on MFD_SPMI_PMIC || COMPILE_TEST diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index 7f02f36..8c95276 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -82,6 +82,7 @@ obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o obj-$(CONFIG_CHARGER_MP2629) += mp2629_charger.o obj-$(CONFIG_CHARGER_MT6360) += mt6360_charger.o +obj-$(CONFIG_CHARGER_MT6370) += mt6370-charger.o obj-$(CONFIG_CHARGER_QCOM_SMBB)+= qcom_smbb.o obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o diff --git a/drivers/power/supply/mt6370-charger.c b/drivers/power/supply/mt6370-charger.c new file mode 100644 index 000..76a8c91 --- /dev/null +++ b/drivers/power/supply/mt6370-charger.c @@ -0,0 +1,1062 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Richtek Technology Corp. + * + * Author: ChiaEn Wu + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define MT6370_REG_CHG_CTRL1 0x111 +#define MT6370_REG_CHG_CTRL2 0x112 +#define MT6370_REG_CHG_CTRL3 0x113 +#define MT6370_REG_CHG_CTRL4 0x114 +#define MT6370_REG_CHG_CTRL5 0x115 +#define MT6370_REG_CHG_CTRL6 0x116 +#define MT6370_REG_CHG_CTRL7 0x117 +#define MT6370_REG_CHG_CTRL8 0x118 +#define MT6370_REG_CHG_CTRL9 0x119 +#define MT6370_REG_CHG_CTRL10 0x11A +#define MT6370_REG_DEVICE_TYPE 0x122 +#define MT6370_REG_USB_STATUS1 0x127 +#define MT6370_REG_CHG_STAT0x14A +#define MT6370_REG_FLED_EN 0x17E +#define MT6370_REG_CHG_STAT1 0X1D0 +#define MT6370_REG_OVPCTRL_STAT0x1D8 + +#define MT6370_VOBST_MASK GENMASK(7, 2) +#define MT6370_OTG_PIN_EN_MASK BIT(1) +#define MT6370_OPA_MODE_MASK BIT(0) +#define MT6370_OTG_OC_MASK GENMASK(2, 0) + +#define MT6370_MIVR_IBUS_TH_100_mA 10 +#define MT6370_ADC_CHAN_IBUS 5 +#define MT6370_ADC_CHAN_MAX9 + +enum mt6370_chg_reg_field { + /* MT6370_REG_CHG_CTRL2 */ + F_IINLMTSEL, F_CFO_EN, F_CHG_EN, + /* MT6370_REG_CHG_CTRL3 */ + F_IAICR, F_AICR_EN, F_ILIM_EN, + /* MT6370_REG_CHG_CTRL4 */ + F_VOREG, + /* MT6370_REG_CHG_CTRL6 */ + F_VMIVR, + /* MT6370_REG_CHG_CTRL7 */ + F_ICHG, + /* MT6370_REG_CHG_CTRL8 */ + F_IPREC, + /* MT6370_REG_CHG_CTRL9 */ + F_IEOC, + /* MT6370_REG_DEVICE_TYPE */ + F_USBCHGEN, +
[PATCH v5 11/13] leds: mt6370: Add MediaTek MT6370 current sink type LED Indicator support
From: ChiYuan Huang The MediaTek MT6370 is a highly-integrated smart power management IC, which includes a single cell Li-Ion/Li-Polymer switching battery charger, a USB Type-C & Power Delivery (PD) controller, dual Flash LED current sources, a RGB LED driver, a backlight WLED driver, a display bias driver and a general LDO for portable devices. In MediaTek MT6370, there are four channel current-sink RGB LEDs that support hardware pattern for constant current, PWM, and breath mode. Isink4 channel can also be used as a CHG_VIN power good indicator. Signed-off-by: ChiYuan Huang --- drivers/leds/Kconfig | 14 + drivers/leds/Makefile | 1 + drivers/leds/leds-mt6370.c | 994 + 3 files changed, 1009 insertions(+) create mode 100644 drivers/leds/leds-mt6370.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a49979f..71bacb5 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -244,6 +244,20 @@ config LEDS_MT6323 This option enables support for on-chip LED drivers found on Mediatek MT6323 PMIC. +config LEDS_MT6370_RGB + tristate "LED Support for MediaTek MT6370 PMIC" + depends on LEDS_CLASS + depends on MFD_MT6370 + select LINEAR_RANGE + help + Say Y here to enable support for MT6370_RGB LED device. + In MT6370, there are four channel current-sink LED drivers that + support hardware pattern for constant current, PWM, and breath mode. + Isink4 channel can also be used as a CHG_VIN power good indicator. + + This driver can also be built as a module. If so the module + will be called "leds-mt6370.ko". + config LEDS_S3C24XX tristate "LED Support for Samsung S3C24XX GPIO LEDs" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 4fd2f92..557be42 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -62,6 +62,7 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532) += leds-rb532.o obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o +obj-$(CONFIG_LEDS_MT6370_RGB) += leds-mt6370.o obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o diff --git a/drivers/leds/leds-mt6370.c b/drivers/leds/leds-mt6370.c new file mode 100644 index 000..1038232 --- /dev/null +++ b/drivers/leds/leds-mt6370.c @@ -0,0 +1,994 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Richtek Technology Corp. + * + * Author: Alice Chen + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +enum { + MT6370_LED_ISNK1 = 0, + MT6370_LED_ISNK2, + MT6370_LED_ISNK3, + MT6370_LED_ISNK4, + MT6370_MAX_LEDS +}; + +enum mt6370_led_mode { + MT6370_LED_PWM_MODE = 0, + MT6370_LED_BREATH_MODE, + MT6370_LED_REG_MODE, + MT6370_LED_MAX_MODE +}; + +enum mt6370_led_field { + F_RGB_EN = 0, + F_CHGIND_EN, + F_LED1_CURR, + F_LED2_CURR, + F_LED3_CURR, + F_LED4_CURR, + F_LED1_MODE, + F_LED2_MODE, + F_LED3_MODE, + F_LED4_MODE, + F_LED1_DUTY, + F_LED2_DUTY, + F_LED3_DUTY, + F_LED4_DUTY, + F_LED1_FREQ, + F_LED2_FREQ, + F_LED3_FREQ, + F_LED4_FREQ, + F_MAX_FIELDS +}; + +enum mt6370_led_ranges { + R_LED123_CURR = 0, + R_LED4_CURR, + R_LED_TRFON, + R_LED_TOFF, + R_MAX_RANGES, +}; + +enum mt6370_pattern { + P_LED_TR1 = 0, + P_LED_TR2, + P_LED_TF1, + P_LED_TF2, + P_LED_TON, + P_LED_TOFF, + P_MAX_PATTERNS +}; + +#define MT6370_REG_DEV_INFO0x100 +#define MT6370_REG_RGB1_DIM0x182 +#define MT6370_REG_RGB2_DIM0x183 +#define MT6370_REG_RGB3_DIM0x184 +#define MT6370_REG_RGB_EN 0x185 +#define MT6370_REG_RGB1_ISNK 0x186 +#define MT6370_REG_RGB2_ISNK 0x187 +#define MT6370_REG_RGB3_ISNK 0x188 +#define MT6370_REG_RGB1_TR 0x189 +#define MT6370_REG_RGB_CHRIND_DIM 0x192 +#define MT6370_REG_RGB_CHRIND_CTRL 0x193 +#define MT6370_REG_RGB_CHRIND_TR 0x194 + +#define MT6372_REG_RGB_EN 0x182 +#define MT6372_REG_RGB1_ISNK 0x183 +#define MT6372_REG_RGB2_ISNK 0x184 +#define MT6372_REG_RGB3_ISNK 0x185 +#define MT6372_REG_RGB4_ISNK 0x186 +#define MT6372_REG_RGB1_DIM0x187 +#define MT6372_REG_RGB2_DIM0x188 +#define MT6372_REG_RGB3_DIM0x189 +#define MT637
[PATCH v5 12/13] leds: flashlight: mt6370: Add MediaTek MT6370 flashlight support
From: Alice Chen The MediaTek MT6370 is a highly-integrated smart power management IC, whichincludes a single cell Li-Ion/Li-Polymer switching battery charger, a USB Type-C & Power Delivery (PD) controller, dual Flash LED current sources, a RGB LED driver, a backlight WLED driver, a display bias driver and a general LDO for portable devices. The Flash LED in MT6370 has 2 channel and support torch/strobe mode. The commit add the support of MT6370 FLASH LED. Signed-off-by: Alice Chen --- v5 - Refine descriptions. - Refine the macro name. - Refine the brackets and blanks. --- drivers/leds/flash/Kconfig | 12 + drivers/leds/flash/Makefile| 1 + drivers/leds/flash/leds-mt6370-flash.c | 661 + 3 files changed, 674 insertions(+) create mode 100644 drivers/leds/flash/leds-mt6370-flash.c diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig index d3eb689..d5761ed 100644 --- a/drivers/leds/flash/Kconfig +++ b/drivers/leds/flash/Kconfig @@ -90,4 +90,16 @@ config LEDS_SGM3140 This option enables support for the SGM3140 500mA Buck/Boost Charge Pump LED Driver. +config LEDS_MT6370_FLASHLIGHT + tristate "Flash LED Support for MediaTek MT6370 PMIC" + depends on LEDS_CLASS + depends on MFD_MT6370 + help + Support 2 channels and torch/strobe mode. + Say Y here to enable support for + MT6370_FLASH_LED device. + + This driver can also be built as a module. If so, the module + will be called "leds-mt6370-flash". + endif # LEDS_CLASS_FLASH diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile index 0acbddc..4c4c171 100644 --- a/drivers/leds/flash/Makefile +++ b/drivers/leds/flash/Makefile @@ -9,3 +9,4 @@ obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o obj-$(CONFIG_LEDS_RT4505) += leds-rt4505.o obj-$(CONFIG_LEDS_RT8515) += leds-rt8515.o obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o +obj-$(CONFIG_LEDS_MT6370_FLASHLIGHT) += leds-mt6370-flash.o diff --git a/drivers/leds/flash/leds-mt6370-flash.c b/drivers/leds/flash/leds-mt6370-flash.c new file mode 100644 index 000..eccda69 --- /dev/null +++ b/drivers/leds/flash/leds-mt6370-flash.c @@ -0,0 +1,661 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Richtek Technology Corp. + * + * Author: Alice Chen +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +enum { + MT6370_LED_FLASH1, + MT6370_LED_FLASH2, + MT6370_MAX_LEDS +}; + +/* Virtual definition for multicolor */ + +#define MT6370_REG_FLEDEN 0x17E +#define MT6370_REG_STRBTO 0x173 +#define MT6370_REG_CHGSTAT20x1D1 +#define MT6370_REG_FLEDSTAT1 0x1D9 +#defineMT6370_REG_FLEDISTRB(_id) (0x174 + 4 * _id) +#define MT6370_REG_FLEDITOR(_id) (0x175 + 4 * _id) +#define MT6370_ITORCH_MASK GENMASK(4, 0) +#define MT6370_ISTROBE_MASKGENMASK(6, 0) +#define MT6370_STRBTO_MASK GENMASK(6, 0) +#define MT6370_TORCHEN_MASKBIT(3) +#define MT6370_STROBEN_MASKBIT(2) +#define MT6370_FLCSEN_MASK(_id)BIT(MT6370_LED_FLASH2 - _id) +#define MT6370_FLCSEN_MASK_ALL (BIT(0) | BIT(1)) +#define MT6370_FLEDCHGVINOVP_MASK BIT(3) +#define MT6370_FLED1STRBTO_MASKBIT(11) +#define MT6370_FLED2STRBTO_MASKBIT(10) +#define MT6370_FLED1STRB_MASK BIT(9) +#define MT6370_FLED2STRB_MASK BIT(8) +#define MT6370_FLED1SHORT_MASK BIT(7) +#define MT6370_FLED2SHORT_MASK BIT(6) +#define MT6370_FLEDLVF_MASKBIT(3) + +#define MT6370_LED_JOINT 2 +#define MT6370_RANGE_FLED_REG 4 +#define MT6370_ITORCH_MIN_UA 25000 +#define MT6370_ITORCH_STEP_UA 12500 +#define MT6370_ITORCH_MAX_UA 40 +#define MT6370_ITORCH_DOUBLE_MAX_UA80 +#define MT6370_ISTRB_MIN_UA5 +#define MT6370_ISTRB_STEP_UA 12500 +#define MT6370_ISTRB_MAX_UA150 +#define MT6370_ISTRB_DOUBLE_MAX_UA 300 +#define MT6370_STRBTO_MIN_US 64000 +#define MT6370_STRBTO_STEP_US 32000 +#define MT6370_STRBTO_MAX_US 2432000 + +#define STATE_OFF 0 +#define STATE_KEEP 1 +#define STATE_ON 2 + +#define to_mt6370_led(ptr, member) container_of(ptr, struct mt6370_led, member) + +struct mt6370_led { + struct led_classdev_flash flash; + struct v4l2_flash *v4l2_flash; + struct mt6370_priv *priv; + u32 led_no; + u32 default_state; +}; + +struct mt6370_priv { + struct device *dev; + struct regmap *regmap; + struct mutex lock; + unsigned int fled_strobe_used; + unsigned int fled_torch_used; + unsigned int leds_active; + unsigned int leds_coun
[PATCH v5 13/13] video: backlight: mt6370: Add MediaTek MT6370 support
From: ChiaEn Wu MediaTek MT6370 is a SubPMIC consisting of a single cell battery charger with ADC monitoring, RGB LEDs, dual channel flashlight, WLED backlight driver, display bias voltage supply, one general purpose LDO, and the USB Type-C & PD controller complies with the latest USB Type-C and PD standards. This adds support for MediaTek MT6370 Backlight driver. It's commonly used to drive the display WLED. There are 4 channels inside, and each channel supports up to 30mA of current capability with 2048 current steps in exponential or linear mapping curves. Signed-off-by: ChiaEn Wu --- v5 - Add missed - Add struct device *dev in probe() to make code cleaning - Remove useless including header file , - Remove useless variable uasage in mt6370_init_backlight_properties() - Remove redundant checking enable_gpio in mt6370_bl_update_status() - Remove redundant parentheses in mt6370_bl_get_brightness() - Revise the description of Kconfig help text - Revise the calculation of hys_th_steps --- drivers/video/backlight/Kconfig| 12 + drivers/video/backlight/Makefile | 1 + drivers/video/backlight/mt6370-backlight.c | 339 + 3 files changed, 352 insertions(+) create mode 100644 drivers/video/backlight/mt6370-backlight.c diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig index a003e02..846dbe7 100644 --- a/drivers/video/backlight/Kconfig +++ b/drivers/video/backlight/Kconfig @@ -268,6 +268,18 @@ config BACKLIGHT_MAX8925 If you have a LCD backlight connected to the WLED output of MAX8925 WLED output, say Y here to enable this driver. +config BACKLIGHT_MT6370 + tristate "MediaTek MT6370 Backlight Driver" + depends on MFD_MT6370 + help + This enables support for Mediatek MT6370 Backlight driver. + It's commonly used to drive the display WLED. There are 4 channels + inside, and each channel supports up to 30mA of current capability + with 2048 current steps in exponential or linear mapping curves. + + This driver can also be built as a module. If so, the module + will be called "mt6370-backlight". + config BACKLIGHT_APPLE tristate "Apple Backlight Driver" depends on X86 && ACPI diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile index cae2c83..e815f3f 100644 --- a/drivers/video/backlight/Makefile +++ b/drivers/video/backlight/Makefile @@ -44,6 +44,7 @@ obj-$(CONFIG_BACKLIGHT_LP855X)+= lp855x_bl.o obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o obj-$(CONFIG_BACKLIGHT_MAX8925)+= max8925_bl.o +obj-$(CONFIG_BACKLIGHT_MT6370) += mt6370-backlight.o obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o obj-$(CONFIG_BACKLIGHT_PANDORA)+= pandora_bl.o obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o diff --git a/drivers/video/backlight/mt6370-backlight.c b/drivers/video/backlight/mt6370-backlight.c new file mode 100644 index 000..ba00a8f --- /dev/null +++ b/drivers/video/backlight/mt6370-backlight.c @@ -0,0 +1,339 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Richtek Technology Corp. + * + * Author: ChiaEn Wu + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define MT6370_REG_DEV_INFO0x100 +#define MT6370_REG_BL_EN 0x1A0 +#define MT6370_REG_BL_BSTCTRL 0x1A1 +#define MT6370_REG_BL_PWM 0x1A2 +#define MT6370_REG_BL_DIM2 0x1A4 + +#define MT6370_VENID_MASK GENMASK(7, 4) +#define MT6370_BL_EXT_EN_MASK BIT(7) +#define MT6370_BL_EN_MASK BIT(6) +#define MT6370_BL_CONFIG_MASK BIT(0) +#define MT6370_BL_CH_MASK GENMASK(5, 2) +#define MT6370_BL_DIM2_MASKGENMASK(2, 0) +#define MT6370_BL_DUMMY_6372_MASK GENMASK(2, 0) +#define MT6370_BL_DIM2_6372_SHIFT 3 +#define MT6370_BL_PWM_EN_MASK BIT(7) +#define MT6370_BL_PWM_HYS_EN_MASK BIT(2) +#define MT6370_BL_PWM_HYS_SEL_MASK GENMASK(1, 0) +#define MT6370_BL_OVP_EN_MASK BIT(7) +#define MT6370_BL_OVP_SEL_MASK GENMASK(6, 5) +#define MT6370_BL_OC_EN_MASK BIT(3) +#define MT6370_BL_OC_SEL_MASK GENMASK(2, 1) + +#define MT6370_BL_PWM_HYS_TH_MIN_STEP 1 +#define MT6370_BL_PWM_HYS_TH_MAX_STEP 64 +#define MT6370_BL_OVP_MIN_UV 1700 +#define MT6370_BL_OVP_MAX_UV 2900 +#define MT6370_BL_OVP_STEP_UV 400 +#define MT6370_BL_OCP_MIN_UA 90 +#define MT6370_BL_OCP_MAX_UA 180 +#define MT6370_BL_OCP_STEP_UA 30 +#define MT6370_BL_MAX_BRIGHTNESS 2048 +#define MT6370_BL_MAX_CH 15 + +enum { + MT6370_VID_COMMON = 0, + MT6370_VID_6372, +}; + +struct mt6370_priv { + int
Re: [PATCH 1/2] drm/ttm: fix locking in vmap/vunmap TTM GEM helpers
On 7/15/22 14:15, Christian König wrote: > I've stumbled over this while reviewing patches for DMA-buf and it looks > like we completely messed the locking up here. > > In general most TTM function should only be called while holding the > appropriate BO resv lock. Without this we could break the internal > buffer object state here. Could you please clarify which part of the TTM state is affected? -- Best regards, Dmitry
Re: [PATCH 1/2] drm/ttm: fix locking in vmap/vunmap TTM GEM helpers
Am 15.07.22 um 13:31 schrieb Dmitry Osipenko: On 7/15/22 14:15, Christian König wrote: I've stumbled over this while reviewing patches for DMA-buf and it looks like we completely messed the locking up here. In general most TTM function should only be called while holding the appropriate BO resv lock. Without this we could break the internal buffer object state here. Could you please clarify which part of the TTM state is affected? The ttm_bo_vmap() function calls ttm_mem_io_reserve() with bo->resource as parameter. Since bo->resource is protected by the lock we must make sure that we are holding the lock while doing this. Regards, Christian.
Re: [PATCH v5 4/9] drm: selftest: convert drm_format selftest to KUnit
On 7/15/22 03:59, Javier Martinez Canillas wrote: > On 7/15/22 02:03, Daniel Latypov wrote: >> On Thu, Jul 14, 2022 at 4:51 PM Guenter Roeck wrote: >>> >>> On Fri, Jul 08, 2022 at 05:30:47PM -0300, Maíra Canal wrote: Considering the current adoption of the KUnit framework, convert the DRM format selftest to the KUnit API. Tested-by: David Gow Acked-by: Daniel Latypov Reviewed-by: Javier Martinez Canillas Signed-off-by: Maíra Canal >>> >>> This patch results in: >>> >>> Building powerpc:allmodconfig ... failed >>> -- >>> Error log: >>> drivers/gpu/drm/tests/drm_format_test.c: In function >>> 'igt_check_drm_format_min_pitch': >>> drivers/gpu/drm/tests/drm_format_test.c:271:1: error: the frame size of >>> 3712 bytes is larger than 2048 bytes >>> >>> presumably due to function nesting. >> >> This can happen when there's a lot of KUNIT_EXPECT_* calls in a single >> function. >> See [1] for some related context. >> There were a number of patches that went into 5.18 ([2] and others) to >> try and mitigate this, but it's not always enough. >> >> Ideally the compiler would see that the stack-local variables used in >> these macros don't need to stick around, but it doesn't always >> happen... > > Thanks Daniel for the explanation. > >> One workaround would be to split up the test case functions into smaller >> chunks. >> > > Maíra, > > Could you please look at splitting in smaller chunks to mitigate this issue ? I'll look into this during the weekend. Thanks Guenter for the report. Best Regards, - Maíra Canal >
Re: [PATCH 05/11] fbdev: Convert drivers to aperture helpers
Hi Am 11.07.22 um 13:01 schrieb Javier Martinez Canillas: On 7/7/22 17:39, Thomas Zimmermann wrote: Convert fbdev drivers from fbdev's remove_conflicting_framebuffers() to the framework-independent aperture_remove_conflicting_devices(). Calling this function will also remove conflicting DRM drivers. Signed-off-by: Thomas Zimmermann --- [...] static int lynxfb_kick_out_firmware_fb(struct pci_dev *pdev) { - struct apertures_struct *ap; + resource_size_t base = pci_resource_start(pdev, 0); + resource_size_t size = pci_resource_len(pdev, 0); bool primary = false; - ap = alloc_apertures(1); - if (!ap) - return -ENOMEM; - - ap->ranges[0].base = pci_resource_start(pdev, 0); - ap->ranges[0].size = pci_resource_len(pdev, 0); #ifdef CONFIG_X86 primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW; #endif - remove_conflicting_framebuffers(ap, "sm750_fb1", primary); - kfree(ap); - return 0; + + return aperture_remove_conflicting_devices(base, size, primary, "sm750_fb1"); Do you know why this can't just use aperture_remove_conflicting_pci_devices() ? I simply don't want change too much at once, because there was this problem with the PCI helper on ast. At some point we can make a push to really fix this throughout the code base. And that would include an update to fb_is_primary_device(), [1] which doesn't fill well into the new interfaces. Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/arch/x86/video/fbdev.c#L14 It seems that the driver is open coding part of the logic already in that helper. For example, figuring out if is a primary by checking the IORESOURCE_ROM_SHADOW flag in the PCI_ROM_RESOURCE. But also getting the base and size for PCI BAR 0, since the loop in that helper would already take care of that (it also starts at BAR 0). } static int lynxfb_pci_probe(struct pci_dev *pdev, diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c index b311c07fe66d..e5e362b8c9da 100644 --- a/drivers/video/fbdev/aty/radeon_base.c +++ b/drivers/video/fbdev/aty/radeon_base.c @@ -54,6 +54,7 @@ #include "radeonfb.h" +#include #include #include #include @@ -2239,20 +2240,10 @@ static const struct bin_attribute edid2_attr = { static int radeon_kick_out_firmware_fb(struct pci_dev *pdev) { - struct apertures_struct *ap; + resource_size_t base = pci_resource_start(pdev, 0); + resource_size_t size = pci_resource_len(pdev, 0); - ap = alloc_apertures(1); - if (!ap) - return -ENOMEM; - - ap->ranges[0].base = pci_resource_start(pdev, 0); - ap->ranges[0].size = pci_resource_len(pdev, 0); - - remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false); - - kfree(ap); - - return 0; + return aperture_remove_conflicting_devices(base, size, KBUILD_MODNAME, false); Same for this. -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 06/11] fbdev: Remove conflicting devices on PCI bus
Hi Am 11.07.22 um 13:13 schrieb Javier Martinez Canillas: On 7/7/22 17:39, Thomas Zimmermann wrote: Remove firmware devices on the PCI bus, by calling aperture_remove_conflicting_pci_devices() in the probe function of each related fbdev driver. iSo far, most of these drivers did not remove conflicting VESA or EFI devices, or outride failed for resource conflicts (i.e., matroxfb.) This must have been broken for quite some time. Signed-off-by: Thomas Zimmermann --- [...] @@ -949,6 +950,10 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) int rc; u8 regval; + rc = aperture_remove_conflicting_pci_devices(dev, "arkfb"); + if (rc < 0) + return rc; + I wonder if we could think of a trick to avoid open coding the same check in all drivers. Maybe a combination of using KBUILD_MODNAME for the name and a probe callback wrapper or something ? Originally, I tried to hack this into register_framebuffer(), where the removal was originally located. But that's too late as there is conflicting resource access in most driver's probe function. The top of the probe function is the only place where this call can realistically be located. It's the same rule as with DRM drivers: the firmware driver needs to be gone before the native driver touches hardware. Best regards Thomas But probably not worth to invest more in the fbdev drivers and could be done as a follow-up anyways if someone feels like it. Reviewed-by: Javier Martinez Canillas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 05/11] fbdev: Convert drivers to aperture helpers
Hello Thomas, On 7/15/22 13:48, Thomas Zimmermann wrote: [...] >>> + >>> + return aperture_remove_conflicting_devices(base, size, primary, >>> "sm750_fb1"); >> >> Do you know why this can't just use >> aperture_remove_conflicting_pci_devices() ? > > I simply don't want change too much at once, because there was this > problem with the PCI helper on ast. > Makes sense. Feel free to add: Reviewed-by: Javier Martinez Canillas > At some point we can make a push to really fix this throughout the code > base. And that would include an update to fb_is_primary_device(), [1] > which doesn't fill well into the new interfaces. > Agreed. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 10/11] fbdev: Acquire framebuffer apertures for firmware devices
Hi Am 11.07.22 um 13:29 schrieb Javier Martinez Canillas: On 7/7/22 17:39, Thomas Zimmermann wrote: When registering a generic framebuffer, automatically acquire ownership of the framebuffer's I/O range. The device will now be handled by the aperture helpers. Fbdev-based conflict handling is no longer required. Signed-off-by: Thomas Zimmermann --- drivers/video/fbdev/core/fbmem.c | 33 1 file changed, 33 insertions(+) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 2237049327db..e556ad69f48f 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -13,6 +13,7 @@ #include +#include #include #include #include @@ -1739,6 +1740,32 @@ static void do_unregister_framebuffer(struct fb_info *fb_info) put_fb_info(fb_info); } +static int fbm_aperture_acquire_for_platform_device(struct fb_info *fb_info) +{ What's the meaning of 'm' here ? Misc, memory ? I would just call it 'fb_'. 'managed' as in drmm_ But using fb_ is also good. I actually wasn't sure about this naming. Best regards Thomas Reviewed-by: Javier Martinez Canillas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v2 16/29] ACPI: video: Add Nvidia WMI EC brightness control detection
Hi Daniel, On 7/12/22 22:13, Daniel Dadap wrote: > Thanks, Hans: > > On 7/12/22 14:38, Hans de Goede wrote: >> On some new laptop designs a new Nvidia specific WMI interface is present >> which gives info about panel brightness control and may allow controlling >> the brightness through this interface when the embedded controller is used >> for brightness control. >> >> When this WMI interface is present and indicates that the EC is used, >> then this interface should be used for brightness control. >> >> Signed-off-by: Hans de Goede >> --- >> drivers/acpi/Kconfig | 1 + >> drivers/acpi/video_detect.c | 35 ++ >> drivers/gpu/drm/gma500/Kconfig | 2 ++ >> drivers/gpu/drm/i915/Kconfig | 2 ++ >> include/acpi/video.h | 1 + >> 5 files changed, 41 insertions(+) >> >> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig >> index 1e34f846508f..c372385cfc3f 100644 >> --- a/drivers/acpi/Kconfig >> +++ b/drivers/acpi/Kconfig >> @@ -212,6 +212,7 @@ config ACPI_VIDEO >> tristate "Video" >> depends on X86 && BACKLIGHT_CLASS_DEVICE >> depends on INPUT >> + depends on ACPI_WMI >> select THERMAL >> help >> This driver implements the ACPI Extensions For Display Adapters >> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c >> index 8c2863403040..7b89dc9a04a2 100644 >> --- a/drivers/acpi/video_detect.c >> +++ b/drivers/acpi/video_detect.c >> @@ -75,6 +75,35 @@ find_video(acpi_handle handle, u32 lvl, void *context, >> void **rv) >> return AE_OK; >> } >> +#define WMI_BRIGHTNESS_METHOD_SOURCE 2 >> +#define WMI_BRIGHTNESS_MODE_GET 0 >> +#define WMI_BRIGHTNESS_SOURCE_EC 2 >> + >> +struct wmi_brightness_args { >> + u32 mode; >> + u32 val; >> + u32 ret; >> + u32 ignored[3]; >> +}; >> + >> +static bool nvidia_wmi_ec_supported(void) >> +{ >> + struct wmi_brightness_args args = { >> + .mode = WMI_BRIGHTNESS_MODE_GET, >> + .val = 0, >> + .ret = 0, >> + }; >> + struct acpi_buffer buf = { (acpi_size)sizeof(args), &args }; >> + acpi_status status; >> + >> + status = wmi_evaluate_method("603E9613-EF25-4338-A3D0-C46177516DB7", 0, >> + WMI_BRIGHTNESS_METHOD_SOURCE, &buf, &buf); >> + if (ACPI_FAILURE(status)) >> + return false; >> + >> + return args.ret == WMI_BRIGHTNESS_SOURCE_EC; >> +} >> + > > > The code duplication here with nvidia-wmi-ec-backlight.c is a little > unfortunate. Can we move the constants, struct definition, and WMI GUID from > that file to a header file that's used both by the EC backlight driver and > the ACPI video driver? Yes that is a good idea. I suggest using include/linux/platform_data/x86/nvidia-wmi-ec-backlight.h to move the shared definitions there. If you can submit 2 patches on top of this series: 1. Moving the definitions from drivers/platform/x86/nvidia-wmi-ec-backlight.c to include/linux/platform_data/x86/nvidia-wmi-ec-backlight.h 2. Switching the code from this patch over to using the new nvidia-wmi-ec-backlight.h Then for the next version I'll add patch 1. to the series and squash patch 2. into this one. > I was thinking it might be nice to add a wrapper around > wmi_brightness_notify() in nvidia-wmi-ec-backlight.c that does this source == > WMI_BRIGHTNESS_SOURCE_EC test, and then export it so that it can be called > both here and in the EC backlight driver's probe routine, but then I guess > that would make video.ko depend on nvidia-wmi-ec-backlight.ko, which seems > wrong. It also seems wrong to implement the WMI plumbing in the ACPI video > driver, and export it so that the EC backlight driver can use it, so I guess > I can live with the duplication of the relatively simple WMI stuff here, it > would just be nice to not have to define all of the API constants, structure, > and GUID twice. Agreed. > > >> /* Force to use vendor driver when the ACPI device is known to be >> * buggy */ >> static int video_detect_force_vendor(const struct dmi_system_id *d) >> @@ -518,6 +547,7 @@ static const struct dmi_system_id >> video_detect_dmi_table[] = { >> static enum acpi_backlight_type __acpi_video_get_backlight_type(bool >> native) >> { >> static DEFINE_MUTEX(init_mutex); >> + static bool nvidia_wmi_ec_present; >> static bool native_available; >> static bool init_done; >> static long video_caps; >> @@ -530,6 +560,7 @@ static enum acpi_backlight_type >> __acpi_video_get_backlight_type(bool native) >> acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, >> ACPI_UINT32_MAX, find_video, NULL, >> &video_caps, NULL); >> + nvidia_wmi_ec_present = nvidia_wmi_ec_supported(); >> init_done = true; >> } >> if (native) >> @@ -547,6 +578,10 @@ static enum acpi_backlight_type >> __acpi_video_get_backlight_type(bool
Re: [PATCH v2 20/29] platform/x86: acer-wmi: Move backlight DMI quirks to acpi/video_detect.c
Hi, On 7/12/22 22:24, Daniel Dadap wrote: > I'll ask around to see if there's some DMI property we can match in order to > detect whether a system is expected to use the EC backlight driver: if so, > maybe we can avoid the WMI interactions in patch 16/29 of this series. > Although I suppose even if there were a DMI property, we'd still need to call > the WMI-wrapped ACPI method to check whether the system is currently > configured to drive the backlight through the EC, unless the system somehow > exports a different DMI table depending on the current backlight control > configuration, which I imagine to be unlikely. IMHO the duplication is fine, it is also important that the video_detect.c code and the actual backlight driver use the same detection mechanism where possible. Otherwise acpi_video_get_backlight_type() may return acpi_backlight_nvidia_wmi_ec while the EC backlight driver refuses to load... Regards, Hans > > This change looks fine to me, although I suppose somebody who maintains the > acer-wmi driver should comment. The bugzilla links are a nice touch. > > On 7/12/22 14:39, Hans de Goede wrote: >> Move the backlight DMI quirks to acpi/video_detect.c, so that >> the driver no longer needs to call acpi_video_set_dmi_backlight_type(). >> >> acpi_video_set_dmi_backlight_type() is troublesome because it may end up >> getting called after other backlight drivers have already called >> acpi_video_get_backlight_type() resulting in the other drivers >> already being registered even though they should not. >> >> Note that even though the DMI quirk table name was video_vendor_dmi_table, >> 5/6 quirks were actually quirks to use the GPU native backlight. >> >> These 5 quirks also had a callback in their dmi_system_id entry which >> disabled the acer-wmi vendor driver; and any DMI match resulted in: >> >> acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); >> >> which disabled the acpi_video driver, so only the native driver was left. >> The new entries for these 5/6 devices correctly marks these as needing >> the native backlight driver. >> >> Also note that other changes in this series change the native backlight >> drivers to no longer unconditionally register their backlight. Instead >> these drivers now do this check: >> >> if (acpi_video_get_backlight_type(false) != acpi_backlight_native) >> return 0; /* bail */ >> >> which without this patch would have broken these 5/6 "special" quirks. >> >> Since I had to look at all the commits adding the quirks anyways, to make >> sure that I understood the code correctly, I've also added links to >> the various original bugzillas for these quirks to the new entries. >> >> Signed-off-by: Hans de Goede >> --- >> drivers/acpi/video_detect.c | 53 ++ >> drivers/platform/x86/acer-wmi.c | 66 - >> 2 files changed, 53 insertions(+), 66 deletions(-) >> >> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c >> index a514adaec14d..cd51cb0d7821 100644 >> --- a/drivers/acpi/video_detect.c >> +++ b/drivers/acpi/video_detect.c >> @@ -147,6 +147,15 @@ static const struct dmi_system_id >> video_detect_dmi_table[] = { >> DMI_MATCH(DMI_BOARD_NAME, "X360"), >> }, >> }, >> + { >> + /* https://bugzilla.redhat.com/show_bug.cgi?id=1128309 */ >> + .callback = video_detect_force_vendor, >> + /* Acer KAV80 */ >> + .matches = { >> + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "KAV80"), >> + }, >> + }, >> { >> .callback = video_detect_force_vendor, >> /* Asus UL30VT */ >> @@ -427,6 +436,41 @@ static const struct dmi_system_id >> video_detect_dmi_table[] = { >> DMI_MATCH(DMI_BOARD_NAME, "JV50"), >> }, >> }, >> + { >> + /* https://bugzilla.redhat.com/show_bug.cgi?id=1012674 */ >> + .callback = video_detect_force_native, >> + /* Acer Aspire 5741 */ >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "Acer"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5741"), >> + }, >> + }, >> + { >> + /* https://bugzilla.kernel.org/show_bug.cgi?id=42993 */ >> + .callback = video_detect_force_native, >> + /* Acer Aspire 5750 */ >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "Acer"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5750"), >> + }, >> + }, >> + { >> + /* https://bugzilla.kernel.org/show_bug.cgi?id=42833 */ >> + .callback = video_detect_force_native, >> + /* Acer Extensa 5235 */ >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "Acer"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "Extensa 5235"), >> + }, >> + }, >> + { >> + .callback = video_detect_force_native, >> + /* Acer TravelMate 4750 */ >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "Acer"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 4