Re: [PATCH] kernel.h: Add non_block_start/end()
On Tue, May 21, 2019 at 11:06 PM Tetsuo Handa wrote: > > On 2019/05/21 23:44, Daniel Vetter wrote: > OOM notifiers should not depend on any locks or sleepable conditionals. > If some lock directly or indirectly depended on __GFP_DIRECT_RECLAIM, > it will deadlock. Thus, despite blocking API, this should effectively be > non-blocking. All OOM notifier users except i915 seems to be atomic, but > I can't evaluate i915 part... > >>> > >>> Read again what I've written, please > >>> > >> > >> Question to Daniel: Is i915's oom_notifier function atomic? > > > > It's supposed to not block too much at least, I don't think it's entirely > > atomic. Waking up the device (which we need to write some of the ptes) > > will take some time and I think acquires a few mutexes, but not 100% sure. > > > > If you want to see, send a patch to intel-gfx m-l and CI will pick it up > > and test with our farm of machines. > > As soon as a mutex is held, we can't expect it is atomic. We need to > manually inspect whether there is __GFP_DIRECT_RECLAIM dependency... > > Since OOM notifier will be called after shrinkers are attempted, > can i915 move from OOM notifier to shrinker? We also have a shrinker. The trouble is a bit that locking design in i915 is still not great (it's a lot better than it's bit), and iirc that's why we had the oom fallback. It unconditionally throws out a bunch of things we can do with less locking. Maybe we could stuff that into the shrinker now. Adding Chris. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] kernel.h: Add non_block_start/end()
Quoting Michal Hocko (2019-05-22 07:34:42) > On Wed 22-05-19 06:06:31, Tetsuo Handa wrote: > [...] > > Since OOM notifier will be called after shrinkers are attempted, > > can i915 move from OOM notifier to shrinker? > > That would be indeed preferable. OOM notifier is an API from hell. We were^W are still trying to make the shrinker nonblocking to avoid incurring horrible latencies for light direct reclaim. The consequence of avoiding heavy work in the shrinker is that we moved it to the oom notifier as being the last chance we have to return all (can be literally all) the system memory. The alternative to using a separate oom notifier would be more reclaim/shrinker phases? -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] dma-buf: start caching of sg_table objects v2
On Thu, May 16, 2019 at 08:26:36AM +0200, Daniel Vetter wrote: > On Wed, May 15, 2019 at 11:25 AM Christian König > wrote: > > > > Am 15.05.19 um 10:58 schrieb Daniel Vetter: > > > On Tue, May 14, 2019 at 04:05:14PM +0200, Christian König wrote: > > >> To allow a smooth transition from pinning buffer objects to dynamic > > >> invalidation we first start to cache the sg_table for an attachment. > > >> > > >> v2: keep closer to the DRM implementation > > >> > > >> Signed-off-by: Christian König > > > i915 doesn't use the prime helpers, so this would be a change for us. > > > > Not a problem at all, see below. I've added an cache_sgt_mapping member > > into the dma_buf_ops structure. Only when this member is true the > > functionality is enabled. > > Hm right I missed that. > > > So I'm just moving code around in those two patches and no functional > > change at all. > > Still not sure. I'll look at your latest dma-buf series to see how it > all fits together. Ok I'm still procrastinating on that one, but I guess either way we need some caching or another in dma-buf.c, and this series here at least looks like it's moving in that direction. On both patches Reviewed-by: Daniel Vetter btw might be good to throw this at intel-gfx-trybot too, just to see whether anything is amiss. I think we've beaten down all the -rc1 regressions now. -Daniel > -Daniel > > > > > Regards, > > Christian. > > > > > Not > > > sure that matters. Since we can't use this as an easy way out of the > > > locking problem around reservation_object I'm not sure how useful it is to > > > move this ahead. At least until we have a clearer picture on how we plan > > > to roll out the dynamic dma-buf stuff. Once we have a bit more clarity > > > there I'm all for merging prep work, but as-is I'd be cautious ... This > > > stuff is a lot more tricky than it seems at first :-/ > > > -Daniel > > >> --- > > >> drivers/dma-buf/dma-buf.c | 27 +-- > > >> include/linux/dma-buf.h | 13 + > > >> 2 files changed, 38 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > > >> index 7c858020d14b..a2d34c6b80a5 100644 > > >> --- a/drivers/dma-buf/dma-buf.c > > >> +++ b/drivers/dma-buf/dma-buf.c > > >> @@ -573,6 +573,7 @@ struct dma_buf_attachment *dma_buf_attach(struct > > >> dma_buf *dmabuf, > > >> list_add(&attach->node, &dmabuf->attachments); > > >> > > >> mutex_unlock(&dmabuf->lock); > > >> + > > >> return attach; > > >> > > >> err_attach: > > >> @@ -595,6 +596,9 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct > > >> dma_buf_attachment *attach) > > >> if (WARN_ON(!dmabuf || !attach)) > > >> return; > > >> > > >> +if (attach->sgt) > > >> +dmabuf->ops->unmap_dma_buf(attach, attach->sgt, > > >> attach->dir); > > >> + > > >> mutex_lock(&dmabuf->lock); > > >> list_del(&attach->node); > > >> if (dmabuf->ops->detach) > > >> @@ -630,10 +634,27 @@ struct sg_table *dma_buf_map_attachment(struct > > >> dma_buf_attachment *attach, > > >> if (WARN_ON(!attach || !attach->dmabuf)) > > >> return ERR_PTR(-EINVAL); > > >> > > >> +if (attach->sgt) { > > >> +/* > > >> + * Two mappings with different directions for the same > > >> + * attachment are not allowed. > > >> + */ > > >> +if (attach->dir != direction && > > >> +attach->dir != DMA_BIDIRECTIONAL) > > >> +return ERR_PTR(-EBUSY); > > >> + > > >> +return attach->sgt; > > >> +} > > >> + > > >> sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); > > >> if (!sg_table) > > >> sg_table = ERR_PTR(-ENOMEM); > > >> > > >> +if (!IS_ERR(sg_table) && attach->dmabuf->ops->cache_sgt_mapping) { > > >> +attach->sgt = sg_table; > > >> +attach->dir = direction; > > >> +} > > >> + > > >> return sg_table; > > >> } > > >> EXPORT_SYMBOL_GPL(dma_buf_map_attachment); > > >> @@ -657,8 +678,10 @@ void dma_buf_unmap_attachment(struct > > >> dma_buf_attachment *attach, > > >> if (WARN_ON(!attach || !attach->dmabuf || !sg_table)) > > >> return; > > >> > > >> -attach->dmabuf->ops->unmap_dma_buf(attach, sg_table, > > >> -direction); > > >> +if (attach->sgt == sg_table) > > >> +return; > > >> + > > >> +attach->dmabuf->ops->unmap_dma_buf(attach, sg_table, direction); > > >> } > > >> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment); > > >> > > >> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > > >> index 58725f890b5b..45b9767e67ec 100644 > > >> --- a/include/linux/dma-buf.h > > >> +++ b/include/linux/dma-buf.h > > >> @@ -51,6 +51,15 @@ struct dma_buf_attachment; > > >>* @vunmap: [optional] unmaps a vmap from the buffer > > >>*/ > > >> struct dma_buf_op
Re: [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v8
On Wed, May 15, 2019 at 04:38:26PM +0200, Christian König wrote: > On the exporter side we add optional explicit pinning callbacks. If those > callbacks are implemented the framework no longer caches sg tables and the > map/unmap callbacks are always called with the lock of the reservation object > held. > > On the importer side we add an optional invalidate callback. This callback is > used by the exporter to inform the importers that their mappings should be > destroyed as soon as possible. > > This allows the exporter to provide the mappings without the need to pin > the backing store. > > v2: don't try to invalidate mappings when the callback is NULL, > lock the reservation obj while using the attachments, > add helper to set the callback > v3: move flag for invalidation support into the DMA-buf, > use new attach_info structure to set the callback > v4: use importer_priv field instead of mangling exporter priv. > v5: drop invalidation_supported flag > v6: squash together with pin/unpin changes > v7: pin/unpin takes an attachment now > v8: nuke dma_buf_attachment_(map|unmap)_locked, > everything is now handled backward compatible > > Signed-off-by: Christian König Sorry for taking so long, I'm kinda starting to dread this series a bit so engaged into some good old fashioned procrastinating :-/ > --- > drivers/dma-buf/dma-buf.c | 157 -- > include/linux/dma-buf.h | 109 -- > 2 files changed, 253 insertions(+), 13 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index a2d34c6b80a5..85026d9e978d 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -409,6 +409,9 @@ struct dma_buf *dma_buf_export(const struct > dma_buf_export_info *exp_info) > return ERR_PTR(-EINVAL); > } > > + if (WARN_ON(exp_info->ops->cache_sgt_mapping && exp_info->ops->pin)) > + return ERR_PTR(-EINVAL); > + > if (!try_module_get(exp_info->owner)) > return ERR_PTR(-ENOENT); > > @@ -530,10 +533,12 @@ void dma_buf_put(struct dma_buf *dmabuf) > EXPORT_SYMBOL_GPL(dma_buf_put); > > /** > - * dma_buf_attach - Add the device to dma_buf's attachments list; optionally, > + * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list; > optionally, > * calls attach() of dma_buf_ops to allow device-specific attach > functionality > - * @dmabuf: [in]buffer to attach device to. > - * @dev: [in]device to be attached. > + * @dmabuf: [in]buffer to attach device to. > + * @dev: [in]device to be attached. > + * @importer_ops [in]importer operations for the attachment > + * @importer_priv[in]importer private pointer for the attachment > * > * Returns struct dma_buf_attachment pointer for this attachment. Attachments > * must be cleaned up by calling dma_buf_detach(). > @@ -547,8 +552,10 @@ EXPORT_SYMBOL_GPL(dma_buf_put); > * accessible to @dev, and cannot be moved to a more suitable place. This is > * indicated with the error code -EBUSY. > */ > -struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, > - struct device *dev) > +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) > { > struct dma_buf_attachment *attach; > int ret; > @@ -562,6 +569,8 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf > *dmabuf, > > attach->dev = dev; > attach->dmabuf = dmabuf; > + attach->importer_ops = importer_ops; > + attach->importer_priv = importer_priv; > > mutex_lock(&dmabuf->lock); > > @@ -570,10 +579,31 @@ struct dma_buf_attachment *dma_buf_attach(struct > dma_buf *dmabuf, > if (ret) > goto err_attach; > } > + reservation_object_lock(dmabuf->resv, NULL); > list_add(&attach->node, &dmabuf->attachments); > + reservation_object_unlock(dmabuf->resv); > > mutex_unlock(&dmabuf->lock); > Just this functional comment, since I think api detail polishing is premature if we're not yet aware of how this works. > + /* When the importer is dynamic but the exporter isn't we need to cache > + * the mapping or otherwise would run into issues with the reservation > + * object lock. > + */ > + if (dma_buf_attachment_is_dynamic(attach) && > + !dma_buf_is_dynamic(dmabuf)) { Isn't this the wrong way round? dynamic importers should be perfectly fine with the reservation locks in their map/unmap paths, it's importers calling exporters there. The real problem is a not-dynamic importer, which hasn't be adjusted to allow the reservation lock in their paths where they map/unmap a buffer, with a dynamic exporter. That's where we need to cache
[Bug 109206] Kernel 4.20 amdgpu fails to load firmware on Ryzen 2500U
https://bugs.freedesktop.org/show_bug.cgi?id=109206 --- Comment #44 from Ondrej Lang --- Created attachment 144316 --> https://bugs.freedesktop.org/attachment.cgi?id=144316&action=edit Kernel log 5.1.3 showing amdgpu drm crash Experiencing the same problem. On boot with any kernel > 4.20, the graphics is not initialized, few scrambled lines appear at the bottom of the screen and then the screen goes blank. The system actually boots as when I entered my credentials (on the black screen) and did "cat dmesg > dmesg.txt", when I rebooted with kernel 4.19 the file was there (I have attached it to this thread). The relevant portion of the dmesg log is: [5.133929] [drm] REG_WAIT timeout 1us * 10 tries - mpc1_assert_idle_mpcc line:103 [5.134034] WARNING: CPU: 2 PID: 367 at drivers/gpu/drm/amd/amdgpu/../display/dc/dc_helper.c:277 generic_reg_wait.cold.0+0x29/0x30 [amdgpu] ... [5.134925] drm_dev_register+0x111/0x150 [drm] [5.135335] [drm] Display Core initialized with v3.2.17! [5.642999] [drm:hwss_edp_wait_for_hpd_ready [amdgpu]] *ERROR* hwss_edp_wait_for_hpd_ready: wait timed out! [6.142512] [drm:hwss_edp_wait_for_hpd_ready [amdgpu]] *ERROR* hwss_edp_wait_for_hpd_ready: wait timed out! [6.143119] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [6.143121] [drm] Driver supports precise vblank timestamp query. [6.158613] [drm] VCN decode and encode initialized successfully(under SPG Mode). [6.161822] [drm] Cannot find any crtc or sizes [6.188933] [drm] Initialized amdgpu 3.30.0 20150101 for :04:00.0 on minor 0 I can also confirm that as a workaround, removing/moving file /lib/firmware/amdgpu/raven_dmcu.bin and regenerating the initramfs ("mkinitcpio -p linux" on Arch linux) allows the 5.1.3 kernel to boot normally (I can also start an X session and everything seems to be fine) and there are no drm errors in dmesg anymore. System: HP ENVY x360 Convertible 15-bq1xx/83C6, BIOS F.20 12/25/2018 Kernel: 5.1.3-arch1-1-ARCH Grub kernel parameters: amd_iommu=on iommu=pt idle=nomwait If you need anything else let me know -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v8
Am 22.05.19 um 10:19 schrieb Daniel Vetter: On Wed, May 15, 2019 at 04:38:26PM +0200, Christian König wrote: [SNAP] Just this functional comment, since I think api detail polishing is premature if we're not yet aware of how this works. + /* When the importer is dynamic but the exporter isn't we need to cache +* the mapping or otherwise would run into issues with the reservation +* object lock. +*/ + if (dma_buf_attachment_is_dynamic(attach) && + !dma_buf_is_dynamic(dmabuf)) { Isn't this the wrong way round? dynamic importers should be perfectly fine with the reservation locks in their map/unmap paths, it's importers calling exporters there. The real problem is a not-dynamic importer, which hasn't be adjusted to allow the reservation lock in their paths where they map/unmap a buffer, with a dynamic exporter. That's where we need to cache the mapping to avoid the deadlock (or having to change everyone) Well could be that this is also a problem, but I actually don't think so. The case I'm describing here certainly is the more obvious problem because the importer is already holding the lock the exporter wants to take. On the other hand we could rather easily change that check to dma_buf_attachment_is_dynamic() != dma_buf_is_dynamic() if that is indeed a problem. + struct sg_table *sgt; + + sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL); And unfortunately the non-dynamic, i.e. legacy/current code importer is also the one which uses other flags than DMA_BIDIRECTIONAL. At least on ARM, and that's the only place where this matters because there the dma api might do cache flushing. Well the only implementer for now is amdgpu, and amdgpu always requires a coherent bidirectional mapping. So this won't be a problem unless the ARM drivers start to implement dynamic DMA-buf handling themselves or start to talk to amdgpu (which wouldn't have worked before anyway). Christian. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] dt-bindings: gpu: add #cooling-cells property to the ARM Mali Midgard GPU binding
Am Donnerstag, 16. Mai 2019, 19:25:08 CEST schrieb Matthias Kaehlcke: > The GPU can be used as a thermal cooling device, add an optional > '#cooling-cells' property. > > Signed-off-by: Matthias Kaehlcke applied to drm-misc-next for 5.3
Re: [PATCH v2 2/3] ARM: dts: rockchip: Add #cooling-cells entry for rk3288 GPU
Am Donnerstag, 16. Mai 2019, 19:25:09 CEST schrieb Matthias Kaehlcke: > The Mali GPU of the rk3288 can be used as cooling device, add > a #cooling-cells entry for it. > > Signed-off-by: Matthias Kaehlcke > Reviewed-by: Douglas Anderson applied patches 2+3 for 5.3 Thanks Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] enable syncobj test depending on capability
Feature is controlled by DRM_CAP_SYNCOBJ_TIMELINE drm capability. Signed-off-by: Chunming Zhou Reviewed-by: Christian König --- tests/amdgpu/syncobj_tests.c | 8 1 file changed, 8 insertions(+) diff --git a/tests/amdgpu/syncobj_tests.c b/tests/amdgpu/syncobj_tests.c index a0c627d7..869ed88e 100644 --- a/tests/amdgpu/syncobj_tests.c +++ b/tests/amdgpu/syncobj_tests.c @@ -22,6 +22,7 @@ */ #include "CUnit/Basic.h" +#include "xf86drm.h" #include "amdgpu_test.h" #include "amdgpu_drm.h" @@ -36,6 +37,13 @@ static void amdgpu_syncobj_timeline_test(void); CU_BOOL suite_syncobj_timeline_tests_enable(void) { + int r; + uint64_t cap = 0; + + r = drmGetCap(drm_amdgpu[0], DRM_CAP_SYNCOBJ_TIMELINE, &cap); + if (r || cap == 0) + return CU_FALSE; + return CU_TRUE; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] update drm.h
a) delta: only DRM_CAP_SYNCOBJ_TIMELINE b) Generated using make headers_install. c) Generated from origin/drm-misc-next commit 982c0500fd1a8012c31d3c9dd8de285129904656" Signed-off-by: Chunming Zhou Suggested-by: Michel Dänzer --- include/drm/drm.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/drm/drm.h b/include/drm/drm.h index c893f3b4..438abde3 100644 --- a/include/drm/drm.h +++ b/include/drm/drm.h @@ -44,6 +44,7 @@ typedef unsigned int drm_handle_t; #else /* One of the BSDs */ +#include #include #include typedef int8_t __s8; @@ -643,6 +644,7 @@ struct drm_gem_open { #define DRM_CAP_PAGE_FLIP_TARGET 0x11 #define DRM_CAP_CRTC_IN_VBLANK_EVENT 0x12 #define DRM_CAP_SYNCOBJ0x13 +#define DRM_CAP_SYNCOBJ_TIMELINE 0x14 /** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap { -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/etnaviv: lock MMU while dumping core
The devcoredump needs to operate on a stable state of the MMU while it is writing the MMU state to the coredump. The missing lock allowed both the userspace submit, as well as the GPU job finish paths to mutate the MMU state while a coredump is under way. Fixes: a8c21a5451d8 (drm/etnaviv: add initial etnaviv DRM driver) Reported-by: David Jander Signed-off-by: Lucas Stach Tested-by: David Jander --- drivers/gpu/drm/etnaviv/etnaviv_dump.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c b/drivers/gpu/drm/etnaviv/etnaviv_dump.c index 33854c94cb85..515515ef24f9 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c @@ -125,6 +125,8 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) return; etnaviv_dump_core = false; + mutex_lock(&gpu->mmu->lock); + mmu_size = etnaviv_iommu_dump_size(gpu->mmu); /* We always dump registers, mmu, ring and end marker */ @@ -167,6 +169,7 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY, PAGE_KERNEL); if (!iter.start) { + mutex_unlock(&gpu->mmu->lock); dev_warn(gpu->dev, "failed to allocate devcoredump file\n"); return; } @@ -234,6 +237,8 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) obj->base.size); } + mutex_unlock(&gpu->mmu->lock); + etnaviv_core_dump_header(&iter, ETDUMP_BUF_END, iter.data); dev_coredumpv(gpu->dev, iter.start, iter.data - iter.start, GFP_KERNEL); -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: lock MMU while dumping core
Hi Lucas, Seems I'm not getting a reply, so I'm hijacking one of your more recent patches in the hope of attracting your attention. A while back I sent a fix for a regression that recently occurred with etnaviv, where the kernel spat out a warning when importing buffers into etnaviv. You apparently merged this as "development", queuing it up for the last merge window. Since it is a regression (although not directly attributable to etnaviv), please ensure that it is merged into stable kernels. Thanks. On Wed, May 22, 2019 at 11:55:14AM +0200, Lucas Stach wrote: > The devcoredump needs to operate on a stable state of the MMU while > it is writing the MMU state to the coredump. The missing lock > allowed both the userspace submit, as well as the GPU job finish > paths to mutate the MMU state while a coredump is under way. > > Fixes: a8c21a5451d8 (drm/etnaviv: add initial etnaviv DRM driver) > Reported-by: David Jander > Signed-off-by: Lucas Stach > Tested-by: David Jander > --- > drivers/gpu/drm/etnaviv/etnaviv_dump.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c > b/drivers/gpu/drm/etnaviv/etnaviv_dump.c > index 33854c94cb85..515515ef24f9 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c > @@ -125,6 +125,8 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) > return; > etnaviv_dump_core = false; > > + mutex_lock(&gpu->mmu->lock); > + > mmu_size = etnaviv_iommu_dump_size(gpu->mmu); > > /* We always dump registers, mmu, ring and end marker */ > @@ -167,6 +169,7 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) > iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN | > __GFP_NORETRY, > PAGE_KERNEL); > if (!iter.start) { > + mutex_unlock(&gpu->mmu->lock); > dev_warn(gpu->dev, "failed to allocate devcoredump file\n"); > return; > } > @@ -234,6 +237,8 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu) >obj->base.size); > } > > + mutex_unlock(&gpu->mmu->lock); > + > etnaviv_core_dump_header(&iter, ETDUMP_BUF_END, iter.data); > > dev_coredumpv(gpu->dev, iter.start, iter.data - iter.start, GFP_KERNEL); > -- > 2.20.1 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbcon: Remove fbcon_has_exited
Hi Daniel, On 5/21/19 4:23 PM, Daniel Vetter wrote: > This is unused code since > > commit 6104c37094e729f3d4ce65797002112735d49cd1 > Author: Daniel Vetter > Date: Tue Aug 1 17:32:07 2017 +0200 > > fbcon: Make fbcon a built-time depency for fbdev > > when fbcon was made a compile-time static dependency of fbdev. We > can't exit fbcon anymore without exiting fbdev first, which only works > if all fbdev drivers have unloaded already. Hence this is all dead > code. > > v2: I missed that fbcon_exit is also called from con_deinit stuff, and > there fbcon_has_exited prevents double-cleanup. But we can fix that > by properly resetting con2fb_map[] to all -1, which is used everywhere > else to indicate "no fb_info allocate to this console". With that > change the double-cleanup (which resulted in a module refcount underflow, > among other things) is prevented. > > Aside: con2fb_map is a signed char, so don't register more than 128 fb_info > or hilarity will ensue. > > v3: CI showed me that I still didn't fully understand what's going on > here. The leaked references in con2fb_map have been used upon > rebinding the fb console in fbcon_init. It worked because fbdev > unregistering still cleaned out con2fb_map, and reset it to info_idx. > If the last fbdev driver unregistered, then it also reset info_idx, > and unregistered the fbcon driver. > > Imo that's all a bit fragile, so let's keep the con2fb_map reset to > -1, and in fbcon_init pick info_idx if we're starting fresh. That > means unbinding and rebinding will cleanse the mapping, but why are > you doing that if you want to retain the mapping, so should be fine. > > Also, I think info_idx == -1 is impossible in fbcon_init - we > unregister the fbcon in that case. So catch&warn about that. > > Signed-off-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Bartlomiej Zolnierkiewicz > Cc: Daniel Vetter > Cc: Hans de Goede > Cc: "Noralf Trønnes" > Cc: Yisheng Xie > Cc: Konstantin Khorenko > Cc: Prarit Bhargava > Cc: Kees Cook > --- > drivers/video/fbdev/core/fbcon.c | 39 +--- > 1 file changed, 6 insertions(+), 33 deletions(-) This patch was #09/33 in your patch series, now it is independent change. Do you want me to apply it now or should I wait for the new version of the whole series? [ I looked at all patches in the series and they look fine to me. After outstanding issues are fixed I'll be happy to apply them all to fbdev-for-next (I can create immutable branch if needed). ] Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: etnaviv: avoid DMA API warning when importing buffers
Hi Russell, Am Samstag, den 18.05.2019, 22:37 +0100 schrieb Russell King - ARM Linux admin: > On Sat, May 18, 2019 at 06:04:42PM -0300, Fabio Estevam wrote: > > Hi Russell, > > > > On Sat, May 18, 2019 at 2:51 PM Russell King - ARM Linux admin > > wrote: > > > > > > Ping. > > > > This patch is present in Lucas' pull request: > > https://lists.freedesktop.org/archives/etnaviv/2019-May/002490.html > > I'm wondering why it didn't make 5.1 since it's a regression. I didn't see the importance to put this into fixes, as it's getting rid of a warning which will only be present when a debug option is enabled. So it should be invisible to most users and it doesn't regress functionality. Regards, Lucas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: etnaviv: avoid DMA API warning when importing buffers
On Wed, May 22, 2019 at 12:04:27PM +0200, Lucas Stach wrote: > Hi Russell, > > Am Samstag, den 18.05.2019, 22:37 +0100 schrieb Russell King - ARM > Linux admin: > > On Sat, May 18, 2019 at 06:04:42PM -0300, Fabio Estevam wrote: > > > Hi Russell, > > > > > > On Sat, May 18, 2019 at 2:51 PM Russell King - ARM Linux admin > > > wrote: > > > > > > > > Ping. > > > > > > This patch is present in Lucas' pull request: > > > https://lists.freedesktop.org/archives/etnaviv/2019-May/002490.html > > > > I'm wondering why it didn't make 5.1 since it's a regression. > > I didn't see the importance to put this into fixes, as it's getting rid > of a warning which will only be present when a debug option is enabled. > So it should be invisible to most users and it doesn't regress > functionality. That depends on your point of view, how you use the kernel, and what you're using the kernel for. If you're trying to use dma debugging (which should always be enabled when doing driver development) and you have a subsystem that keeps triggering it, then it is a serious problem. Given that we want developers to have such options on, having false complaints is counter-productive. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] video: fbdev: pvr2fb: remove function prototypes
Reorder code a bit and then remove no longer needed function prototypes. Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/video/fbdev/pvr2fb.c | 141 ++- 1 file changed, 61 insertions(+), 80 deletions(-) Index: b/drivers/video/fbdev/pvr2fb.c === --- a/drivers/video/fbdev/pvr2fb.c +++ b/drivers/video/fbdev/pvr2fb.c @@ -193,39 +193,6 @@ static unsigned int shdma = PVR2_CASCADE static unsigned int pvr2dma = ONCHIP_NR_DMA_CHANNELS; #endif -static int pvr2fb_setcolreg(unsigned int regno, unsigned int red, unsigned int green, unsigned int blue, -unsigned int transp, struct fb_info *info); -static int pvr2fb_blank(int blank, struct fb_info *info); -static unsigned long get_line_length(int xres_virtual, int bpp); -static void set_color_bitfields(struct fb_var_screeninfo *var); -static int pvr2fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info); -static int pvr2fb_set_par(struct fb_info *info); -static void pvr2_update_display(struct fb_info *info); -static void pvr2_init_display(struct fb_info *info); -static void pvr2_do_blank(void); -static irqreturn_t pvr2fb_interrupt(int irq, void *dev_id); -static int pvr2_init_cable(void); -static int pvr2_get_param(const struct pvr2_params *p, const char *s, -int val, int size); -#ifdef CONFIG_PVR2_DMA -static ssize_t pvr2fb_write(struct fb_info *info, const char *buf, - size_t count, loff_t *ppos); -#endif - -static struct fb_ops pvr2fb_ops = { - .owner = THIS_MODULE, - .fb_setcolreg = pvr2fb_setcolreg, - .fb_blank = pvr2fb_blank, - .fb_check_var = pvr2fb_check_var, - .fb_set_par = pvr2fb_set_par, -#ifdef CONFIG_PVR2_DMA - .fb_write = pvr2fb_write, -#endif - .fb_fillrect= cfb_fillrect, - .fb_copyarea= cfb_copyarea, - .fb_imageblit = cfb_imageblit, -}; - static struct fb_videomode pvr2_modedb[] = { /* * Broadcast video modes (PAL and NTSC). I'm unfamiliar with @@ -353,6 +320,36 @@ static int pvr2fb_setcolreg(unsigned int return 0; } +/* + * Determine the cable type and initialize the cable output format. Don't do + * anything if the cable type has been overidden (via "cable:XX"). + */ + +#define PCTRA 0xff80002c +#define PDTRA 0xff800030 +#define VOUTC 0xa0702c00 + +static int pvr2_init_cable(void) +{ + if (cable_type < 0) { + fb_writel((fb_readl(PCTRA) & 0xfff0) | 0x000a, + PCTRA); + cable_type = (fb_readw(PDTRA) >> 8) & 3; + } + + /* Now select the output format (either composite or other) */ + /* XXX: Save the previous val first, as this reg is also AICA + related */ + if (cable_type == CT_COMPOSITE) + fb_writel(3 << 8, VOUTC); + else if (cable_type == CT_RGB) + fb_writel(1 << 9, VOUTC); + else + fb_writel(0, VOUTC); + + return cable_type; +} + static int pvr2fb_set_par(struct fb_info *info) { struct pvr2fb_par *par = (struct pvr2fb_par *)info->par; @@ -641,36 +638,6 @@ static irqreturn_t pvr2fb_interrupt(int return IRQ_HANDLED; } -/* - * Determine the cable type and initialize the cable output format. Don't do - * anything if the cable type has been overidden (via "cable:XX"). - */ - -#define PCTRA 0xff80002c -#define PDTRA 0xff800030 -#define VOUTC 0xa0702c00 - -static int pvr2_init_cable(void) -{ - if (cable_type < 0) { - fb_writel((fb_readl(PCTRA) & 0xfff0) | 0x000a, - PCTRA); - cable_type = (fb_readw(PDTRA) >> 8) & 3; - } - - /* Now select the output format (either composite or other) */ - /* XXX: Save the previous val first, as this reg is also AICA - related */ - if (cable_type == CT_COMPOSITE) - fb_writel(3 << 8, VOUTC); - else if (cable_type == CT_RGB) - fb_writel(1 << 9, VOUTC); - else - fb_writel(0, VOUTC); - - return cable_type; -} - #ifdef CONFIG_PVR2_DMA static ssize_t pvr2fb_write(struct fb_info *info, const char *buf, size_t count, loff_t *ppos) @@ -741,6 +708,37 @@ out_unmap: } #endif /* CONFIG_PVR2_DMA */ +static struct fb_ops pvr2fb_ops = { + .owner = THIS_MODULE, + .fb_setcolreg = pvr2fb_setcolreg, + .fb_blank = pvr2fb_blank, + .fb_check_var = pvr2fb_check_var, + .fb_set_par = pvr2fb_set_par, +#ifdef CONFIG_PVR2_DMA + .fb_write = pvr2fb_write, +#endif + .fb_fillrect= cfb_fillrect, + .fb_copyarea= cfb_copyarea, + .fb_imageblit = cfb_imageblit, +}; + +static int pvr2_get_param(const struct pvr2_params *p, const char *s, int val, + int size) +{ +
[PATCH 2/2] video: fbdev: pvr2fb: add COMPILE_TEST support
Add COMPILE_TEST support to pvr2fb driver for better compile testing coverage. While at it mark pvr2fb_interrupt() and pvr2fb_common_init() with __maybe_unused tag (to silence build warnings when !SH_DREAMCAST), also convert mmio_base in struct pvr2fb_par to 'void __iomem *' from 'unsigned long' (needed to silence build warnings on ARM). Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/video/fbdev/Kconfig |3 ++- drivers/video/fbdev/pvr2fb.c | 26 +- 2 files changed, 15 insertions(+), 14 deletions(-) Index: b/drivers/video/fbdev/Kconfig === --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -807,7 +807,8 @@ config FB_XVR1000 config FB_PVR2 tristate "NEC PowerVR 2 display support" - depends on FB && SH_DREAMCAST + depends on FB && HAS_IOMEM + depends on SH_DREAMCAST || COMPILE_TEST select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT Index: b/drivers/video/fbdev/pvr2fb.c === --- a/drivers/video/fbdev/pvr2fb.c +++ b/drivers/video/fbdev/pvr2fb.c @@ -139,7 +139,7 @@ static struct pvr2fb_par { unsigned char is_doublescan;/* Are scanlines output twice? (doublescan) */ unsigned char is_lowres;/* Is horizontal pixel-doubling enabled? */ - unsigned long mmio_base;/* MMIO base */ + void __iomem *mmio_base;/* MMIO base */ u32 palette[16]; } *currentpar; @@ -325,9 +325,9 @@ static int pvr2fb_setcolreg(unsigned int * anything if the cable type has been overidden (via "cable:XX"). */ -#define PCTRA 0xff80002c -#define PDTRA 0xff800030 -#define VOUTC 0xa0702c00 +#define PCTRA ((void __iomem *)0xff80002c) +#define PDTRA ((void __iomem *)0xff800030) +#define VOUTC ((void __iomem *)0xa0702c00) static int pvr2_init_cable(void) { @@ -619,7 +619,7 @@ static void pvr2_do_blank(void) is_blanked = do_blank > 0 ? do_blank : 0; } -static irqreturn_t pvr2fb_interrupt(int irq, void *dev_id) +static irqreturn_t __maybe_unused pvr2fb_interrupt(int irq, void *dev_id) { struct fb_info *info = dev_id; @@ -757,7 +757,7 @@ static int pvr2_get_param(const struct p * in for flexibility anyways. Who knows, maybe someone has tv-out on a * PCI-based version of these things ;-) */ -static int pvr2fb_common_init(void) +static int __maybe_unused pvr2fb_common_init(void) { struct pvr2fb_par *par = currentpar; unsigned long modememused, rev; @@ -770,8 +770,8 @@ static int pvr2fb_common_init(void) goto out_err; } - par->mmio_base = (unsigned long)ioremap_nocache(pvr2_fix.mmio_start, - pvr2_fix.mmio_len); + par->mmio_base = ioremap_nocache(pvr2_fix.mmio_start, +pvr2_fix.mmio_len); if (!par->mmio_base) { printk(KERN_ERR "pvr2fb: Failed to remap mmio space\n"); goto out_err; @@ -838,7 +838,7 @@ out_err: if (fb_info->screen_base) iounmap(fb_info->screen_base); if (par->mmio_base) - iounmap((void *)par->mmio_base); + iounmap(par->mmio_base); return -ENXIO; } @@ -905,8 +905,8 @@ static void __exit pvr2fb_dc_exit(void) fb_info->screen_base = NULL; } if (currentpar->mmio_base) { - iounmap((void *)currentpar->mmio_base); - currentpar->mmio_base = 0; + iounmap(currentpar->mmio_base); + currentpar->mmio_base = NULL; } free_irq(HW_EVENT_VSYNC, fb_info); @@ -955,8 +955,8 @@ static void pvr2fb_pci_remove(struct pci fb_info->screen_base = NULL; } if (currentpar->mmio_base) { - iounmap((void *)currentpar->mmio_base); - currentpar->mmio_base = 0; + iounmap(currentpar->mmio_base); + currentpar->mmio_base = NULL; } pci_release_regions(pdev);
Re: [PATCH] drm/vc4: Fix with pm_runtime synchronization on DSI
On 4/10/19 1:24 AM, Eric Anholt wrote: > Hoegeun Kwon writes: > >> On 4/2/19 2:48 AM, Eric Anholt wrote: >>> Hoegeun Kwon writes: >>> There is a problem when often dpms goes from off to on. pm idle is not in sync and the problem occurs. Modify pm_runtime_put from asynchronous to synchronous. >>> Why would we need the power domain to go to off before we try to come >>> back? Any idea? Also, please specify what "the problem" is. >> Hi Eric, >> >> >> First thank you for your review. >> >> There is a problem failed to runtime PM enable on DSI when often dpms > What do you mean by "failed to runtime PM enable"? The > pm_runtime_enable() returned an error? Have you investigated the source > that error, if so? I'm sorry for the late reply. The pm_runtime_enable() is not returned because return value is void. The problem is that if the error log is not output and the DPMS off on is repeated, the screen will stop. As a result of debugging the problem, I think that synchronization is a problem caused by dsi pm_suspend and resume. So when we entered the pm runtime idle state, if used with sync, the problem does not occur. Best regards, Hoegeun ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/cirrus: remove leftover files
cirrus_drv.h and cirrus_ttm.c are unused since commit ab3e023b1b4c ("drm/cirrus: rewrite and modernize driver"), apparently I ran "rm" instead of "git rm" on them so they are still in present the tree. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/cirrus/cirrus_drv.h | 250 - drivers/gpu/drm/cirrus/cirrus_ttm.c | 337 2 files changed, 587 deletions(-) delete mode 100644 drivers/gpu/drm/cirrus/cirrus_drv.h delete mode 100644 drivers/gpu/drm/cirrus/cirrus_ttm.c diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h b/drivers/gpu/drm/cirrus/cirrus_drv.h deleted file mode 100644 index 1bd816be3aae.. --- a/drivers/gpu/drm/cirrus/cirrus_drv.h +++ /dev/null @@ -1,250 +0,0 @@ -/* - * Copyright 2012 Red Hat - * - * This file is subject to the terms and conditions of the GNU General - * Public License version 2. See the file COPYING in the main - * directory of this archive for more details. - * - * Authors: Matthew Garrett - * Dave Airlie - */ -#ifndef __CIRRUS_DRV_H__ -#define __CIRRUS_DRV_H__ - -#include - -#include -#include - -#include -#include -#include -#include -#include - -#include - -#define DRIVER_AUTHOR "Matthew Garrett" - -#define DRIVER_NAME"cirrus" -#define DRIVER_DESC"qemu Cirrus emulation" -#define DRIVER_DATE"20110418" - -#define DRIVER_MAJOR 1 -#define DRIVER_MINOR 0 -#define DRIVER_PATCHLEVEL 0 - -#define CIRRUSFB_CONN_LIMIT 1 - -#define RREG8(reg) ioread8(((void __iomem *)cdev->rmmio) + (reg)) -#define WREG8(reg, v) iowrite8(v, ((void __iomem *)cdev->rmmio) + (reg)) -#define RREG32(reg) ioread32(((void __iomem *)cdev->rmmio) + (reg)) -#define WREG32(reg, v) iowrite32(v, ((void __iomem *)cdev->rmmio) + (reg)) - -#define SEQ_INDEX 4 -#define SEQ_DATA 5 - -#define WREG_SEQ(reg, v) \ - do {\ - WREG8(SEQ_INDEX, reg); \ - WREG8(SEQ_DATA, v); \ - } while (0) \ - -#define CRT_INDEX 0x14 -#define CRT_DATA 0x15 - -#define WREG_CRT(reg, v) \ - do {\ - WREG8(CRT_INDEX, reg); \ - WREG8(CRT_DATA, v); \ - } while (0) \ - -#define GFX_INDEX 0xe -#define GFX_DATA 0xf - -#define WREG_GFX(reg, v) \ - do {\ - WREG8(GFX_INDEX, reg); \ - WREG8(GFX_DATA, v); \ - } while (0) \ - -/* - * Cirrus has a "hidden" DAC register that can be accessed by writing to - * the pixel mask register to reset the state, then reading from the register - * four times. The next write will then pass to the DAC - */ -#define VGA_DAC_MASK 0x6 - -#define WREG_HDR(v)\ - do {\ - RREG8(VGA_DAC_MASK);\ - RREG8(VGA_DAC_MASK);\ - RREG8(VGA_DAC_MASK);\ - RREG8(VGA_DAC_MASK);\ - WREG8(VGA_DAC_MASK, v); \ - } while (0) \ - - -#define CIRRUS_MAX_FB_HEIGHT 4096 -#define CIRRUS_MAX_FB_WIDTH 4096 - -#define CIRRUS_DPMS_CLEARED (-1) - -#define to_cirrus_crtc(x) container_of(x, struct cirrus_crtc, base) -#define to_cirrus_encoder(x) container_of(x, struct cirrus_encoder, base) - -struct cirrus_crtc { - struct drm_crtc base; - int last_dpms; - boolenabled; -}; - -struct cirrus_fbdev; -struct cirrus_mode_info { - struct cirrus_crtc *crtc; - /* pointer to fbdev info structure */ - struct cirrus_fbdev *gfbdev; -}; - -struct cirrus_encoder { - struct drm_encoder base; - int last_dpms; -}; - -struct cirrus_connector { - struct drm_connectorbase; -}; - -struct cirrus_mc { - resource_size_t vram_size; - resource_size_t vram_base; -}; - -struct cirrus_device { - struct drm_device *dev; - unsigned long flags; - - resource_size_t rmmio_base; - resource_size_t rmmio_size; - void __iomem*rmmio; - - st
Re: [PATCH] fbcon: Remove fbcon_has_exited
On Wed, May 22, 2019 at 12:04 PM Bartlomiej Zolnierkiewicz wrote: > > > Hi Daniel, > > On 5/21/19 4:23 PM, Daniel Vetter wrote: > > This is unused code since > > > > commit 6104c37094e729f3d4ce65797002112735d49cd1 > > Author: Daniel Vetter > > Date: Tue Aug 1 17:32:07 2017 +0200 > > > > fbcon: Make fbcon a built-time depency for fbdev > > > > when fbcon was made a compile-time static dependency of fbdev. We > > can't exit fbcon anymore without exiting fbdev first, which only works > > if all fbdev drivers have unloaded already. Hence this is all dead > > code. > > > > v2: I missed that fbcon_exit is also called from con_deinit stuff, and > > there fbcon_has_exited prevents double-cleanup. But we can fix that > > by properly resetting con2fb_map[] to all -1, which is used everywhere > > else to indicate "no fb_info allocate to this console". With that > > change the double-cleanup (which resulted in a module refcount underflow, > > among other things) is prevented. > > > > Aside: con2fb_map is a signed char, so don't register more than 128 fb_info > > or hilarity will ensue. > > > > v3: CI showed me that I still didn't fully understand what's going on > > here. The leaked references in con2fb_map have been used upon > > rebinding the fb console in fbcon_init. It worked because fbdev > > unregistering still cleaned out con2fb_map, and reset it to info_idx. > > If the last fbdev driver unregistered, then it also reset info_idx, > > and unregistered the fbcon driver. > > > > Imo that's all a bit fragile, so let's keep the con2fb_map reset to > > -1, and in fbcon_init pick info_idx if we're starting fresh. That > > means unbinding and rebinding will cleanse the mapping, but why are > > you doing that if you want to retain the mapping, so should be fine. > > > > Also, I think info_idx == -1 is impossible in fbcon_init - we > > unregister the fbcon in that case. So catch&warn about that. > > > > Signed-off-by: Daniel Vetter > > Cc: Maarten Lankhorst > > Cc: Bartlomiej Zolnierkiewicz > > Cc: Daniel Vetter > > Cc: Hans de Goede > > Cc: "Noralf Trønnes" > > Cc: Yisheng Xie > > Cc: Konstantin Khorenko > > Cc: Prarit Bhargava > > Cc: Kees Cook > > --- > > drivers/video/fbdev/core/fbcon.c | 39 +--- > > 1 file changed, 6 insertions(+), 33 deletions(-) > This patch was #09/33 in your patch series, now it is independent change. > > Do you want me to apply it now or should I wait for the new version of > the whole series? It's an in-reply-to replacement for the totally broken one, so that patchwork picks things up correctly (and therefore our CI). I'm not sure how far that convention is used beyond dri-devel ... I did fix up all the issues pointed out thus far, but I haven't fully appeased our CI just yet. Once that's done I'll resend. Thanks, Daniel > [ I looked at all patches in the series and they look fine to me. > After outstanding issues are fixed I'll be happy to apply them all > to fbdev-for-next (I can create immutable branch if needed). ] > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] update drm.h
Am 22.05.19 um 11:07 schrieb Chunming Zhou: > a) delta: only DRM_CAP_SYNCOBJ_TIMELINE > b) Generated using make headers_install. > c) Generated from origin/drm-misc-next commit > 982c0500fd1a8012c31d3c9dd8de285129904656" > > Signed-off-by: Chunming Zhou > Suggested-by: Michel Dänzer Reviewed-by: Christian König > --- > include/drm/drm.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/drm.h b/include/drm/drm.h > index c893f3b4..438abde3 100644 > --- a/include/drm/drm.h > +++ b/include/drm/drm.h > @@ -44,6 +44,7 @@ typedef unsigned int drm_handle_t; > > #else /* One of the BSDs */ > > +#include > #include > #include > typedef int8_t __s8; > @@ -643,6 +644,7 @@ struct drm_gem_open { > #define DRM_CAP_PAGE_FLIP_TARGET0x11 > #define DRM_CAP_CRTC_IN_VBLANK_EVENT0x12 > #define DRM_CAP_SYNCOBJ 0x13 > +#define DRM_CAP_SYNCOBJ_TIMELINE 0x14 > > /** DRM_IOCTL_GET_CAP ioctl argument type */ > struct drm_get_cap { ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] Various clean-up patches for GEM VRAM
On Tue, May 21, 2019 at 02:40:22PM +0200, Daniel Vetter wrote: > On Tue, May 21, 2019 at 01:08:28PM +0200, Thomas Zimmermann wrote: > > Replacing drm_gem_vram_push_to_system() moves policy from drivers back > > to the memory manager. Now, unused BOs are only evicted when the space > > is required. > > > > The lock/unlock-renaming patch aligns the interface with other names > > in DRM. No functional changes are done. > > > > Finally, there's now a lockdep assert that ensures we don't call the > > GEM VRAM _locked() functions with an unlocked BO. > > > > Patches are against a recent drm-tip and tested on mgag200 and ast HW. > > > > Thomas Zimmermann (3): > > drm: Replace drm_gem_vram_push_to_system() with kunmap + unpin > > drm: Rename reserve/unreserve to lock/unlock in GEM VRAM helpers > > drm: Assert that BO is locked in drm_gem_vram_{pin,unpin}_locked() > > Awesome, thanks a lot for quickly working on this. On the series: > > Acked-by: Daniel Vetter > > But definitely get someone with more knowledge of the details to check > this all again. Done & pushed to drm-misc-next. thanks, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v8
On Wed, May 22, 2019 at 10:49 AM Christian König wrote: > > Am 22.05.19 um 10:19 schrieb Daniel Vetter: > > On Wed, May 15, 2019 at 04:38:26PM +0200, Christian König wrote: > > [SNAP] > > Just this functional comment, since I think api detail polishing is > > premature if we're not yet aware of how this works. > > > >> +/* When the importer is dynamic but the exporter isn't we need to > >> cache > >> + * the mapping or otherwise would run into issues with the reservation > >> + * object lock. > >> + */ > >> +if (dma_buf_attachment_is_dynamic(attach) && > >> +!dma_buf_is_dynamic(dmabuf)) { > > Isn't this the wrong way round? dynamic importers should be perfectly fine > > with the reservation locks in their map/unmap paths, it's importers > > calling exporters there. > > > > The real problem is a not-dynamic importer, which hasn't be adjusted to > > allow the reservation lock in their paths where they map/unmap a buffer, > > with a dynamic exporter. That's where we need to cache the mapping to > > avoid the deadlock (or having to change everyone) > > Well could be that this is also a problem, but I actually don't think so. > > The case I'm describing here certainly is the more obvious problem > because the importer is already holding the lock the exporter wants to take. Hm, isn't that papered over by such exporters enabling the dma-buf caching you've just moved from drm_prime to dma-buf? > On the other hand we could rather easily change that check to > dma_buf_attachment_is_dynamic() != dma_buf_is_dynamic() if that is > indeed a problem. Hm yeah. > >> +struct sg_table *sgt; > >> + > >> +sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL); > > And unfortunately the non-dynamic, i.e. legacy/current code importer is > > also the one which uses other flags than DMA_BIDIRECTIONAL. At least on > > ARM, and that's the only place where this matters because there the dma > > api might do cache flushing. > > Well the only implementer for now is amdgpu, and amdgpu always requires > a coherent bidirectional mapping. > > So this won't be a problem unless the ARM drivers start to implement > dynamic DMA-buf handling themselves or start to talk to amdgpu (which > wouldn't have worked before anyway). Hm, feels a bit evil. I just heard a some soc people tell me that drm isn't cool because it likes to ignore soc at the expensive of x86. Otoh I don't see a way out either, and maybe this will be motivation enough to make the dma-api cache management a bit more explicit. Not that I expect much change there ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v8
On Wed, May 15, 2019 at 4:38 PM Christian König wrote: > > On the exporter side we add optional explicit pinning callbacks. If those > callbacks are implemented the framework no longer caches sg tables and the > map/unmap callbacks are always called with the lock of the reservation object > held. > > On the importer side we add an optional invalidate callback. This callback is > used by the exporter to inform the importers that their mappings should be > destroyed as soon as possible. > > This allows the exporter to provide the mappings without the need to pin > the backing store. > > v2: don't try to invalidate mappings when the callback is NULL, > lock the reservation obj while using the attachments, > add helper to set the callback > v3: move flag for invalidation support into the DMA-buf, > use new attach_info structure to set the callback > v4: use importer_priv field instead of mangling exporter priv. > v5: drop invalidation_supported flag > v6: squash together with pin/unpin changes > v7: pin/unpin takes an attachment now > v8: nuke dma_buf_attachment_(map|unmap)_locked, > everything is now handled backward compatible > > Signed-off-by: Christian König > --- > drivers/dma-buf/dma-buf.c | 157 -- > include/linux/dma-buf.h | 109 -- > 2 files changed, 253 insertions(+), 13 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index a2d34c6b80a5..85026d9e978d 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -409,6 +409,9 @@ struct dma_buf *dma_buf_export(const struct > dma_buf_export_info *exp_info) > return ERR_PTR(-EINVAL); > } > > + if (WARN_ON(exp_info->ops->cache_sgt_mapping && exp_info->ops->pin)) > + return ERR_PTR(-EINVAL); > + > if (!try_module_get(exp_info->owner)) > return ERR_PTR(-ENOENT); > > @@ -530,10 +533,12 @@ void dma_buf_put(struct dma_buf *dmabuf) > EXPORT_SYMBOL_GPL(dma_buf_put); > > /** > - * dma_buf_attach - Add the device to dma_buf's attachments list; optionally, > + * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list; > optionally, > * calls attach() of dma_buf_ops to allow device-specific attach > functionality > - * @dmabuf:[in]buffer to attach device to. > - * @dev: [in]device to be attached. > + * @dmabuf:[in]buffer to attach device to. > + * @dev: [in]device to be attached. > + * @importer_ops [in]importer operations for the attachment > + * @importer_priv [in]importer private pointer for the attachment > * > * Returns struct dma_buf_attachment pointer for this attachment. Attachments > * must be cleaned up by calling dma_buf_detach(). > @@ -547,8 +552,10 @@ EXPORT_SYMBOL_GPL(dma_buf_put); > * accessible to @dev, and cannot be moved to a more suitable place. This is > * indicated with the error code -EBUSY. > */ > -struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, > - struct device *dev) > +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) > { > struct dma_buf_attachment *attach; > int ret; > @@ -562,6 +569,8 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf > *dmabuf, > > attach->dev = dev; > attach->dmabuf = dmabuf; > + attach->importer_ops = importer_ops; > + attach->importer_priv = importer_priv; > > mutex_lock(&dmabuf->lock); > > @@ -570,10 +579,31 @@ struct dma_buf_attachment *dma_buf_attach(struct > dma_buf *dmabuf, > if (ret) > goto err_attach; > } > + reservation_object_lock(dmabuf->resv, NULL); > list_add(&attach->node, &dmabuf->attachments); > + reservation_object_unlock(dmabuf->resv); > > mutex_unlock(&dmabuf->lock); > > + /* When the importer is dynamic but the exporter isn't we need to > cache > +* the mapping or otherwise would run into issues with the reservation > +* object lock. > +*/ > + if (dma_buf_attachment_is_dynamic(attach) && > + !dma_buf_is_dynamic(dmabuf)) { > + struct sg_table *sgt; > + > + sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL); > + if (!sgt) > + sgt = ERR_PTR(-ENOMEM); > + if (IS_ERR(sgt)) { > + dma_buf_detach(dmabuf, attach); > + return ERR_CAST(sgt); > + } > + attach->sgt = sgt; > + attach->dir = DMA_BIDIRECTIONAL; > + } > + > return attach; > > err_attach: > @@ -581,6 +611,21 @@ struct dma_buf_attachment *dma_buf_attach(struct
Re: [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v8
On Wed, May 22, 2019 at 1:27 PM Daniel Vetter wrote: > > On Wed, May 22, 2019 at 10:49 AM Christian König > wrote: > > > > Am 22.05.19 um 10:19 schrieb Daniel Vetter: > > > On Wed, May 15, 2019 at 04:38:26PM +0200, Christian König wrote: > > > [SNAP] > > > Just this functional comment, since I think api detail polishing is > > > premature if we're not yet aware of how this works. > > > > > >> +/* When the importer is dynamic but the exporter isn't we need to > > >> cache > > >> + * the mapping or otherwise would run into issues with the > > >> reservation > > >> + * object lock. > > >> + */ > > >> +if (dma_buf_attachment_is_dynamic(attach) && > > >> +!dma_buf_is_dynamic(dmabuf)) { > > > Isn't this the wrong way round? dynamic importers should be perfectly fine > > > with the reservation locks in their map/unmap paths, it's importers > > > calling exporters there. > > > > > > The real problem is a not-dynamic importer, which hasn't be adjusted to > > > allow the reservation lock in their paths where they map/unmap a buffer, > > > with a dynamic exporter. That's where we need to cache the mapping to > > > avoid the deadlock (or having to change everyone) > > > > Well could be that this is also a problem, but I actually don't think so. > > > > The case I'm describing here certainly is the more obvious problem > > because the importer is already holding the lock the exporter wants to take. > > Hm, isn't that papered over by such exporters enabling the dma-buf > caching you've just moved from drm_prime to dma-buf? > > > On the other hand we could rather easily change that check to > > dma_buf_attachment_is_dynamic() != dma_buf_is_dynamic() if that is > > indeed a problem. > > Hm yeah. Forgot to add: Iirc it was buffer sharing between i915 and amdgpu that hits this. Can't say for sure since intel-gfx isn't cc'ed on this version, so our CI hasn't picked this up. -Daniel > > > >> +struct sg_table *sgt; > > >> + > > >> +sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL); > > > And unfortunately the non-dynamic, i.e. legacy/current code importer is > > > also the one which uses other flags than DMA_BIDIRECTIONAL. At least on > > > ARM, and that's the only place where this matters because there the dma > > > api might do cache flushing. > > > > Well the only implementer for now is amdgpu, and amdgpu always requires > > a coherent bidirectional mapping. > > > > So this won't be a problem unless the ARM drivers start to implement > > dynamic DMA-buf handling themselves or start to talk to amdgpu (which > > wouldn't have worked before anyway). > > Hm, feels a bit evil. I just heard a some soc people tell me that drm > isn't cool because it likes to ignore soc at the expensive of x86. > > Otoh I don't see a way out either, and maybe this will be motivation > enough to make the dma-api cache management a bit more explicit. Not > that I expect much change there ... > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: Fix with pm_runtime synchronization on DSI
On Wed, May 22, 2019 at 12:24 PM Hoegeun Kwon wrote: > On 4/10/19 1:24 AM, Eric Anholt wrote: > > Hoegeun Kwon writes: > > > On 4/2/19 2:48 AM, Eric Anholt wrote: > > Hoegeun Kwon writes: > > > There is a problem when often dpms goes from off to on. pm idle is not > in sync and the problem occurs. Modify pm_runtime_put from > asynchronous to synchronous. > > Why would we need the power domain to go to off before we try to come > back? Any idea? Also, please specify what "the problem" is. > > Hi Eric, > > > First thank you for your review. > > There is a problem failed to runtime PM enable on DSI when often dpms > > What do you mean by "failed to runtime PM enable"? The > pm_runtime_enable() returned an error? Have you investigated the source > that error, if so? > > I'm sorry for the late reply. > > The pm_runtime_enable() is not returned because return value is void. > > The problem is that if the error log is not output > and the DPMS off on is repeated, the screen will stop. > > As a result of debugging the problem, I think that synchronization > is a problem caused by dsi pm_suspend and resume. > > So when we entered the pm runtime idle state, if used with sync, > the problem does not occur. > > Sounds like not enough hystersis between disable/enable, i.e. some msleep missing somewhere. That's at least my guess, we've had a bunch of these with delayed runtime suspend of various things in i915. -Daniel > > Best regards, > Hoegeun > > > > > > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] vmwgfx-fixes-5.2
Dave, Daniel A set of misc fixes for various issues that have surfaced recently. All Cc'd stable except the dma iterator fix which shouldn't really cause any real issues on older kernels. The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9: Linux 5.2-rc1 (2019-05-19 15:47:09 -0700) are available in the Git repository at: git://people.freedesktop.org/~thomash/linux vmwgfx-fixes-5.2 for you to fetch changes up to 5ed7f4b5eca11c3c69e7c8b53e4321812bc1ee1e: drm/vmwgfx: integer underflow in vmw_cmd_dx_set_shader() leading to an invalid read (2019-05-21 10:23:10 +0200) Murray McAllister (2): drm/vmwgfx: NULL pointer dereference from vmw_cmd_dx_view_define() drm/vmwgfx: integer underflow in vmw_cmd_dx_set_shader() leading to an invalid read Thomas Hellstrom (4): drm/vmwgfx: Don't send drm sysfs hotplug events on initial master set drm/vmwgfx: Fix user space handle equal to zero drm/vmwgfx: Fix compat mode shader operation drm/vmwgfx: Use the dma scatter-gather iterator to get dma addresses drivers/gpu/drm/vmwgfx/ttm_object.c| 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 8 +++- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c| 20 +++- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 27 +++ 5 files changed, 35 insertions(+), 24 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106302] [radeonsi] Garbage content when accessing a texture in multiple shared EGL contexts
https://bugs.freedesktop.org/show_bug.cgi?id=106302 --- Comment #1 from Pierre-Eric Pelloux-Prayer --- I can reproduce but I don't think it's a bug in Mesa: your createTexture() function doesn't use any synchronization mechanisms so you can't expect the other thread/context to pick up the changes mades to the texture. Adding a call to glFlush or glFinish at the end of createTexture() is enough in this case to fix the issue. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH 3/6] drm/sun4i: dsi: Add bridge support
Hi Paul and Maxime, On Fri, Mar 15, 2019 at 7:03 PM Paul Kocialkowski wrote: > > Hi, > > On Fri, 2019-03-15 at 18:38 +0530, Jagan Teki wrote: > > Some display panels would come up with a non-DSI output which > > can have an option to connect DSI interface by means of bridge > > convertor. > > > > This DSI to non-DSI bridge convertor would require a bridge > > driver that would communicate the DSI controller for bridge > > functionalities. > > > > So, add support for bridge functionalities in Allwinner DSI > > controller. > > See a few comments below. > > > Signed-off-by: Jagan Teki > > --- > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 65 +++--- > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 1 + > > 2 files changed, 49 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > index 0960b96b62cc..64d74313b842 100644 > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > @@ -781,6 +781,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder > > *encoder) > > if (!IS_ERR(dsi->panel)) > > drm_panel_prepare(dsi->panel); > > > > + if (!IS_ERR(dsi->bridge)) > > + drm_bridge_pre_enable(dsi->bridge); > > + > > /* > >* FIXME: This should be moved after the switch to HS mode. > >* > > @@ -796,6 +799,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder > > *encoder) > > if (!IS_ERR(dsi->panel)) > > drm_panel_enable(dsi->panel); > > > > + if (!IS_ERR(dsi->bridge)) > > + drm_bridge_enable(dsi->bridge); > > + > > sun6i_dsi_start(dsi, DSI_START_HSC); > > > > udelay(1000); > > @@ -812,6 +818,9 @@ static void sun6i_dsi_encoder_disable(struct > > drm_encoder *encoder) > > if (!IS_ERR(dsi->panel)) { > > drm_panel_disable(dsi->panel); > > drm_panel_unprepare(dsi->panel); > > + } else if (!IS_ERR(dsi->bridge)) { > > + drm_bridge_disable(dsi->bridge); > > + drm_bridge_post_disable(dsi->bridge); > > } > > > > phy_power_off(dsi->dphy); > > @@ -973,11 +982,16 @@ static int sun6i_dsi_attach(struct mipi_dsi_host > > *host, > > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > > > > dsi->device = device; > > - dsi->panel = of_drm_find_panel(device->dev.of_node); > > - if (IS_ERR(dsi->panel)) > > - return PTR_ERR(dsi->panel); > > > > - dev_info(host->dev, "Attached device %s\n", device->name); > > + dsi->bridge = of_drm_find_bridge(device->dev.of_node); > > + if (!dsi->bridge) { > > You are using IS_ERR to check that the bridge is alive in the changes > above, but switch to checking that it's non-NULL at this point. > > Are both guaranteed to be interchangeable? > > > + dsi->panel = of_drm_find_panel(device->dev.of_node); > > + if (IS_ERR(dsi->panel)) > > + return PTR_ERR(dsi->panel); > > + } > > You should probably use drm_of_find_panel_or_bridge instead of > duplicating the logic here. True, In-fact I did try this API. but pipeline were unable to bound. Usually the panel and bridge were attached first and then the pipeline bound would start from front-end (in A33) But in my below cases I have seen only panel or bridge attached but no pipeline bound at all. And I'm using drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0, &dsi->panel, &dsi->bridge); in dsi attach API. Case-1, panel: &dsi { vcc-dsi-supply = <®_dcdc1>;/* VCC3V3-DSI */ status = "okay"; ports { dsi_out: port@1 { reg = <1>; dsi_out_panel: endpoint { remote-endpoint = <&panel_out_dsi>; }; }; }; panel@0 { compatible = "bananapi,s070wv20-ct16-icn6211"; reg = <0>; enable-gpios = <&pio 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PB7 */ reset-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */ backlight = <&backlight>; port { panel_out_dsi: endpoint { remote-endpoint = <&dsi_out_panel>; }; }; }; }; Case-2, bridge: panel { compatible = "bananapi,s070wv20-ct16", "simple-panel"; enable-gpios = <&pio 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PB7 */ backlight = <&backlight>; port { panel_out_bridge: endpoint { remote-endpoint = <&bridge_out_panel>; }; }; }; &dsi { vcc-dsi-supply = <®_dcdc1>;/* VCC-DSI */ status = "okay"; ports { dsi_out: port@1 { reg = <1>; dsi_out_bridge: endpoint { remote-endpoint = <&bridge_out_dsi>; }; }; }; bridge@0 { reg = <0>; compatible = "bananapi,icn6211", "chipone,icn6211"; reset-gpios = <&r_pio 0 5 GPIO_ACTIVE_
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #2 from alvarex --- now the bug has hit 19.0.5. There is a fix for steam if you uncheck "Enable Gpu accelarated rendering in web views" -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/panfrost: Use drm_gem_shmem_map_offset()
On 21/05/2019 19:23, Chris Wilson wrote: > Quoting Rob Herring (2019-05-21 16:24:27) >> On Mon, May 20, 2019 at 4:23 AM Steven Price wrote: >>> >> >> You forgot to update the subject. I can fixup when applying, but I'd >> like an ack from Chris on patch 1. Sorry about that - I'll try to be more careful in the future. > I still think it is incorrect as the limitation is purely an issue with > the shmem backend and not a generic GEM limitation. It matters if you Do you prefer the previous version of this series[1] with the shmem helper? [1] https://lore.kernel.org/lkml/20190516141447.46839-1-steven.pr...@arm.com/ Although this isn't a generic GEM limitation it's currently the same limitation that applies to the 'dumb' drivers as well as shmem backend, so I'd prefer not implementing two identical functions purely because this limitation could be removed in the future. > have a history of passing names and want a consistent api across objects > when the apps themselves no longer can tell the difference, and > certainly do not have access to the dmabuf fd. i915 provides an indirect > mmap interface that uses the dma mapping regardless of source. I don't understand the i915 driver, but from a quick look at the source of i915_gem_mmap_ioctl(): /* prime objects have no backing filp to GEM mmap * pages from. */ if (!obj->base.filp) { addr = -ENXIO; goto err; } it looks to me that an attempt to map an imported dmabuf from user space using the ioctl will fail. Am I missing something? exynos_drm_gem_mmap() is the only (mainline[2]) instance I can see where a transparent mapping of a dma_buf is supported. [2] mali_kbase does this too - but I'm not convinced it was a good idea. I could instead add support in shmem/panfrost for transparently passing the request to the exporter (i.e. call dma_buf_mmap()) - but I'm not sure we want to implement this if there isn't going to be a user of the support. Steve
[PATCH] drm_edid-load: Fix a missing-check bug in drivers/gpu/drm/drm_edid_load.c
In drm_load_edid_firmware(), fwstr is allocated by kstrdup(). And fwstr is dereferenced in the following codes. However, memory allocation functions such as kstrdup() may fail and returns NULL. Dereferencing this null pointer may cause the kernel go wrong. Thus we should check this kstrdup() operation. Further, if kstrdup() returns NULL, we should return ERR_PTR(-ENOMEM) to the caller site. Signed-off-by: Gen Zhang --- diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index a491509..a0e107a 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -290,6 +290,8 @@ struct edid *drm_load_edid_firmware(struct drm_connector *connector) * the last one found one as a fallback. */ fwstr = kstrdup(edid_firmware, GFP_KERNEL); + if (!fwstr) + return ERR_PTR(-ENOMEM); edidstr = fwstr; while ((edidname = strsep(&edidstr, ","))) { ---
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 alvarex changed: What|Removed |Added Version|unspecified |19.0 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/10] drm/ttm: Make LRU removal optional.
We are already doing this for DMA-buf imports and also for amdgpu VM BOs for quite a while now. If this doesn't run into any problems we are probably going to stop removing BOs from the LRU altogether. Signed-off-by: Christian König --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 + drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++-- drivers/gpu/drm/qxl/qxl_release.c | 2 +- drivers/gpu/drm/radeon/radeon_gem.c | 2 +- drivers/gpu/drm/radeon/radeon_object.c| 2 +- drivers/gpu/drm/ttm/ttm_execbuf_util.c| 20 +++ drivers/gpu/drm/virtio/virtgpu_ioctl.c| 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 3 ++- drivers/gpu/drm/vmwgfx/vmwgfx_validation.h| 2 +- include/drm/ttm/ttm_bo_driver.h | 5 - include/drm/ttm/ttm_execbuf_util.h| 3 ++- 13 files changed, 34 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index e1cae4a37113..647e18f9e136 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -574,7 +574,7 @@ static int reserve_bo_and_vm(struct kgd_mem *mem, amdgpu_vm_get_pd_bo(vm, &ctx->list, &ctx->vm_pd[0]); ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list, -false, &ctx->duplicates); +false, &ctx->duplicates, true); if (!ret) ctx->reserved = true; else { @@ -647,7 +647,7 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem, } ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list, -false, &ctx->duplicates); +false, &ctx->duplicates, true); if (!ret) ctx->reserved = true; else @@ -1800,7 +1800,8 @@ static int validate_invalid_user_pages(struct amdkfd_process_info *process_info) } /* Reserve all BOs and page tables for validation */ - ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates); + ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates, +true); WARN(!list_empty(&duplicates), "Duplicates should be empty"); if (ret) goto out_free; @@ -2006,7 +2007,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) } ret = ttm_eu_reserve_buffers(&ctx.ticket, &ctx.list, -false, &duplicate_save); +false, &duplicate_save, true); if (ret) { pr_debug("Memory eviction: TTM Reserve Failed. Try again\n"); goto ttm_reserve_fail; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index d72cc583ebd1..fff558cf385b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true, - &duplicates); + &duplicates, true); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) DRM_ERROR("ttm_eu_reserve_buffers failed.\n"); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c index 54dd02a898b9..06f83cac0d3a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c @@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, list_add(&csa_tv.head, &list); amdgpu_vm_get_pd_bo(vm, &list, &pd); - r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL); + r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true); if (r) { DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r); return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 7b840367004c..d513a5ad03dd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj, amdgpu_vm_get_pd_bo(vm, &list, &vm_pd); - r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates); + r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, true); if (r) { dev_err(adev->dev, "leaking bo va because " "we fail to reserve bo (%d)\n", r); @@ -608,7 +608,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *d
[PATCH 02/10] drm/ttm: return immediately in case of a signal
When a signal arrives we should return immediately for handling it and not try other placements first. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 2845fceb2fbd..4336893cb35e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -978,7 +978,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, uint32_t cur_flags = 0; bool type_found = false; bool type_ok = false; - bool has_erestartsys = false; int i, ret; ret = reservation_object_reserve_shared(bo->resv, 1); @@ -1069,8 +1068,8 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, mem->placement = cur_flags; return 0; } - if (ret == -ERESTARTSYS) - has_erestartsys = true; + if (ret && ret != -EBUSY) + return ret; } if (!type_found) { @@ -1078,7 +1077,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, return -EINVAL; } - return (has_erestartsys) ? -ERESTARTSYS : -ENOMEM; + return -ENOMEM; } EXPORT_SYMBOL(ttm_bo_mem_space); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/10] drm/ttm: remove manual placement preference
If drivers don't prefer a system memory placement they should not but it into the placement list first. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 4336893cb35e..b29799aedb71 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1011,8 +1011,12 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, ttm_flag_masked(&cur_flags, place->flags, ~TTM_PL_MASK_MEMTYPE); - if (mem_type == TTM_PL_SYSTEM) - break; + if (mem_type == TTM_PL_SYSTEM) { + mem->mem_type = mem_type; + mem->placement = cur_flags; + mem->mm_node = NULL; + return 0; + } ret = (*man->func->get_node)(man, bo, place, mem); if (unlikely(ret)) @@ -1024,16 +1028,12 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, (*man->func->put_node)(man, mem); return ret; } - break; + mem->mem_type = mem_type; + mem->placement = cur_flags; + return 0; } } - if ((type_ok && (mem_type == TTM_PL_SYSTEM)) || mem->mm_node) { - mem->mem_type = mem_type; - mem->placement = cur_flags; - return 0; - } - for (i = 0; i < placement->num_busy_placement; ++i) { const struct ttm_place *place = &placement->busy_placement[i]; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/10] drm/amdgpu: create GDS, GWS and OA in system domain
And only move them in on validation. This allows for better control when multiple processes are fighting over those resources. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 93b2c5a48a71..30493429851e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -495,7 +495,11 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, #endif bo->tbo.bdev = &adev->mman.bdev; - amdgpu_bo_placement_from_domain(bo, bp->domain); + if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA | + AMDGPU_GEM_DOMAIN_GDS)) + amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU); + else + amdgpu_bo_placement_from_domain(bo, bp->domain); if (bp->type == ttm_bo_type_kernel) bo->tbo.priority = 1; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/10] drm/ttm: cleanup ttm_bo_mem_space
We tried this once before, but that turned out to be more complicated than thought. With all the right prerequisites it looks like we can do this now. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 127 ++- 1 file changed, 66 insertions(+), 61 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index b29799aedb71..fd6dbebea430 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -892,13 +892,12 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, * space, or we've evicted everything and there isn't enough space. */ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, - uint32_t mem_type, - const struct ttm_place *place, - struct ttm_mem_reg *mem, - struct ttm_operation_ctx *ctx) + const struct ttm_place *place, + struct ttm_mem_reg *mem, + struct ttm_operation_ctx *ctx) { struct ttm_bo_device *bdev = bo->bdev; - struct ttm_mem_type_manager *man = &bdev->man[mem_type]; + struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type]; int ret; do { @@ -907,11 +906,11 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, return ret; if (mem->mm_node) break; - ret = ttm_mem_evict_first(bdev, mem_type, place, ctx); + ret = ttm_mem_evict_first(bdev, mem->mem_type, place, ctx); if (unlikely(ret != 0)) return ret; } while (1); - mem->mem_type = mem_type; + return ttm_bo_add_move_fence(bo, man, mem); } @@ -959,6 +958,51 @@ static bool ttm_bo_mt_compatible(struct ttm_mem_type_manager *man, return true; } +/** + * ttm_bo_mem_placement - check if placement is compatible + * @bo: BO to find memory for + * @place: where to search + * @mem: the memory object to fill in + * @ctx: operation context + * + * Check if placement is compatible and fill in mem structure. + * Returns -EBUSY if placement won't work or negative error code. + * 0 when placement can be used. + */ +static int ttm_bo_mem_placement(struct ttm_buffer_object *bo, + const struct ttm_place *place, + struct ttm_mem_reg *mem, + struct ttm_operation_ctx *ctx) +{ + struct ttm_bo_device *bdev = bo->bdev; + uint32_t mem_type = TTM_PL_SYSTEM; + struct ttm_mem_type_manager *man; + uint32_t cur_flags = 0; + int ret; + + ret = ttm_mem_type_from_place(place, &mem_type); + if (ret) + return ret; + + man = &bdev->man[mem_type]; + if (!man->has_type || !man->use_type) + return -EBUSY; + + if (!ttm_bo_mt_compatible(man, mem_type, place, &cur_flags)) + return -EBUSY; + + cur_flags = ttm_bo_select_caching(man, bo->mem.placement, cur_flags); + /* +* Use the access and other non-mapping-related flag bits from +* the memory placement flags to the current flags +*/ + ttm_flag_masked(&cur_flags, place->flags, ~TTM_PL_MASK_MEMTYPE); + + mem->mem_type = mem_type; + mem->placement = cur_flags; + return 0; +} + /** * Creates space for memory region @mem according to its type. * @@ -973,11 +1017,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx) { struct ttm_bo_device *bdev = bo->bdev; - struct ttm_mem_type_manager *man; - uint32_t mem_type = TTM_PL_SYSTEM; - uint32_t cur_flags = 0; bool type_found = false; - bool type_ok = false; int i, ret; ret = reservation_object_reserve_shared(bo->resv, 1); @@ -987,37 +1027,20 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, mem->mm_node = NULL; for (i = 0; i < placement->num_placement; ++i) { const struct ttm_place *place = &placement->placement[i]; + struct ttm_mem_type_manager *man; - ret = ttm_mem_type_from_place(place, &mem_type); + ret = ttm_bo_mem_placement(bo, place, mem, ctx); + if (ret == -EBUSY) + continue; if (ret) return ret; - man = &bdev->man[mem_type]; - if (!man->has_type || !man->use_type) - continue; - - type_ok = ttm_bo_mt_compatible(man, mem_type, place, - &cur_flags); - - if (!type_ok) - continue; type_found = true; - cur_fl
[PATCH 06/10] drm/ttm: fix busy memory to fail other user v10
BOs on the LRU might be blocked during command submission and cause OOM situations. Avoid this by blocking for the first busy BO not locked by the same ticket as the BO we are searching space for. v10: completely start over with the patch since we didn't handled a whole bunch of corner cases. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 77 ++-- 1 file changed, 66 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 4c6389d849ed..861facac33d4 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -771,32 +771,72 @@ EXPORT_SYMBOL(ttm_bo_eviction_valuable); * b. Otherwise, trylock it. */ static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo, - struct ttm_operation_ctx *ctx, bool *locked) + struct ttm_operation_ctx *ctx, bool *locked, bool *busy) { bool ret = false; - *locked = false; if (bo->resv == ctx->resv) { reservation_object_assert_held(bo->resv); if (ctx->flags & TTM_OPT_FLAG_ALLOW_RES_EVICT || !list_empty(&bo->ddestroy)) ret = true; + *locked = false; + if (busy) + *busy = false; } else { - *locked = reservation_object_trylock(bo->resv); - ret = *locked; + ret = reservation_object_trylock(bo->resv); + *locked = ret; + if (busy) + *busy = !ret; } return ret; } +/** + * ttm_mem_evict_wait_busy - wait for a busy BO to become available + * + * @busy_bo: BO which couldn't be locked with trylock + * @ctx: operation context + * @ticket: acquire ticket + * + * Try to lock a busy buffer object to avoid failing eviction. + */ +static int ttm_mem_evict_wait_busy(struct ttm_buffer_object *busy_bo, + struct ttm_operation_ctx *ctx, + struct ww_acquire_ctx *ticket) +{ + int r; + + if (!busy_bo || !ticket) + return -EBUSY; + + if (ctx->interruptible) + r = reservation_object_lock_interruptible(busy_bo->resv, + ticket); + else + r = reservation_object_lock(busy_bo->resv, ticket); + + /* +* TODO: It would be better to keep the BO locked until allocation is at +* least tried one more time, but that would mean a much larger rework +* of TTM. +*/ + if (!r) + reservation_object_unlock(busy_bo->resv); + + return r == -EDEADLK ? -EAGAIN : r; +} + static int ttm_mem_evict_first(struct ttm_bo_device *bdev, uint32_t mem_type, const struct ttm_place *place, - struct ttm_operation_ctx *ctx) + struct ttm_operation_ctx *ctx, + struct ww_acquire_ctx *ticket) { + struct ttm_buffer_object *bo = NULL, *busy_bo = NULL; struct ttm_bo_global *glob = bdev->glob; struct ttm_mem_type_manager *man = &bdev->man[mem_type]; - struct ttm_buffer_object *bo = NULL; bool locked = false; unsigned i; int ret; @@ -804,8 +844,15 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, spin_lock(&glob->lru_lock); for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { list_for_each_entry(bo, &man->lru[i], lru) { - if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked)) + bool busy; + + if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked, + &busy)) { + if (busy && !busy_bo && + bo->resv->lock.ctx != ticket) + busy_bo = bo; continue; + } if (place && !bdev->driver->eviction_valuable(bo, place)) { @@ -824,8 +871,13 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, } if (!bo) { + if (busy_bo) + ttm_bo_get(busy_bo); spin_unlock(&glob->lru_lock); - return -EBUSY; + ret = ttm_mem_evict_wait_busy(busy_bo, ctx, ticket); + if (busy_bo) + ttm_bo_put(busy_bo); + return ret; } kref_get(&bo->list_kref); @@ -911,7 +963,8 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, return ret; if (mem->mm_node)
[PATCH 08/10] drm/amdgpu: drop some validation failure messages
The messages about amdgpu_cs_list_validate are duplicated because the caller will complain into the logs as well and we can also get interrupted by a signal here. Also fix the the caller to not report -EAGAIN from validation. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index fff558cf385b..20f2955d2a55 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -671,16 +671,12 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } r = amdgpu_cs_list_validate(p, &duplicates); - if (r) { - DRM_ERROR("amdgpu_cs_list_validate(duplicates) failed.\n"); + if (r) goto error_validate; - } r = amdgpu_cs_list_validate(p, &p->validated); - if (r) { - DRM_ERROR("amdgpu_cs_list_validate(validated) failed.\n"); + if (r) goto error_validate; - } amdgpu_cs_report_moved_bytes(p->adev, p->bytes_moved, p->bytes_moved_vis); @@ -1383,7 +1379,7 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r) { if (r == -ENOMEM) DRM_ERROR("Not enough memory for command submission!\n"); - else if (r != -ERESTARTSYS) + else if (r != -ERESTARTSYS && r != -EAGAIN) DRM_ERROR("Failed to process the buffer list %d!\n", r); goto out; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/10] drm/ttm: immediately move BOs to the new LRU v2
Move BOs which are currently in a lower domain to the new LRU before allocating backing space while validating. This makes sure that we always have enough entries on the LRU to allow for other processes to wait for an operation to complete. v2: generalize the test Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 45 ++-- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index fd6dbebea430..4c6389d849ed 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -166,17 +166,17 @@ static void ttm_bo_release_list(struct kref *list_kref) ttm_mem_global_free(bdev->glob->mem_glob, acc_size); } -void ttm_bo_add_to_lru(struct ttm_buffer_object *bo) +static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo, + struct ttm_mem_reg *mem) { struct ttm_bo_device *bdev = bo->bdev; struct ttm_mem_type_manager *man; reservation_object_assert_held(bo->resv); + BUG_ON(!list_empty(&bo->lru)); - if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) { - BUG_ON(!list_empty(&bo->lru)); - - man = &bdev->man[bo->mem.mem_type]; + if (!(mem->placement & TTM_PL_FLAG_NO_EVICT)) { + man = &bdev->man[mem->mem_type]; list_add_tail(&bo->lru, &man->lru[bo->priority]); kref_get(&bo->list_kref); @@ -188,6 +188,11 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo) } } } + +void ttm_bo_add_to_lru(struct ttm_buffer_object *bo) +{ + ttm_bo_add_mem_to_lru(bo, &bo->mem); +} EXPORT_SYMBOL(ttm_bo_add_to_lru); static void ttm_bo_ref_bug(struct kref *list_kref) @@ -1000,6 +1005,14 @@ static int ttm_bo_mem_placement(struct ttm_buffer_object *bo, mem->mem_type = mem_type; mem->placement = cur_flags; + + if (bo->mem.mem_type < mem_type && !list_empty(&bo->lru)) { + spin_lock(&bo->bdev->glob->lru_lock); + ttm_bo_del_from_lru(bo); + ttm_bo_add_mem_to_lru(bo, mem); + spin_unlock(&bo->bdev->glob->lru_lock); + } + return 0; } @@ -1033,7 +1046,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, if (ret == -EBUSY) continue; if (ret) - return ret; + goto error; type_found = true; mem->mm_node = NULL; @@ -1043,13 +1056,13 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, man = &bdev->man[mem->mem_type]; ret = (*man->func->get_node)(man, bo, place, mem); if (unlikely(ret)) - return ret; + goto error; if (mem->mm_node) { ret = ttm_bo_add_move_fence(bo, man, mem); if (unlikely(ret)) { (*man->func->put_node)(man, mem); - return ret; + goto error; } return 0; } @@ -1062,7 +1075,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, if (ret == -EBUSY) continue; if (ret) - return ret; + goto error; type_found = true; mem->mm_node = NULL; @@ -1074,15 +1087,23 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, return 0; if (ret && ret != -EBUSY) - return ret; + goto error; } + ret = -ENOMEM; if (!type_found) { pr_err(TTM_PFX "No compatible memory type found\n"); - return -EINVAL; + ret = -EINVAL; } - return -ENOMEM; +error: + if (bo->mem.mem_type == TTM_PL_SYSTEM && !list_empty(&bo->lru)) { + spin_lock(&bo->bdev->glob->lru_lock); + ttm_bo_move_to_lru_tail(bo, NULL); + spin_unlock(&bo->bdev->glob->lru_lock); + } + + return ret; } EXPORT_SYMBOL(ttm_bo_mem_space); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/10] drm/amdgpu: stop removing BOs from the LRU v3
This avoids OOM situations when we have lots of threads submitting at the same time. v3: apply this to the whole driver, not just CS Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 20f2955d2a55..3e2da24cd17a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true, - &duplicates, true); + &duplicates, false); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) DRM_ERROR("ttm_eu_reserve_buffers failed.\n"); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c index 06f83cac0d3a..f660628e6af9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c @@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, list_add(&csa_tv.head, &list); amdgpu_vm_get_pd_bo(vm, &list, &pd); - r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true); + r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, false); if (r) { DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r); return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index d513a5ad03dd..ed25a4e14404 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj, amdgpu_vm_get_pd_bo(vm, &list, &vm_pd); - r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, true); + r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, false); if (r) { dev_err(adev->dev, "leaking bo va because " "we fail to reserve bo (%d)\n", r); @@ -608,7 +608,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data, amdgpu_vm_get_pd_bo(&fpriv->vm, &list, &vm_pd); - r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates, true); + r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates, false); if (r) goto error_unref; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index c430e8259038..d60593cc436e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -155,7 +155,7 @@ static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo, bool no_intr) struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); int r; - r = ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL); + r = __ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) dev_err(adev->dev, "%p reserve failed\n", bo); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/10] drm/amd/display: use ttm_eu_reserve_buffers instead of amdgpu_bo_reserve v2
From: Chunming Zhou add ticket for display bo, so that it can preempt busy bo. v2: fix stupid rebase error Change-Id: I9f031cdcc8267de00e819ae303baa0a52df8ebb9 Signed-off-by: Chunming Zhou Reviewed-by: Christian König --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 21 ++- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 4a1755bce96c..56f320f3fd72 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4182,6 +4182,9 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane, struct amdgpu_device *adev; struct amdgpu_bo *rbo; struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old; + struct list_head list; + struct ttm_validate_buffer tv; + struct ww_acquire_ctx ticket; uint64_t tiling_flags; uint32_t domain; int r; @@ -4198,9 +4201,17 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane, obj = new_state->fb->obj[0]; rbo = gem_to_amdgpu_bo(obj); adev = amdgpu_ttm_adev(rbo->tbo.bdev); - r = amdgpu_bo_reserve(rbo, false); - if (unlikely(r != 0)) + INIT_LIST_HEAD(&list); + + tv.bo = &rbo->tbo; + tv.num_shared = 1; + list_add(&tv.head, &list); + + r = ttm_eu_reserve_buffers(&ticket, &list, false, NULL, true); + if (r) { + dev_err(adev->dev, "fail to reserve bo (%d)\n", r); return r; + } if (plane->type != DRM_PLANE_TYPE_CURSOR) domain = amdgpu_display_supported_domains(adev); @@ -4211,21 +4222,21 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane, if (unlikely(r != 0)) { if (r != -ERESTARTSYS) DRM_ERROR("Failed to pin framebuffer with error %d\n", r); - amdgpu_bo_unreserve(rbo); + ttm_eu_backoff_reservation(&ticket, &list); return r; } r = amdgpu_ttm_alloc_gart(&rbo->tbo); if (unlikely(r != 0)) { amdgpu_bo_unpin(rbo); - amdgpu_bo_unreserve(rbo); + ttm_eu_backoff_reservation(&ticket, &list); DRM_ERROR("%p bind failed\n", rbo); return r; } amdgpu_bo_get_tiling_flags(rbo, &tiling_flags); - amdgpu_bo_unreserve(rbo); + ttm_eu_backoff_reservation(&ticket, &list); afb->address = amdgpu_bo_gpu_offset(rbo); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[bug report] drm/scheduler: rework job destruction
Hello Christian König, The patch 5918045c4ed4: "drm/scheduler: rework job destruction" from Apr 18, 2019, leads to the following static checker warning: drivers/gpu/drm/scheduler/sched_main.c:297 drm_sched_job_timedout() error: potential NULL dereference 'job'. drivers/gpu/drm/scheduler/sched_main.c 279 static void drm_sched_job_timedout(struct work_struct *work) 280 { 281 struct drm_gpu_scheduler *sched; 282 struct drm_sched_job *job; 283 unsigned long flags; 284 285 sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work); 286 job = list_first_entry_or_null(&sched->ring_mirror_list, 287 struct drm_sched_job, node); 288 289 if (job) ^^^ We assume that job can be NULL. 290 job->sched->ops->timedout_job(job); 291 292 /* 293 * Guilty job did complete and hence needs to be manually removed 294 * See drm_sched_stop doc. 295 */ 296 if (sched->free_guilty) { Originally (last week) this check was "if (list_empty(&job->node))" which is obviously problematic if job is NULL. It's not clear to me that this new check ensures that job is non-NULL either. 297 job->sched->ops->free_job(job); ^ Dereference. 298 sched->free_guilty = false; 299 } 300 301 spin_lock_irqsave(&sched->job_list_lock, flags); 302 drm_sched_start_timeout(sched); 303 spin_unlock_irqrestore(&sched->job_list_lock, flags); 304 } regards, dan carpenter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [bug report] drm/scheduler: rework job destruction
Thanks for letting know, I will send a fix soon. Andrey On 5/22/19 9:07 AM, Dan Carpenter wrote: > [CAUTION: External Email] > > Hello Christian König, > > The patch 5918045c4ed4: "drm/scheduler: rework job destruction" from > Apr 18, 2019, leads to the following static checker warning: > > drivers/gpu/drm/scheduler/sched_main.c:297 drm_sched_job_timedout() > error: potential NULL dereference 'job'. > > drivers/gpu/drm/scheduler/sched_main.c > 279 static void drm_sched_job_timedout(struct work_struct *work) > 280 { > 281 struct drm_gpu_scheduler *sched; > 282 struct drm_sched_job *job; > 283 unsigned long flags; > 284 > 285 sched = container_of(work, struct drm_gpu_scheduler, > work_tdr.work); > 286 job = list_first_entry_or_null(&sched->ring_mirror_list, > 287 struct drm_sched_job, node); > 288 > 289 if (job) > ^^^ > We assume that job can be NULL. > > 290 job->sched->ops->timedout_job(job); > 291 > 292 /* > 293 * Guilty job did complete and hence needs to be manually > removed > 294 * See drm_sched_stop doc. > 295 */ > 296 if (sched->free_guilty) { > > Originally (last week) this check was "if (list_empty(&job->node))" > which is obviously problematic if job is NULL. It's not clear to me > that this new check ensures that job is non-NULL either. > > 297 job->sched->ops->free_job(job); > ^ > Dereference. > > 298 sched->free_guilty = false; > 299 } > 300 > 301 spin_lock_irqsave(&sched->job_list_lock, flags); > 302 drm_sched_start_timeout(sched); > 303 spin_unlock_irqrestore(&sched->job_list_lock, flags); > 304 } > > regards, > dan carpenter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sched: Fix static checker warning for potential NULL ptr
Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/scheduler/sched_main.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 90d7a82..ec7faca 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -286,16 +286,17 @@ static void drm_sched_job_timedout(struct work_struct *work) job = list_first_entry_or_null(&sched->ring_mirror_list, struct drm_sched_job, node); - if (job) + if (job) { job->sched->ops->timedout_job(job); - /* -* Guilty job did complete and hence needs to be manually removed -* See drm_sched_stop doc. -*/ - if (sched->free_guilty) { - job->sched->ops->free_job(job); - sched->free_guilty = false; + /* +* Guilty job did complete and hence needs to be manually removed +* See drm_sched_stop doc. +*/ + if (sched->free_guilty) { + job->sched->ops->free_job(job); + sched->free_guilty = false; + } } spin_lock_irqsave(&sched->job_list_lock, flags); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #3 from Eric Engestrom --- (In reply to alvarex from comment #2) > now the bug has hit 19.0.5. Are you saying the bug was not present in 19.0.4, but is now present in 19.0.5? If so, that's only 22 new commits, 10 if you exclude obvious non-candidates. If you can reliably test this, that's at most 5 build & test you need to do when bisecting mesa-19.0.4..mesa-19.0.5; could you do this please? It would greatly help figure out the cause :) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/sched: Fix static checker warning for potential NULL ptr
Am 22.05.19 um 15:57 schrieb Andrey Grodzovsky: Signed-off-by: Andrey Grodzovsky Reviewed-by: Christian König --- drivers/gpu/drm/scheduler/sched_main.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 90d7a82..ec7faca 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -286,16 +286,17 @@ static void drm_sched_job_timedout(struct work_struct *work) job = list_first_entry_or_null(&sched->ring_mirror_list, struct drm_sched_job, node); - if (job) + if (job) { job->sched->ops->timedout_job(job); - /* -* Guilty job did complete and hence needs to be manually removed -* See drm_sched_stop doc. -*/ - if (sched->free_guilty) { - job->sched->ops->free_job(job); - sched->free_guilty = false; + /* +* Guilty job did complete and hence needs to be manually removed +* See drm_sched_stop doc. +*/ + if (sched->free_guilty) { + job->sched->ops->free_job(job); + sched->free_guilty = false; + } } spin_lock_irqsave(&sched->job_list_lock, flags); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #4 from alvarex --- yes the bug was not present on 19.0.4. I'll give it a shot, I don't compile mesa from git in a standard way, I compile it on opensuse build service. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110729] Broken links on Linux Man Pages
https://bugs.freedesktop.org/show_bug.cgi?id=110729 Eric Engestrom changed: What|Removed |Added Product|Mesa|DRI Version|19.1|unspecified Component|Other |libdrm QA Contact|mesa-dev@lists.freedesktop. | |org | Assignee|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop |org |.org --- Comment #1 from Eric Engestrom --- We have no control over that website :) Beside, this is a libdrm issue, not Mesa, so reassigning. The actual issue is that not enough documentation has been written, including several (most) functions that have none. This is something that we've known for a while but haven't dealt with yet. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 4/6] dt-bindings: gpu: mali-midgard: Add H6 mali gpu compatible
This add the H6 mali compatible in the dt-bindings to later support specific implementation. Signed-off-by: Clément Péron Reviewed-by: Rob Herring --- .../devicetree/bindings/gpu/arm,mali-midgard.txt | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt index 2e8bbce35695..4bf17e1cf555 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt @@ -15,6 +15,7 @@ Required properties: + "arm,mali-t860" + "arm,mali-t880" * which must be preceded by one of the following vendor specifics: ++ "allwinner,sun50i-h6-mali" + "amlogic,meson-gxm-mali" + "rockchip,rk3288-mali" + "rockchip,rk3399-mali" @@ -49,9 +50,15 @@ Vendor-specific bindings The Mali GPU is integrated very differently from one SoC to -another. In order to accomodate those differences, you have the option +another. In order to accommodate those differences, you have the option to specify one more vendor-specific compatible, among: +- "allwinner,sun50i-h6-mali" + Required properties: + - clocks : phandles to core and bus clocks + - clock-names : must contain "core" and "bus" + - resets: phandle to GPU reset line + - "amlogic,meson-gxm-mali" Required properties: - resets : Should contain phandles of : -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RE-RESEND 1/2] drm/panel: Add support for Armadeus ST0700 Adapt
Hello Sam, On 5/8/19 11:06 AM, Daniel Vetter wrote: > On Wed, May 08, 2019 at 10:33:03AM +0200, Daniel Vetter wrote: >> On Tue, May 07, 2019 at 06:19:50PM +0200, Sam Ravnborg wrote: >>> Hi Fabio >>> >>> On Tue, May 07, 2019 at 12:33:39PM -0300, Fabio Estevam wrote: [Adding Sam, who is helping to review/collect panel-simple patches] On Tue, May 7, 2019 at 12:27 PM Sébastien Szymanski wrote: > > This patch adds support for the Armadeus ST0700 Adapt. It comes with a > Santek ST0700I5Y-RBSLW 7.0" WVGA (800x480) TFT and an adapter board so > that it can be connected on the TFT header of Armadeus Dev boards. > > Cc: sta...@vger.kernel.org # v4.19 > Reviewed-by: Rob Herring > Signed-off-by: Sébastien Szymanski >>> Reviewed-by: Sam Ravnborg >>> >>> If you wil lresend the patch I can apply it. >>> I have lost the original mail. >> >> Usually patchwork should have it already (and you can pipe the raw >> patchwork mbox into dim apply), but somehow it's not there either. >> Not sure why, sometimes this is because mails are stuck in moderation, >> sometimes because people do interesting things with their mails (e.g. smtp >> servers mangling formatting). > > patchwork was just a bit slow, it's there now: > > https://patchwork.freedesktop.org/series/60408/ > Will you take the patch from patchwork or should I resent it ? Regards, > Cheers, Daniel > -- Sébastien Szymanski Software engineer, Armadeus Systems Tel: +33 (0)9 72 29 41 44 Fax: +33 (0)9 72 28 79 26 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
lima_bo memory leak after drm_sched job destruction rework
Hello, I have recently observed a memory leak issue with lima using drm-misc-next, which I initially reported here: https://gitlab.freedesktop.org/lima/linux/issues/24 It is an easily reproduceable memory leak which I was able to bisect to commit: 5918045c4ed4 drm/scheduler: rework job destruction After some investigation, it seems that after the refactor, sched->ops->free_job (in lima: lima_sched_free_job) is no longer called. With some more debugging I found that it is not being called because the job freeing is now in drm_sched_cleanup_jobs, which for lima always aborts in the initial "Don't destroy jobs while the timeout worker is running" condition. Lima currently defaults to an "infinite" timeout. Setting a 500ms default timeout like most other drm_sched users do fixed the leak for me. I can send a patch to set a 500ms timeout and have it probably working again, but I am wondering now if this is expected behaviour for drm_sched after the refactor. In particular I also noticed that drm_sched_suspend_timeout is not called anywhere. Is it expected that we now rely on a timeout parameter to cleanup jobs that ran successfully? Erico ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu: fix typos in code comments
fix eror to error Signed-off-by: Weitao Hou --- drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c index f70437aae8e0..df422440845b 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c @@ -183,8 +183,8 @@ static bool calculate_fb_and_fractional_fb_divider( *RETURNS: * It fills the PLLSettings structure with PLL Dividers values * if calculated values are within required tolerance -* It returns - true if eror is within tolerance -* - false if eror is not within tolerance +* It returns - true if error is within tolerance +* - false if error is not within tolerance */ static bool calc_fb_divider_checking_tolerance( struct calc_pll_clock_source *calc_pll_cs, -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RESEND] drm/msm/mdp5: Fix mdp5_cfg_init error return
If mdp5_cfg_init fails because of an unknown major version, a null pointer dereference occurs. This is because the caller of init expects error pointers, but init returns NULL on error. Fix this by returning the expected values on error. Fixes: 2e362e1772b8 (drm/msm/mdp5: introduce mdp5_cfg module) Signed-off-by: Jeffrey Hugo Reviewed-by: Bjorn Andersson --- drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c index ea8f7d7daf7f..52e23780fce1 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c @@ -721,7 +721,7 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms, if (cfg_handler) mdp5_cfg_destroy(cfg_handler); - return NULL; + return ERR_PTR(ret); } static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: lima_bo memory leak after drm_sched job destruction rework
On Fri, May 17, 2019 at 10:43 PM Grodzovsky, Andrey wrote: > On 5/17/19 3:35 PM, Erico Nunes wrote: > > Lima currently defaults to an "infinite" timeout. Setting a 500ms > > default timeout like most other drm_sched users do fixed the leak for > > me. > > I am not very clear about the problem - so you basically never allow a > time out handler to run ? And then when the job hangs for ever you get > this memory leak ? How it worked for you before this refactoring ? As > far as I remember sched->ops->free_job before this change was called > from drm_sched_job_finish which is the work scheduled from HW fence > signaled callback - drm_sched_process_job so if your job hangs for ever > anyway and this work is never scheduled when your free_job callback was > called ? In this particular case, the jobs run successfully, nothing hangs. Lima currently specifies an "infinite" timeout to the drm scheduler, so if a job did did hang, it would hang forever, I suppose. But this is not the issue. If I understand correctly it worked well before the rework because the cleanup was triggered at the end of drm_sched_process_job independently on the timeout. What I'm observing now is that even when jobs run successfully, they are not cleaned by the drm scheduler because drm_sched_cleanup_jobs seems to give up based on the status of a timeout worker. I would expect the timeout value to only be relevant in error/hung job cases. I will probably set the timeout to a reasonable value anyway, I just posted here to report that this can possibly be a bug in the drm scheduler after that rework. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dt-bindings: backlight: lm3630a: correct schema validation
The '#address-cells' and '#size-cells' properties were not defined in the lm3630a bindings and would cause the following error when attempting to validate the examples against the schema: Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml: '#address-cells', '#size-cells' do not match any of the regexes: '^led@[01]$', 'pinctrl-[0-9]+' Correct this by adding those two properties. While we're here, move the ti,linear-mapping-mode property to the led@[01] child nodes to correct the following validation error: Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml: led@0: 'ti,linear-mapping-mode' does not match any of the regexes: 'pinctrl-[0-9]+' Fixes: 32fcb75c66a0 ("dt-bindings: backlight: Add lm3630a bindings") Signed-off-by: Brian Masney Reported-by: Rob Herring --- .../leds/backlight/lm3630a-backlight.yaml | 20 +-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml index 4d61fe0a98a4..f0855e248ae5 100644 --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -23,16 +23,17 @@ properties: reg: maxItems: 1 - ti,linear-mapping-mode: -description: | - Enable linear mapping mode. If disabled, then it will use exponential - mapping mode in which the ramp up/down appears to have a more uniform - transition to the human eye. -type: boolean + '#address-cells': +const: 1 + + '#size-cells': +const: 0 required: - compatible - reg + - '#address-cells' + - '#size-cells' patternProperties: "^led@[01]$": @@ -73,6 +74,13 @@ patternProperties: minimum: 0 maximum: 255 + ti,linear-mapping-mode: +description: | + Enable linear mapping mode. If disabled, then it will use exponential + mapping mode in which the ramp up/down appears to have a more uniform + transition to the human eye. +type: boolean + required: - reg -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/msm: Fix improper uses of smp_mb__{before, after}_atomic()
These barriers only apply to the read-modify-write operations; in particular, they do not apply to the atomic_set() primitive. Replace the barriers with smp_mb()s. Fixes: b1fc2839d2f92 ("drm/msm: Implement preemption for A5XX targets") Cc: sta...@vger.kernel.org Reported-by: "Paul E. McKenney" Reported-by: Peter Zijlstra Signed-off-by: Andrea Parri Cc: Rob Clark Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Cc: Jordan Crouse Cc: linux-arm-...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: freedr...@lists.freedesktop.org Cc: "Paul E. McKenney" Cc: Peter Zijlstra --- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c index 3d62310a535fb..ee0820ee0c664 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c @@ -39,10 +39,10 @@ static inline void set_preempt_state(struct a5xx_gpu *gpu, * preemption or in the interrupt handler so barriers are needed * before... */ - smp_mb__before_atomic(); + smp_mb(); atomic_set(&gpu->preempt_state, new); /* ... and after*/ - smp_mb__after_atomic(); + smp_mb(); } /* Write the most recent wptr for the given ring into the hardware */ -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu: drm: use struct_size() in kmalloc()
Use struct_size() helper to keep code simple. Signed-off-by: xiaolinkui --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 22bd21e..4717a64 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1375,8 +1375,7 @@ int amdgpu_ras_init(struct amdgpu_device *adev) if (con) return 0; - con = kmalloc(sizeof(struct amdgpu_ras) + - sizeof(struct ras_manager) * AMDGPU_RAS_BLOCK_COUNT, + con = kmalloc(struct_size(con, objs, AMDGPU_RAS_BLOCK_COUNT), GFP_KERNEL|__GFP_ZERO); if (!con) return -ENOMEM; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 1/6] drm: panfrost: add optional bus_clock
Allwinner H6 has an ARM Mali-T720 MP2 which required a bus_clock. Add an optional bus_clock at the init of the panfrost driver. Signed-off-by: Clément Péron --- drivers/gpu/drm/panfrost/panfrost_device.c | 22 ++ drivers/gpu/drm/panfrost/panfrost_device.h | 1 + 2 files changed, 23 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 3b2bced1b015..ccb8eb2a518c 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -55,11 +55,33 @@ static int panfrost_clk_init(struct panfrost_device *pfdev) if (err) return err; + pfdev->bus_clock = devm_clk_get_optional(pfdev->dev, "bus"); + if (IS_ERR(pfdev->bus_clock)) { + dev_err(pfdev->dev, "get bus_clock failed %ld\n", + PTR_ERR(pfdev->bus_clock)); + return PTR_ERR(pfdev->bus_clock); + } + + if (pfdev->bus_clock) { + rate = clk_get_rate(pfdev->bus_clock); + dev_info(pfdev->dev, "bus_clock rate = %lu\n", rate); + + err = clk_prepare_enable(pfdev->bus_clock); + if (err) + goto disable_clock; + } + return 0; + +disable_clock: + clk_disable_unprepare(pfdev->clock); + + return err; } static void panfrost_clk_fini(struct panfrost_device *pfdev) { + clk_disable_unprepare(pfdev->bus_clock); clk_disable_unprepare(pfdev->clock); } diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 56f452dfb490..8074f221034b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -66,6 +66,7 @@ struct panfrost_device { void __iomem *iomem; struct clk *clock; + struct clk *bus_clock; struct regulator *regulator; struct reset_control *rstc; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 3/6] dt-bindings: gpu: add bus clock for Mali Midgard GPUs
From: Icenowy Zheng Some SoCs adds a bus clock gate to the Mali Midgard GPU. Add the binding for the bus clock. Signed-off-by: Icenowy Zheng Signed-off-by: Clément Péron Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt index 1b1a74129141..2e8bbce35695 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt @@ -31,6 +31,12 @@ Optional properties: - clocks : Phandle to clock for the Mali Midgard device. +- clock-names : Specify the names of the clocks specified in clocks + when multiple clocks are present. +* core: clock driving the GPU itself (When only one clock is present, + assume it's this clock.) +* bus: bus clock for the GPU + - mali-supply : Phandle to regulator for the Mali device. Refer to Documentation/devicetree/bindings/regulator/regulator.txt for details. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/2] drm/panfrost: drm_gem_map_offset() helper
Panfrost has a re-implementation of drm_gem_dumb_map_offset() with an extra bug regarding the handling of imported buffers. However we don't really want Panfrost calling _dumb functions because it's not a KMS driver. This series renames drm_gem_dumb_map_offset() to drop the '_dumb' and updates Panfrost to use it rather than it's own implementation. v2: https://lore.kernel.org/lkml/20190516141447.46839-1-steven.pr...@arm.com/ Changes since v2: * Drop the shmem helper v1: https://lore.kernel.org/lkml/20190513143244.16478-1-steven.pr...@arm.com/ Changes since v1: * Rename drm_gem_dumb_map_offset to drop _dumb * Add a shmem helper Steven Price (2): drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset() drm/panfrost: Use drm_gem_shmem_map_offset() drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 6 +++--- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 +-- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-- include/drm/drm_gem.h | 4 ++-- 5 files changed, 10 insertions(+), 23 deletions(-) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: fbdev: mxsfb: Remove driver
On 5/20/19 3:06 PM, Fabio Estevam wrote: > There is a DRM version of the mxsfb driver for quite some time > at drivers/gpu/drm/mxsfb/, so there is no need to keep maintaining > the fbdev version any longer. > > Remove the fbdev mxsfb driver in favour of the DRM version. > > Signed-off-by: Fabio Estevam Acked-by: Marek Vasut -- Best regards, Marek Vasut ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 0/6] Allwinner H6 Mali GPU support
Hi, The Allwinner H6 has a Mali-T720 MP2 which should be supported by the new panfrost driver. This series fix two issues and introduce the dt-bindings but a simple benchmark show that it's still NOT WORKING. I'm pushing it in case someone want to continue the work. This has been tested with Mesa3D 19.1.0-RC2 and a GPU bitness patch[1]. One patch is from Icenowy Zheng where I changed the order as required by Rob Herring[2]. Thanks, Clement [1] https://gitlab.freedesktop.org/kszaq/mesa/tree/panfrost_64_32 [2] https://patchwork.kernel.org/patch/10699829/ [ 345.204813] panfrost 180.gpu: mmu irq status=1 [ 345.209617] panfrost 180.gpu: Unhandled Page fault in AS0 at VA 0x02400400 [ 345.209617] Reason: TODO [ 345.209617] raw fault status: 0x82C1 [ 345.209617] decoded fault status: SLAVE FAULT [ 345.209617] exception type 0xC1: TRANSLATION_FAULT_LEVEL1 [ 345.209617] access type 0x2: READ [ 345.209617] source id 0x8000 [ 345.729957] panfrost 180.gpu: gpu sched timeout, js=0, status=0x8, head=0x2400400, tail=0x2400400, sched_job=9e204de9 [ 346.055876] panfrost 180.gpu: mmu irq status=1 [ 346.060680] panfrost 180.gpu: Unhandled Page fault in AS0 at VA 0x02C00A00 [ 346.060680] Reason: TODO [ 346.060680] raw fault status: 0x810002C1 [ 346.060680] decoded fault status: SLAVE FAULT [ 346.060680] exception type 0xC1: TRANSLATION_FAULT_LEVEL1 [ 346.060680] access type 0x2: READ [ 346.060680] source id 0x8100 [ 346.561955] panfrost 180.gpu: gpu sched timeout, js=1, status=0x8, head=0x2c00a00, tail=0x2c00a00, sched_job=b55a9a85 [ 346.573913] panfrost 180.gpu: mmu irq status=1 [ 346.578707] panfrost 180.gpu: Unhandled Page fault in AS0 at VA 0x02C00B80 Change in v5: - Remove fix indent Changes in v4: - Add bus_clock probe - Fix sanity check in io-pgtable - Add vramp-delay - Merge all boards into one patch - Remove upstreamed Neil A. patch Change in v3 (Thanks to Maxime Ripard): - Reauthor Icenowy for her path Changes in v2 (Thanks to Maxime Ripard): - Drop GPU OPP Table - Add clocks and clock-names in required Clément Péron (5): drm: panfrost: add optional bus_clock iommu: io-pgtable: fix sanity check for non 48-bit mali iommu dt-bindings: gpu: mali-midgard: Add H6 mali gpu compatible arm64: dts: allwinner: Add ARM Mali GPU node for H6 arm64: dts: allwinner: Add mali GPU supply for H6 boards Icenowy Zheng (1): dt-bindings: gpu: add bus clock for Mali Midgard GPUs .../bindings/gpu/arm,mali-midgard.txt | 15 - .../dts/allwinner/sun50i-h6-beelink-gs1.dts | 6 + .../dts/allwinner/sun50i-h6-orangepi-3.dts| 6 + .../dts/allwinner/sun50i-h6-orangepi.dtsi | 6 + .../boot/dts/allwinner/sun50i-h6-pine-h64.dts | 6 + arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 14 drivers/gpu/drm/panfrost/panfrost_device.c| 22 +++ drivers/gpu/drm/panfrost/panfrost_device.h| 1 + drivers/iommu/io-pgtable-arm.c| 2 +- 9 files changed, 76 insertions(+), 2 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/nouveau/svm: Convert to use hmm_range_fault()
Convert to use hmm_range_fault(). Signed-off-by: Souptick Joarder --- drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index 93ed43c..8d56bd6 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -649,7 +649,7 @@ struct nouveau_svmm { range.values = nouveau_svm_pfn_values; range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT; again: - ret = hmm_vma_fault(&range, true); + ret = hmm_range_fault(&range, true); if (ret == 0) { mutex_lock(&svmm->mutex); if (!hmm_vma_range_done(&range)) { -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/2] drm/panfrost: Use drm_gem_shmem_map_offset()
panfrost_ioctl_mmap_bo() contains a reimplementation of drm_gem_map_offset() but with a bug - it allows mapping imported objects (without going through the exporter). Fix this by switching to use the newly renamed drm_gem_map_offset() function instead which has the bonus of simplifying the code. CC: Alyssa Rosenzweig Signed-off-by: Steven Price Reviewed-by: Alyssa Rosenzweig --- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index d11e2281dde6..8be0cd5d6c7a 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -255,26 +255,14 @@ static int panfrost_ioctl_mmap_bo(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_panfrost_mmap_bo *args = data; - struct drm_gem_object *gem_obj; - int ret; if (args->flags != 0) { DRM_INFO("unknown mmap_bo flags: %d\n", args->flags); return -EINVAL; } - gem_obj = drm_gem_object_lookup(file_priv, args->handle); - if (!gem_obj) { - DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle); - return -ENOENT; - } - - ret = drm_gem_create_mmap_offset(gem_obj); - if (ret == 0) - args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node); - drm_gem_object_put_unlocked(gem_obj); - - return ret; + return drm_gem_map_offset(file_priv, dev, args->handle, + &args->offset); } static int panfrost_ioctl_get_bo_offset(struct drm_device *dev, void *data, -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
XDC 2019: Registration & Call for Proposals now open!
Hello! Registration & Call for Proposals are now open for XDC 2019, which will take place at the Concordia University Conference Centre in Montréal, Canada on October 2-4, 2019. Thanks to LWN.net, this year we have a brand new website using the Indico platform, a fully open source event management tool! https://xdc2019.x.org If you plan on attending, please make sure to register as early as possible as space is limited! As usual, the conference is free of charge and open to the general public. Please note, as we are now based on Indico, we will no longer use the wiki for registration. You will therefore need to register via the XDC website. However, as XDC is sharing the same Indico infrastructure as LPC, if you previously registered on the LPC website (linuxplumbersconference.org), then you already have an active account and can use the same username & password to login! https://xdc2019.x.org/event/5/registrations/2/ In addition to registration, the CfP is now open for talks, workshops and demos at XDC 2019. While any serious proposal will be gratefully considered, topics of interest to X.Org and freedesktop.org developers are encouraged. The program focus is on new development, ongoing challenges and anything else that will spark discussions among attendees in the hallway track. We are open to talks across all layers of the graphics stack, from the kernel to desktop environments / graphical applications and about how to make things better for the developers who build them. Head to the CfP page to learn more: https://xdc2019.x.org/event/5/abstracts/ The deadline for submissions Sunday, 7 July 2019. We are looking forward to seeing you (and sharing a poutine!) in beautiful Montréal! If you have any questions, please send me an email to mark.fil...@collabora.com, adding on CC the X.org board (board at foundation.x.org). And don't forget, you can follow us on Twitter for all the latest updates and to stay connected: https://twitter.com/xdc2019 Best, Mark ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: WARNING: locking bug in inet_autobind
syzbot has bisected this bug to: commit c0d9271ecbd891cdeb0fad1edcdd99ee717a655f Author: Yong Zhao Date: Fri Feb 1 23:36:21 2019 + drm/amdgpu: Delete user queue doorbell variables bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1433ece4a0 start commit: f49aa1de Merge tag 'for-5.2-rc1-tag' of git://git.kernel.o.. git tree: net-next final crash:https://syzkaller.appspot.com/x/report.txt?x=1633ece4a0 console output: https://syzkaller.appspot.com/x/log.txt?x=1233ece4a0 kernel config: https://syzkaller.appspot.com/x/.config?x=fc045131472947d7 dashboard link: https://syzkaller.appspot.com/bug?extid=94cc2a66fc228b23f360 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=163731f8a0 Reported-by: syzbot+94cc2a66fc228b23f...@syzkaller.appspotmail.com Fixes: c0d9271ecbd8 ("drm/amdgpu: Delete user queue doorbell variables") For information about bisection process see: https://goo.gl/tpsmEJ#bisection ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2 03/22] drm/bridge: tc358767: fix ansi 8b10b use
On Mon, May 6, 2019 at 2:59 AM Tomi Valkeinen wrote: > > Hi Laurent, Andrey, > > On 03/05/2019 20:11, Laurent Pinchart wrote: > >> I agree that if the panel Andrey mentioned is still used, we need to > >> handle it somehow. But I think explicitly handling such a case is better > >> than guessing. > > > > The risk may not be worth it, I agree. I would explain this in details > > in the commit message though, and add a comment to the code as well, to > > ease future debugging. > > Andrey, do you still have the panel that doesn't work with 8b10b? Is it > used in real life (i.e. it was not just a temporary development phase > panel)? What's the model, and is there a public datasheet? Note that I am a different Andrey, and I can't speak about the original panel that caused the issue. However, production units are shipped with this panel: https://www.data-modul.com/productfinder/sites/default/files/products/NL192108AC13-02D_specification_12023727.pdf which seems to do pretty standard DP stuff. In all of my testing of Tomi's patches, I haven't seen any problems so far (I still have to test v3 though), so I think we can carefully proceed assuming that that weird panel was just a fluke. Thanks, Andrey Smirnov ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm_edid-load: Fix a missing-check bug in drivers/gpu/drm/drm_edid_load.c
On Wed, 22 May 2019, Gen Zhang wrote: > In drm_load_edid_firmware(), fwstr is allocated by kstrdup(). And fwstr > is dereferenced in the following codes. However, memory allocation > functions such as kstrdup() may fail and returns NULL. Dereferencing > this null pointer may cause the kernel go wrong. Thus we should check > this kstrdup() operation. > Further, if kstrdup() returns NULL, we should return ERR_PTR(-ENOMEM) to > the caller site. strsep() handles the NULL pointer just fine, so there won't be a NULL dereference. However this patch seems like the right thing to do anyway. Reviewed-by: Jani Nikula > > Signed-off-by: Gen Zhang > > --- > diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c > index a491509..a0e107a 100644 > --- a/drivers/gpu/drm/drm_edid_load.c > +++ b/drivers/gpu/drm/drm_edid_load.c > @@ -290,6 +290,8 @@ struct edid *drm_load_edid_firmware(struct drm_connector > *connector) >* the last one found one as a fallback. >*/ > fwstr = kstrdup(edid_firmware, GFP_KERNEL); > + if (!fwstr) > + return ERR_PTR(-ENOMEM); > edidstr = fwstr; > > while ((edidname = strsep(&edidstr, ","))) { > --- > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110575] [R9 380X] Artifacts in CSGO
https://bugs.freedesktop.org/show_bug.cgi?id=110575 --- Comment #3 from tempel.jul...@gmail.com --- Can you try R600_DEBUG=nohyperz? I have this kind of glitches in almost every game with OpenGL/Gallium Nine on Polaris and this variable seems to help at least in Skyrim with Gallium Nine (haven't tested anything else so far). My bug report: https://bugs.freedesktop.org/show_bug.cgi?id=110635 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/nouveau: remove open-coded drm_invalid_op()
From: Emil Velikov Cc: Ben Skeggs Cc: nouv...@lists.freedesktop.org Signed-off-by: Emil Velikov --- drivers/gpu/drm/nouveau/nouveau_abi16.c | 6 -- drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 - drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c index c3fd5dd39ed9..0c585dc5f5c3 100644 --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c @@ -244,12 +244,6 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS) return 0; } -int -nouveau_abi16_ioctl_setparam(ABI16_IOCTL_ARGS) -{ - return -EINVAL; -} - int nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS) { diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.h b/drivers/gpu/drm/nouveau/nouveau_abi16.h index 36fde1ff3ad5..9275d529b947 100644 --- a/drivers/gpu/drm/nouveau/nouveau_abi16.h +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.h @@ -6,7 +6,6 @@ struct drm_device *dev, void *data, struct drm_file *file_priv int nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS); -int nouveau_abi16_ioctl_setparam(ABI16_IOCTL_ARGS); int nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS); int nouveau_abi16_ioctl_channel_free(ABI16_IOCTL_ARGS); int nouveau_abi16_ioctl_grobj_alloc(ABI16_IOCTL_ARGS); diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 22cd45845e07..ed45ad2b72f2 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -1046,7 +1046,7 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv) static const struct drm_ioctl_desc nouveau_ioctls[] = { DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_SETPARAM, nouveau_abi16_ioctl_setparam, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), + DRM_IOCTL_DEF_DRV(NOUVEAU_SETPARAM, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_AUTH|DRM_RENDER_ALLOW), -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/omap: remove open-coded drm_invalid_op()
From: Emil Velikov Cc: Tomi Valkeinen Signed-off-by: Emil Velikov --- drivers/gpu/drm/omapdrm/omap_drv.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index 1b9b6f5e48e1..672e0f8ad11c 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -439,20 +439,6 @@ static int ioctl_get_param(struct drm_device *dev, void *data, return 0; } -static int ioctl_set_param(struct drm_device *dev, void *data, - struct drm_file *file_priv) -{ - struct drm_omap_param *args = data; - - switch (args->param) { - default: - DBG("unknown parameter %lld", args->param); - return -EINVAL; - } - - return 0; -} - #define OMAP_BO_USER_MASK 0x00ff /* flags settable by userspace */ static int ioctl_gem_new(struct drm_device *dev, void *data, @@ -492,7 +478,7 @@ static int ioctl_gem_info(struct drm_device *dev, void *data, static const struct drm_ioctl_desc ioctls[DRM_COMMAND_END - DRM_COMMAND_BASE] = { DRM_IOCTL_DEF_DRV(OMAP_GET_PARAM, ioctl_get_param, DRM_AUTH | DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, ioctl_set_param, + DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, drm_invalid_op, DRM_AUTH | DRM_MASTER | DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(OMAP_GEM_NEW, ioctl_gem_new, DRM_AUTH | DRM_RENDER_ALLOW), -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107612] [Vega10] Hard Lock [gfxhub] VMC page fault when opening Mario Kart 8 in Cemu under wine
https://bugs.freedesktop.org/show_bug.cgi?id=107612 --- Comment #7 from bibitocarlos --- Yeah, it's fixed. thanks -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: remove leftover files
On Wed, May 22, 2019 at 12:33:07PM +0200, Gerd Hoffmann wrote: > cirrus_drv.h and cirrus_ttm.c are unused since commit ab3e023b1b4c > ("drm/cirrus: rewrite and modernize driver"), apparently I ran "rm" > instead of "git rm" on them so they are still in present the tree. > > Signed-off-by: Gerd Hoffmann Reviewed-by: Sam Ravnborg Always nice with the code removal patches. Will you apply yourself? Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110575] [R9 380X] Artifacts in CSGO
https://bugs.freedesktop.org/show_bug.cgi?id=110575 --- Comment #4 from Danylo --- I'll try it later when will come home, thanks. It also happens in Minecraft but much rarer (so I wasn't patient enough to bisect with it). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag
From: Emil Velikov DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it. Cc: Thierry Reding Cc: linux-te...@vger.kernel.org Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/tegra/drm.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 0c5f1e6a0446..8836c113b392 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -891,33 +891,33 @@ static int tegra_gem_get_flags(struct drm_device *drm, void *data, static const struct drm_ioctl_desc tegra_drm_ioctls[] = { #ifdef CONFIG_DRM_TEGRA_STAGING DRM_IOCTL_DEF_DRV(TEGRA_GEM_CREATE, tegra_gem_create, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_GEM_MMAP, tegra_gem_mmap, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_READ, tegra_syncpt_read, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_INCR, tegra_syncpt_incr, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_WAIT, tegra_syncpt_wait, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_OPEN_CHANNEL, tegra_open_channel, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_CLOSE_CHANNEL, tegra_close_channel, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT, tegra_get_syncpt, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_SUBMIT, tegra_submit, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT_BASE, tegra_get_syncpt_base, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_TILING, tegra_gem_set_tiling, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_FLAGS, tegra_gem_set_flags, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_FLAGS, tegra_gem_get_flags, - DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), #endif }; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED
From: Emil Velikov Should minimise the copy/paste mistakes, fixed with previous patches. Cc: Daniel Vetter Signed-off-by: Emil Velikov --- Daniel, not 100% sold on the idea. That plus listing you as a contact point ;-) What do you thing? Emil --- Documentation/gpu/todo.rst | 19 +++ 1 file changed, 19 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 66f05f4e469f..9e67d125f2fd 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -397,6 +397,25 @@ Some of these date from the very introduction of KMS in 2008 ... end, for which we could add drm_*_cleanup_kfree(). And then there's the (for historical reasons) misnamed drm_primary_helper_destroy() function. +Use DRM_LOCKED instead of DRM_UNLOCKED +-- + +DRM_UNLOCKED is a remainder from the legacy DRM drivers. Seemingly drivers get +tricked by it and it ends up in the driver private ioctls. + +Today no more legacy drivers are allowed and most core DRM ioctls are unlocked. + +Introduce DRM_LOCKED, use it to annotate only the relevant ioctls and kill the +old DRM_UNLOCKED. + +Patch series should be split as follows: + - Patch 1: drm: add the new DRM_LOCKED flag and honour it + - Patch 2: drm: convert core ioctls from DRM_UNLOCKED to DRM_LOCKED + - Patch 3-...: drm/driverX: convert driver ioctls from ... + - Patch X: drm: remove no longer used DRM_UNLOCKED, drop todo item + +Contact: Emil Velikov, Daniel Vetter + Better Testing == -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/virtio: remove irrelevant DRM_UNLOCKED flag
From: Emil Velikov DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it. Cc: David Airlie Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 949a264985fc..b7f9dfe61d1c 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -553,34 +553,34 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS] = { DRM_IOCTL_DEF_DRV(VIRTGPU_MAP, virtio_gpu_map_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_EXECBUFFER, virtio_gpu_execbuffer_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_GETPARAM, virtio_gpu_getparam_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_RESOURCE_CREATE, virtio_gpu_resource_create_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_RESOURCE_INFO, virtio_gpu_resource_info_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), /* make transfer async to the main ring? - no sure, can we * thread these in the underlying GL */ DRM_IOCTL_DEF_DRV(VIRTGPU_TRANSFER_FROM_HOST, virtio_gpu_transfer_from_host_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_TRANSFER_TO_HOST, virtio_gpu_transfer_to_host_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_WAIT, virtio_gpu_wait_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_GET_CAPS, virtio_gpu_get_caps_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), }; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/i915: remove irrelevant DRM_UNLOCKED flag
From: Emil Velikov DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it. Cc: intel-...@lists.freedesktop.org Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/i915/i915_drv.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 1ad88e6d7c04..a18c155cff88 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -3145,9 +3145,9 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_RENDER_ALLOW), }; static struct drm_driver driver = { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #5 from alvarex --- I need help, I build up mesa and set the git bisect good and bad commits, but steam does not start because it needs 32bits libs and I can't seem to build them. What option should I pass to meson or ninja to compile 32 bit? I can't trigger the bug on chrome, chrome or chromium do launch but the bug doesn't trigger . @Ropid what do you mean with Visual Studio Code? where should I find that extension? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED
On Wed, May 22, 2019 at 04:47:02PM +0100, Emil Velikov wrote: > From: Emil Velikov > > Should minimise the copy/paste mistakes, fixed with previous patches. > > Cc: Daniel Vetter > Signed-off-by: Emil Velikov > --- > Daniel, not 100% sold on the idea. That plus listing you as a contact > point ;-) > > What do you thing? > Emil > --- > Documentation/gpu/todo.rst | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 66f05f4e469f..9e67d125f2fd 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -397,6 +397,25 @@ Some of these date from the very introduction of KMS in > 2008 ... >end, for which we could add drm_*_cleanup_kfree(). And then there's the > (for >historical reasons) misnamed drm_primary_helper_destroy() function. > > +Use DRM_LOCKED instead of DRM_UNLOCKED > +-- > + > +DRM_UNLOCKED is a remainder from the legacy DRM drivers. Seemingly drivers > get > +tricked by it and it ends up in the driver private ioctls. > + > +Today no more legacy drivers are allowed and most core DRM ioctls are > unlocked. > + > +Introduce DRM_LOCKED, use it to annotate only the relevant ioctls and kill > the > +old DRM_UNLOCKED. > + > +Patch series should be split as follows: > + - Patch 1: drm: add the new DRM_LOCKED flag and honour it > + - Patch 2: drm: convert core ioctls from DRM_UNLOCKED to DRM_LOCKED > + - Patch 3-...: drm/driverX: convert driver ioctls from ... > + - Patch X: drm: remove no longer used DRM_UNLOCKED, drop todo item Seems like too much bother for legacy drivers ... What I'd do is something a lot cheaper, since all we're touching here are legacy drivers: Remove DRM_UNLOCKED from everything except the old vblank ioctl. That one we need to keep, because it freezes X: commit 8f4ff2b06afcd6f151868474a432c603057eaf56 Author: Ilija Hadzic Date: Mon Oct 31 17:46:18 2011 -0400 drm: do not sleep on vblank while holding a mutex Anything else I think is either never used by legacy userspace, or just doesn't matter. That's simple enough that I don't think we really need a todo for it :-) I guess if you want to kill DRM_UNLOCKED you could replace the check with ioctl->func == drm_vblank_ioctl, ofc only in the DRIVER_LEGACY path. On patches 1-3 in your series: Reviewed-by: Daniel Vetter Cheers, Daniel > + > +Contact: Emil Velikov, Daniel Vetter > + > Better Testing > == > > -- > 2.21.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/12] dma-buf: add dynamic caching of sg_table
Hi Christian, On Sat, 27 Apr 2019 at 05:31, Liam Mark wrote: > > On Tue, 16 Apr 2019, Christian König wrote: > > > To allow a smooth transition from pinning buffer objects to dynamic > > invalidation we first start to cache the sg_table for an attachment > > unless the driver explicitly says to not do so. > > > > --- > > drivers/dma-buf/dma-buf.c | 24 > > include/linux/dma-buf.h | 11 +++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > > index 7c858020d14b..65161a82d4d5 100644 > > --- a/drivers/dma-buf/dma-buf.c > > +++ b/drivers/dma-buf/dma-buf.c > > @@ -573,6 +573,20 @@ struct dma_buf_attachment *dma_buf_attach(struct > > dma_buf *dmabuf, > > list_add(&attach->node, &dmabuf->attachments); > > > > mutex_unlock(&dmabuf->lock); > > + > > + if (!dmabuf->ops->dynamic_sgt_mapping) { > > + struct sg_table *sgt; > > + > > + sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL); > > + if (!sgt) > > + sgt = ERR_PTR(-ENOMEM); > > + if (IS_ERR(sgt)) { > > + dma_buf_detach(dmabuf, attach); > > + return ERR_CAST(sgt); > > + } > > + attach->sgt = sgt; > > + } > > + > > return attach; > > > > err_attach: > > @@ -595,6 +609,10 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct > > dma_buf_attachment *attach) > > if (WARN_ON(!dmabuf || !attach)) > > return; > > > > + if (attach->sgt) > > + dmabuf->ops->unmap_dma_buf(attach, attach->sgt, > > +DMA_BIDIRECTIONAL); > > + > > mutex_lock(&dmabuf->lock); > > list_del(&attach->node); > > if (dmabuf->ops->detach) > > @@ -630,6 +648,9 @@ struct sg_table *dma_buf_map_attachment(struct > > dma_buf_attachment *attach, > > if (WARN_ON(!attach || !attach->dmabuf)) > > return ERR_PTR(-EINVAL); > > > > + if (attach->sgt) > > + return attach->sgt; > > + > > I am concerned by this change to make caching the sg_table the default > behavior as this will result in the exporter's map_dma_buf/unmap_dma_buf > calls are no longer being called in > dma_buf_map_attachment/dma_buf_unmap_attachment. Probably this concern from Liam got lost between versions of your patches; could we please request a reply to these points here? > > This seems concerning to me as it appears to ignore the cache maintenance > aspect of the map_dma_buf/unmap_dma_buf calls. > For example won't this potentially cause issues for clients of ION. > > If we had the following > - #1 dma_buf_attach coherent_device > - #2 dma_buf attach non_coherent_device > - #3 dma_buf_map_attachment non_coherent_device > - #4 non_coherent_device writes to buffer > - #5 dma_buf_unmap_attachment non_coherent_device > - #6 dma_buf_map_attachment coherent_device > - #7 coherent_device reads buffer > - #8 dma_buf_unmap_attachment coherent_device > > There wouldn't be any CMO at step #5 anymore (specifically no invalidate) > so now at step #7 the coherent_device could read a stale cache line. > > Also, now by default dma_buf_unmap_attachment no longer removes the > mappings from the iommu, so now by default dma_buf_unmap_attachment is not > doing what I would expect and clients are losing the potential sandboxing > benefits of removing the mappings. > Shouldn't this caching behavior be something that clients opt into instead > of being the default? > > Liam > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > Best, Sumit. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #6 from Ropid --- @alvarex: Try starting chromium with this command line here, it makes it show corruption everywhere for me here: chromium --ignore-gpu-blacklist --enable-gpu-rasterization --enable-native-gpu-memory-buffers --enable-zero-copy --disable-gpu-driver-bug-workarounds About Visual Studio Code, I see corruption when I click on the main menu and move the mouse around to open and close the different menus there. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #7 from alvarex --- (In reply to Ropid from comment #6) > @alvarex: > > Try starting chromium with this command line here, it makes it show > corruption everywhere for me here: > > chromium --ignore-gpu-blacklist --enable-gpu-rasterization > --enable-native-gpu-memory-buffers --enable-zero-copy > --disable-gpu-driver-bug-workarounds > > About Visual Studio Code, I see corruption when I click on the main menu and > move the mouse around to open and close the different menus there. no the bug doesn't trigger with chromium unless this error has something to do maybe chrome is not detecting the gpu (same as steam) > ac: Unknown GPU, using 0 for raster_config > [1:7:0522/132124.876161:ERROR:command_buffer_proxy_impl.cc(125)] > ContextResult::kTransientFailure: Failed to send > GpuChannelMsg_CreateCommandBuffer. > [31337:31346:0522/132131.668335:ERROR:browser_process_sub_thread.cc(217)] > Waited 322 ms for network service @Ropid what version of chromium are u using?? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #8 from Ropid --- $ chromium --version Chromium 74.0.3729.157 Arch Linux -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] backlight: pwm_bl: Set pin to sleep state when powered down
When the driver probes, the PWM pin is automatically configured to its default state, which should be the "pwm" function. However, at this point we don't know the actual level of the pin, which may be active or inactive. As a result, if the driver probes without enabling the backlight, the PWM pin might be active, and the backlight would be lit way before being officially enabled. To work around this, if the probe function doesn't enable the backlight, the pin is set to its sleep state instead of the default one, until the backlight is enabled. When the backlight is disabled, the pin is reset to its sleep state. Signed-off-by: Paul Cercueil --- drivers/video/backlight/pwm_bl.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index fb45f866b923..422f7903b382 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -50,6 +51,8 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb) struct pwm_state state; int err; + pinctrl_pm_select_default_state(pb->dev); + pwm_get_state(pb->pwm, &state); if (pb->enabled) return; @@ -90,6 +93,8 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb) regulator_disable(pb->power_supply); pb->enabled = false; + + pinctrl_pm_select_sleep_state(pb->dev); } static int compute_duty_cycle(struct pwm_bl_data *pb, int brightness) @@ -626,6 +631,10 @@ static int pwm_backlight_probe(struct platform_device *pdev) backlight_update_status(bl); platform_set_drvdata(pdev, bl); + + if (bl->props.power == FB_BLANK_POWERDOWN) + pinctrl_pm_select_sleep_state(&pdev->dev); + return 0; err_alloc: -- 2.21.0.593.g511ec345e18
[PATCH 1/5] vmwgfx: drop empty lastclose stub
From: Emil Velikov Core DRM is safe when the callback is NULL. Cc: "VMware Graphics" Cc: Thomas Hellstrom Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index be25ce9440ad..a38f06909fb6 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -1200,10 +1200,6 @@ static long vmw_compat_ioctl(struct file *filp, unsigned int cmd, } #endif -static void vmw_lastclose(struct drm_device *dev) -{ -} - static void vmw_master_init(struct vmw_master *vmaster) { ttm_lock_init(&vmaster->lock); @@ -1568,7 +1564,6 @@ static struct drm_driver driver = { DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER | DRIVER_ATOMIC, .load = vmw_driver_load, .unload = vmw_driver_unload, - .lastclose = vmw_lastclose, .get_vblank_counter = vmw_get_vblank_counter, .enable_vblank = vmw_enable_vblank, .disable_vblank = vmw_disable_vblank, -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/vmwgfx: use core drm to extend/check vmw_execbuf_ioctl
From: Emil Velikov Currently vmw_execbuf_ioctl() open-codes the permission checking, size extending and copying that is already done in core drm. Kill all the duplication, adding a few comments for clarity. Cc: "VMware Graphics" Cc: Thomas Hellstrom Cc: Daniel Vetter Signed-off-by: Emil Velikov --- Thomas, VMware team, Please give this some testing on your end. I've only tested it against mesa-master. Thanks Emil --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 12 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 52 + 3 files changed, 23 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index d3f108f7e52d..2cb6ae219e43 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -186,7 +186,7 @@ static const struct drm_ioctl_desc vmw_ioctls[] = { DRM_RENDER_ALLOW), VMW_IOCTL_DEF(VMW_REF_SURFACE, vmw_surface_reference_ioctl, DRM_AUTH | DRM_RENDER_ALLOW), - VMW_IOCTL_DEF(VMW_EXECBUF, NULL, DRM_AUTH | + VMW_IOCTL_DEF(VMW_EXECBUF, vmw_execbuf_ioctl, DRM_AUTH | DRM_RENDER_ALLOW), VMW_IOCTL_DEF(VMW_FENCE_WAIT, vmw_fence_obj_wait_ioctl, DRM_RENDER_ALLOW), @@ -1140,15 +1140,7 @@ static long vmw_generic_ioctl(struct file *filp, unsigned int cmd, &vmw_ioctls[nr - DRM_COMMAND_BASE]; if (nr == DRM_COMMAND_BASE + DRM_VMW_EXECBUF) { - ret = (long) drm_ioctl_permit(ioctl->flags, file_priv); - if (unlikely(ret != 0)) - return ret; - - if (unlikely((cmd & (IOC_IN | IOC_OUT)) != IOC_IN)) - goto out_io_encoding; - - return (long) vmw_execbuf_ioctl(dev, arg, file_priv, - _IOC_SIZE(cmd)); + return ioctl_func(filp, cmd, arg); } else if (nr == DRM_COMMAND_BASE + DRM_VMW_UPDATE_LAYOUT) { if (!drm_is_current_master(file_priv) && !capable(CAP_SYS_ADMIN)) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index 9be2176cc260..f5bfac85f793 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -910,8 +910,8 @@ static inline struct page *vmw_piter_page(struct vmw_piter *viter) * Command submission - vmwgfx_execbuf.c */ -extern int vmw_execbuf_ioctl(struct drm_device *dev, unsigned long data, -struct drm_file *file_priv, size_t size); +extern int vmw_execbuf_ioctl(struct drm_device *dev, void *data, +struct drm_file *file_priv); extern int vmw_execbuf_process(struct drm_file *file_priv, struct vmw_private *dev_priv, void __user *user_commands, diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c index 2ff7ba04d8c8..767e2b99618d 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c @@ -3977,54 +3977,40 @@ void vmw_execbuf_release_pinned_bo(struct vmw_private *dev_priv) mutex_unlock(&dev_priv->cmdbuf_mutex); } -int vmw_execbuf_ioctl(struct drm_device *dev, unsigned long data, - struct drm_file *file_priv, size_t size) +int vmw_execbuf_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) { struct vmw_private *dev_priv = vmw_priv(dev); - struct drm_vmw_execbuf_arg arg; + struct drm_vmw_execbuf_arg *arg = data; int ret; - static const size_t copy_offset[] = { - offsetof(struct drm_vmw_execbuf_arg, context_handle), - sizeof(struct drm_vmw_execbuf_arg)}; struct dma_fence *in_fence = NULL; - if (unlikely(size < copy_offset[0])) { - VMW_DEBUG_USER("Invalid command size, ioctl %d\n", - DRM_VMW_EXECBUF); - return -EINVAL; - } - - if (copy_from_user(&arg, (void __user *) data, copy_offset[0]) != 0) - return -EFAULT; - /* * Extend the ioctl argument while maintaining backwards compatibility: -* We take different code paths depending on the value of arg.version. +* We take different code paths depending on the value of arg->version. +* +* Note: The ioctl argument is extended and zeropadded by core DRM. */ - if (unlikely(arg.version > DRM_VMW_EXECBUF_VERSION || -arg.version == 0)) { + if (unlikely(arg->version > DRM_VMW_EXECBUF_VERSION || +arg->version == 0)) { VMW_DEBUG_USER("Incorrect exec
[PATCH 2/5] drm/vmgfx: kill off unused init_mutex
From: Emil Velikov According to the docs - prevents firstopen/lastclose races. Yet never used in practise. Cc: "VMware Graphics" Cc: Thomas Hellstrom Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 5 - 2 files changed, 6 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index a38f06909fb6..d3f108f7e52d 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -664,7 +664,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) INIT_LIST_HEAD(&dev_priv->res_lru[i]); } - mutex_init(&dev_priv->init_mutex); init_waitqueue_head(&dev_priv->fence_queue); init_waitqueue_head(&dev_priv->fifo_queue); dev_priv->fence_queue_waiters = 0; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index 96983c47fb40..9be2176cc260 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -484,11 +484,6 @@ struct vmw_private { spinlock_t resource_lock; struct idr res_idr[vmw_res_max]; - /* -* Block lastclose from racing with firstopen. -*/ - - struct mutex init_mutex; /* * A resource manager for kernel-only surfaces and -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm: make drm_ioctl_permit() internal
From: Emil Velikov As of earlier commit the function is used only within drm core. Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/drm_ioctl.c | 3 +-- include/drm/drm_ioctl.h | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 2263e3ddd822..0646c0bd5535 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -523,7 +523,7 @@ int drm_version(struct drm_device *dev, void *data, * Returns: * Zero if allowed, -EACCES otherwise. */ -int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) +static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) { /* ROOT_ONLY is only for CAP_SYS_ADMIN */ if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) @@ -546,7 +546,6 @@ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) return 0; } -EXPORT_SYMBOL(drm_ioctl_permit); #define DRM_IOCTL_DEF(ioctl, _func, _flags)\ [DRM_IOCTL_NR(ioctl)] = { \ diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h index fafb6f592c4b..f3fba529cb1b 100644 --- a/include/drm/drm_ioctl.h +++ b/include/drm/drm_ioctl.h @@ -163,7 +163,6 @@ struct drm_ioctl_desc { .name = #ioctl \ } -int drm_ioctl_permit(u32 flags, struct drm_file *file_priv); long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); long drm_ioctl_kernel(struct file *, drm_ioctl_t, void *, u32); #ifdef CONFIG_COMPAT -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
From: Emil Velikov Drop the custom ioctl io encoding check - core drm does it for us. Cc: "VMware Graphics" Cc: Thomas Hellstrom Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 2cb6ae219e43..f65542639b55 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -1147,9 +1147,6 @@ static long vmw_generic_ioctl(struct file *filp, unsigned int cmd, return -EACCES; } - if (unlikely(ioctl->cmd != cmd)) - goto out_io_encoding; - flags = ioctl->flags; } else if (!drm_ioctl_flags(nr, &flags)) return -EINVAL; @@ -1169,12 +1166,6 @@ static long vmw_generic_ioctl(struct file *filp, unsigned int cmd, ttm_read_unlock(&vmaster->lock); return ret; - -out_io_encoding: - DRM_ERROR("Invalid command format, ioctl %d\n", - nr - DRM_COMMAND_BASE); - - return -EINVAL; } static long vmw_unlocked_ioctl(struct file *filp, unsigned int cmd, -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #9 from Pierre-Eric Pelloux-Prayer --- > (In reply to Ropid from comment #6) > @alvarex: > > Try starting chromium with this command line here, it makes it show > corruption everywhere for me here: > > chromium --ignore-gpu-blacklist --enable-gpu-rasterization > --enable-native-gpu-memory-buffers --enable-zero-copy > --disable-gpu-driver-bug-workarounds > Same here so I did a bisect. There are 2 problematic commits, making the bisect a bit more complicated. The first one has already been solved by d6053bf2a170. The second one is 811fa9a79cf ("mesa: unreference current winsys buffers when unbinding winsys buffers"). Using master + this commit reverted: no more corruption in chromium. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110731] ui issues
https://bugs.freedesktop.org/show_bug.cgi?id=110731 Andre Klapper changed: What|Removed |Added Product|DRI |Spam Resolution|--- |INVALID Status|NEW |RESOLVED Component|DRM/AMDgpu-pro |Two Group||spam --- Comment #1 from Andre Klapper --- Go away and test somewhere else. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/12] dma-buf: add dynamic caching of sg_table
Am 22.05.19 um 18:17 schrieb Sumit Semwal: Hi Christian, On Sat, 27 Apr 2019 at 05:31, Liam Mark wrote: On Tue, 16 Apr 2019, Christian König wrote: To allow a smooth transition from pinning buffer objects to dynamic invalidation we first start to cache the sg_table for an attachment unless the driver explicitly says to not do so. --- drivers/dma-buf/dma-buf.c | 24 include/linux/dma-buf.h | 11 +++ 2 files changed, 35 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 7c858020d14b..65161a82d4d5 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -573,6 +573,20 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, list_add(&attach->node, &dmabuf->attachments); mutex_unlock(&dmabuf->lock); + + if (!dmabuf->ops->dynamic_sgt_mapping) { + struct sg_table *sgt; + + sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL); + if (!sgt) + sgt = ERR_PTR(-ENOMEM); + if (IS_ERR(sgt)) { + dma_buf_detach(dmabuf, attach); + return ERR_CAST(sgt); + } + attach->sgt = sgt; + } + return attach; err_attach: @@ -595,6 +609,10 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) if (WARN_ON(!dmabuf || !attach)) return; + if (attach->sgt) + dmabuf->ops->unmap_dma_buf(attach, attach->sgt, +DMA_BIDIRECTIONAL); + mutex_lock(&dmabuf->lock); list_del(&attach->node); if (dmabuf->ops->detach) @@ -630,6 +648,9 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, if (WARN_ON(!attach || !attach->dmabuf)) return ERR_PTR(-EINVAL); + if (attach->sgt) + return attach->sgt; + I am concerned by this change to make caching the sg_table the default behavior as this will result in the exporter's map_dma_buf/unmap_dma_buf calls are no longer being called in dma_buf_map_attachment/dma_buf_unmap_attachment. Probably this concern from Liam got lost between versions of your patches; could we please request a reply to these points here? Sorry I indeed never got this mail, but this is actually not an issue because Daniel had similar concerns and we didn't made this the default in the final version. This seems concerning to me as it appears to ignore the cache maintenance aspect of the map_dma_buf/unmap_dma_buf calls. For example won't this potentially cause issues for clients of ION. If we had the following - #1 dma_buf_attach coherent_device - #2 dma_buf attach non_coherent_device - #3 dma_buf_map_attachment non_coherent_device - #4 non_coherent_device writes to buffer - #5 dma_buf_unmap_attachment non_coherent_device - #6 dma_buf_map_attachment coherent_device - #7 coherent_device reads buffer - #8 dma_buf_unmap_attachment coherent_device There wouldn't be any CMO at step #5 anymore (specifically no invalidate) so now at step #7 the coherent_device could read a stale cache line. Also, now by default dma_buf_unmap_attachment no longer removes the mappings from the iommu, so now by default dma_buf_unmap_attachment is not doing what I would expect and clients are losing the potential sandboxing benefits of removing the mappings. Shouldn't this caching behavior be something that clients opt into instead of being the default? Well, it seems you are making incorrect assumptions about the cache maintenance of DMA-buf here. At least for all DRM devices I'm aware of mapping/unmapping an attachment does *NOT* have any cache maintenance implications. E.g. the use case you describe above would certainly fail with amdgpu, radeon, nouveau and i915 because mapping a DMA-buf doesn't stop the exporter from reading/writing to that buffer (just the opposite actually). All of them assume perfectly coherent access to the underlying memory. As far as I know there is no documented cache maintenance requirements for DMA-buf. The IOMMU concern on the other hand is certainly valid and I perfectly agree that keeping the mapping time as short as possible is desirable. Regards, Christian. Liam Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project Best, Sumit. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110721] graphics corruption on steam client with mesa 19.1.0 rc3 on polaris
https://bugs.freedesktop.org/show_bug.cgi?id=110721 --- Comment #10 from alvarex --- (In reply to Pierre-Eric Pelloux-Prayer from comment #9) > > (In reply to Ropid from comment #6) > > @alvarex: > > > > Try starting chromium with this command line here, it makes it show > > corruption everywhere for me here: > > > > chromium --ignore-gpu-blacklist --enable-gpu-rasterization > > --enable-native-gpu-memory-buffers --enable-zero-copy > > --disable-gpu-driver-bug-workarounds > > > > Same here so I did a bisect. > There are 2 problematic commits, making the bisect a bit more complicated. > > The first one has already been solved by d6053bf2a170. > The second one is 811fa9a79cf ("mesa: unreference current winsys buffers > when unbinding winsys buffers"). > > Using master + this commit reverted: no more corruption in chromium. for me it doesn't happen on chrome with 64 bits libs, (I couldn't compile 32bits) I tried several versions of chrome, but not that precise one. not sure if it is the same bug. what version of chrome does steam use? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm/msm/a6xx: Remove duplicate irq disable from remove
From: Sean Paul a6xx_gmu_stop() already calls this function via shutdown or force_stop, so it's not necessary to call it twice. This also knocks the irq depth count out of sync, so nice to avoid. Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index aa84edb25d91..742c8ff9a61c 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -1239,7 +1239,6 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) dev_pm_domain_detach(gmu->gxpd, false); } - a6xx_gmu_irq_disable(gmu); a6xx_gmu_memory_free(gmu, gmu->hfi); iommu_detach_device(gmu->domain, gmu->dev); -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RESEND PATCH 1/5] drm/msm/a6xx: Avoid freeing gmu resources multiple times
From: Sean Paul The driver checks for gmu->mmio as a sign that the device has been initialized, however there are failures in probe below the mmio init. If one of those is hit, mmio will be non-null but freed. In that case, a6xx_gmu_probe will return an error to a6xx_gpu_init which will in turn call a6xx_gmu_remove which checks gmu->mmio and tries to free resources for a second time. This causes a great boom. Fix this by adding an initialized member to gmu which is set on successful probe and cleared on removal. Signed-off-by: Sean Paul --- Resending as part of the set since some later patches depend on it drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 14 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 38e2cfa9cec7..aa84edb25d91 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -74,7 +74,7 @@ bool a6xx_gmu_sptprac_is_on(struct a6xx_gmu *gmu) u32 val; /* This can be called from gpu state code so make sure GMU is valid */ - if (IS_ERR_OR_NULL(gmu->mmio)) + if (!gmu->initialized) return false; val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS); @@ -90,7 +90,7 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) u32 val; /* This can be called from gpu state code so make sure GMU is valid */ - if (IS_ERR_OR_NULL(gmu->mmio)) + if (!gmu->initialized) return false; val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS); @@ -695,7 +695,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) struct a6xx_gmu *gmu = &a6xx_gpu->gmu; int status, ret; - if (WARN(!gmu->mmio, "The GMU is not set up yet\n")) + if (WARN(!gmu->initialized, "The GMU is not set up yet\n")) return 0; gmu->hung = false; @@ -765,7 +765,7 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu) { u32 reg; - if (!gmu->mmio) + if (!gmu->initialized) return true; reg = gmu_read(gmu, REG_A6XX_GPU_GMU_AO_GPU_CX_BUSY_STATUS); @@ -1227,7 +1227,7 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) { struct a6xx_gmu *gmu = &a6xx_gpu->gmu; - if (IS_ERR_OR_NULL(gmu->mmio)) + if (!gmu->initialized) return; a6xx_gmu_stop(a6xx_gpu); @@ -1245,6 +1245,8 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) iommu_detach_device(gmu->domain, gmu->dev); iommu_domain_free(gmu->domain); + + gmu->initialized = false; } int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node) @@ -1309,6 +1311,8 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node) /* Set up the HFI queues */ a6xx_hfi_init(gmu); + gmu->initialized = true; + return 0; err: a6xx_gmu_memory_free(gmu, gmu->hfi); diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h index bedd8e6a63aa..39a26dd63674 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h @@ -75,6 +75,7 @@ struct a6xx_gmu { struct a6xx_hfi_queue queues[2]; + bool initialized; bool hung; }; -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/msm/a6xx: Remove devm calls from gmu driver
From: Sean Paul The gmu driver is initalized and cleaned up with calls from the gpu driver. As such, the platform device stays valid after a6xx_gmu_remove is called and the device managed resources are not freed. In the case of gpu probe failures or unbind, these resources will remain managed. If the gpu bind is run again (eg: if there's a probe defer somewhere in msm), these resources will be initialized again for the same device, creating multiple references. In the case of irqs, this causes failures since the irqs are not shared (nor should they be). This patch removes all devm_* calls and manually cleans things up in gmu_remove. Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 7465423e9b71..701b813fa38a 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -505,9 +505,9 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu) err: if (!IS_ERR_OR_NULL(pdcptr)) - devm_iounmap(gmu->dev, pdcptr); + iounmap(pdcptr); if (!IS_ERR_OR_NULL(seqptr)) - devm_iounmap(gmu->dev, seqptr); + iounmap(seqptr); } /* @@ -1197,7 +1197,7 @@ static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev, return ERR_PTR(-EINVAL); } - ret = devm_ioremap(&pdev->dev, res->start, resource_size(res)); + ret = ioremap(res->start, resource_size(res)); if (!ret) { DRM_DEV_ERROR(&pdev->dev, "Unable to map the %s registers\n", name); return ERR_PTR(-EINVAL); @@ -1213,10 +1213,10 @@ static int a6xx_gmu_get_irq(struct a6xx_gmu *gmu, struct platform_device *pdev, irq = platform_get_irq_byname(pdev, name); - ret = devm_request_irq(&pdev->dev, irq, handler, IRQF_TRIGGER_HIGH, - name, gmu); + ret = request_irq(irq, handler, IRQF_TRIGGER_HIGH, name, gmu); if (ret) { - DRM_DEV_ERROR(&pdev->dev, "Unable to get interrupt %s\n", name); + DRM_DEV_ERROR(&pdev->dev, "Unable to get interrupt %s %d\n", + name, ret); return ret; } @@ -1241,12 +1241,18 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) dev_pm_domain_detach(gmu->gxpd, false); } + iounmap(gmu->mmio); + gmu->mmio = NULL; + a6xx_gmu_memory_free(gmu, gmu->hfi); iommu_detach_device(gmu->domain, gmu->dev); iommu_domain_free(gmu->domain); + free_irq(gmu->gmu_irq, gmu); + free_irq(gmu->hfi_irq, gmu); + gmu->initialized = false; } -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/msm/a6xx: Check for ERR or NULL before iounmap
From: Sean Paul pdcptr and seqptr aren't necessarily valid, check them before trying to unmap them. Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 742c8ff9a61c..7465423e9b71 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -504,8 +504,10 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu) wmb(); err: - devm_iounmap(gmu->dev, pdcptr); - devm_iounmap(gmu->dev, seqptr); + if (!IS_ERR_OR_NULL(pdcptr)) + devm_iounmap(gmu->dev, pdcptr); + if (!IS_ERR_OR_NULL(seqptr)) + devm_iounmap(gmu->dev, seqptr); } /* -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel