Re: [PATCH] drm/exynos: Drop local dma_parms
Hi Robin, As Marek said, he posted same patch[1] before so I merged it instead of you. I totally forgot to merge it. :( Sorry for confusing. [1] https://lore.kernel.org/dri-devel/20200707110827.3760-1-m.szyprow...@samsung.com/ Thanks, Inki Dae 20. 9. 4. 오전 5:51에 Robin Murphy 이(가) 쓴 글: > Since commit 9495b7e92f71 ("driver core: platform: Initialize dma_parms > for platform devices"), struct platform_device already provides a > dma_parms structure, so we can save allocating another one. > > Also the DMA segment size is simply a size, not a bitmask. > > Signed-off-by: Robin Murphy > --- > drivers/gpu/drm/exynos/exynos_drm_dma.c | 26 + > 1 file changed, 1 insertion(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c > b/drivers/gpu/drm/exynos/exynos_drm_dma.c > index 58b89ec11b0e..9f25a5ebbf7d 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c > @@ -31,23 +31,6 @@ > #define EXYNOS_DEV_ADDR_START0x2000 > #define EXYNOS_DEV_ADDR_SIZE 0x4000 > > -static inline int configure_dma_max_seg_size(struct device *dev) > -{ > - if (!dev->dma_parms) > - dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL); > - if (!dev->dma_parms) > - return -ENOMEM; > - > - dma_set_max_seg_size(dev, DMA_BIT_MASK(32)); > - return 0; > -} > - > -static inline void clear_dma_max_seg_size(struct device *dev) > -{ > - kfree(dev->dma_parms); > - dev->dma_parms = NULL; > -} > - > /* > * drm_iommu_attach_device- attach device to iommu mapping > * > @@ -69,9 +52,7 @@ static int drm_iommu_attach_device(struct drm_device > *drm_dev, > return -EINVAL; > } > > - ret = configure_dma_max_seg_size(subdrv_dev); > - if (ret) > - return ret; > + dma_set_max_seg_size(subdrv_dev, UINT_MAX); > > if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) { > /* > @@ -89,9 +70,6 @@ static int drm_iommu_attach_device(struct drm_device > *drm_dev, > ret = iommu_attach_device(priv->mapping, subdrv_dev); > } > > - if (ret) > - clear_dma_max_seg_size(subdrv_dev); > - > return ret; > } > > @@ -114,8 +92,6 @@ static void drm_iommu_detach_device(struct drm_device > *drm_dev, > arm_iommu_attach_device(subdrv_dev, *dma_priv); > } else if (IS_ENABLED(CONFIG_IOMMU_DMA)) > iommu_detach_device(priv->mapping, subdrv_dev); > - > - clear_dma_max_seg_size(subdrv_dev); > } > > int exynos_drm_register_dma(struct drm_device *drm, struct device *dev, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)
https://bugzilla.kernel.org/show_bug.cgi?id=209345 --- Comment #6 from Alexander von Gluck (kallis...@unixzen.com) --- Created attachment 292557 --> https://bugzilla.kernel.org/attachment.cgi?id=292557=edit tesla k80 patch -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)
https://bugzilla.kernel.org/show_bug.cgi?id=209345 --- Comment #5 from Alexander von Gluck (kallis...@unixzen.com) --- [ 2208.130049] nouveau: version magic '5.8.10 SMP mod_unload ' should be '5.8.10-200.fc32.x86_64 SMP mod_unload ' [ 2460.923164] ACPI Warning: \_SB.PCI0.GFX0._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20200528/nsarguments-59) [ 2460.923220] nouveau :08:00.0: can't change power state from D3hot to D0 (config space inaccessible) [ 2460.923393] nouveau :08:00.0: GPU not supported on big-endian [ 2460.923411] nouveau: probe of :08:00.0 failed with error -38 [ 2460.923424] nouveau :09:00.0: can't change power state from D3hot to D0 (config space inaccessible) [ 2460.923504] nouveau :09:00.0: GPU not supported on big-endian [ 2460.923507] nouveau: probe of :09:00.0 failed with error -38 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: manual merge of the drm-misc tree with the drm-intel tree
Hi all, Today's linux-next merge of the drm-misc tree got a conflict in: drivers/gpu/drm/i915/selftests/mock_gem_device.c between commit: 9f9f4101fc98 ("drm/i915/selftests: Push the fake iommu device from the stack to data") from the drm-intel tree and commit: cd01269d11a3 ("drm/i915/selftests: align more to real device lifetimes") from the drm-misc tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/gpu/drm/i915/selftests/mock_gem_device.c index 397c313a8b69,c207d2239791.. --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@@ -118,12 -116,11 +116,11 @@@ static struct dev_pm_domain pm_domain struct drm_i915_private *mock_gem_device(void) { - struct drm_i915_private *i915; - struct pci_dev *pdev; #if IS_ENABLED(CONFIG_IOMMU_API) && defined(CONFIG_INTEL_IOMMU) - struct dev_iommu iommu; + static struct dev_iommu fake_iommu = { .priv = (void *)-1 }; #endif + struct drm_i915_private *i915; + struct pci_dev *pdev; - int err; pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); if (!pdev) @@@ -141,11 -132,28 +132,26 @@@ dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64)); #if IS_ENABLED(CONFIG_IOMMU_API) && defined(CONFIG_INTEL_IOMMU) - /* HACK HACK HACK to disable iommu for the fake device; force identity mapping */ - memset(, 0, sizeof(iommu)); - iommu.priv = (void *)-1; - pdev->dev.iommu = + /* HACK to disable iommu for the fake device; force identity mapping */ + pdev->dev.iommu = _iommu; #endif + if (!devres_open_group(>dev, NULL, GFP_KERNEL)) { + put_device(>dev); + return NULL; + } + + i915 = devm_drm_dev_alloc(>dev, _driver, + struct drm_i915_private, drm); + if (IS_ERR(i915)) { + pr_err("Failed to allocate mock GEM device: err=%ld\n", PTR_ERR(i915)); + devres_release_group(>dev, NULL); + put_device(>dev); + + return NULL; + } pci_set_drvdata(pdev, i915); + i915->drm.pdev = pdev; dev_pm_domain_set(>dev, _domain); pm_runtime_enable(>dev); pgpzYUS1WDHAb.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v17 00/20] iommu/arm-smmu + drm/msm: per-process GPU pgtables
On Mon, Sep 21, 2020 at 10:30:57PM +0100, Will Deacon wrote: > On Sat, Sep 05, 2020 at 01:04:06PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > NOTE: I have re-ordered the series, and propose that we could merge this > > series in the following order: > > > >1) 01-11 - merge via drm / msm-next > >2) 12-15 - merge via iommu, no dependency on msm-next pull req > > Thanks, I've queued 12-15 in the smmu tree. > > https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates > > Will Will, thanks for your help and patience in getting these merged. Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)
https://bugzilla.kernel.org/show_bug.cgi?id=209345 --- Comment #4 from Alexander von Gluck (kallis...@unixzen.com) --- Physical K80 board in my possession. They go for cheap now-a-days on ebay :-) Memory size (GDDR5): 24GB CUDA cores: 4992 Number Of GPUs: 2x GK120 GPUs I'll try adding the nvf2 and see what happens. I have it in a TB3 enclosure plugged into my Dell XPS 13, so it makes testing things pretty easy. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v17 00/20] iommu/arm-smmu + drm/msm: per-process GPU pgtables
On Mon, Sep 21, 2020 at 2:31 PM Will Deacon wrote: > > On Sat, Sep 05, 2020 at 01:04:06PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > NOTE: I have re-ordered the series, and propose that we could merge this > > series in the following order: > > > >1) 01-11 - merge via drm / msm-next > >2) 12-15 - merge via iommu, no dependency on msm-next pull req > > Thanks, I've queued 12-15 in the smmu tree. > > https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates > Thx, let me know if you are up for trying a post-rc1 pull req for 16-18 (plus Bjorn's series if it is reposted in time).. I can certainly help rebase/wrangle patches if that helps.. otherwise we can try for those next cycle BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)
https://bugzilla.kernel.org/show_bug.cgi?id=209345 Ilia Mirkin (imir...@alum.mit.edu) changed: What|Removed |Added CC||imir...@alum.mit.edu --- Comment #3 from Ilia Mirkin (imir...@alum.mit.edu) --- Is this a physical K80 board, or a virtual one via some sort of cloud provider? We have nvf0 = GK110, nvf1 = what we call GK110B, but I'm not sure that's an official name - basically the GTX 780 Ti and related Titans. We don't have explicit support for nvf2 -- as I understand the K80 (GK210) actually has some slight differences, e.g. more shared memory, etc... not sure if that translates into some ctxsw fw differences or if it should just work -- you can check drivers/gpu/drm/nouveau/nvkm/engine/device/base.c, should be easy to add in 0xf2 support based on the nvf1 if you want to play with it. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)
https://bugzilla.kernel.org/show_bug.cgi?id=209345 --- Comment #2 from Alexander von Gluck (kallis...@unixzen.com) --- Created attachment 292555 --> https://bugzilla.kernel.org/attachment.cgi?id=292555=edit nvidia thunderbolt 3 attachment logs from attachment -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)
https://bugzilla.kernel.org/show_bug.cgi?id=209345 --- Comment #1 from Alexander von Gluck (kallis...@unixzen.com) --- lspci -vvn 08:00.0 0302: 10de:102d (rev ff) (prog-if ff) !!! Unknown header type 7f Kernel driver in use: nouveau Kernel modules: nouveau 09:00.0 0302: 10de:102d (rev ff) (prog-if ff) !!! Unknown header type 7f Kernel modules: nouveau 08:00.0 3D controller: NVIDIA Corporation GK210GL [Tesla K80] (rev ff) 09:00.0 3D controller: NVIDIA Corporation GK210GL [Tesla K80] (rev ff) -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209345] New: [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)
https://bugzilla.kernel.org/show_bug.cgi?id=209345 Bug ID: 209345 Summary: [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80) Product: Drivers Version: 2.5 Kernel Version: 5.8.9 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: kallis...@unixzen.com Regression: No Attaching an nVidia Tesla K80 compute card to a Fedora 32 machine. Getting: nouveau :08:00.0: unknown chipset (0f22d0a1) -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 05/38] drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays()
Tested-by: Alex Goins This change fixes a regression with drm_prime_sg_to_page_addr_arrays() and AMDGPU in v5.9. Commit 39913934 similarly revamped AMDGPU to use sgtable helper functions. When it changed from dma_map_sg_attrs() to dma_map_sgtable(), as a side effect it started correctly updating sgt->nents to the return value of dma_map_sg_attrs(). However, drm_prime_sg_to_page_addr_arrays() incorrectly uses sgt->nents to iterate over pages, rather than sgt->orig_nents, resulting in it now returning the incorrect number of pages on AMDGPU. I had written a patch that changes drm_prime_sg_to_page_addr_arrays() to use for_each_sgtable_sg() instead of for_each_sg(), iterating using sgt->orig_nents: - for_each_sg(sgt->sgl, sg, sgt->nents, count) { + for_each_sgtable_sg(sgt, sg, count) { This patch takes it further, but still has the effect of fixing the number of pages that drm_prime_sg_to_page_addr_arrays() returns. Something like this should be included in v5.9 to prevent a regression with AMDGPU. Thanks, Alex On Wed, 13 May 2020, Marek Szyprowski wrote: > Replace the current hand-crafted code for extracting pages and DMA > addresses from the given scatterlist by the much more robust > code based on the generic scatterlist iterators and recently > introduced sg_table-based wrappers. The resulting code is simple and > easy to understand, so the comment describing the old code is no > longer needed. > > Signed-off-by: Marek Szyprowski > --- > For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents > vs. orig_nents misuse' thread: > https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprow...@samsung.com/T/ > --- > drivers/gpu/drm/drm_prime.c | 47 > ++--- > 1 file changed, 14 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 1d2e5fe..dfdf4d4 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -985,45 +985,26 @@ struct drm_gem_object *drm_gem_prime_import(struct > drm_device *dev, > int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page > **pages, >dma_addr_t *addrs, int max_entries) > { > - unsigned count; > - struct scatterlist *sg; > - struct page *page; > - u32 page_len, page_index; > - dma_addr_t addr; > - u32 dma_len, dma_index; > + struct sg_dma_page_iter dma_iter; > + struct sg_page_iter page_iter; > + struct page **p = pages; > + dma_addr_t *a = addrs; > > - /* > - * Scatterlist elements contains both pages and DMA addresses, but > - * one shoud not assume 1:1 relation between them. The sg->length is > - * the size of the physical memory chunk described by the sg->page, > - * while sg_dma_len(sg) is the size of the DMA (IO virtual) chunk > - * described by the sg_dma_address(sg). > - */ > - page_index = 0; > - dma_index = 0; > - for_each_sg(sgt->sgl, sg, sgt->nents, count) { > - page_len = sg->length; > - page = sg_page(sg); > - dma_len = sg_dma_len(sg); > - addr = sg_dma_address(sg); > - > - while (pages && page_len > 0) { > - if (WARN_ON(page_index >= max_entries)) > + if (pages) { > + for_each_sgtable_page(sgt, _iter, 0) { > + if (p - pages >= max_entries) > return -1; > - pages[page_index] = page; > - page++; > - page_len -= PAGE_SIZE; > - page_index++; > + *p++ = sg_page_iter_page(_iter); > } > - while (addrs && dma_len > 0) { > - if (WARN_ON(dma_index >= max_entries)) > + } > + if (addrs) { > + for_each_sgtable_dma_page(sgt, _iter, 0) { > + if (a - addrs >= max_entries) > return -1; > - addrs[dma_index] = addr; > - addr += PAGE_SIZE; > - dma_len -= PAGE_SIZE; > - dma_index++; > + *a++ = sg_page_iter_dma_address(_iter); > } > } > + > return 0; > } > EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays); > -- > 1.9.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915/dp: Tweak initial dpcd backlight.enabled value
So if I understand this correctly, it sounds like that some Pixelbooks boot up with DP_EDP_BACKLIGHT_BRIGHTNESS_MSB set to a non-zero value, without the panel actually having DPCD backlight controls enabled? If I'm understanding that correctly, then this patch looks good to me: Reviewed-by: Lyude Paul On Thu, 2020-09-17 at 20:28 -0400, Sean Paul wrote: > From: Sean Paul > > In commit 79946723092b ("drm/i915: Assume 100% brightness when not in > DPCD control mode"), we fixed the brightness level when DPCD control was > not active to max brightness. This is as good as we can guess since most > backlights go on full when uncontrolled. > > However in doing so we changed the semantics of the initial > 'backlight.enabled' value. At least on Pixelbooks, they were relying > on the brightness level in DP_EDP_BACKLIGHT_BRIGHTNESS_MSB to be 0 on > boot such that enabled would be false. This causes the device to be > enabled when the brightness is set. Without this, brightness control > doesn't work. So by changing brightness to max, we also flipped enabled > to be true on boot. > > To fix this, make enabled a function of brightness and backlight control > mechanism. > > Fixes: 79946723092b ("drm/i915: Assume 100% brightness when not in DPCD > control mode") > Cc: Lyude Paul > Cc: Jani Nikula > Cc: Juha-Pekka Heikkila > Cc: "Ville Syrjälä" > Cc: Rodrigo Vivi > Cc: Kevin Chowski > > Signed-off-by: Sean Paul > --- > .../drm/i915/display/intel_dp_aux_backlight.c | 31 --- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > index acbd7eb66cbe..036f504ac7db 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > @@ -52,17 +52,11 @@ static void set_aux_backlight_enable(struct intel_dp > *intel_dp, bool enable) > } > } > > -/* > - * Read the current backlight value from DPCD register(s) based > - * on if 8-bit(MSB) or 16-bit(MSB and LSB) values are supported > - */ > -static u32 intel_dp_aux_get_backlight(struct intel_connector *connector) > +static bool intel_dp_aux_backlight_dpcd_mode(struct intel_connector > *connector) > { > struct intel_dp *intel_dp = intel_attached_dp(connector); > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > - u8 read_val[2] = { 0x0 }; > u8 mode_reg; > - u16 level = 0; > > if (drm_dp_dpcd_readb(_dp->aux, > DP_EDP_BACKLIGHT_MODE_SET_REGISTER, > @@ -70,15 +64,29 @@ static u32 intel_dp_aux_get_backlight(struct > intel_connector *connector) > drm_dbg_kms(>drm, > "Failed to read the DPCD register 0x%x\n", > DP_EDP_BACKLIGHT_MODE_SET_REGISTER); > - return 0; > + return false; > } > > + return (mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) == > +DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD; > +} > + > +/* > + * Read the current backlight value from DPCD register(s) based > + * on if 8-bit(MSB) or 16-bit(MSB and LSB) values are supported > + */ > +static u32 intel_dp_aux_get_backlight(struct intel_connector *connector) > +{ > + struct intel_dp *intel_dp = intel_attached_dp(connector); > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > + u8 read_val[2] = { 0x0 }; > + u16 level = 0; > + > /* >* If we're not in DPCD control mode yet, the programmed brightness >* value is meaningless and we should assume max brightness >*/ > - if ((mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) != > - DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) > + if (!intel_dp_aux_backlight_dpcd_mode(connector)) > return connector->panel.backlight.max; > > if (drm_dp_dpcd_read(_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, > @@ -319,7 +327,8 @@ static int intel_dp_aux_setup_backlight(struct > intel_connector *connector, > > panel->backlight.min = 0; > panel->backlight.level = intel_dp_aux_get_backlight(connector); > - panel->backlight.enabled = panel->backlight.level != 0; > + panel->backlight.enabled = intel_dp_aux_backlight_dpcd_mode(connector) > && > +panel->backlight.level != 0; > > return 0; > } -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] iommu/io-pgtable-arm: Support coherency for Mali LPAE
On Mon, Sep 21, 2020 at 10:53:23PM +0100, Robin Murphy wrote: > On 2020-09-21 18:57, Will Deacon wrote: > > On Wed, Sep 16, 2020 at 12:51:05AM +0100, Robin Murphy wrote: > > > Midgard GPUs have ACE-Lite master interfaces which allows systems to > > > integrate them in an I/O-coherent manner. It seems that from the GPU's > > > viewpoint, the rest of the system is its outer shareable domain, and so > > > even when snoop signals are wired up, they are only emitted for outer > > > shareable accesses. As such, setting the TTBR_SHARE_OUTER bit does > > > indeed get coherent pagetable walks working nicely for the coherent > > > T620 in the Arm Juno SoC. > > > > I can't help but think some of this commentary deserves to be in the code > > as well. > > Sure, if you want. Yes, please. > > Do you know if this sort of thing is done for other SoCs too, or is this > > just a Juno quirk? > > Yup, this is a "Midgard working as designed" thing. Juno is the coherent > example I have to hand, but off the top of my head I believe some of the > Exynos SoCs can also use their GPUs coherently if a switch is flipped in the > interconnect to change routing between the CCI and a direct-to-RAM path; I > expect there are probably further Midgard examples that I'm not aware of. > Then there are definitely coherent Bifrost GPUs like the Amlogic S922/A311 > that prompted me to revive this patch, which we currently drive in "Legacy" > mode and thus behave the same way as Midgard (Bifrost's "AArch64" mode > realigns Ish and Osh with the rest of the system, and instead invents a new > "Internal Shareable" value in between Nsh and Ish to represent the > shareability between cores within the GPU for which Midgard hijacked Ish). That is more than I wanted to know :) "Internal Shareable", jeez... Thanks, Will ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv4 1/6] iommu/io-pgtable-arm: Add support to use system cache
On Mon, Sep 21, 2020 at 11:03:49PM +0100, Robin Murphy wrote: > On 2020-09-21 19:03, Will Deacon wrote: > > On Fri, Sep 11, 2020 at 07:57:18PM +0530, Sai Prakash Ranjan wrote: > > > Add a quirk IO_PGTABLE_QUIRK_SYS_CACHE to override the > > > attributes set in TCR for the page table walker when > > > using system cache. > > > > I wonder if the panfrost folks can reuse this for the issue discussed > > over at: > > > > https://lore.kernel.org/r/cover.1600213517.git.robin.mur...@arm.com > > Isn't this all hinged around the outer cacheability attribute, rather than > shareability (since these are nominally NC mappings and thus already > properly Osh)? The Panfrost issue is just about shareability domains being a > bit wonky; the cacheability attributes there are actually reasonably normal > (other than not having a non-cacheable type at all, only a choice of > allocation policies...) Hmm, yes, this quirk _also_ changes the cacheability settings which isn't what we need. It's a bit grotty having two different ways to configure these TCR bits (i.e. a quirk and a format), but at least the mali format rejects all of the quirks so I suppose it's not the end of the world. Will ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv4 1/6] iommu/io-pgtable-arm: Add support to use system cache
On 2020-09-21 19:03, Will Deacon wrote: On Fri, Sep 11, 2020 at 07:57:18PM +0530, Sai Prakash Ranjan wrote: Add a quirk IO_PGTABLE_QUIRK_SYS_CACHE to override the attributes set in TCR for the page table walker when using system cache. I wonder if the panfrost folks can reuse this for the issue discussed over at: https://lore.kernel.org/r/cover.1600213517.git.robin.mur...@arm.com Isn't this all hinged around the outer cacheability attribute, rather than shareability (since these are nominally NC mappings and thus already properly Osh)? The Panfrost issue is just about shareability domains being a bit wonky; the cacheability attributes there are actually reasonably normal (other than not having a non-cacheable type at all, only a choice of allocation policies...) Robin. However, Sai, your email setup went wrong when you posted this so you probably need to repost now that you have that fixed. Will ___ iommu mailing list io...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] iommu/io-pgtable-arm: Support coherency for Mali LPAE
On 2020-09-21 18:57, Will Deacon wrote: On Wed, Sep 16, 2020 at 12:51:05AM +0100, Robin Murphy wrote: Midgard GPUs have ACE-Lite master interfaces which allows systems to integrate them in an I/O-coherent manner. It seems that from the GPU's viewpoint, the rest of the system is its outer shareable domain, and so even when snoop signals are wired up, they are only emitted for outer shareable accesses. As such, setting the TTBR_SHARE_OUTER bit does indeed get coherent pagetable walks working nicely for the coherent T620 in the Arm Juno SoC. I can't help but think some of this commentary deserves to be in the code as well. Sure, if you want. Do you know if this sort of thing is done for other SoCs too, or is this just a Juno quirk? Yup, this is a "Midgard working as designed" thing. Juno is the coherent example I have to hand, but off the top of my head I believe some of the Exynos SoCs can also use their GPUs coherently if a switch is flipped in the interconnect to change routing between the CCI and a direct-to-RAM path; I expect there are probably further Midgard examples that I'm not aware of. Then there are definitely coherent Bifrost GPUs like the Amlogic S922/A311 that prompted me to revive this patch, which we currently drive in "Legacy" mode and thus behave the same way as Midgard (Bifrost's "AArch64" mode realigns Ish and Osh with the rest of the system, and instead invents a new "Internal Shareable" value in between Nsh and Ish to represent the shareability between cores within the GPU for which Midgard hijacked Ish). Robin. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] video: fbdev: amba-clcd: Retire elder CLCD driver
On Tue, Sep 15, 2020 at 3:10 AM Peter Collingbourne wrote: > On Tue, Jun 9, 2020 at 1:08 PM Linus Walleij wrote: > > > > All the functionality in this driver has been reimplemented > > in the new DRM driver in drivers/gpu/drm/pl111/* and all > > the boards using it have been migrated to use the DRM driver > > with all configuration coming from the device tree. > > Android's FVP configuration still uses FBDEV. But all DRM drivers support frame buffer emulation so I don't see how this can be a problem? The new DRM driver provides a framebuffer. I'm also confused about how this can be in use. fvp-base-revc.dts includes rtsm_ve-motherboard.dtsi where the PL111 is defined. commit f1fe12c8bf33241e04c57a0fc5b25b16ba77ba2d "ARM: dts: Modernize the Vexpress PL111 integration" changes the DTS for the FVP such that only the new DRM driver really works with it: it removes the panel-dpi encoded panel and defines the panel "arm,rtsm-display" for FVP and that is *only* supported by drivers/gpu/drm/panel/panel-simple.c from DRM. The upstream FVP has been using the new DRM driver by default since commit 37ad688497785c9ad1471236e28efda1e2f39741 "arm64: defconfig: Switch to PL11x DRM driver" in january 2019. Are you using old or outoftree devicetrees with a newer kernel? > While it would be great > to see it migrated to DRM, this first requires resolving a design > incompatibility between Android's composer and DRM devices that only > support software rendering. I proposed a patch that would help resolve > this [1], but there was disagreement about the approach and I haven't > had time to get back to this. > > Can this please be reverted until FVP on Android can be migrated to DRM? We need a clear technical reason, I understand that using DRM directly might be a problem, but DRM supports full framebuffer emulation and from a userspace point of view, what the new driver provides should be equivalent. I can think of problems like Android seeing the new fancy DRM userspace ABI but surely it can be instructed to ignore that and just use the framebuffer emulation instead? If there is anything else making DRMs framebuffer emulation substandard I am sure the DRM developers would like to know, especially if it makes Android unhappy. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v17 00/20] iommu/arm-smmu + drm/msm: per-process GPU pgtables
On Sat, Sep 05, 2020 at 01:04:06PM -0700, Rob Clark wrote: > From: Rob Clark > > NOTE: I have re-ordered the series, and propose that we could merge this > series in the following order: > >1) 01-11 - merge via drm / msm-next >2) 12-15 - merge via iommu, no dependency on msm-next pull req Thanks, I've queued 12-15 in the smmu tree. https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates Will ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: move Milo Kim to credits
Hi! > Milo Kim's email in TI bounces with permanent error (550: Invalid > recipient). Last email from him on LKML was in 2017. Move Milo Kim to > credits and remove the separate driver entries for: > > - TI LP855x backlight driver, > - TI LP8727 charger driver, > - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers. > > Signed-off-by: Krzysztof Kozlowski I believe normal way would be to mark the entries "orphaned", not to drop them altogether. Plus, I believe someone from TI is likely to step up. Best regards, Pavel > diff --git a/MAINTAINERS b/MAINTAINERS > index 5b9621ca2b31..031adeff29cc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -17471,29 +17471,6 @@ S: Maintained > F: sound/soc/codecs/isabelle* > F: sound/soc/codecs/lm49453* > > -TI LP855x BACKLIGHT DRIVER > -M: Milo Kim > -S: Maintained > -F: Documentation/driver-api/backlight/lp855x-driver.rst > -F: drivers/video/backlight/lp855x_bl.c > -F: include/linux/platform_data/lp855x.h > - > -TI LP8727 CHARGER DRIVER > -M: Milo Kim > -S: Maintained > -F: drivers/power/supply/lp8727_charger.c > -F: include/linux/platform_data/lp8727.h > - > -TI LP8788 MFD DRIVER > -M: Milo Kim > -S: Maintained > -F: drivers/iio/adc/lp8788_adc.c > -F: drivers/leds/leds-lp8788.c > -F: drivers/mfd/lp8788*.c > -F: drivers/power/supply/lp8788-charger.c > -F: drivers/regulator/lp8788-*.c > -F: include/linux/mfd/lp8788*.h > - > TI NETCP ETHERNET DRIVER > M: Wingman Kwok > M: Murali Karicheri -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] MAINTAINERS: move Milo Kim to credits
Milo Kim's email in TI bounces with permanent error (550: Invalid recipient). Last email from him on LKML was in 2017. Move Milo Kim to credits and remove the separate driver entries for: - TI LP855x backlight driver, - TI LP8727 charger driver, - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers. Signed-off-by: Krzysztof Kozlowski --- CREDITS | 3 +++ MAINTAINERS | 23 --- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/CREDITS b/CREDITS index 1df63cdf71df..46384b11f258 100644 --- a/CREDITS +++ b/CREDITS @@ -1910,6 +1910,9 @@ S: 660 Harvard Ave. #7 S: Santa Clara, CA 95051 S: USA +N: Milo Kim +D: TI LP855x, LP8727 and LP8788 drivers + N: Russell King E: r...@arm.linux.org.uk D: Linux/arm integrator, maintainer & hacker diff --git a/MAINTAINERS b/MAINTAINERS index 5b9621ca2b31..031adeff29cc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17471,29 +17471,6 @@ S: Maintained F: sound/soc/codecs/isabelle* F: sound/soc/codecs/lm49453* -TI LP855x BACKLIGHT DRIVER -M: Milo Kim -S: Maintained -F: Documentation/driver-api/backlight/lp855x-driver.rst -F: drivers/video/backlight/lp855x_bl.c -F: include/linux/platform_data/lp855x.h - -TI LP8727 CHARGER DRIVER -M: Milo Kim -S: Maintained -F: drivers/power/supply/lp8727_charger.c -F: include/linux/platform_data/lp8727.h - -TI LP8788 MFD DRIVER -M: Milo Kim -S: Maintained -F: drivers/iio/adc/lp8788_adc.c -F: drivers/leds/leds-lp8788.c -F: drivers/mfd/lp8788*.c -F: drivers/power/supply/lp8788-charger.c -F: drivers/regulator/lp8788-*.c -F: include/linux/mfd/lp8788*.h - TI NETCP ETHERNET DRIVER M: Wingman Kwok M: Murali Karicheri -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/6] x86/xen: open code alloc_vm_area in arch_gnttab_valloc
On 9/18/20 12:37 PM, Christoph Hellwig wrote: > > +static int gnttab_apply(pte_t *pte, unsigned long addr, void *data) > +{ > + pte_t ***p = data; > + > + **p = pte; > + (*p)++; > + return 0; > +} > + > static int arch_gnttab_valloc(struct gnttab_vm_area *area, unsigned > nr_frames) > { > area->ptes = kmalloc_array(nr_frames, sizeof(*area->ptes), GFP_KERNEL); > if (area->ptes == NULL) > return -ENOMEM; > - > - area->area = alloc_vm_area(PAGE_SIZE * nr_frames, area->ptes); > - if (area->area == NULL) { > - kfree(area->ptes); > - return -ENOMEM; > - } > - > + area->area = get_vm_area(PAGE_SIZE * nr_frames, VM_IOREMAP); > + if (!area->area) > + goto out_free_ptes; > + if (apply_to_page_range(_mm, (unsigned long)area->area->addr, > + PAGE_SIZE * nr_frames, gnttab_apply, >ptes)) This will end up incrementing area->ptes pointer. So perhaps something like pte_t **ptes = area->ptes; if (apply_to_page_range(_mm, (unsigned long)area->area->addr, PAGE_SIZE * nr_frames, gnttab_apply, )) { ... } -boris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/panel: samsung: make vint_table static const
On Wed, Sep 16, 2020 at 3:22 PM Jason Yan wrote: > This eliminates the following sparse warning: > > drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c:217:15: warning: symbol > 'vint_table' was not declared. Should it be static? > > While at it, make the table const as it is never modified. > > Reported-by: Hulk Robot > Signed-off-by: Jason Yan > Reviewed-by: Laurent Pinchart Patch applied to drm-misc-next. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Sat, Sep 19, 2020 at 06:39:06PM +0100, Matthew Wilcox wrote: > On Sat, Sep 19, 2020 at 10:18:54AM -0700, Linus Torvalds wrote: > > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: > > > > > > this provides a preemptible variant of kmap_atomic & related > > > interfaces. This is achieved by: > > > > Ack. This looks really nice, even apart from the new capability. > > > > The only thing I really reacted to is that the name doesn't make sense > > to me: "kmap_temporary()" seems a bit odd. > > > > Particularly for an interface that really is basically meant as a > > better replacement of "kmap_atomic()" (but is perhaps also a better > > replacement for "kmap()"). > > > > I think I understand how the name came about: I think the "temporary" > > is there as a distinction from the "longterm" regular kmap(). So I > > think it makes some sense from an internal implementation angle, but I > > don't think it makes a lot of sense from an interface name. > > > > I don't know what might be a better name, but if we want to emphasize > > that it's thread-private and a one-off, maybe "local" would be a > > better naming, and make it distinct from the "global" nature of the > > old kmap() interface? > > > > However, another solution might be to just use this new preemptible > > "local" kmap(), and remove the old global one entirely. Yes, the old > > global one caches the page table mapping and that sounds really > > efficient and nice. But it's actually horribly horribly bad, because > > it means that we need to use locking for them. Your new "temporary" > > implementation seems to be fundamentally better locking-wise, and only > > need preemption disabling as locking (and is equally fast for the > > non-highmem case). > > > > So I wonder if the single-page TLB flush isn't a better model, and > > whether it wouldn't be a lot simpler to just get rid of the old > > complex kmap() entirely, and replace it with this? > > > > I agree we can't replace the kmap_atomic() version, because maybe > > people depend on the preemption disabling it also implied. But what > > about replacing the non-atomic kmap()? > > My concern with that is people might use kmap() and then pass the address > to a different task. So we need to audit the current users of kmap() > and convert any that do that into using vmap() instead. > I've done some of this work.[3] PKS and pmem stray write protection[2] depend on kmap to enable the correct PKS settings. After working through the exception handling we realized that some users of kmap() seem to be doing just this; passing the address to a different task. >From what I have found ~90% of kmap() callers are 'kmap_thread()' and the other ~10% are kmap().[3] But of those 10% I'm not familiar with the code enough to know if they really require a 'global' map. What I do know is they save an address which appears to be used in other threads. But I could be wrong. For PKS I added a 'global' implementation which could then be called by kmap() and added a new kmap_thread() call which used the original 'local' version of the PKS interface. The PKS work is still being reviewed internally for the TIP core code. But I've pushed it all to git hub for purposes of this discussion.[1] > I like kmap_local(). Or kmap_thread(). I chose kmap_thread() so that makes sense to me. I also thought about using kmap_global() as an alternative interface which would change just ~10% of the callers and make the series much smaller. But internal discussions lead me to chose kmap_thread() as the new interface so that we don't change the semantics of kmap(). Ira [1] https://github.com/weiny2/linux-kernel/tree/lm-pks-pmem-for-5.10-v3 [2] https://lore.kernel.org/lkml/20200717072056.73134-1-ira.we...@intel.com/ [3] 12:42:06 > git grep ' kmap(' *.c | grep -v '* ' | wc -l 22 12:43:32 > git grep ' kmap_thread(' *.c | grep -v '* ' | wc -l 204 Here are the callers which hand an address to another thread. 12:45:25 > git grep ' kmap(' *.c | grep -v '* ' arch/x86/mm/dump_pagetables.c: [PKMAP_BASE_NR] = { 0UL, "Persistent kmap() Area" }, drivers/firewire/net.c: ptr = kmap(dev->broadcast_rcv_buffer.pages[u]); drivers/gpu/drm/i915/gem/i915_gem_pages.c: return kmap(sg_page(sgt->sgl)); drivers/gpu/drm/i915/selftests/i915_perf.c: scratch = kmap(ce->vm->scratch[0].base.page); drivers/gpu/drm/ttm/ttm_bo_util.c: map->virtual = kmap(map->page); drivers/infiniband/hw/qib/qib_user_sdma.c: mpage = kmap(page); drivers/misc/vmw_vmci/vmci_host.c: context->notify = kmap(context->notify_page) + (uva & (PAGE_SIZE - 1)); drivers/misc/xilinx_sdfec.c:addr = kmap(pages[i]); drivers/mmc/host/usdhi6rol0.c: host->pg.mapped = kmap(host->pg.page); drivers/mmc/host/usdhi6rol0.c: host->pg.mapped = kmap(host->pg.page); drivers/mmc/host/usdhi6rol0.c: host->pg.mapped = kmap(host->pg.page); drivers/nvme/target/tcp.c: iov->iov_base =
Re: [PATCH v1 ] drm/amd/pm: Removed fixed clock in auto mode DPM
Applied with fixed up whitespace. Thanks, Alex On Tue, Sep 15, 2020 at 3:45 AM Christian König wrote: > > Am 15.09.20 um 09:18 schrieb Sudheesh Mavila: > > SMU10_UMD_PSTATE_PEAK_FCLK value should not be used to set the DPM. > > > > Change suggested by evan.q...@amd.com > > Can't say much about the change itself, but the Commit message is > indented and the indentation in the code doesn't look consistent either. > > Christian. > > > > > Signed-off-by: Sudheesh Mavila > > --- > > drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 10 ++ > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c > > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c > > index c9cfe90a2947..081cb9b1b7c8 100644 > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c > > @@ -566,6 +566,8 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr > > *hwmgr, > > struct smu10_hwmgr *data = hwmgr->backend; > > uint32_t min_sclk = hwmgr->display_config->min_core_set_clock; > > uint32_t min_mclk = hwmgr->display_config->min_mem_set_clock/100; > > + uint32_t index_fclk = data->clock_vol_info.vdd_dep_on_fclk->count - 1; > > + uint32_t index_socclk = data->clock_vol_info.vdd_dep_on_socclk->count > > - 1; > > > > if (hwmgr->smu_version < 0x1E3700) { > > pr_info("smu firmware version too old, can not set dpm > > level\n"); > > @@ -679,13 +681,13 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr > > *hwmgr, > > smum_send_msg_to_smc_with_parameter(hwmgr, > > > > PPSMC_MSG_SetHardMinFclkByFreq, > > > > hwmgr->display_config->num_display > 3 ? > > - SMU10_UMD_PSTATE_PEAK_FCLK : > > + > > data->clock_vol_info.vdd_dep_on_fclk->entries[0].clk : > > min_mclk, > > NULL); > > > > smum_send_msg_to_smc_with_parameter(hwmgr, > > > > PPSMC_MSG_SetHardMinSocclkByFreq, > > - SMU10_UMD_PSTATE_MIN_SOCCLK, > > + > > data->clock_vol_info.vdd_dep_on_socclk->entries[0].clk, > > NULL); > > smum_send_msg_to_smc_with_parameter(hwmgr, > > PPSMC_MSG_SetHardMinVcn, > > @@ -698,11 +700,11 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr > > *hwmgr, > > NULL); > > smum_send_msg_to_smc_with_parameter(hwmgr, > > > > PPSMC_MSG_SetSoftMaxFclkByFreq, > > - SMU10_UMD_PSTATE_PEAK_FCLK, > > + > > data->clock_vol_info.vdd_dep_on_fclk->entries[index_fclk].clk, > > NULL); > > smum_send_msg_to_smc_with_parameter(hwmgr, > > > > PPSMC_MSG_SetSoftMaxSocclkByFreq, > > - SMU10_UMD_PSTATE_PEAK_SOCCLK, > > + > > data->clock_vol_info.vdd_dep_on_socclk->entries[index_socclk].clk, > > NULL); > > smum_send_msg_to_smc_with_parameter(hwmgr, > > PPSMC_MSG_SetSoftMaxVcn, > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display: optimize code runtime a bit
On Mon, Sep 21, 2020 at 9:14 AM Bernard Zhao wrote: > > Static function dal_ddc_i2c_payloads_destroy is only called > in dal_ddc_service_query_ddc_data, the parameter is > , there is no point NULL risk, so no need to check. > This change is to make the code run a bit fast. > How about just getting rid of dal_ddc_i2c_payloads_destroy() and just call dal_vector_destruct() directly. Alex > Signed-off-by: Bernard Zhao > --- > drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c > b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c > index b984eecca58b..6dcc666738fc 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c > @@ -150,9 +150,6 @@ static uint32_t dal_ddc_i2c_payloads_get_count(struct > i2c_payloads *p) > > static void dal_ddc_i2c_payloads_destroy(struct i2c_payloads *p) > { > - if (!p) > - return; > - > dal_vector_destruct(>payloads); > } > > -- > 2.28.0 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/amdgpu/gmc9: simplify the return expression of gmc_v9_0_suspend
Applied. Thanks! Alex On Mon, Sep 21, 2020 at 9:14 AM Liu Shixin wrote: > > Simplify the return expression. > > Signed-off-by: Liu Shixin > --- > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > index 5400cac02087..cb9e9e5afa5a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > @@ -1683,14 +1683,9 @@ static int gmc_v9_0_hw_fini(void *handle) > > static int gmc_v9_0_suspend(void *handle) > { > - int r; > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > - r = gmc_v9_0_hw_fini(adev); > - if (r) > - return r; > - > - return 0; > + return gmc_v9_0_hw_fini(adev); > } > > static int gmc_v9_0_resume(void *handle) > -- > 2.25.1 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/amd/pm: simplify the return expression of smu_hw_fini
Applied. Thanks! Alex On Mon, Sep 21, 2020 at 9:14 AM Liu Shixin wrote: > > Simplify the return expression. > > Signed-off-by: Liu Shixin > --- > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > index 5c4b74f964fc..3612841d40dc 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > @@ -1214,7 +1214,6 @@ static int smu_hw_fini(void *handle) > { > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > struct smu_context *smu = >smu; > - int ret = 0; > > if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev)) > return 0; > @@ -1230,11 +1229,7 @@ static int smu_hw_fini(void *handle) > > adev->pm.dpm_enabled = false; > > - ret = smu_smc_hw_cleanup(smu); > - if (ret) > - return ret; > - > - return 0; > + return smu_smc_hw_cleanup(smu); > } > > int smu_reset(struct smu_context *smu) > -- > 2.25.1 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/amdgpu: simplify the return expression
Applied. Thanks! Alex On Mon, Sep 21, 2020 at 9:14 AM Qinglang Miao wrote: > > Simplify the return expression. > > Signed-off-by: Qinglang Miao > --- > drivers/gpu/drm/amd/amdgpu/cik_ih.c | 7 +-- > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 10 ++ > 2 files changed, 3 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c > b/drivers/gpu/drm/amd/amdgpu/cik_ih.c > index 401c99f0b..db953e95f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c > +++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c > @@ -316,14 +316,9 @@ static int cik_ih_sw_fini(void *handle) > > static int cik_ih_hw_init(void *handle) > { > - int r; > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > - r = cik_ih_irq_init(adev); > - if (r) > - return r; > - > - return 0; > + return cik_ih_irq_init(adev); > } > > static int cik_ih_hw_fini(void *handle) > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index 20d8a03ca..56ed108b2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -2198,7 +2198,6 @@ static int gfx_v9_0_gpu_early_init(struct amdgpu_device > *adev) > static int gfx_v9_0_compute_ring_init(struct amdgpu_device *adev, int > ring_id, > int mec, int pipe, int queue) > { > - int r; > unsigned irq_type; > struct amdgpu_ring *ring = >gfx.compute_ring[ring_id]; > unsigned int hw_prio; > @@ -2223,13 +,8 @@ static int gfx_v9_0_compute_ring_init(struct > amdgpu_device *adev, int ring_id, > hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, > ring->queue) ? > AMDGPU_GFX_PIPE_PRIO_HIGH : > AMDGPU_GFX_PIPE_PRIO_NORMAL; > /* type-2 packets are deprecated on MEC, use type-3 instead */ > - r = amdgpu_ring_init(adev, ring, 1024, > ->gfx.eop_irq, irq_type, hw_prio); > - if (r) > - return r; > - > - > - return 0; > + return amdgpu_ring_init(adev, ring, 1024, > + >gfx.eop_irq, irq_type, hw_prio); > } > > static int gfx_v9_0_sw_init(void *handle) > -- > 2.23.0 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/amdgpu/mes: simplify the return expression of mes_v10_1_ring_init
Applied. Thanks! Alex On Mon, Sep 21, 2020 at 9:14 AM Qinglang Miao wrote: > > Simplify the return expression. > > Signed-off-by: Qinglang Miao > --- > drivers/gpu/drm/amd/amdgpu/mes_v10_1.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c > b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c > index 4b746584a..1c22d8393 100644 > --- a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c > +++ b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c > @@ -832,7 +832,6 @@ static int mes_v10_1_queue_init(struct amdgpu_device > *adev) > static int mes_v10_1_ring_init(struct amdgpu_device *adev) > { > struct amdgpu_ring *ring; > - int r; > > ring = >mes.ring; > > @@ -849,11 +848,7 @@ static int mes_v10_1_ring_init(struct amdgpu_device > *adev) > ring->no_scheduler = true; > sprintf(ring->name, "mes_%d.%d.%d", ring->me, ring->pipe, > ring->queue); > > - r = amdgpu_ring_init(adev, ring, 1024, NULL, 0, > AMDGPU_RING_PRIO_DEFAULT); > - if (r) > - return r; > - > - return 0; > + return amdgpu_ring_init(adev, ring, 1024, NULL, 0, > AMDGPU_RING_PRIO_DEFAULT); > } > > static int mes_v10_1_mqd_sw_init(struct amdgpu_device *adev) > -- > 2.23.0 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #30 from Lyude Paul (ly...@redhat.com) --- Hi! Sorry for the long wait, I got distracted with some other work that needed to be done. Also, sorry for making you buy that adapter since it looks like things aren't working still. So I took a look at the logs that you gave me and noticed some interesting things that didn't catch my eye before. In the last log you gave me with the new DP adapter I can't see nouveau noticing any of the monitors. On the log before that though, I actually see it detect all four monitors. I also see it actually turn all four on successfully, huh. The other thing I noticed is that it looks like you're using the nouveau DDX in your X config. That makes me wonder, do the screens come up -at all- during the boot process before the GUI comes up? As well, have you tried the modesetting DDX to see if that makes any difference, or perhaps wayland? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] zsmalloc: switch from alloc_vm_area to get_vm_area
On Mon, Sep 21, 2020 at 08:17:08PM +0200, Christoph Hellwig wrote: > On Mon, Sep 21, 2020 at 10:42:56AM -0700, Minchan Kim wrote: > > IIRC, the problem was runtime pte popluating needs GFP_KERNEL but > > zs_map_object API runs under non-preemtible section. > > Make sense. > > > > - area->vm = alloc_vm_area(PAGE_SIZE * 2, NULL); > > > + area->vm = get_vm_area(PAGE_SIZE * 2, 0); > > > if (!area->vm) > > > return -ENOMEM; > > > return 0; > > > > I think it shoud work. > > > > diff --git a/mm/memory.c b/mm/memory.c > > index 05789aa4af12..6a1e4d854593 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -2232,7 +2232,7 @@ static int apply_to_pte_range(struct mm_struct *mm, > > pmd_t *pmd, > > arch_enter_lazy_mmu_mode(); > > > > do { > > - if (create || !pte_none(*pte)) { > > + if ((create || !pte_none(*pte)) && fn) { > > err = fn(pte++, addr, data); > > if (err) > > break; > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > > index 3e4fe3259612..9ef7daf3d279 100644 > > --- a/mm/zsmalloc.c > > +++ b/mm/zsmalloc.c > > @@ -1116,6 +1116,8 @@ static struct zspage *find_get_zspage(struct > > size_class *class) > > #ifdef CONFIG_ZSMALLOC_PGTABLE_MAPPING > > static inline int __zs_cpu_up(struct mapping_area *area) > > { > > + int ret; > > + > > /* > > * Make sure we don't leak memory if a cpu UP notification > > * and zs_init() race and both call zs_cpu_up() on the same cpu > > @@ -1125,7 +1127,13 @@ static inline int __zs_cpu_up(struct mapping_area > > *area) > > area->vm = get_vm_area(PAGE_SIZE * 2, 0); > > if (!area->vm) > > return -ENOMEM; > > - return 0; > > + > > + /* > > +* Populate ptes in advance to avoid pte allocation with GFP_KERNEL > > +* in non-preemtible context of zs_map_object. > > +*/ > > + ret = apply_to_page_range(_mm, NULL, PAGE_SIZE * 2, NULL, NULL); > > + return ret; > > I think this needs the addr from the vm area somewhere.. Yeah, let's assign the addres we got get_vm_area. > > We probaby want to add a trivial helper to prefault an area instead of > the open coded variant. It seems zsmalloc is only customer the function so let's have the helper when we see another customer. If we don't have objection, I'd like to ask to Andrew fold this up. --- mm/memory.c | 2 +- mm/zsmalloc.c | 8 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 05789aa4af12..6a1e4d854593 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -2232,7 +2232,7 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd, arch_enter_lazy_mmu_mode(); do { - if (create || !pte_none(*pte)) { + if ((create || !pte_none(*pte)) && fn) { err = fn(pte++, addr, data); if (err) break; diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index 3e4fe3259612..918c7b019b3d 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -1125,7 +1125,13 @@ static inline int __zs_cpu_up(struct mapping_area *area) area->vm = get_vm_area(PAGE_SIZE * 2, 0); if (!area->vm) return -ENOMEM; - return 0; + + /* +* Populate ptes in advance to avoid pte allocation with GFP_KERNEL +* in non-preemtible context of zs_map_object. +*/ + return apply_to_page_range(_mm, (unsigned long)area->vm->addr, + PAGE_SIZE * 2, NULL, NULL); } static inline void __zs_cpu_down(struct mapping_area *area) -- 2.28.0.681.g6f77f65b4e-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dp: start using more of the extended receiver caps
On Mon, 21 Sep 2020, Lyude Paul wrote: > On Tue, 2020-09-01 at 21:01 +0300, Jani Nikula wrote: >> I guess we can try without the change, and fix later if we hit issues. > > I'm fine with the change if it doesn't break things btw - just as long as > we're making sure that we don't zero things out by accident My conclusion was to go without, it's a trivial change to add afterwards as needed. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dp: start using more of the extended receiver caps
On Tue, 2020-09-01 at 21:01 +0300, Jani Nikula wrote: > On Tue, 01 Sep 2020, Lyude Paul wrote: > > On Tue, 2020-09-01 at 15:32 +0300, Jani Nikula wrote: > > > In the future, we'll be needing more of the extended receiver capability > > > field starting at DPCD address 0x2200. (Specifically, we'll need main > > > link channel coding cap for DP 2.0.) Start using it now to not miss out > > > later on. > > > > > > Cc: Lyude Paul > > > Signed-off-by: Jani Nikula > > > > > > --- > > > > > > I guess this can be merged after the topic branch to drm-misc-next or > > > so, but I'd prefer to have this fairly early on to catch any potential > > > issues. > > > --- > > > drivers/gpu/drm/drm_dp_helper.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > > > b/drivers/gpu/drm/drm_dp_helper.c > > > index 1e7c638873c8..3a3c238452df 100644 > > > --- a/drivers/gpu/drm/drm_dp_helper.c > > > +++ b/drivers/gpu/drm/drm_dp_helper.c > > > @@ -436,7 +436,7 @@ static u8 drm_dp_downstream_port_count(const u8 > > > dpcd[DP_RECEIVER_CAP_SIZE]) > > > static int drm_dp_read_extended_dpcd_caps(struct drm_dp_aux *aux, > > > u8 dpcd[DP_RECEIVER_CAP_SIZE]) > > > { > > > - u8 dpcd_ext[6]; > > > + u8 dpcd_ext[DP_RECEIVER_CAP_SIZE]; > > > > Not 100% sure this is right? It's not clear at first glance of the 2.0 > > spec, but > > my assumption would be that on < DP2.0 devices that everything but those > > first 6 > > bytes are zeroed out in the extended DPRX field. Since we memcpy() > > dpcd_ext > > using sizeof(dpcd_ext), we'd potentially end up zeroing out all of the > > DPCD caps > > that comes after those 6 bytes. > > Re-reading stuff... AFAICT everything in 0x2200..0x220F should be > valid. They should match what's in 0x..0x000F except for 0x, > 0x0001, and 0x0005, for backwards compatibility. > > Apparently there are no such backwards compatibility concerns with the > other receiver cap fields then. > > But it gives me an uneasy feeling that many places in the spec refer to > 0x2200+ even though they should per spec be the same in 0x+. > > I guess we can try without the change, and fix later if we hit issues. I'm fine with the change if it doesn't break things btw - just as long as we're making sure that we don't zero things out by accident > > > BR, > Jani. > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/dp_mst: Retrieve extended DPCD caps for topology manager
Hi, sorry I lost track of this until just now. Comments down below: On Fri, 2020-09-11 at 11:44 +0800, Koba Ko wrote: > As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT. > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV to > get the faster capability. > If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV. > > Signed-off-by: Koba Ko > Reviewed-by: Lyude Paul > --- > Changelog: > 1. Adjust the commit message. > 2. use drm_dbg_kms instead and print the return code. > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++--- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 7753c718ddf9..63f8809b9aa4 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -3671,8 +3671,6 @@ EXPORT_SYMBOL(drm_dp_read_mst_cap); > int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, > bool mst_state) > { > int ret = 0; > - u8 dpcd_ext = 0; > - unsigned int dpcd_offset = 0; > struct drm_dp_mst_branch *mstb = NULL; > > mutex_lock(>payload_lock); > @@ -3686,17 +3684,11 @@ int drm_dp_mst_topology_mgr_set_mst(struct > drm_dp_mst_topology_mgr *mgr, bool ms > struct drm_dp_payload reset_pay; > > WARN_ON(mgr->mst_primary); > - drm_dp_dpcd_read(mgr->aux, > - DP_TRAINING_AUX_RD_INTERVAL, > - _ext, sizeof(dpcd_ext)); > - > - dpcd_offset = > - ((dpcd_ext & DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) > ? DP_DP13_DPCD_REV : DP_DPCD_REV); Uh, are you sure you formatted this patch correctly? None of these hunks are present upstream, this looks like it's a diff for the previous version of this patch that you sent out > > /* get dpcd info */ > - ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd, > DP_RECEIVER_CAP_SIZE); > - if (ret != DP_RECEIVER_CAP_SIZE) { > - DRM_DEBUG_KMS("failed to read DPCD\n"); > + ret = drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd); > + if (ret < 0) { > + drm_dbg_kms(mgr->dev, "failed to read DPCD, ret %d\n", > ret); > goto out_unlock; > } > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv4 1/6] iommu/io-pgtable-arm: Add support to use system cache
On Fri, Sep 11, 2020 at 07:57:18PM +0530, Sai Prakash Ranjan wrote: > Add a quirk IO_PGTABLE_QUIRK_SYS_CACHE to override the > attributes set in TCR for the page table walker when > using system cache. I wonder if the panfrost folks can reuse this for the issue discussed over at: https://lore.kernel.org/r/cover.1600213517.git.robin.mur...@arm.com However, Sai, your email setup went wrong when you posted this so you probably need to repost now that you have that fixed. Will ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] iommu/io-pgtable-arm: Support coherency for Mali LPAE
On Wed, Sep 16, 2020 at 12:51:05AM +0100, Robin Murphy wrote: > Midgard GPUs have ACE-Lite master interfaces which allows systems to > integrate them in an I/O-coherent manner. It seems that from the GPU's > viewpoint, the rest of the system is its outer shareable domain, and so > even when snoop signals are wired up, they are only emitted for outer > shareable accesses. As such, setting the TTBR_SHARE_OUTER bit does > indeed get coherent pagetable walks working nicely for the coherent > T620 in the Arm Juno SoC. I can't help but think some of this commentary deserves to be in the code as well. Do you know if this sort of thing is done for other SoCs too, or is this just a Juno quirk? Will ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] zsmalloc: switch from alloc_vm_area to get_vm_area
On Fri, Sep 18, 2020 at 06:37:19PM +0200, Christoph Hellwig wrote: > There is no obvious reason why zsmalloc needs to pre-fault the PTEs > given that it later uses map_kernel_range to just like vmap(). IIRC, the problem was runtime pte popluating needs GFP_KERNEL but zs_map_object API runs under non-preemtible section. > > Signed-off-by: Christoph Hellwig > --- > mm/zsmalloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > index c36fdff9a37131..3e4fe3259612fd 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -1122,7 +1122,7 @@ static inline int __zs_cpu_up(struct mapping_area *area) >*/ > if (area->vm) > return 0; > - area->vm = alloc_vm_area(PAGE_SIZE * 2, NULL); > + area->vm = get_vm_area(PAGE_SIZE * 2, 0); > if (!area->vm) > return -ENOMEM; > return 0; I think it shoud work. diff --git a/mm/memory.c b/mm/memory.c index 05789aa4af12..6a1e4d854593 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -2232,7 +2232,7 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd, arch_enter_lazy_mmu_mode(); do { - if (create || !pte_none(*pte)) { + if ((create || !pte_none(*pte)) && fn) { err = fn(pte++, addr, data); if (err) break; diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index 3e4fe3259612..9ef7daf3d279 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -1116,6 +1116,8 @@ static struct zspage *find_get_zspage(struct size_class *class) #ifdef CONFIG_ZSMALLOC_PGTABLE_MAPPING static inline int __zs_cpu_up(struct mapping_area *area) { + int ret; + /* * Make sure we don't leak memory if a cpu UP notification * and zs_init() race and both call zs_cpu_up() on the same cpu @@ -1125,7 +1127,13 @@ static inline int __zs_cpu_up(struct mapping_area *area) area->vm = get_vm_area(PAGE_SIZE * 2, 0); if (!area->vm) return -ENOMEM; - return 0; + + /* +* Populate ptes in advance to avoid pte allocation with GFP_KERNEL +* in non-preemtible context of zs_map_object. +*/ + ret = apply_to_page_range(_mm, NULL, PAGE_SIZE * 2, NULL, NULL); + return ret; } static inline void __zs_cpu_down(struct mapping_area *area) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm: omapdrm: dss: simplify the return expression of hdmi_init_pll_data
Hi, On 21/09/2020 16:10, Qinglang Miao wrote: > Simplify the return expression. > > Signed-off-by: Qinglang Miao > --- > drivers/gpu/drm/omapdrm/dss/hdmi_pll.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_pll.c > b/drivers/gpu/drm/omapdrm/dss/hdmi_pll.c > index cf2b000f3..c3e85b636 100644 > --- a/drivers/gpu/drm/omapdrm/dss/hdmi_pll.c > +++ b/drivers/gpu/drm/omapdrm/dss/hdmi_pll.c > @@ -131,7 +131,6 @@ static int hdmi_init_pll_data(struct dss_device *dss, > { > struct dss_pll *pll = >pll; > struct clk *clk; > - int r; > > clk = devm_clk_get(>dev, "sys_clk"); > if (IS_ERR(clk)) { > @@ -151,11 +150,7 @@ static int hdmi_init_pll_data(struct dss_device *dss, > > pll->ops = _pll_ops; > > - r = dss_pll_register(dss, pll); > - if (r) > - return r; > - > - return 0; > + return dss_pll_register(dss, pll); > } > > int hdmi_pll_init(struct dss_device *dss, struct platform_device *pdev, > I like it more when there's a return 0 at the end of the function, especially in functions where there are multiple cases of if(...) return r. It's more easily readable, at least to my eyes. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
Hi Guido, On Mon, Sep 21, 2020 at 1:56 PM Guido Günther wrote: > > We need to reset both for the panel to show an image. There is a typo in "resets" in the Subject line. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v1 1/3] drm/panel: mantix: Don't dereference NULL mode
Don't dereference mode which was just NULL checked. Signed-off-by: Guido Günther Reported-by: Dan Carpenter --- drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c index 3482e28e30fc..4a7fbf64bb7a 100644 --- a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c +++ b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c @@ -204,7 +204,7 @@ static int mantix_get_modes(struct drm_panel *panel, if (!mode) { dev_err(ctx->dev, "Failed to add mode %ux%u@%u\n", default_mode.hdisplay, default_mode.vdisplay, - drm_mode_vrefresh(mode)); + drm_mode_vrefresh(_mode)); return -ENOMEM; } -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
We need to reset both for the panel to show an image. Signed-off-by: Guido Günther --- .../bindings/display/panel/mantix,mlaf057we51-x.yaml | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml index 937323cc9aaa..ba5a18fac9f9 100644 --- a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +++ b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml @@ -35,7 +35,9 @@ properties: vddi-supply: description: 1.8V I/O voltage supply - reset-gpios: true + reset-gpios: +minItems: 2 +maxItems: 2 backlight: true @@ -62,7 +64,8 @@ examples: avdd-supply = <_avdd>; avee-supply = <_avee>; vddi-supply = <_1v8_p>; -reset-gpios = < 29 GPIO_ACTIVE_LOW>; +reset-gpios = < 29 GPIO_ACTIVE_LOW>, + < 24 GPIO_ACTIVE_LOW>; backlight = <>; }; }; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v1 2/3] drm/panel: mantix: Fix panel reset
The mantix panel needs two reset lines (RESX and TP_RSTN) deasserted to output an image. Only deasserting RESX is not enough and the display will stay blank. Deassert in prepare() and assert in unprepare() to keep device held in reset when off. Signed-off-by: Guido Günther --- .../gpu/drm/panel/panel-mantix-mlaf057we51.c | 37 --- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c index 4a7fbf64bb7a..29d28c63bb72 100644 --- a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c +++ b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c @@ -23,10 +23,13 @@ #define MANTIX_CMD_OTP_STOP_RELOAD_MIPI 0x41 #define MANTIX_CMD_INT_CANCEL 0x4C +#define MANTIX_NUM_RESETS 2 + struct mantix { struct device *dev; struct drm_panel panel; - struct gpio_desc *reset_gpio; + /* RESX and TP_RSTN */ + struct gpio_descs *reset_gpios; struct regulator *avdd; struct regulator *avee; @@ -122,8 +125,13 @@ static int mantix_disable(struct drm_panel *panel) static int mantix_unprepare(struct drm_panel *panel) { + DECLARE_BITMAP(asserted_values, BITS_PER_TYPE(1)); struct mantix *ctx = panel_to_mantix(panel); + gpiod_set_array_value_cansleep(ctx->reset_gpios->ndescs, + ctx->reset_gpios->desc, + ctx->reset_gpios->info, asserted_values); + regulator_disable(ctx->avee); regulator_disable(ctx->avdd); /* T11 */ @@ -137,6 +145,7 @@ static int mantix_unprepare(struct drm_panel *panel) static int mantix_prepare(struct drm_panel *panel) { + DECLARE_BITMAP(deasserted_values, BITS_PER_TYPE(0)); struct mantix *ctx = panel_to_mantix(panel); int ret; @@ -165,14 +174,11 @@ static int mantix_prepare(struct drm_panel *panel) return ret; } - /* T3+T5 */ - usleep_range(1, 12000); - - gpiod_set_value_cansleep(ctx->reset_gpio, 1); - usleep_range(5150, 7000); - - gpiod_set_value_cansleep(ctx->reset_gpio, 0); - + /* T3 + T4 + time for voltage to become stable: */ + usleep_range(6000, 7000); + gpiod_set_array_value_cansleep(ctx->reset_gpios->ndescs, + ctx->reset_gpios->desc, + ctx->reset_gpios->info, deasserted_values); /* T6 */ msleep(50); @@ -236,10 +242,15 @@ static int mantix_probe(struct mipi_dsi_device *dsi) if (!ctx) return -ENOMEM; - ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); - if (IS_ERR(ctx->reset_gpio)) { - dev_err(dev, "cannot get reset gpio\n"); - return PTR_ERR(ctx->reset_gpio); + ctx->reset_gpios = devm_gpiod_get_array(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(ctx->reset_gpios)) { + dev_err(dev, "cannot get reset gpios\n"); + return PTR_ERR(ctx->reset_gpios); + } + + if (ctx->reset_gpios->ndescs != MANTIX_NUM_RESETS) { + dev_err(dev, "Need exactly %d reset-gpios\n", MANTIX_NUM_RESETS); + return -EINVAL; } mipi_dsi_set_drvdata(dsi, ctx); -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v1 0/3] drm/panel: mantix panel reset fixes
Posting as RFC since I'm not sure how to handle the bindings, please see below. The first patch in this series fixes dereferencing a NULL mode in the error path. The second one extends the resets to not only reset RESX but also TP_RSTN since otherwise the display will stay completely blank. I didn't spot that before initial submission since the reset line was bound to the touch controller and although that failed to probe it came far enough to deassert the reset line (at an arbitrary point in time during boot) and hence we got a picture. Since touch and panel are on the same IC they're not completely independent and i might have to turn the whole thing into an MFD at some point but this series gets the panel to a reliably working state on boot and on fb blank/unblank. Since the reset-gpios are active low we can deassert in prepare and assert in unprepare simplifying the code making sure lines are kept low when the panel is off. As for the bindings it seems I can't override the maxItems of reset-gpios since that is already set in bindings/display/panel/panel-common.yaml to maxItems: 1`. Is there a way to do that instead of side stepping the issue and using a different name or using enable-gpios for one of them or forcing everyone else to set `maxItems: 1`? The binding were not part of a stable kernel so I hope it's okay to not worry about backward compatibility. Guido Günther (3): drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset dt-binding: display: Require two rests on mantix panel .../display/panel/mantix,mlaf057we51-x.yaml | 7 +++- .../gpu/drm/panel/panel-mantix-mlaf057we51.c | 39 --- 2 files changed, 30 insertions(+), 16 deletions(-) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 9/9] drm/mediatek: reduce clear event
On 08/07/2020 01:01, Chun-Kuang Hu wrote: Hi, Dennis: Dennis YC Hsieh 於 2020年7月7日 週二 下午11:47寫道: No need to clear event again since event always clear before wait. This fix depend on patch: "soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api" Acked-by: Chun-Kuang Hu Pushed now to v5.9-next/soc Thanks! Fixes: 2f965be7f9008 ("drm/mediatek: apply CMDQ control flow") Signed-off-by: Dennis YC Hsieh --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index c84e7a14d4a8..ba6cf956b239 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc) mbox_flush(mtk_crtc->cmdq_client->chan, 2000); cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE); cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event); - cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true); + cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false); mtk_crtc_ddp_config(crtc, cmdq_handle); cmdq_pkt_finalize(cmdq_handle); cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle); -- 1.7.9.5 ___ Linux-mediatek mailing list linux-media...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 9/9] drm/mediatek: reduce clear event
On 09/07/2020 04:39, Bibby Hsieh wrote: Hi, Dennis, Thanks for this patch. It's better to send another tree for this patch. Because this tree is only for soc/mediatek. Please do not forget to add the dependency information. Normally you are right. This time I took this patch as well into my tree. I verified with the merge tag from CKs DRM tree that we don't have any conflicts, so we should be Ok. Regards, Matthias Bibby On Tue, 2020-07-07 at 23:45 +0800, Dennis YC Hsieh wrote: No need to clear event again since event always clear before wait. This fix depend on patch: "soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api" Fixes: 2f965be7f9008 ("drm/mediatek: apply CMDQ control flow") Signed-off-by: Dennis YC Hsieh Reviewed-by: Bibby Hsieh --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index c84e7a14d4a8..ba6cf956b239 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc) mbox_flush(mtk_crtc->cmdq_client->chan, 2000); cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE); cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event); - cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true); + cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false); mtk_crtc_ddp_config(crtc, cmdq_handle); cmdq_pkt_finalize(cmdq_handle); cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Mon, Sep 21, 2020 at 12:39 AM Thomas Gleixner wrote: > > If a task is migrated to a different CPU then the mapping address will > change which will explode in colourful ways. Heh. Right you are. Maybe we really *could* call this new kmap functionality something like "kmap_percpu()" (or maybe "local" is good enough), and make it act like your RT code does for spinlocks - not disable preemption, but only disabling CPU migration. That would probably be good enough for a lot of users that don't want to expose excessive latencies, but where it's really not a huge deal to say "stick to this CPU for a short while". The crypto code certainly sounds like one such case. Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: commit_work scheduling
On Mon, Sep 21, 2020 at 9:10 AM Qais Yousef wrote: > > On 09/19/20 12:37, Rob Clark wrote: > > From: Rob Clark > > > > The android userspace treats the display pipeline as a realtime problem. > > And arguably, if your goal is to not miss frame deadlines (ie. vblank), > > it is. (See https://lwn.net/Articles/809545/ for the best explaination > > that I found.) > > > > But this presents a problem with using workqueues for non-blocking > > atomic commit_work(), because the SCHED_FIFO userspace thread(s) can > > preempt the worker. Which is not really the outcome you want.. once > > the required fences are scheduled, you want to push the atomic commit > > down to hw ASAP. > > > > But the decision of whether commit_work should be RT or not really > > depends on what userspace is doing. For a pure CFS userspace display > > pipeline, commit_work() should remain SCHED_NORMAL. > > Just a side note; this RT vs CFS inter-operatability is an issue that > creeps up every now and again. > > https://lore.kernel.org/lkml/1567048502-6064-1-git-send-email-jing-ting...@mediatek.com/ > > Does the UI thread in Android ever run as RT by the way? I always suspected it > is one susceptible to such potential delays since it is part of the > application > thread and thought it can't be trusted to become RT. The application itself is not RT, since there is no good way to know what other things the app may be doing. Although that is mentioned in the lwn article (ie. the hypothetical SCHED_DEADLINE + token passing part) But at least once the app / ui thread is done, then it is all (or at least in theory "all" if atomic-helper doesn't gum up the works ;-)) SCHED_FIFO from there to the display. > Those 120MHz displays will stress the pipeline :-) indeed BR, -R > > > > To handle this, convert non-blocking commit_work() to use per-CRTC > > kthread workers, instead of system_unbound_wq. Per-CRTC workers are > > used to avoid serializing commits when userspace is using a per-CRTC > > update loop. > > > > A client-cap is introduced so that userspace can opt-in to SCHED_FIFO > > priority commit work. > > > > A potential issue is that since 616d91b68cd ("sched: Remove > > sched_setscheduler*() EXPORTs") we have limited RT priority levels, > > meaning that commit_work() ends up running at the same priority level > > as vblank-work. This shouldn't be a big problem *yet*, due to limited > > use of vblank-work at this point. And if it could be arranged that > > vblank-work is scheduled before signaling out-fences and/or sending > > pageflip events, it could probably work ok to use a single priority > > level for both commit-work and vblank-work. > > This is a function of num_cpus too. As long as nr_cpus > nr_running_rt_tasks > you should be fine. > > Cheers > > -- > Qais Yousef ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 8/9] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
On 07/07/2020 17:45, Dennis YC Hsieh wrote: Add clear parameter to let client decide if event should be clear to 0 after GCE receive it. Change since v2: - Keep behavior in drm crtc driver and separate bug fix code into another patch. This, should go... Signed-off-by: Dennis YC Hsieh --- ...here :) I fixed to commit message and pushed the patch to v5.9-next/soc Thanks! drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +- drivers/soc/mediatek/mtk-cmdq-helper.c |5 +++-- include/linux/mailbox/mtk-cmdq-mailbox.h |3 +-- include/linux/soc/mediatek/mtk-cmdq.h|5 +++-- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index ec6c9ffbf35e..c84e7a14d4a8 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc) mbox_flush(mtk_crtc->cmdq_client->chan, 2000); cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE); cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event); - cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event); + cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true); mtk_crtc_ddp_config(crtc, cmdq_handle); cmdq_pkt_finalize(cmdq_handle); cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle); diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index d55dc3296105..505651b0d715 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -316,15 +316,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx, } EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value); -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event) +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear) { struct cmdq_instruction inst = { {0} }; + u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0; if (event >= CMDQ_MAX_EVENT) return -EINVAL; inst.op = CMDQ_CODE_WFE; - inst.value = CMDQ_WFE_OPTION; + inst.value = CMDQ_WFE_OPTION | clear_option; inst.event = event; return cmdq_pkt_append_command(pkt, inst); diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h index efbd8a9eb2d1..d5a983d65f05 100644 --- a/include/linux/mailbox/mtk-cmdq-mailbox.h +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h @@ -28,8 +28,7 @@ * bit 16-27: update value * bit 31: 1 - update, 0 - no update */ -#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | \ - CMDQ_WFE_WAIT_VALUE) +#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE) /** cmdq event maximum */ #define CMDQ_MAX_EVENT0x3ff diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index 34354e952f60..960704d75994 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -182,11 +182,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx, /** * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet * @pkt: the CMDQ packet - * @event: the desired event type to "wait and CLEAR" + * @event: the desired event type to wait + * @clear: clear event or not after event arrive * * Return: 0 for success; else the error code is returned */ -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event); +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear); /** * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: commit_work scheduling
On Mon, Sep 21, 2020 at 8:16 AM Rob Clark wrote: > > On Mon, Sep 21, 2020 at 2:21 AM Daniel Vetter wrote: > > > > On Sat, Sep 19, 2020 at 12:37:23PM -0700, Rob Clark wrote: > > > From: Rob Clark > > > > > > The android userspace treats the display pipeline as a realtime problem. > > > And arguably, if your goal is to not miss frame deadlines (ie. vblank), > > > it is. (See https://lwn.net/Articles/809545/ for the best explaination > > > that I found.) > > > > > > But this presents a problem with using workqueues for non-blocking > > > atomic commit_work(), because the SCHED_FIFO userspace thread(s) can > > > preempt the worker. Which is not really the outcome you want.. once > > > the required fences are scheduled, you want to push the atomic commit > > > down to hw ASAP. > > > > > > But the decision of whether commit_work should be RT or not really > > > depends on what userspace is doing. For a pure CFS userspace display > > > pipeline, commit_work() should remain SCHED_NORMAL. > > > > > > To handle this, convert non-blocking commit_work() to use per-CRTC > > > kthread workers, instead of system_unbound_wq. Per-CRTC workers are > > > used to avoid serializing commits when userspace is using a per-CRTC > > > update loop. > > > > > > A client-cap is introduced so that userspace can opt-in to SCHED_FIFO > > > priority commit work. > > > > > > A potential issue is that since 616d91b68cd ("sched: Remove > > > sched_setscheduler*() EXPORTs") we have limited RT priority levels, > > > meaning that commit_work() ends up running at the same priority level > > > as vblank-work. This shouldn't be a big problem *yet*, due to limited > > > use of vblank-work at this point. And if it could be arranged that > > > vblank-work is scheduled before signaling out-fences and/or sending > > > pageflip events, it could probably work ok to use a single priority > > > level for both commit-work and vblank-work. > > > > The part I don't like about this is that it all feels rather hacked > > together, and if we add more stuff (or there's some different thing in the > > system that also needs rt scheduling) then it doesn't compose. > > The ideal thing would be that userspace is in control of the > priorities.. the setclientcap approach seemed like a reasonable way to > give the drm-master a way to opt in. > > I suppose instead userspace could use sched_setscheduler().. but that > would require userspace to be root, and would require some way to find > the tid. > > Is there some way we could arrange for the per-crtc kthread's to be > owned by the drm master? That would solve the "must be root" issue. > And since the target audience is an atomic userspace, I suppose we > could expose the tid as a read-only property on the crtc? Looks like kthread goes out of it's way to *not* be owned by users (to avoid fork, and such complications?) But maybe we could modify the kthread_worker's task->real_cred? I didn't see any examples of anything else doing something similar, so I'm not sure what sorts of dragons there lie.. > BR, > -R > > > So question to rt/worker folks: What's the best way to let userspace set > > the scheduling mode and priorities of things the kernel does on its > > behalf? Surely we're not the first ones where if userspace runs with some > > rt priority it'll starve out the kernel workers that it needs. Hardcoding > > something behind a subsystem ioctl (which just means every time userspace > > changes what it does, we need a new such flag or mode) can't be the right > > thing. > > > > Peter, Tejun? > > > > Thanks, Daniel > > > > > > > > Rob Clark (3): > > > drm/crtc: Introduce per-crtc kworker > > > drm/atomic: Use kthread worker for nonblocking commits > > > drm: Add a client-cap to set scheduling mode > > > > > > drivers/gpu/drm/drm_atomic_helper.c | 13 ++ > > > drivers/gpu/drm/drm_auth.c | 4 > > > drivers/gpu/drm/drm_crtc.c | 37 + > > > drivers/gpu/drm/drm_ioctl.c | 13 ++ > > > include/drm/drm_atomic.h| 31 > > > include/drm/drm_crtc.h | 10 > > > include/uapi/drm/drm.h | 13 ++ > > > 7 files changed, 117 insertions(+), 4 deletions(-) > > > > > > -- > > > 2.26.2 > > > > > > ___ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 5/9] soc: mediatek: cmdq: add write_s value function
On 07/07/2020 17:45, Dennis YC Hsieh wrote: add write_s function in cmdq helper functions which writes a constant value to address with large dma access support. Signed-off-by: Dennis YC Hsieh Now pushed to v5.9-next/soc Thanks! --- drivers/soc/mediatek/mtk-cmdq-helper.c | 14 ++ include/linux/soc/mediatek/mtk-cmdq.h | 13 + 2 files changed, 27 insertions(+) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index ed9f5e63c195..4e86b65815fc 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -280,6 +280,20 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, } EXPORT_SYMBOL(cmdq_pkt_write_s_mask); +int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx, + u16 addr_low, u32 value) +{ + struct cmdq_instruction inst = {}; + + inst.op = CMDQ_CODE_WRITE_S; + inst.sop = high_addr_reg_idx; + inst.offset = addr_low; + inst.value = value; + + return cmdq_pkt_append_command(pkt, inst); +} +EXPORT_SYMBOL(cmdq_pkt_write_s_value); + int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event) { struct cmdq_instruction inst = { {0} }; diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index cd7ec714344e..ae73e10da274 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -152,6 +152,19 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low, u16 src_reg_idx, u32 mask); /** + * cmdq_pkt_write_s_value() - append write_s command to the CMDQ packet which + * write value to a physical address + * @pkt: the CMDQ packet + * @high_addr_reg_idx: internal register ID which contains high address of pa + * @addr_low: low address of pa + * @value: the specified target value + * + * Return: 0 for success; else the error code is returned + */ +int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx, + u16 addr_low, u32 value); + +/** * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet * @pkt: the CMDQ packet * @event:the desired event type to "wait and CLEAR" ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/9] soc: mediatek: cmdq: add write_s_mask function
On 07/07/2020 17:45, Dennis YC Hsieh wrote: add write_s_mask function in cmdq helper functions which writes value contains in internal register to address with mask and large dma access support. Signed-off-by: Dennis YC Hsieh Now pushed to v5.9-next/soc Thanks! --- drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++ include/linux/mailbox/mtk-cmdq-mailbox.h |1 + include/linux/soc/mediatek/mtk-cmdq.h| 18 ++ 3 files changed, 42 insertions(+) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index 880349b3f16c..550e9e7e3ff2 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -242,6 +242,29 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, } EXPORT_SYMBOL(cmdq_pkt_write_s); +int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, + u16 addr_low, u16 src_reg_idx, u32 mask) +{ + struct cmdq_instruction inst = {}; + int err; + + inst.op = CMDQ_CODE_MASK; + inst.mask = ~mask; + err = cmdq_pkt_append_command(pkt, inst); + if (err < 0) + return err; + + inst.mask = 0; + inst.op = CMDQ_CODE_WRITE_S_MASK; + inst.src_t = CMDQ_REG_TYPE; + inst.sop = high_addr_reg_idx; + inst.offset = addr_low; + inst.src_reg = src_reg_idx; + + return cmdq_pkt_append_command(pkt, inst); +} +EXPORT_SYMBOL(cmdq_pkt_write_s_mask); + int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event) { struct cmdq_instruction inst = { {0} }; diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h index 1f76cfedb16d..90d1d8e64412 100644 --- a/include/linux/mailbox/mtk-cmdq-mailbox.h +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h @@ -61,6 +61,7 @@ enum cmdq_code { CMDQ_CODE_WFE = 0x20, CMDQ_CODE_EOC = 0x40, CMDQ_CODE_WRITE_S = 0x90, + CMDQ_CODE_WRITE_S_MASK = 0x91, CMDQ_CODE_LOGIC = 0xa0, }; diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index 9b0c57a0063d..53230341bf94 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -122,6 +122,24 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low, u16 src_reg_idx); /** + * cmdq_pkt_write_s_mask() - append write_s with mask command to the CMDQ packet + * @pkt: the CMDQ packet + * @high_addr_reg_idx: internal register ID which contains high address of pa + * @addr_low: low address of pa + * @src_reg_idx: the CMDQ internal register ID which cache source value + * @mask: the specified target address mask, use U32_MAX if no need + * + * Return: 0 for success; else the error code is returned + * + * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH() + * to get high address and call cmdq_pkt_assign() to assign value into internal + * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when + * call to this function. + */ +int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, + u16 addr_low, u16 src_reg_idx, u32 mask); + +/** * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet * @pkt: the CMDQ packet * @event:the desired event type to "wait and CLEAR" ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/9] soc: mediatek: cmdq: add write_s_mask value function
On 07/07/2020 17:45, Dennis YC Hsieh wrote: add write_s_mask_value function in cmdq helper functions which writes a constant value to address with mask and large dma access support. Signed-off-by: Dennis YC Hsieh Now pushed to v5.9-next/soc Thanks! --- drivers/soc/mediatek/mtk-cmdq-helper.c | 21 + include/linux/soc/mediatek/mtk-cmdq.h | 15 +++ 2 files changed, 36 insertions(+) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index 4e86b65815fc..b6e25f216605 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -294,6 +294,27 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx, } EXPORT_SYMBOL(cmdq_pkt_write_s_value); +int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx, + u16 addr_low, u32 value, u32 mask) +{ + struct cmdq_instruction inst = {}; + int err; + + inst.op = CMDQ_CODE_MASK; + inst.mask = ~mask; + err = cmdq_pkt_append_command(pkt, inst); + if (err < 0) + return err; + + inst.op = CMDQ_CODE_WRITE_S_MASK; + inst.sop = high_addr_reg_idx; + inst.offset = addr_low; + inst.value = value; + + return cmdq_pkt_append_command(pkt, inst); +} +EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value); + int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event) { struct cmdq_instruction inst = { {0} }; diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index ae73e10da274..d9390d76ee14 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -165,6 +165,21 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx, u16 addr_low, u32 value); /** + * cmdq_pkt_write_s_mask_value() - append write_s command with mask to the CMDQ + *packet which write value to a physical + *address + * @pkt: the CMDQ packet + * @high_addr_reg_idx: internal register ID which contains high address of pa + * @addr_low: low address of pa + * @value: the specified target value + * @mask: the specified target mask + * + * Return: 0 for success; else the error code is returned + */ +int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx, + u16 addr_low, u32 value, u32 mask); + +/** * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet * @pkt: the CMDQ packet * @event:the desired event type to "wait and CLEAR" ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 4/9] soc: mediatek: cmdq: add read_s function
On 07/07/2020 17:45, Dennis YC Hsieh wrote: Add read_s function in cmdq helper functions which support read value from register or dma physical address into gce internal register. Signed-off-by: Dennis YC Hsieh Now pushed to v5.9-next/soc Thanks! --- drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++ include/linux/mailbox/mtk-cmdq-mailbox.h |1 + include/linux/soc/mediatek/mtk-cmdq.h| 12 3 files changed, 28 insertions(+) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index 550e9e7e3ff2..ed9f5e63c195 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -227,6 +227,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys, } EXPORT_SYMBOL(cmdq_pkt_write_mask); +int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low, + u16 reg_idx) +{ + struct cmdq_instruction inst = {}; + + inst.op = CMDQ_CODE_READ_S; + inst.dst_t = CMDQ_REG_TYPE; + inst.sop = high_addr_reg_idx; + inst.reg_dst = reg_idx; + inst.src_reg = addr_low; + + return cmdq_pkt_append_command(pkt, inst); +} +EXPORT_SYMBOL(cmdq_pkt_read_s); + int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low, u16 src_reg_idx) { diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h index 90d1d8e64412..efbd8a9eb2d1 100644 --- a/include/linux/mailbox/mtk-cmdq-mailbox.h +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h @@ -60,6 +60,7 @@ enum cmdq_code { CMDQ_CODE_JUMP = 0x10, CMDQ_CODE_WFE = 0x20, CMDQ_CODE_EOC = 0x40, + CMDQ_CODE_READ_S = 0x80, CMDQ_CODE_WRITE_S = 0x90, CMDQ_CODE_WRITE_S_MASK = 0x91, CMDQ_CODE_LOGIC = 0xa0, diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index 53230341bf94..cd7ec714344e 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -104,6 +104,18 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev, int index, int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value, u32 mask); +/* + * cmdq_pkt_read_s() - append read_s command to the CMDQ packet + * @pkt: the CMDQ packet + * @high_addr_reg_idx: internal register ID which contains high address of pa + * @addr_low: low address of pa + * @reg_idx: the CMDQ internal register ID to cache read data + * + * Return: 0 for success; else the error code is returned + */ +int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low, + u16 reg_idx); + /** * cmdq_pkt_write_s() - append write_s command to the CMDQ packet * @pkt: the CMDQ packet ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/9] soc: mediatek: cmdq: add write_s function
On 07/07/2020 17:45, Dennis YC Hsieh wrote: add write_s function in cmdq helper functions which writes value contains in internal register to address with large dma access support. Signed-off-by: Dennis YC Hsieh Now pushed to v5.9-next/soc Thanks! --- drivers/soc/mediatek/mtk-cmdq-helper.c | 19 +++ include/linux/mailbox/mtk-cmdq-mailbox.h |1 + include/linux/soc/mediatek/mtk-cmdq.h| 19 +++ 3 files changed, 39 insertions(+) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index 9faf78fbed3a..880349b3f16c 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -18,6 +18,10 @@ struct cmdq_instruction { union { u32 value; u32 mask; + struct { + u16 arg_c; + u16 src_reg; + }; }; union { u16 offset; @@ -223,6 +227,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys, } EXPORT_SYMBOL(cmdq_pkt_write_mask); +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, +u16 addr_low, u16 src_reg_idx) +{ + struct cmdq_instruction inst = {}; + + inst.op = CMDQ_CODE_WRITE_S; + inst.src_t = CMDQ_REG_TYPE; + inst.sop = high_addr_reg_idx; + inst.offset = addr_low; + inst.src_reg = src_reg_idx; + + return cmdq_pkt_append_command(pkt, inst); +} +EXPORT_SYMBOL(cmdq_pkt_write_s); + int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event) { struct cmdq_instruction inst = { {0} }; diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h index 05eea1aef5aa..1f76cfedb16d 100644 --- a/include/linux/mailbox/mtk-cmdq-mailbox.h +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h @@ -60,6 +60,7 @@ enum cmdq_code { CMDQ_CODE_JUMP = 0x10, CMDQ_CODE_WFE = 0x20, CMDQ_CODE_EOC = 0x40, + CMDQ_CODE_WRITE_S = 0x90, CMDQ_CODE_LOGIC = 0xa0, }; diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index 2249ecaf77e4..9b0c57a0063d 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -12,6 +12,8 @@ #include #define CMDQ_NO_TIMEOUT 0xu +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0))) +#define CMDQ_ADDR_LOW(addr)((u16)(addr) | BIT(1)) struct cmdq_pkt; @@ -103,6 +105,23 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value, u32 mask); /** + * cmdq_pkt_write_s() - append write_s command to the CMDQ packet + * @pkt: the CMDQ packet + * @high_addr_reg_idx: internal register ID which contains high address of pa + * @addr_low: low address of pa + * @src_reg_idx: the CMDQ internal register ID which cache source value + * + * Return: 0 for success; else the error code is returned + * + * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH() + * to get high address and call cmdq_pkt_assign() to assign value into internal + * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when + * call to this function. + */ +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, +u16 addr_low, u16 src_reg_idx); + +/** * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet * @pkt: the CMDQ packet * @event:the desired event type to "wait and CLEAR" ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/9] soc: mediatek: cmdq: add address shift in jump
On 07/07/2020 17:45, Dennis YC Hsieh wrote: Add address shift when compose jump instruction to compatible with 35bit format. Change since v1: - Rename cmdq_mbox_shift() to cmdq_get_shift_pa(). Signed-off-by: Dennis YC Hsieh Now pushed to v5.9-next/soc Thanks! --- drivers/soc/mediatek/mtk-cmdq-helper.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index dc644cfb6419..9faf78fbed3a 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -329,7 +329,8 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) /* JUMP to end */ inst.op = CMDQ_CODE_JUMP; - inst.value = CMDQ_JUMP_PASS; + inst.value = CMDQ_JUMP_PASS >> + cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan); err = cmdq_pkt_append_command(pkt, inst); return err; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] soc: mediatek: cmdq: Remove timeout handler in helper function
On 21/09/2020 17:32, Chun-Kuang Hu wrote: Hi, Matthias: Matthias Brugger 於 2020年9月21日 週一 下午4:53寫道: On 21/09/2020 01:42, Chun-Kuang Hu wrote: For each client driver, its timeout handler need to dump hardware register or its state machine information, so remove timeout handler in helper function and let client driver implement its own timeout handler. I don't see the implementation of a client side handler. Did I miss something? Would it make sense to instead add a callback to the handler in cmdq_mbox_create()? According to the commit message, it make sense to add a callback to the handler in comq_mbox_create(). But for DRM, I would like to check timeout in vblank irq because the register should be applied in vblank. (I have not implement this patch yet) What I want to say is that different client may have different way to detect timeout and different way to handle it. If you want, I would add DRM timeout handle patch with this patch, and modify commit message to include different way to detect timeout. I think that would help me to understand the whole picture. Thanks a lot! Matthias Regards, Chun-Kuang. Regards, Matthias Signed-off-by: Chun-Kuang Hu --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +- drivers/soc/mediatek/mtk-cmdq-helper.c | 41 + include/linux/soc/mediatek/mtk-cmdq.h | 11 +-- 3 files changed, 3 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 3fc5511330b9..cabeb7fea2be 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -824,8 +824,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, #if IS_REACHABLE(CONFIG_MTK_CMDQ) mtk_crtc->cmdq_client = cmdq_mbox_create(mtk_crtc->mmsys_dev, - drm_crtc_index(_crtc->base), - 2000); + drm_crtc_index(_crtc->base)); if (IS_ERR(mtk_crtc->cmdq_client)) { dev_dbg(dev, "mtk_crtc %d failed to create mailbox client, writing register by CPU now\n", drm_crtc_index(_crtc->base)); diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index dc644cfb6419..4f311f035b59 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -65,14 +65,7 @@ int cmdq_dev_get_client_reg(struct device *dev, } EXPORT_SYMBOL(cmdq_dev_get_client_reg); -static void cmdq_client_timeout(struct timer_list *t) -{ - struct cmdq_client *client = from_timer(client, t, timer); - - dev_err(client->client.dev, "cmdq timeout!\n"); -} - -struct cmdq_client *cmdq_mbox_create(struct device *dev, int index, u32 timeout) +struct cmdq_client *cmdq_mbox_create(struct device *dev, int index) { struct cmdq_client *client; @@ -80,12 +73,6 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev, int index, u32 timeout) if (!client) return (struct cmdq_client *)-ENOMEM; - client->timeout_ms = timeout; - if (timeout != CMDQ_NO_TIMEOUT) { - spin_lock_init(>lock); - timer_setup(>timer, cmdq_client_timeout, 0); - } - client->pkt_cnt = 0; client->client.dev = dev; client->client.tx_block = false; client->client.knows_txdone = true; @@ -107,11 +94,6 @@ EXPORT_SYMBOL(cmdq_mbox_create); void cmdq_mbox_destroy(struct cmdq_client *client) { - if (client->timeout_ms != CMDQ_NO_TIMEOUT) { - spin_lock(>lock); - del_timer_sync(>timer); - spin_unlock(>lock); - } mbox_free_channel(client->chan); kfree(client); } @@ -342,18 +324,6 @@ static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data) struct cmdq_task_cb *cb = >cb; struct cmdq_client *client = (struct cmdq_client *)pkt->cl; - if (client->timeout_ms != CMDQ_NO_TIMEOUT) { - unsigned long flags = 0; - - spin_lock_irqsave(>lock, flags); - if (--client->pkt_cnt == 0) - del_timer(>timer); - else - mod_timer(>timer, jiffies + - msecs_to_jiffies(client->timeout_ms)); - spin_unlock_irqrestore(>lock, flags); - } - dma_sync_single_for_cpu(client->chan->mbox->dev, pkt->pa_base, pkt->cmd_buf_size, DMA_TO_DEVICE); if (cb->cb) { @@ -366,7 +336,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb, void *data) { int err; - unsigned long flags = 0; struct cmdq_client *client = (struct cmdq_client *)pkt->cl; pkt->cb.cb = cb; @@ -377,14 +346,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
Re: [PATCH 2/2] drm/msm: Leave inuse count intact on map failure
On Mon, Sep 21, 2020 at 8:27 AM Akhil P Oommen wrote: > > Leave the inuse count intact on map failure to keep the accounting > accurate. > > Signed-off-by: Akhil P Oommen > --- > drivers/gpu/drm/msm/msm_gem_vma.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c > b/drivers/gpu/drm/msm/msm_gem_vma.c > index 80a8a26..8367a1c 100644 > --- a/drivers/gpu/drm/msm/msm_gem_vma.c > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c > @@ -88,8 +88,10 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, > ret = aspace->mmu->funcs->map(aspace->mmu, vma->iova, sgt, > size, prot); > > - if (ret) > + if (ret) { > vma->mapped = false; > + vma->inuse++; vma->inuse-- ? BR, -R > + } > > return ret; > } > -- > 2.7.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/msm: Replace active_list with refcount
On Mon, Sep 21, 2020 at 8:27 AM Akhil P Oommen wrote: > > In the case where we have a back-to-back submission that shares the same > BO, this BO will be prematurely moved to inactive_list while retiring the > first submit. But it will be still part of the second submit which is > being processed by the GPU. Now, if the shrinker happens to be triggered at > this point, it will result in premature purging of this BO. > > To fix this, we can replace the active_list with reference counting and > move the BO to inactive list only when this count becomes zero. > > Signed-off-by: Akhil P Oommen > --- > drivers/gpu/drm/msm/msm_drv.h | 5 ++--- > drivers/gpu/drm/msm/msm_gem.c | 30 -- > drivers/gpu/drm/msm/msm_gem.h | 4 +++- > drivers/gpu/drm/msm/msm_gpu.c | 11 +++ > 4 files changed, 28 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h > index 3193274..28e3c8d 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -309,9 +309,8 @@ void msm_gem_put_vaddr(struct drm_gem_object *obj); > int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv); > int msm_gem_sync_object(struct drm_gem_object *obj, > struct msm_fence_context *fctx, bool exclusive); > -void msm_gem_move_to_active(struct drm_gem_object *obj, > - struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence); > -void msm_gem_move_to_inactive(struct drm_gem_object *obj); > +void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu); > +void msm_gem_active_put(struct drm_gem_object *obj); > int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t > *timeout); > int msm_gem_cpu_fini(struct drm_gem_object *obj); > void msm_gem_free_object(struct drm_gem_object *obj); > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c > index 76a6c52..accc106 100644 > --- a/drivers/gpu/drm/msm/msm_gem.c > +++ b/drivers/gpu/drm/msm/msm_gem.c > @@ -743,33 +743,36 @@ int msm_gem_sync_object(struct drm_gem_object *obj, > return 0; > } > > -void msm_gem_move_to_active(struct drm_gem_object *obj, > - struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence) > +void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu) > { > struct msm_gem_object *msm_obj = to_msm_bo(obj); > + WARN_ON(!mutex_is_locked(>dev->struct_mutex)); > WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED); > + > msm_obj->gpu = gpu; > - if (exclusive) > - dma_resv_add_excl_fence(obj->resv, fence); > - else > - dma_resv_add_shared_fence(obj->resv, fence); > list_del_init(_obj->mm_list); > - list_add_tail(_obj->mm_list, >active_list); > + atomic_inc(_obj->active_count); I don't think we want to get rid of active_list, but just supplement it with a refcnt. I suspect as-is, this breaks $debugfs/gem (which iterates the active and inactive list) > } > > -void msm_gem_move_to_inactive(struct drm_gem_object *obj) > +static void move_to_inactive(struct msm_gem_object *msm_obj) > { > - struct drm_device *dev = obj->dev; > + struct drm_device *dev = msm_obj->base.dev; > struct msm_drm_private *priv = dev->dev_private; > - struct msm_gem_object *msm_obj = to_msm_bo(obj); > - > - WARN_ON(!mutex_is_locked(>struct_mutex)); > > msm_obj->gpu = NULL; > - list_del_init(_obj->mm_list); > list_add_tail(_obj->mm_list, >inactive_list); > } > > +void msm_gem_active_put(struct drm_gem_object *obj) > +{ > + struct msm_gem_object *msm_obj = to_msm_bo(obj); > + > + WARN_ON(!mutex_is_locked(>dev->struct_mutex)); > + > + if (atomic_dec_and_test(_obj->active_count)) > + move_to_inactive(msm_obj); > +} > + > int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t > *timeout) > { > bool write = !!(op & MSM_PREP_WRITE); > @@ -1104,7 +1107,6 @@ static struct drm_gem_object *_msm_gem_new(struct > drm_device *dev, > } > > if (struct_mutex_locked) { > - WARN_ON(!mutex_is_locked(>struct_mutex)); This looks unrelated (and I don't think we want to drop this WARN_ON()) BR, -R > list_add_tail(_obj->mm_list, >inactive_list); > } else { > mutex_lock(>struct_mutex); > diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h > index 7b1c7a5..a1bf741 100644 > --- a/drivers/gpu/drm/msm/msm_gem.h > +++ b/drivers/gpu/drm/msm/msm_gem.h > @@ -88,12 +88,14 @@ struct msm_gem_object { > struct mutex lock; /* Protects resources associated with bo */ > > char name[32]; /* Identifier to print for the debugfs files */ > + > + atomic_t active_count; > }; > #define to_msm_bo(x) container_of(x, struct msm_gem_object, base) > > static inline bool is_active(struct msm_gem_object *msm_obj) > { > -
Re: [PATCH] soc: mediatek: cmdq: Remove timeout handler in helper function
Hi, Matthias: Matthias Brugger 於 2020年9月21日 週一 下午4:53寫道: > > > > On 21/09/2020 01:42, Chun-Kuang Hu wrote: > > For each client driver, its timeout handler need to dump hardware register > > or its state machine information, so remove timeout handler in helper > > function and let client driver implement its own timeout handler. > > > > I don't see the implementation of a client side handler. Did I miss something? > Would it make sense to instead add a callback to the handler in > cmdq_mbox_create()? According to the commit message, it make sense to add a callback to the handler in comq_mbox_create(). But for DRM, I would like to check timeout in vblank irq because the register should be applied in vblank. (I have not implement this patch yet) What I want to say is that different client may have different way to detect timeout and different way to handle it. If you want, I would add DRM timeout handle patch with this patch, and modify commit message to include different way to detect timeout. Regards, Chun-Kuang. > > Regards, > Matthias > > > Signed-off-by: Chun-Kuang Hu > > --- > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +- > > drivers/soc/mediatek/mtk-cmdq-helper.c | 41 + > > include/linux/soc/mediatek/mtk-cmdq.h | 11 +-- > > 3 files changed, 3 insertions(+), 52 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index 3fc5511330b9..cabeb7fea2be 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -824,8 +824,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, > > #if IS_REACHABLE(CONFIG_MTK_CMDQ) > > mtk_crtc->cmdq_client = > > cmdq_mbox_create(mtk_crtc->mmsys_dev, > > - drm_crtc_index(_crtc->base), > > - 2000); > > + drm_crtc_index(_crtc->base)); > > if (IS_ERR(mtk_crtc->cmdq_client)) { > > dev_dbg(dev, "mtk_crtc %d failed to create mailbox client, > > writing register by CPU now\n", > > drm_crtc_index(_crtc->base)); > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > > b/drivers/soc/mediatek/mtk-cmdq-helper.c > > index dc644cfb6419..4f311f035b59 100644 > > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > > @@ -65,14 +65,7 @@ int cmdq_dev_get_client_reg(struct device *dev, > > } > > EXPORT_SYMBOL(cmdq_dev_get_client_reg); > > > > -static void cmdq_client_timeout(struct timer_list *t) > > -{ > > - struct cmdq_client *client = from_timer(client, t, timer); > > - > > - dev_err(client->client.dev, "cmdq timeout!\n"); > > -} > > - > > -struct cmdq_client *cmdq_mbox_create(struct device *dev, int index, u32 > > timeout) > > +struct cmdq_client *cmdq_mbox_create(struct device *dev, int index) > > { > > struct cmdq_client *client; > > > > @@ -80,12 +73,6 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev, > > int index, u32 timeout) > > if (!client) > > return (struct cmdq_client *)-ENOMEM; > > > > - client->timeout_ms = timeout; > > - if (timeout != CMDQ_NO_TIMEOUT) { > > - spin_lock_init(>lock); > > - timer_setup(>timer, cmdq_client_timeout, 0); > > - } > > - client->pkt_cnt = 0; > > client->client.dev = dev; > > client->client.tx_block = false; > > client->client.knows_txdone = true; > > @@ -107,11 +94,6 @@ EXPORT_SYMBOL(cmdq_mbox_create); > > > > void cmdq_mbox_destroy(struct cmdq_client *client) > > { > > - if (client->timeout_ms != CMDQ_NO_TIMEOUT) { > > - spin_lock(>lock); > > - del_timer_sync(>timer); > > - spin_unlock(>lock); > > - } > > mbox_free_channel(client->chan); > > kfree(client); > > } > > @@ -342,18 +324,6 @@ static void cmdq_pkt_flush_async_cb(struct > > cmdq_cb_data data) > > struct cmdq_task_cb *cb = >cb; > > struct cmdq_client *client = (struct cmdq_client *)pkt->cl; > > > > - if (client->timeout_ms != CMDQ_NO_TIMEOUT) { > > - unsigned long flags = 0; > > - > > - spin_lock_irqsave(>lock, flags); > > - if (--client->pkt_cnt == 0) > > - del_timer(>timer); > > - else > > - mod_timer(>timer, jiffies + > > - msecs_to_jiffies(client->timeout_ms)); > > - spin_unlock_irqrestore(>lock, flags); > > - } > > - > > dma_sync_single_for_cpu(client->chan->mbox->dev, pkt->pa_base, > > pkt->cmd_buf_size, DMA_TO_DEVICE); > > if (cb->cb) { > > @@ -366,7 +336,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, > > cmdq_async_flush_cb cb, > >void *data) > > { > > int err; > > - unsigned long flags
[PATCH 1/2] drm/msm: Replace active_list with refcount
In the case where we have a back-to-back submission that shares the same BO, this BO will be prematurely moved to inactive_list while retiring the first submit. But it will be still part of the second submit which is being processed by the GPU. Now, if the shrinker happens to be triggered at this point, it will result in premature purging of this BO. To fix this, we can replace the active_list with reference counting and move the BO to inactive list only when this count becomes zero. Signed-off-by: Akhil P Oommen --- drivers/gpu/drm/msm/msm_drv.h | 5 ++--- drivers/gpu/drm/msm/msm_gem.c | 30 -- drivers/gpu/drm/msm/msm_gem.h | 4 +++- drivers/gpu/drm/msm/msm_gpu.c | 11 +++ 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 3193274..28e3c8d 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -309,9 +309,8 @@ void msm_gem_put_vaddr(struct drm_gem_object *obj); int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv); int msm_gem_sync_object(struct drm_gem_object *obj, struct msm_fence_context *fctx, bool exclusive); -void msm_gem_move_to_active(struct drm_gem_object *obj, - struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence); -void msm_gem_move_to_inactive(struct drm_gem_object *obj); +void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu); +void msm_gem_active_put(struct drm_gem_object *obj); int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t *timeout); int msm_gem_cpu_fini(struct drm_gem_object *obj); void msm_gem_free_object(struct drm_gem_object *obj); diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 76a6c52..accc106 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -743,33 +743,36 @@ int msm_gem_sync_object(struct drm_gem_object *obj, return 0; } -void msm_gem_move_to_active(struct drm_gem_object *obj, - struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence) +void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu) { struct msm_gem_object *msm_obj = to_msm_bo(obj); + WARN_ON(!mutex_is_locked(>dev->struct_mutex)); WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED); + msm_obj->gpu = gpu; - if (exclusive) - dma_resv_add_excl_fence(obj->resv, fence); - else - dma_resv_add_shared_fence(obj->resv, fence); list_del_init(_obj->mm_list); - list_add_tail(_obj->mm_list, >active_list); + atomic_inc(_obj->active_count); } -void msm_gem_move_to_inactive(struct drm_gem_object *obj) +static void move_to_inactive(struct msm_gem_object *msm_obj) { - struct drm_device *dev = obj->dev; + struct drm_device *dev = msm_obj->base.dev; struct msm_drm_private *priv = dev->dev_private; - struct msm_gem_object *msm_obj = to_msm_bo(obj); - - WARN_ON(!mutex_is_locked(>struct_mutex)); msm_obj->gpu = NULL; - list_del_init(_obj->mm_list); list_add_tail(_obj->mm_list, >inactive_list); } +void msm_gem_active_put(struct drm_gem_object *obj) +{ + struct msm_gem_object *msm_obj = to_msm_bo(obj); + + WARN_ON(!mutex_is_locked(>dev->struct_mutex)); + + if (atomic_dec_and_test(_obj->active_count)) + move_to_inactive(msm_obj); +} + int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t *timeout) { bool write = !!(op & MSM_PREP_WRITE); @@ -1104,7 +1107,6 @@ static struct drm_gem_object *_msm_gem_new(struct drm_device *dev, } if (struct_mutex_locked) { - WARN_ON(!mutex_is_locked(>struct_mutex)); list_add_tail(_obj->mm_list, >inactive_list); } else { mutex_lock(>struct_mutex); diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 7b1c7a5..a1bf741 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -88,12 +88,14 @@ struct msm_gem_object { struct mutex lock; /* Protects resources associated with bo */ char name[32]; /* Identifier to print for the debugfs files */ + + atomic_t active_count; }; #define to_msm_bo(x) container_of(x, struct msm_gem_object, base) static inline bool is_active(struct msm_gem_object *msm_obj) { - return msm_obj->gpu != NULL; + return atomic_read(_obj->active_count); } static inline bool is_purgeable(struct msm_gem_object *msm_obj) diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 29c8d73c..55d1648 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -698,8 +698,8 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring, for (i = 0; i < submit->nr_bos; i++) { struct msm_gem_object *msm_obj =
[PATCH 2/2] drm/msm: Leave inuse count intact on map failure
Leave the inuse count intact on map failure to keep the accounting accurate. Signed-off-by: Akhil P Oommen --- drivers/gpu/drm/msm/msm_gem_vma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index 80a8a26..8367a1c 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -88,8 +88,10 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, ret = aspace->mmu->funcs->map(aspace->mmu, vma->iova, sgt, size, prot); - if (ret) + if (ret) { vma->mapped = false; + vma->inuse++; + } return ret; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: commit_work scheduling
On Mon, Sep 21, 2020 at 8:16 AM Rob Clark wrote: > > On Mon, Sep 21, 2020 at 2:21 AM Daniel Vetter wrote: > > > > On Sat, Sep 19, 2020 at 12:37:23PM -0700, Rob Clark wrote: > > > From: Rob Clark > > > > > > The android userspace treats the display pipeline as a realtime problem. > > > And arguably, if your goal is to not miss frame deadlines (ie. vblank), > > > it is. (See https://lwn.net/Articles/809545/ for the best explaination > > > that I found.) > > > > > > But this presents a problem with using workqueues for non-blocking > > > atomic commit_work(), because the SCHED_FIFO userspace thread(s) can > > > preempt the worker. Which is not really the outcome you want.. once > > > the required fences are scheduled, you want to push the atomic commit > > > down to hw ASAP. > > > > > > But the decision of whether commit_work should be RT or not really > > > depends on what userspace is doing. For a pure CFS userspace display > > > pipeline, commit_work() should remain SCHED_NORMAL. > > > > > > To handle this, convert non-blocking commit_work() to use per-CRTC > > > kthread workers, instead of system_unbound_wq. Per-CRTC workers are > > > used to avoid serializing commits when userspace is using a per-CRTC > > > update loop. > > > > > > A client-cap is introduced so that userspace can opt-in to SCHED_FIFO > > > priority commit work. > > > > > > A potential issue is that since 616d91b68cd ("sched: Remove > > > sched_setscheduler*() EXPORTs") we have limited RT priority levels, > > > meaning that commit_work() ends up running at the same priority level > > > as vblank-work. This shouldn't be a big problem *yet*, due to limited > > > use of vblank-work at this point. And if it could be arranged that > > > vblank-work is scheduled before signaling out-fences and/or sending > > > pageflip events, it could probably work ok to use a single priority > > > level for both commit-work and vblank-work. > > > > The part I don't like about this is that it all feels rather hacked > > together, and if we add more stuff (or there's some different thing in the > > system that also needs rt scheduling) then it doesn't compose. > > The ideal thing would be that userspace is in control of the > priorities.. the setclientcap approach seemed like a reasonable way to > give the drm-master a way to opt in. > > I suppose instead userspace could use sched_setscheduler().. but that > would require userspace to be root, and would require some way to find > the tid. > > Is there some way we could arrange for the per-crtc kthread's to be > owned by the drm master? That would solve the "must be root" issue. > And since the target audience is an atomic userspace, I suppose we > could expose the tid as a read-only property on the crtc? Side-note, we have the same issue with work scheduled when GPU completes a batch/submit.. I'm less sure what to do with that, so figured I'd start with the commit_work because that was the "easy" part ;-) The retire_work tends to complete quickly, so maybe sched_set_fifo() is sufficient. (That plus these days things that run android tend to have 8 cores so you can kinda get away with multiple things at the same RT priority level to some degree..) > BR, > -R > > > So question to rt/worker folks: What's the best way to let userspace set > > the scheduling mode and priorities of things the kernel does on its > > behalf? Surely we're not the first ones where if userspace runs with some > > rt priority it'll starve out the kernel workers that it needs. Hardcoding > > something behind a subsystem ioctl (which just means every time userspace > > changes what it does, we need a new such flag or mode) can't be the right > > thing. > > > > Peter, Tejun? > > > > Thanks, Daniel > > > > > > > > Rob Clark (3): > > > drm/crtc: Introduce per-crtc kworker > > > drm/atomic: Use kthread worker for nonblocking commits > > > drm: Add a client-cap to set scheduling mode > > > > > > drivers/gpu/drm/drm_atomic_helper.c | 13 ++ > > > drivers/gpu/drm/drm_auth.c | 4 > > > drivers/gpu/drm/drm_crtc.c | 37 + > > > drivers/gpu/drm/drm_ioctl.c | 13 ++ > > > include/drm/drm_atomic.h| 31 > > > include/drm/drm_crtc.h | 10 > > > include/uapi/drm/drm.h | 13 ++ > > > 7 files changed, 117 insertions(+), 4 deletions(-) > > > > > > -- > > > 2.26.2 > > > > > > ___ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: commit_work scheduling
On Mon, Sep 21, 2020 at 2:21 AM Daniel Vetter wrote: > > On Sat, Sep 19, 2020 at 12:37:23PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > The android userspace treats the display pipeline as a realtime problem. > > And arguably, if your goal is to not miss frame deadlines (ie. vblank), > > it is. (See https://lwn.net/Articles/809545/ for the best explaination > > that I found.) > > > > But this presents a problem with using workqueues for non-blocking > > atomic commit_work(), because the SCHED_FIFO userspace thread(s) can > > preempt the worker. Which is not really the outcome you want.. once > > the required fences are scheduled, you want to push the atomic commit > > down to hw ASAP. > > > > But the decision of whether commit_work should be RT or not really > > depends on what userspace is doing. For a pure CFS userspace display > > pipeline, commit_work() should remain SCHED_NORMAL. > > > > To handle this, convert non-blocking commit_work() to use per-CRTC > > kthread workers, instead of system_unbound_wq. Per-CRTC workers are > > used to avoid serializing commits when userspace is using a per-CRTC > > update loop. > > > > A client-cap is introduced so that userspace can opt-in to SCHED_FIFO > > priority commit work. > > > > A potential issue is that since 616d91b68cd ("sched: Remove > > sched_setscheduler*() EXPORTs") we have limited RT priority levels, > > meaning that commit_work() ends up running at the same priority level > > as vblank-work. This shouldn't be a big problem *yet*, due to limited > > use of vblank-work at this point. And if it could be arranged that > > vblank-work is scheduled before signaling out-fences and/or sending > > pageflip events, it could probably work ok to use a single priority > > level for both commit-work and vblank-work. > > The part I don't like about this is that it all feels rather hacked > together, and if we add more stuff (or there's some different thing in the > system that also needs rt scheduling) then it doesn't compose. The ideal thing would be that userspace is in control of the priorities.. the setclientcap approach seemed like a reasonable way to give the drm-master a way to opt in. I suppose instead userspace could use sched_setscheduler().. but that would require userspace to be root, and would require some way to find the tid. Is there some way we could arrange for the per-crtc kthread's to be owned by the drm master? That would solve the "must be root" issue. And since the target audience is an atomic userspace, I suppose we could expose the tid as a read-only property on the crtc? BR, -R > So question to rt/worker folks: What's the best way to let userspace set > the scheduling mode and priorities of things the kernel does on its > behalf? Surely we're not the first ones where if userspace runs with some > rt priority it'll starve out the kernel workers that it needs. Hardcoding > something behind a subsystem ioctl (which just means every time userspace > changes what it does, we need a new such flag or mode) can't be the right > thing. > > Peter, Tejun? > > Thanks, Daniel > > > > > Rob Clark (3): > > drm/crtc: Introduce per-crtc kworker > > drm/atomic: Use kthread worker for nonblocking commits > > drm: Add a client-cap to set scheduling mode > > > > drivers/gpu/drm/drm_atomic_helper.c | 13 ++ > > drivers/gpu/drm/drm_auth.c | 4 > > drivers/gpu/drm/drm_crtc.c | 37 + > > drivers/gpu/drm/drm_ioctl.c | 13 ++ > > include/drm/drm_atomic.h| 31 > > include/drm/drm_crtc.h | 10 > > include/uapi/drm/drm.h | 13 ++ > > 7 files changed, 117 insertions(+), 4 deletions(-) > > > > -- > > 2.26.2 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/7] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix
Hi, On 04/09/2019 23:20, Ilia Mirkin wrote: >> Implement CTM color management property for OMAP CRTC using DSS >> overlay manager's Color Phase Rotation matrix. The CPR matrix does not >> exactly match the CTM property documentation. On DSS the CPR matrix is >> applied after gamma table look up. However, it seems stupid to add a >> custom property just for that. > > In that case the DRM documentation should be updated to mention that > both options are allowed. Ok, if that is alright. But if we do that, then I guess all the drivers implementing CTM should document the point where it is applied in the pipeline. >>> >>> Whatever solution we end up picking, I think it should at least be >>> discussed with a broader upstream audience and not just swept under the >>> omapdrm carpet :-) >>> >> >> I'll try to write something and send the next series to wider audience. >> Let's see what jury says. > > In case it's useful ... the pipeline normally goes degamma -> ctm -> > gamma. If your ctm is applied after gamma, perhaps you can just rename > "gamma" to "degamma" and be done? (There's the unfortunate case of > legacy gamma which does end up in "GAMMA" when using atomic helpers. > But in such a case, you won't have a CTM.) Waking up old thread, as I started looking at these patches again. So the problem was that DRM defines the output color transformations as: degamma -> ctm -> gamma -> out whereas OMAP DSS has gamma -> ctm -> out The omapdrm driver could declare the gamma table as degamma, as suggested by Ilia, and for the legacy drmModeCrtcSetGamma, the omapdrm driver could implement its own version, and use the degamma table internally (with no ctm). For legacy, that would work fine and as expected, but I think the atomic version would be a bit odd, with only degamma, and no gamma. Quick grep for drm_crtc_enable_color_mgmt shows that there are other drivers that have CTM and gamma, but no degamma. I wonder if all those have ctm -> gamma -> out, or are they similar to OMAP DSS. Any thoughts on how to proceed with this? Should we have a property that describes the order? Or new property name for gamma before ctm (PREGAMMA)? Or just have it as degamma, and let the userspace deal with it (i.e. figure out there's no gamma, but there's degamma, so use degamma)? Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/atomic: Use kthread worker for nonblocking commits
On Mon, Sep 21, 2020 at 2:23 AM Daniel Vetter wrote: > > On Sat, Sep 19, 2020 at 12:37:25PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > This will allow us to more easily switch scheduling rules based on what > > userspace wants. > > > > Signed-off-by: Rob Clark > > I still think switching to the highpriority systemwq as a start (like i915 > already does) would be a good first step no matter what we end up doing > for the android thing. highpri wq is probably better than the current state, but it doesn't really address the problem. You'll still end up with surfaceflinger preempting commit_work.. And with non-RT priority, you'll still occasionally get lower priority threads which haven't had a chance to run for a while preempting you. BR, -R > -Daniel > > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 13 > > include/drm/drm_atomic.h| 31 + > > 2 files changed, 40 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index 9e1ad493e689..75eeec5e7b10 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -1659,11 +1659,11 @@ static void commit_tail(struct drm_atomic_state > > *old_state) > > drm_atomic_state_put(old_state); > > } > > > > -static void commit_work(struct work_struct *work) > > +static void commit_work(struct kthread_work *work) > > { > > struct drm_atomic_state *state = container_of(work, > > struct drm_atomic_state, > > - commit_work); > > + commit_kwork); > > commit_tail(state); > > } > > > > @@ -1797,6 +1797,7 @@ int drm_atomic_helper_commit(struct drm_device *dev, > >struct drm_atomic_state *state, > >bool nonblock) > > { > > + struct kthread_worker *worker = NULL; > > int ret; > > > > if (state->async_update) { > > @@ -1814,7 +1815,7 @@ int drm_atomic_helper_commit(struct drm_device *dev, > > if (ret) > > return ret; > > > > - INIT_WORK(>commit_work, commit_work); > > + kthread_init_work(>commit_kwork, commit_work); > > > > ret = drm_atomic_helper_prepare_planes(dev, state); > > if (ret) > > @@ -1857,8 +1858,12 @@ int drm_atomic_helper_commit(struct drm_device *dev, > >*/ > > > > drm_atomic_state_get(state); > > + > > if (nonblock) > > - queue_work(system_unbound_wq, >commit_work); > > + worker = drm_atomic_pick_worker(state); > > + > > + if (worker) > > + kthread_queue_work(worker, >commit_kwork); > > else > > commit_tail(state); > > > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > > index d07c851d255b..8d0ee19953df 100644 > > --- a/include/drm/drm_atomic.h > > +++ b/include/drm/drm_atomic.h > > @@ -373,8 +373,18 @@ struct drm_atomic_state { > >* > >* Work item which can be used by the driver or helpers to execute the > >* commit without blocking. > > + * > > + * This is deprecated, use commit_kwork. > >*/ > > struct work_struct commit_work; > > + > > + /** > > + * @commit_kwork: > > + * > > + * Work item which can be used by the driver or helpers to execute the > > + * commit without blocking. > > + */ > > + struct kthread_work commit_kwork; > > }; > > > > void __drm_crtc_commit_free(struct kref *kref); > > @@ -954,6 +964,27 @@ void drm_state_dump(struct drm_device *dev, struct > > drm_printer *p); > > (new_obj_state) = > > (__state)->private_objs[__i].new_state, 1); \ > >(__i)++) > > > > +/** > > + * drm_atomic_pick_worker - helper to get kworker to use for nonblocking > > commit > > + * @state: the _atomic_state for the commit > > + * > > + * Pick an appropriate worker for a given atomic update. The first CRTC > > + * invovled in the atomic update is used to pick the worker, to prevent > > + * serializing multiple pageflips / atomic-updates on indenpendent CRTCs. > > + */ > > +static inline struct kthread_worker * > > +drm_atomic_pick_worker(const struct drm_atomic_state *state) > > +{ > > + struct drm_crtc_state *crtc_state; > > + struct drm_crtc *crtc; > > + unsigned i; > > + > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) > > + return crtc->worker; > > + > > + return NULL; > > +} > > + > > /** > > * drm_atomic_crtc_needs_modeset - compute combined modeset need > > * @state: _crtc_state for the CRTC > > -- > > 2.26.2 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org
[PATCH 03/11] drm/vmwgfx: remove unused placement combination
Just some dead code cleanup. Signed-off-by: Christian König --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 1 - drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 30 -- 2 files changed, 31 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index 1523b51a7284..9ceee4eb0b13 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -1008,7 +1008,6 @@ extern struct ttm_placement vmw_vram_placement; extern struct ttm_placement vmw_vram_ne_placement; extern struct ttm_placement vmw_vram_sys_placement; extern struct ttm_placement vmw_vram_gmr_placement; -extern struct ttm_placement vmw_vram_gmr_ne_placement; extern struct ttm_placement vmw_sys_placement; extern struct ttm_placement vmw_sys_ne_placement; extern struct ttm_placement vmw_evictable_placement; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c index 7454f797d37b..d7ea658e9910 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c @@ -65,13 +65,6 @@ static const struct ttm_place gmr_placement_flags = { .flags = TTM_PL_FLAG_CACHED }; -static const struct ttm_place gmr_ne_placement_flags = { - .fpfn = 0, - .lpfn = 0, - .mem_type = VMW_PL_GMR, - .flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_NO_EVICT -}; - static const struct ttm_place mob_placement_flags = { .fpfn = 0, .lpfn = 0, @@ -128,29 +121,6 @@ struct ttm_placement vmw_vram_gmr_placement = { .busy_placement = _placement_flags }; -static const struct ttm_place vram_gmr_ne_placement_flags[] = { - { - .fpfn = 0, - .lpfn = 0, - .mem_type = TTM_PL_VRAM, - .flags = TTM_PL_FLAG_CACHED | -TTM_PL_FLAG_NO_EVICT - }, { - .fpfn = 0, - .lpfn = 0, - .mem_type = VMW_PL_GMR, - .flags = TTM_PL_FLAG_CACHED | -TTM_PL_FLAG_NO_EVICT - } -}; - -struct ttm_placement vmw_vram_gmr_ne_placement = { - .num_placement = 2, - .placement = vram_gmr_ne_placement_flags, - .num_busy_placement = 1, - .busy_placement = _ne_placement_flags -}; - struct ttm_placement vmw_vram_sys_placement = { .num_placement = 1, .placement = _placement_flags, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/11] drm/vmwgfx: stop using ttm_bo_create
Implement in the driver instead since it is the only user of that function. Signed-off-by: Christian König --- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 42 ++ drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 6 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 4 +++ drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 8 ++--- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 813f1b148094..30d19b45b602 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -487,6 +487,48 @@ static void vmw_user_bo_destroy(struct ttm_buffer_object *bo) ttm_prime_object_kfree(vmw_user_bo, prime); } +/** + * vmw_bo_create_kernel - Create a pinned BO for internal kernel use. + * + * @dev_priv: Pointer to the device private struct + * @size: size of the BO we need + * @placement: where to put it + * @p_bo: resulting BO + * + * Creates and pin a simple BO for in kernel use. + */ +int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size, +struct ttm_placement *placement, +struct ttm_buffer_object **p_bo) +{ + unsigned npages = PAGE_ALIGN(size) >> PAGE_SHIFT; + struct ttm_buffer_object *bo; + size_t acc_size; + int ret; + + bo = kzalloc(sizeof(*bo), GFP_KERNEL); + if (unlikely(!bo)) + return -ENOMEM; + + acc_size = ttm_round_pot(sizeof(*bo)); + acc_size += ttm_round_pot(npages * sizeof(void *)); + acc_size += ttm_round_pot(sizeof(struct ttm_tt)); + ret = ttm_bo_init_reserved(_priv->bdev, bo, size, + ttm_bo_type_device, placement, 0, + false, acc_size, NULL, NULL, NULL); + if (unlikely(ret)) + goto error_free; + + ttm_bo_pin(bo); + ttm_bo_unreserve(bo); + *p_bo = bo; + + return 0; + +error_free: + kfree(bo); + return ret; +} /** * vmw_bo_init - Initialize a vmw buffer object diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c index 3b41cf63110a..9a9fe10d829b 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c @@ -1245,9 +1245,9 @@ int vmw_cmdbuf_set_pool_size(struct vmw_cmdbuf_man *man, !dev_priv->has_mob) return -ENOMEM; - ret = ttm_bo_create(_priv->bdev, size, ttm_bo_type_device, - _mob_ne_placement, 0, false, - >cmd_space); + ret = vmw_bo_create_kernel(dev_priv, size, + _mob_placement, + >cmd_space); if (ret) return ret; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index 9ceee4eb0b13..5d07de5183e1 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -845,6 +845,10 @@ extern void vmw_bo_get_guest_ptr(const struct ttm_buffer_object *buf, SVGAGuestPtr *ptr); extern void vmw_bo_pin_reserved(struct vmw_buffer_object *bo, bool pin); extern void vmw_bo_bo_free(struct ttm_buffer_object *bo); +extern int vmw_bo_create_kernel(struct vmw_private *dev_priv, + unsigned long size, + struct ttm_placement *placement, + struct ttm_buffer_object **p_bo); extern int vmw_bo_init(struct vmw_private *dev_priv, struct vmw_buffer_object *vmw_bo, size_t size, struct ttm_placement *placement, diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c index d7ea658e9910..39a2f720f4ed 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c @@ -793,11 +793,9 @@ int vmw_bo_create_and_populate(struct vmw_private *dev_priv, struct ttm_buffer_object *bo; int ret; - ret = ttm_bo_create(_priv->bdev, bo_size, - ttm_bo_type_device, - _sys_ne_placement, - 0, false, ); - + ret = vmw_bo_create_kernel(dev_priv, bo_size, + _sys_placement, + ); if (unlikely(ret != 0)) return ret; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Nuke TTM_PL_FLAG_NO_EVICT
Hi guys, The TTM_PL_FLAG_NO_EVICT flag was never a placement flag to begin with. Instead it affects LRU and eviction handling. So clean this up and provide the common logic of pinning/unpinning a buffer object instead. Since this affects basically all the driver using TTM please comment and/or test the patches. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/11] drm/ttm: add ttm_bo_pin()/ttm_bo_unpin()
As an alternative to the placement flag add a pin count to the ttm buffer object. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 9 ++--- drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +- include/drm/ttm/ttm_bo_api.h | 24 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 70b3bee27850..b82b49d43942 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -115,7 +115,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo, struct ttm_bo_device *bdev = bo->bdev; struct ttm_resource_manager *man; - if (!list_empty(>lru)) + if (!list_empty(>lru) || bo->pin_count) return; if (mem->placement & TTM_PL_FLAG_NO_EVICT) @@ -165,7 +165,8 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, ttm_bo_del_from_lru(bo); ttm_bo_add_mem_to_lru(bo, >mem); - if (bulk && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) { + if (bulk && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT) && + !bo->pin_count) { switch (bo->mem.mem_type) { case TTM_PL_TT: ttm_bo_bulk_move_set_pos(>tt[bo->priority], bo); @@ -544,8 +545,9 @@ static void ttm_bo_release(struct kref *kref) * shrinkers, now that they are queued for * destruction. */ - if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) { + if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT || bo->pin_count) { bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT; + bo->pin_count = 0; ttm_bo_del_from_lru(bo); ttm_bo_add_mem_to_lru(bo, >mem); } @@ -1172,6 +1174,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, bo->moving = NULL; bo->mem.placement = TTM_PL_FLAG_CACHED; bo->acc_size = acc_size; + bo->pin_count = 0; bo->sg = sg; if (resv) { bo->base.resv = resv; diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index fb2a25f8408f..1968df9743fc 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -352,7 +352,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, return -ENOMEM; fbo->base = *bo; - fbo->base.mem.placement |= TTM_PL_FLAG_NO_EVICT; ttm_bo_get(bo); fbo->bo = bo; @@ -372,6 +371,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, kref_init(>base.kref); fbo->base.destroy = _transfered_destroy; fbo->base.acc_size = 0; + fbo->base.pin_count = 1; if (bo->type != ttm_bo_type_sg) fbo->base.base.resv = >base.base._resv; diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 0f7cd21d6d74..83979500df1c 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -157,6 +157,7 @@ struct ttm_buffer_object { struct dma_fence *moving; unsigned priority; + unsigned pin_count; /** * Special members that are protected by the reserve lock @@ -606,6 +607,29 @@ static inline bool ttm_bo_uses_embedded_gem_object(struct ttm_buffer_object *bo) return bo->base.dev != NULL; } +/** + * ttm_bo_pin - Pin the buffer object. + * @bo: The buffer object to pin + * + * Make sure the buffer is not evicted any more during memory pressure. + */ +static inline void ttm_bo_pin(struct ttm_buffer_object *bo) +{ + ++bo->pin_count; +} + +/** + * ttm_bo_unpin - Unpin the buffer object. + * @bo: The buffer object to unpin + * + * Allows the buffer object to be evicted again during memory pressure. + */ +static inline void ttm_bo_unpin(struct ttm_buffer_object *bo) +{ + WARN_ON_ONCE(!bo->pin_count); + --bo->pin_count; +} + int ttm_mem_evict_first(struct ttm_bo_device *bdev, struct ttm_resource_manager *man, const struct ttm_place *place, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/11] drm/qxl: switch over to the new pin interface
Stop using TTM_PL_FLAG_NO_EVICT. Signed-off-by: Christian König --- drivers/gpu/drm/qxl/qxl_debugfs.c | 2 +- drivers/gpu/drm/qxl/qxl_drv.h | 1 - drivers/gpu/drm/qxl/qxl_ioctl.c | 4 +-- drivers/gpu/drm/qxl/qxl_object.c | 44 +-- drivers/gpu/drm/qxl/qxl_object.h | 2 +- drivers/gpu/drm/qxl/qxl_release.c | 4 +-- drivers/gpu/drm/qxl/qxl_ttm.c | 2 +- 7 files changed, 20 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c b/drivers/gpu/drm/qxl/qxl_debugfs.c index 524d35b648d8..183d15e2cf58 100644 --- a/drivers/gpu/drm/qxl/qxl_debugfs.c +++ b/drivers/gpu/drm/qxl/qxl_debugfs.c @@ -67,7 +67,7 @@ qxl_debugfs_buffers_info(struct seq_file *m, void *data) seq_printf(m, "size %ld, pc %d, num releases %d\n", (unsigned long)bo->tbo.base.size, - bo->pin_count, rel); + bo->tbo.pin_count, rel); } return 0; } diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index aae90a9ee1db..3602e8b34189 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -80,7 +80,6 @@ struct qxl_bo { struct ttm_placeplacements[3]; struct ttm_placementplacement; struct ttm_bo_kmap_obj kmap; - unsigned int pin_count; void*kptr; unsigned intmap_count; int type; diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c index 5cea6eea72ab..0bab9ec6adc1 100644 --- a/drivers/gpu/drm/qxl/qxl_ioctl.c +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c @@ -326,8 +326,8 @@ static int qxl_update_area_ioctl(struct drm_device *dev, void *data, if (ret) goto out; - if (!qobj->pin_count) { - qxl_ttm_placement_from_domain(qobj, qobj->type, false); + if (!qobj->tbo.pin_count) { + qxl_ttm_placement_from_domain(qobj, qobj->type); ret = ttm_bo_validate(>tbo, >placement, ); if (unlikely(ret)) goto out; diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index 2bc364412e8b..d3635e3e3267 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -51,14 +51,12 @@ bool qxl_ttm_bo_is_qxl_bo(struct ttm_buffer_object *bo) return false; } -void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain, bool pinned) +void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain) { u32 c = 0; u32 pflag = 0; unsigned int i; - if (pinned) - pflag |= TTM_PL_FLAG_NO_EVICT; if (qbo->tbo.base.size <= PAGE_SIZE) pflag |= TTM_PL_FLAG_TOPDOWN; @@ -128,14 +126,13 @@ int qxl_bo_create(struct qxl_device *qdev, } bo->tbo.base.funcs = _object_funcs; bo->type = domain; - bo->pin_count = pinned ? 1 : 0; bo->surface_id = 0; INIT_LIST_HEAD(>list); if (surf) bo->surf = *surf; - qxl_ttm_placement_from_domain(bo, domain, pinned); + qxl_ttm_placement_from_domain(bo, domain); r = ttm_bo_init(>mman.bdev, >tbo, size, type, >placement, 0, !kernel, size, @@ -147,6 +144,8 @@ int qxl_bo_create(struct qxl_device *qdev, size, domain); return r; } + if (pinned) + ttm_bo_pin(>tbo); *bo_ptr = bo; return 0; } @@ -248,39 +247,22 @@ static int __qxl_bo_pin(struct qxl_bo *bo) struct drm_device *ddev = bo->tbo.base.dev; int r; - if (bo->pin_count) { - bo->pin_count++; + if (bo->tbo.pin_count) { + ttm_bo_pin(>tbo); return 0; } - qxl_ttm_placement_from_domain(bo, bo->type, true); + qxl_ttm_placement_from_domain(bo, bo->type); r = ttm_bo_validate(>tbo, >placement, ); - if (likely(r == 0)) { - bo->pin_count = 1; - } + if (likely(r == 0)) + ttm_bo_pin(>tbo); if (unlikely(r != 0)) dev_err(ddev->dev, "%p pin failed\n", bo); return r; } -static int __qxl_bo_unpin(struct qxl_bo *bo) +static void __qxl_bo_unpin(struct qxl_bo *bo) { - struct ttm_operation_ctx ctx = { false, false }; - struct drm_device *ddev = bo->tbo.base.dev; - int r, i; - - if (!bo->pin_count) { - dev_warn(ddev->dev, "%p unpin not necessary\n", bo); - return 0; - } - bo->pin_count--; - if (bo->pin_count) - return 0; - for (i = 0; i < bo->placement.num_placement; i++) - bo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT; - r = ttm_bo_validate(>tbo, >placement, ); - if (unlikely(r !=
[PATCH 09/11] drm/amdgpu: switch over to the new pin interface
Stop using TTM_PL_FLAG_NO_EVICT. Signed-off-by: Christian König --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 5 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 +--- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 5 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 41 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h| 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 2 +- 9 files changed, 24 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index b6b821500d30..64d4b5ff95d6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -1479,7 +1479,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu( } } - if (!amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) && !bo->pin_count) + if (!amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) && !bo->tbo.pin_count) amdgpu_bo_fence(bo, >process_info->eviction_fence->base, true); @@ -1558,7 +1558,8 @@ int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu( * required. */ if (mem->mapped_to_gpu_memory == 0 && - !amdgpu_ttm_tt_get_usermm(mem->bo->tbo.ttm) && !mem->bo->pin_count) + !amdgpu_ttm_tt_get_usermm(mem->bo->tbo.ttm) && + !mem->bo->tbo.pin_count) amdgpu_amdkfd_remove_eviction_fence(mem->bo, process_info->eviction_fence); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 12598a4b5c78..d50b63a93d37 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -410,7 +410,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p, uint32_t domain; int r; - if (bo->pin_count) + if (bo->tbo.pin_count) return 0; /* Don't move this buffer if we have depleted our allowance diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index c81206e6096f..4cba095b6c44 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -132,10 +132,7 @@ static void amdgpu_display_unpin_work_func(struct work_struct *__work) /* unpin of the old buffer */ r = amdgpu_bo_reserve(work->old_abo, true); if (likely(r == 0)) { - r = amdgpu_bo_unpin(work->old_abo); - if (unlikely(r != 0)) { - DRM_ERROR("failed to unpin buffer after flip\n"); - } + amdgpu_bo_unpin(work->old_abo); amdgpu_bo_unreserve(work->old_abo); } else DRM_ERROR("failed to reserve buffer after flip\n"); @@ -249,8 +246,7 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc *crtc, } unpin: if (!adev->enable_virtual_display) - if (unlikely(amdgpu_bo_unpin(new_abo) != 0)) - DRM_ERROR("failed to unpin new abo in error path\n"); + amdgpu_bo_unpin(new_abo); unreserve: amdgpu_bo_unreserve(new_abo); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index 957934926b24..5b465ab774d1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -281,7 +281,7 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach, struct sg_table *sgt; long r; - if (!bo->pin_count) { + if (!bo->tbo.pin_count) { /* move buffer into GTT or VRAM */ struct ttm_operation_ctx ctx = { false, false }; unsigned domains = AMDGPU_GEM_DOMAIN_GTT; @@ -390,7 +390,8 @@ static int amdgpu_dma_buf_begin_cpu_access(struct dma_buf *dma_buf, if (unlikely(ret != 0)) return ret; - if (!bo->pin_count && (bo->allowed_domains & AMDGPU_GEM_DOMAIN_GTT)) { + if (!bo->tbo.pin_count && + (bo->allowed_domains & AMDGPU_GEM_DOMAIN_GTT)) { amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT); ret = ttm_bo_validate(>tbo, >placement, ); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index aa7f230c71bf..59b52804622d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -860,7 +860,7 @@ static int amdgpu_debugfs_gem_bo_info(int id, void *ptr, void *data) seq_printf(m, "\t0x%08x: %12ld byte %s", id, amdgpu_bo_size(bo), placement); - pin_count =
[PATCH 10/11] drm/ttm: remove ttm_bo_create
Not used any more. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 40 include/drm/ttm/ttm_bo_api.h | 24 -- 2 files changed, 64 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index b82b49d43942..1a4b25083326 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1254,19 +1254,6 @@ int ttm_bo_init(struct ttm_bo_device *bdev, } EXPORT_SYMBOL(ttm_bo_init); -static size_t ttm_bo_acc_size(struct ttm_bo_device *bdev, - unsigned long bo_size, - unsigned struct_size) -{ - unsigned npages = (PAGE_ALIGN(bo_size)) >> PAGE_SHIFT; - size_t size = 0; - - size += ttm_round_pot(struct_size); - size += ttm_round_pot(npages * sizeof(void *)); - size += ttm_round_pot(sizeof(struct ttm_tt)); - return size; -} - size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev, unsigned long bo_size, unsigned struct_size) @@ -1281,33 +1268,6 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev, } EXPORT_SYMBOL(ttm_bo_dma_acc_size); -int ttm_bo_create(struct ttm_bo_device *bdev, - unsigned long size, - enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, - bool interruptible, - struct ttm_buffer_object **p_bo) -{ - struct ttm_buffer_object *bo; - size_t acc_size; - int ret; - - bo = kzalloc(sizeof(*bo), GFP_KERNEL); - if (unlikely(bo == NULL)) - return -ENOMEM; - - acc_size = ttm_bo_acc_size(bdev, size, sizeof(struct ttm_buffer_object)); - ret = ttm_bo_init(bdev, bo, size, type, placement, page_alignment, - interruptible, acc_size, - NULL, NULL, NULL); - if (likely(ret == 0)) - *p_bo = bo; - - return ret; -} -EXPORT_SYMBOL(ttm_bo_create); - int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type) { struct ttm_resource_manager *man = ttm_manager_type(bdev, mem_type); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 83979500df1c..ce592c9061e8 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -447,30 +447,6 @@ int ttm_bo_init(struct ttm_bo_device *bdev, struct ttm_buffer_object *bo, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)); -/** - * ttm_bo_create - * - * @bdev: Pointer to a ttm_bo_device struct. - * @size: Requested size of buffer object. - * @type: Requested type of buffer object. - * @placement: Initial placement. - * @page_alignment: Data alignment in pages. - * @interruptible: If needing to sleep while waiting for GPU resources, - * sleep interruptible. - * @p_bo: On successful completion *p_bo points to the created object. - * - * This function allocates a ttm_buffer_object, and then calls ttm_bo_init - * on that object. The destroy function is set to kfree(). - * Returns - * -ENOMEM: Out of memory. - * -EINVAL: Invalid placement flags. - * -ERESTARTSYS: Interrupted by signal while waiting for resources. - */ -int ttm_bo_create(struct ttm_bo_device *bdev, unsigned long size, - enum ttm_bo_type type, struct ttm_placement *placement, - uint32_t page_alignment, bool interruptible, - struct ttm_buffer_object **p_bo); - /** * ttm_bo_evict_mm * -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/11] drm/vmwgfx: switch over to the new pin interface
Stop using TTM_PL_FLAG_NO_EVICT. Signed-off-by: Christian König --- drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 48 +++--- drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c| 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 7 +--- drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 42 --- drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 2 +- 11 files changed, 38 insertions(+), 85 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c index e8d66182cd7b..ea2f2f937eb3 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c @@ -459,9 +459,9 @@ int vmw_bo_cpu_blit(struct ttm_buffer_object *dst, int ret = 0; /* Buffer objects need to be either pinned or reserved: */ - if (!(dst->mem.placement & TTM_PL_FLAG_NO_EVICT)) + if (!(dst->pin_count)) dma_resv_assert_held(dst->base.resv); - if (!(src->mem.placement & TTM_PL_FLAG_NO_EVICT)) + if (!(src->pin_count)) dma_resv_assert_held(src->base.resv); if (!ttm_tt_is_populated(dst->ttm)) { diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 30d19b45b602..236eaa163917 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -106,7 +106,7 @@ int vmw_bo_pin_in_placement(struct vmw_private *dev_priv, if (unlikely(ret != 0)) goto err; - if (buf->pin_count > 0) + if (buf->base.pin_count > 0) ret = ttm_bo_mem_compat(placement, >mem, _flags) == true ? 0 : -EINVAL; else @@ -155,7 +155,7 @@ int vmw_bo_pin_in_vram_or_gmr(struct vmw_private *dev_priv, if (unlikely(ret != 0)) goto err; - if (buf->pin_count > 0) { + if (buf->base.pin_count > 0) { ret = ttm_bo_mem_compat(_vram_gmr_placement, >mem, _flags) == true ? 0 : -EINVAL; goto out_unreserve; @@ -246,12 +246,12 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private *dev_priv, if (bo->mem.mem_type == TTM_PL_VRAM && bo->mem.start < bo->num_pages && bo->mem.start > 0 && - buf->pin_count == 0) { + buf->base.pin_count == 0) { ctx.interruptible = false; (void) ttm_bo_validate(bo, _sys_placement, ); } - if (buf->pin_count > 0) + if (buf->base.pin_count > 0) ret = ttm_bo_mem_compat(, >mem, _flags) == true ? 0 : -EINVAL; else @@ -343,23 +343,13 @@ void vmw_bo_pin_reserved(struct vmw_buffer_object *vbo, bool pin) dma_resv_assert_held(bo->base.resv); - if (pin) { - if (vbo->pin_count++ > 0) - return; - } else { - WARN_ON(vbo->pin_count <= 0); - if (--vbo->pin_count > 0) - return; - } + if (pin == !!bo->pin_count) + return; pl.fpfn = 0; pl.lpfn = 0; pl.mem_type = bo->mem.mem_type; pl.flags = bo->mem.placement; - if (pin) - pl.flags |= TTM_PL_FLAG_NO_EVICT; - else - pl.flags &= ~TTM_PL_FLAG_NO_EVICT; memset(, 0, sizeof(placement)); placement.num_placement = 1; @@ -368,8 +358,12 @@ void vmw_bo_pin_reserved(struct vmw_buffer_object *vbo, bool pin) ret = ttm_bo_validate(bo, , ); BUG_ON(ret != 0 || bo->mem.mem_type != old_mem_type); -} + if (pin) + ttm_bo_pin(bo); + else + ttm_bo_unpin(bo); +} /** * vmw_bo_map_and_cache - Map a buffer object and cache the map @@ -538,6 +532,7 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size, * @size: Buffer object size in bytes. * @placement: Initial placement. * @interruptible: Whether waits should be performed interruptible. + * @pin: If the BO should be created pinned at a fixed location. * @bo_free: The buffer object destructor. * Returns: Zero on success, negative error code on error. * @@ -546,9 +541,10 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size, int vmw_bo_init(struct vmw_private *dev_priv, struct vmw_buffer_object *vmw_bo, size_t size, struct ttm_placement *placement, - bool interruptible, + bool interruptible, bool pin, void (*bo_free)(struct ttm_buffer_object *bo)) { + struct ttm_operation_ctx ctx = {
[PATCH 11/11] drm/ttm: remove TTM_PL_FLAG_NO_EVICT
Not used any more. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c| 11 +++ include/drm/ttm/ttm_placement.h | 1 - 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 1a4b25083326..5737b3fae1b3 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -118,9 +118,6 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo, if (!list_empty(>lru) || bo->pin_count) return; - if (mem->placement & TTM_PL_FLAG_NO_EVICT) - return; - man = ttm_manager_type(bdev, mem->mem_type); list_add_tail(>lru, >lru[bo->priority]); @@ -165,8 +162,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, ttm_bo_del_from_lru(bo); ttm_bo_add_mem_to_lru(bo, >mem); - if (bulk && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT) && - !bo->pin_count) { + if (bulk && !bo->pin_count) { switch (bo->mem.mem_type) { case TTM_PL_TT: ttm_bo_bulk_move_set_pos(>tt[bo->priority], bo); @@ -541,12 +537,11 @@ static void ttm_bo_release(struct kref *kref) spin_lock(_bo_glob.lru_lock); /* -* Make NO_EVICT bos immediately available to +* Make pinned bos immediately available to * shrinkers, now that they are queued for * destruction. */ - if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT || bo->pin_count) { - bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT; + if (bo->pin_count) { bo->pin_count = 0; ttm_bo_del_from_lru(bo); ttm_bo_add_mem_to_lru(bo, >mem); diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h index d4022655eae4..50e72df48b8d 100644 --- a/include/drm/ttm/ttm_placement.h +++ b/include/drm/ttm/ttm_placement.h @@ -58,7 +58,6 @@ #define TTM_PL_FLAG_UNCACHED(1 << 17) #define TTM_PL_FLAG_WC (1 << 18) #define TTM_PL_FLAG_CONTIGUOUS (1 << 19) -#define TTM_PL_FLAG_NO_EVICT(1 << 21) #define TTM_PL_FLAG_TOPDOWN (1 << 22) #define TTM_PL_MASK_CACHING (TTM_PL_FLAG_CACHED | \ -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/11] drm/vram-helper: switch over to the new pin interface
Stop using TTM_PL_FLAG_NO_EVICT. Signed-off-by: Christian König --- drivers/gpu/drm/drm_gem_vram_helper.c | 37 +++ include/drm/drm_gem_vram_helper.h | 3 --- 2 files changed, 9 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 50cad0e4a92e..bd3dd17ad81d 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -301,7 +301,7 @@ static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo) */ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo) { - if (WARN_ON_ONCE(!gbo->pin_count)) + if (WARN_ON_ONCE(!gbo->bo.pin_count)) return (s64)-ENODEV; return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT; } @@ -310,24 +310,21 @@ EXPORT_SYMBOL(drm_gem_vram_offset); static int drm_gem_vram_pin_locked(struct drm_gem_vram_object *gbo, unsigned long pl_flag) { - int i, ret; struct ttm_operation_ctx ctx = { false, false }; + int ret; - if (gbo->pin_count) + if (gbo->bo.pin_count) goto out; if (pl_flag) drm_gem_vram_placement(gbo, pl_flag); - for (i = 0; i < gbo->placement.num_placement; ++i) - gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; - ret = ttm_bo_validate(>bo, >placement, ); if (ret < 0) return ret; out: - ++gbo->pin_count; + ttm_bo_pin(>bo); return 0; } @@ -369,26 +366,9 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag) } EXPORT_SYMBOL(drm_gem_vram_pin); -static int drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo) +static void drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo) { - int i, ret; - struct ttm_operation_ctx ctx = { false, false }; - - if (WARN_ON_ONCE(!gbo->pin_count)) - return 0; - - --gbo->pin_count; - if (gbo->pin_count) - return 0; - - for (i = 0; i < gbo->placement.num_placement ; ++i) - gbo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT; - - ret = ttm_bo_validate(>bo, >placement, ); - if (ret < 0) - return ret; - - return 0; + ttm_bo_unpin(>bo); } /** @@ -406,10 +386,11 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo) ret = ttm_bo_reserve(>bo, true, false, NULL); if (ret) return ret; - ret = drm_gem_vram_unpin_locked(gbo); + + drm_gem_vram_unpin_locked(gbo); ttm_bo_unreserve(>bo); - return ret; + return 0; } EXPORT_SYMBOL(drm_gem_vram_unpin); diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 62cc6e6c3a4f..128f88174d32 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -35,7 +35,6 @@ struct vm_area_struct; * @placement: TTM placement information. Supported placements are \ %TTM_PL_VRAM and %TTM_PL_SYSTEM * @placements:TTM placement information. - * @pin_count: Pin counter * * The type struct drm_gem_vram_object represents a GEM object that is * backed by VRAM. It can be used for simple framebuffer devices with @@ -64,8 +63,6 @@ struct drm_gem_vram_object { /* Supported placements are %TTM_PL_VRAM and %TTM_PL_SYSTEM */ struct ttm_placement placement; struct ttm_place placements[2]; - - int pin_count; }; /** -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/11] drm/radeon: switch over to the new pin interface
Stop using TTM_PL_FLAG_NO_EVICT. Signed-off-by: Christian König --- drivers/gpu/drm/radeon/radeon.h | 1 - drivers/gpu/drm/radeon/radeon_display.c | 9 ++ drivers/gpu/drm/radeon/radeon_object.c | 37 ++--- drivers/gpu/drm/radeon/radeon_object.h | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- 5 files changed, 13 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index a6d8de01194a..5d54bccebd4d 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -497,7 +497,6 @@ struct radeon_bo { struct ttm_buffer_objecttbo; struct ttm_bo_kmap_obj kmap; u32 flags; - unsignedpin_count; void*kptr; u32 tiling_flags; u32 pitch; diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 7b69d6dfe44a..3eacf33bbe48 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -273,10 +273,7 @@ static void radeon_unpin_work_func(struct work_struct *__work) /* unpin of the old buffer */ r = radeon_bo_reserve(work->old_rbo, false); if (likely(r == 0)) { - r = radeon_bo_unpin(work->old_rbo); - if (unlikely(r != 0)) { - DRM_ERROR("failed to unpin buffer after flip\n"); - } + radeon_bo_unpin(work->old_rbo); radeon_bo_unreserve(work->old_rbo); } else DRM_ERROR("failed to reserve buffer after flip\n"); @@ -607,9 +604,7 @@ static int radeon_crtc_page_flip_target(struct drm_crtc *crtc, DRM_ERROR("failed to reserve new rbo in error path\n"); goto cleanup; } - if (unlikely(radeon_bo_unpin(new_rbo) != 0)) { - DRM_ERROR("failed to unpin new rbo in error path\n"); - } + radeon_bo_unpin(new_rbo); radeon_bo_unreserve(new_rbo); cleanup: diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 316e35d3f8a9..0de267ab3913 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -334,8 +334,8 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, if (radeon_ttm_tt_has_userptr(bo->rdev, bo->tbo.ttm)) return -EPERM; - if (bo->pin_count) { - bo->pin_count++; + if (bo->tbo.pin_count) { + ttm_bo_pin(>tbo); if (gpu_addr) *gpu_addr = radeon_bo_gpu_offset(bo); @@ -367,13 +367,11 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; else bo->placements[i].lpfn = max_offset >> PAGE_SHIFT; - - bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; } r = ttm_bo_validate(>tbo, >placement, ); if (likely(r == 0)) { - bo->pin_count = 1; + ttm_bo_pin(>tbo); if (gpu_addr != NULL) *gpu_addr = radeon_bo_gpu_offset(bo); if (domain == RADEON_GEM_DOMAIN_VRAM) @@ -391,32 +389,15 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) return radeon_bo_pin_restricted(bo, domain, 0, gpu_addr); } -int radeon_bo_unpin(struct radeon_bo *bo) +void radeon_bo_unpin(struct radeon_bo *bo) { - struct ttm_operation_ctx ctx = { false, false }; - int r, i; - - if (!bo->pin_count) { - dev_warn(bo->rdev->dev, "%p unpin not necessary\n", bo); - return 0; - } - bo->pin_count--; - if (bo->pin_count) - return 0; - for (i = 0; i < bo->placement.num_placement; i++) { - bo->placements[i].lpfn = 0; - bo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT; - } - r = ttm_bo_validate(>tbo, >placement, ); - if (likely(r == 0)) { + ttm_bo_unpin(>tbo); + if (!bo->tbo.pin_count) { if (bo->tbo.mem.mem_type == TTM_PL_VRAM) bo->rdev->vram_pin_size -= radeon_bo_size(bo); else bo->rdev->gart_pin_size -= radeon_bo_size(bo); - } else { - dev_err(bo->rdev->dev, "%p validate failed for unpin\n", bo); } - return r; } int radeon_bo_evict_vram(struct radeon_device *rdev) @@ -549,7 +530,7 @@ int radeon_bo_list_validate(struct radeon_device *rdev, list_for_each_entry(lobj, head, tv.head) { struct radeon_bo *bo = lobj->robj; - if (!bo->pin_count) { + if
[PATCH 08/11] drm/nouveau: switch over to the new pin interface
Stop using TTM_PL_FLAG_NO_EVICT. Signed-off-by: Christian König --- drivers/gpu/drm/nouveau/nouveau_bo.c | 48 +++--- drivers/gpu/drm/nouveau/nouveau_bo.h | 3 -- drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +- 3 files changed, 13 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 2ee75646ad6f..bcae4514952f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -139,7 +139,7 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo) struct drm_device *dev = drm->dev; struct nouveau_bo *nvbo = nouveau_bo(bo); - WARN_ON(nvbo->pin_refcnt > 0); + WARN_ON(nvbo->bo.pin_count > 0); nouveau_bo_del_io_reserve_lru(bo); nv10_bo_put_tile_region(dev, nvbo->tile, NULL); @@ -417,9 +417,8 @@ nouveau_bo_placement_set(struct nouveau_bo *nvbo, uint32_t domain, { struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev); struct ttm_placement *pl = >placement; - uint32_t flags = (nvbo->force_coherent ? TTM_PL_FLAG_UNCACHED : -TTM_PL_MASK_CACHING) | -(nvbo->pin_refcnt ? TTM_PL_FLAG_NO_EVICT : 0); + uint32_t flags = nvbo->force_coherent ? TTM_PL_FLAG_UNCACHED : + TTM_PL_MASK_CACHING; pl->placement = nvbo->placements; set_placement_list(drm, nvbo->placements, >num_placement, @@ -453,7 +452,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, bool contig) } } - if (nvbo->pin_refcnt) { + if (nvbo->bo.pin_count) { bool error = evict; switch (bo->mem.mem_type) { @@ -472,7 +471,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, bool contig) bo->mem.mem_type, domain); ret = -EBUSY; } - nvbo->pin_refcnt++; + ttm_bo_pin(>bo); goto out; } @@ -483,18 +482,12 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, bool contig) goto out; } - nvbo->pin_refcnt++; nouveau_bo_placement_set(nvbo, domain, 0); - - /* drop pin_refcnt temporarily, so we don't trip the assertion -* in nouveau_bo_move() that makes sure we're not trying to -* move a pinned buffer -*/ - nvbo->pin_refcnt--; ret = nouveau_bo_validate(nvbo, false, false); if (ret) goto out; - nvbo->pin_refcnt++; + + ttm_bo_pin(>bo); switch (bo->mem.mem_type) { case TTM_PL_VRAM: @@ -519,30 +512,14 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo) { struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev); struct ttm_buffer_object *bo = >bo; - int ret, ref; + int ret; ret = ttm_bo_reserve(bo, false, false, NULL); if (ret) return ret; - ref = --nvbo->pin_refcnt; - WARN_ON_ONCE(ref < 0); - if (ref) - goto out; - - switch (bo->mem.mem_type) { - case TTM_PL_VRAM: - nouveau_bo_placement_set(nvbo, NOUVEAU_GEM_DOMAIN_VRAM, 0); - break; - case TTM_PL_TT: - nouveau_bo_placement_set(nvbo, NOUVEAU_GEM_DOMAIN_GART, 0); - break; - default: - break; - } - - ret = nouveau_bo_validate(nvbo, false, false); - if (ret == 0) { + ttm_bo_unpin(>bo); + if (!nvbo->bo.pin_count) { switch (bo->mem.mem_type) { case TTM_PL_VRAM: drm->gem.vram_available += bo->mem.size; @@ -555,9 +532,8 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo) } } -out: ttm_bo_unreserve(bo); - return ret; + return 0; } int @@ -1065,7 +1041,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, if (ret) return ret; - if (nvbo->pin_refcnt) + if (nvbo->bo.pin_count) NV_WARN(drm, "Moving pinned object %p!\n", nvbo); if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA) { diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h index 2a23c8207436..ff68ded8d590 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.h +++ b/drivers/gpu/drm/nouveau/nouveau_bo.h @@ -40,9 +40,6 @@ struct nouveau_bo { struct nouveau_drm_tile *tile; - /* protect by the ttm reservation lock */ - int pin_refcnt; - struct ttm_bo_kmap_obj dma_buf_vmap; }; diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c index 8f099601d2f2..5d191e58edf1 100644 --- a/drivers/gpu/drm/nouveau/nouveau_chan.c +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c @@ -107,7 +107,7 @@ nouveau_channel_del(struct
Re: [PATCH AUTOSEL 5.4 13/15] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
On 2020-09-21 4:40 p.m., Sasha Levin wrote: From: Michel Dänzer [ Upstream commit 2f228aab21bbc74e90e267a721215ec8be51daf7 ] Don't check drm_crtc_state::active for this either, per its documentation in include/drm/drm_crtc.h: * Hence drivers must not consult @active in their various * _mode_config_funcs.atomic_check callback to reject an atomic * commit. atomic_remove_fb disables the CRTC as needed for disabling the primary plane. This prevents at least the following problems if the primary plane gets disabled (e.g. due to destroying the FB assigned to the primary plane, as happens e.g. with mutter in Wayland mode): * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID (which enables the cursor plane). * If the cursor plane was enabled, changing the legacy DPMS property value from off to on returned EINVAL. v2: * Minor changes to code comment and commit log, per review feedback. GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344 Suggested-by: Daniel Vetter Acked-by: Daniel Vetter Reviewed-by: Nicholas Kazlauskas Signed-off-by: Michel Dänzer Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin I'm a bit nervous about this getting backported so far back so quickly. I'd prefer waiting for 5.9 final first at least. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.4 12/15] drm/amd/display: update nv1x stutter latencies
From: Jun Lei [ Upstream commit c4790a8894232f39c25c7c546c06efe074e63384 ] [why] Recent characterization shows increased stutter latencies on some SKUs, leading to underflow. [how] Update SOC params to account for this worst case latency. Signed-off-by: Jun Lei Acked-by: Aurabindo Pillai Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c index 05b98eadc2899..46afd048c1ea3 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c @@ -340,8 +340,8 @@ struct _vcs_dpi_soc_bounding_box_st dcn2_0_nv14_soc = { }, }, .num_states = 5, - .sr_exit_time_us = 8.6, - .sr_enter_plus_exit_time_us = 10.9, + .sr_exit_time_us = 11.6, + .sr_enter_plus_exit_time_us = 13.9, .urgent_latency_us = 4.0, .urgent_latency_pixel_data_only_us = 4.0, .urgent_latency_pixel_mixed_with_vm_data_us = 4.0, -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.8 13/20] drm/amd/display: Don't use DRM_ERROR() for DTM add topology
From: Bhawanpreet Lakha [ Upstream commit 4cdd7b332ed139b1e37faeb82409a14490adb644 ] [Why] Previously we were only calling add_topology when hdcp was being enabled. Now we call add_topology by default so the ERROR messages are printed if the firmware is not loaded. This error message is not relevant for normal display functionality so no need to print a ERROR message. [How] Change DRM_ERROR to DRM_INFO Signed-off-by: Bhawanpreet Lakha Acked-by: Aurabindo Pillai Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c index fb1161dd7ea80..3a367a5968ae1 100644 --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c @@ -88,7 +88,7 @@ enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct mod_hdcp *hdcp, enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS; if (!psp->dtm_context.dtm_initialized) { - DRM_ERROR("Failed to add display topology, DTM TA is not initialized."); + DRM_INFO("Failed to add display topology, DTM TA is not initialized."); display->state = MOD_HDCP_DISPLAY_INACTIVE; return MOD_HDCP_STATUS_FAILURE; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.8 14/20] drm/amd/display: update nv1x stutter latencies
From: Jun Lei [ Upstream commit c4790a8894232f39c25c7c546c06efe074e63384 ] [why] Recent characterization shows increased stutter latencies on some SKUs, leading to underflow. [how] Update SOC params to account for this worst case latency. Signed-off-by: Jun Lei Acked-by: Aurabindo Pillai Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c index 2d9055eb3ce92..20bdabebbc434 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c @@ -409,8 +409,8 @@ struct _vcs_dpi_soc_bounding_box_st dcn2_0_nv14_soc = { }, }, .num_states = 5, - .sr_exit_time_us = 8.6, - .sr_enter_plus_exit_time_us = 10.9, + .sr_exit_time_us = 11.6, + .sr_enter_plus_exit_time_us = 13.9, .urgent_latency_us = 4.0, .urgent_latency_pixel_data_only_us = 4.0, .urgent_latency_pixel_mixed_with_vm_data_us = 4.0, -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 4.19 7/9] drm/amdkfd: fix a memory leak issue
From: Dennis Li [ Upstream commit 087d764159996ae378b08c0fdd557537adfd6899 ] In the resume stage of GPU recovery, start_cpsch will call pm_init which set pm->allocated as false, cause the next pm_release_ib has no chance to release ib memory. Add pm_release_ib in stop_cpsch which will be called in the suspend stage of GPU recovery. Reviewed-by: Felix Kuehling Signed-off-by: Dennis Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index 189212cb35475..bff39f561264e 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -1101,6 +1101,8 @@ static int stop_cpsch(struct device_queue_manager *dqm) unmap_queues_cpsch(dqm, KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES, 0); dqm_unlock(dqm); + pm_release_ib(>packets); + kfd_gtt_sa_free(dqm->dev, dqm->fence_mem); pm_uninit(>packets); -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.8 12/20] drm/amdkfd: fix a memory leak issue
From: Dennis Li [ Upstream commit 087d764159996ae378b08c0fdd557537adfd6899 ] In the resume stage of GPU recovery, start_cpsch will call pm_init which set pm->allocated as false, cause the next pm_release_ib has no chance to release ib memory. Add pm_release_ib in stop_cpsch which will be called in the suspend stage of GPU recovery. Reviewed-by: Felix Kuehling Signed-off-by: Dennis Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index e9c4867abeffb..00b042a15373a 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -1177,6 +1177,8 @@ static int stop_cpsch(struct device_queue_manager *dqm) dqm->sched_running = false; dqm_unlock(dqm); + pm_release_ib(>packets); + kfd_gtt_sa_free(dqm->dev, dqm->fence_mem); pm_uninit(>packets, hanging); -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.8 16/20] drm/amd/display: Don't log hdcp module warnings in dmesg
From: Bhawanpreet Lakha [ Upstream commit 875d369d8f75275d30e59421602d9366426abff7 ] [Why] DTM topology updates happens by default now. This results in DTM warnings when hdcp is not even being enabled. This spams the dmesg and doesn't effect normal display functionality so it is better to log it using DRM_DEBUG_KMS() [How] Change the DRM_WARN() to DRM_DEBUG_KMS() Signed-off-by: Bhawanpreet Lakha Acked-by: Alex Deucher Reviewed-by: Rodrigo Siqueira Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h index d3192b9d0c3d8..47f8ee2832ff0 100644 --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h @@ -27,7 +27,7 @@ #define MOD_HDCP_LOG_H_ #ifdef CONFIG_DRM_AMD_DC_HDCP -#define HDCP_LOG_ERR(hdcp, ...) DRM_WARN(__VA_ARGS__) +#define HDCP_LOG_ERR(hdcp, ...) DRM_DEBUG_KMS(__VA_ARGS__) #define HDCP_LOG_VER(hdcp, ...) DRM_DEBUG_KMS(__VA_ARGS__) #define HDCP_LOG_FSM(hdcp, ...) DRM_DEBUG_KMS(__VA_ARGS__) #define HDCP_LOG_TOP(hdcp, ...) pr_debug("[HDCP_TOP]:"__VA_ARGS__) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.8 15/20] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
From: Michel Dänzer [ Upstream commit 2f228aab21bbc74e90e267a721215ec8be51daf7 ] Don't check drm_crtc_state::active for this either, per its documentation in include/drm/drm_crtc.h: * Hence drivers must not consult @active in their various * _mode_config_funcs.atomic_check callback to reject an atomic * commit. atomic_remove_fb disables the CRTC as needed for disabling the primary plane. This prevents at least the following problems if the primary plane gets disabled (e.g. due to destroying the FB assigned to the primary plane, as happens e.g. with mutter in Wayland mode): * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID (which enables the cursor plane). * If the cursor plane was enabled, changing the legacy DPMS property value from off to on returned EINVAL. v2: * Minor changes to code comment and commit log, per review feedback. GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344 Suggested-by: Daniel Vetter Acked-by: Daniel Vetter Reviewed-by: Nicholas Kazlauskas Signed-off-by: Michel Dänzer Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++- 1 file changed, 10 insertions(+), 22 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 3f7eced92c0c8..7c1cc0ba30a55 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -5257,19 +5257,6 @@ static void dm_crtc_helper_disable(struct drm_crtc *crtc) { } -static bool does_crtc_have_active_cursor(struct drm_crtc_state *new_crtc_state) -{ - struct drm_device *dev = new_crtc_state->crtc->dev; - struct drm_plane *plane; - - drm_for_each_plane_mask(plane, dev, new_crtc_state->plane_mask) { - if (plane->type == DRM_PLANE_TYPE_CURSOR) - return true; - } - - return false; -} - static int count_crtc_active_planes(struct drm_crtc_state *new_crtc_state) { struct drm_atomic_state *state = new_crtc_state->state; @@ -5349,19 +5336,20 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, return ret; } - /* In some use cases, like reset, no stream is attached */ - if (!dm_crtc_state->stream) - return 0; - /* -* We want at least one hardware plane enabled to use -* the stream with a cursor enabled. +* We require the primary plane to be enabled whenever the CRTC is, otherwise +* drm_mode_cursor_universal may end up trying to enable the cursor plane while all other +* planes are disabled, which is not supported by the hardware. And there is legacy +* userspace which stops using the HW cursor altogether in response to the resulting EINVAL. */ - if (state->enable && state->active && - does_crtc_have_active_cursor(state) && - dm_crtc_state->active_planes == 0) + if (state->enable && + !(state->plane_mask & drm_plane_mask(crtc->primary))) return -EINVAL; + /* In some use cases, like reset, no stream is attached */ + if (!dm_crtc_state->stream) + return 0; + if (dc_validate_stream(dc, dm_crtc_state->stream) == DC_OK) return 0; -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.4 11/15] drm/amdkfd: fix a memory leak issue
From: Dennis Li [ Upstream commit 087d764159996ae378b08c0fdd557537adfd6899 ] In the resume stage of GPU recovery, start_cpsch will call pm_init which set pm->allocated as false, cause the next pm_release_ib has no chance to release ib memory. Add pm_release_ib in stop_cpsch which will be called in the suspend stage of GPU recovery. Reviewed-by: Felix Kuehling Signed-off-by: Dennis Li Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index a2ed9c257cb0d..e9a2784400792 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -1075,6 +1075,8 @@ static int stop_cpsch(struct device_queue_manager *dqm) unmap_queues_cpsch(dqm, KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES, 0); dqm_unlock(dqm); + pm_release_ib(>packets); + kfd_gtt_sa_free(dqm->dev, dqm->fence_mem); pm_uninit(>packets); -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 5.4 13/15] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
From: Michel Dänzer [ Upstream commit 2f228aab21bbc74e90e267a721215ec8be51daf7 ] Don't check drm_crtc_state::active for this either, per its documentation in include/drm/drm_crtc.h: * Hence drivers must not consult @active in their various * _mode_config_funcs.atomic_check callback to reject an atomic * commit. atomic_remove_fb disables the CRTC as needed for disabling the primary plane. This prevents at least the following problems if the primary plane gets disabled (e.g. due to destroying the FB assigned to the primary plane, as happens e.g. with mutter in Wayland mode): * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID (which enables the cursor plane). * If the cursor plane was enabled, changing the legacy DPMS property value from off to on returned EINVAL. v2: * Minor changes to code comment and commit log, per review feedback. GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344 Suggested-by: Daniel Vetter Acked-by: Daniel Vetter Reviewed-by: Nicholas Kazlauskas Signed-off-by: Michel Dänzer Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++- 1 file changed, 10 insertions(+), 22 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 60e50181f6d39..2384aa018993d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4299,19 +4299,6 @@ static void dm_crtc_helper_disable(struct drm_crtc *crtc) { } -static bool does_crtc_have_active_cursor(struct drm_crtc_state *new_crtc_state) -{ - struct drm_device *dev = new_crtc_state->crtc->dev; - struct drm_plane *plane; - - drm_for_each_plane_mask(plane, dev, new_crtc_state->plane_mask) { - if (plane->type == DRM_PLANE_TYPE_CURSOR) - return true; - } - - return false; -} - static int count_crtc_active_planes(struct drm_crtc_state *new_crtc_state) { struct drm_atomic_state *state = new_crtc_state->state; @@ -4391,19 +4378,20 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, return ret; } - /* In some use cases, like reset, no stream is attached */ - if (!dm_crtc_state->stream) - return 0; - /* -* We want at least one hardware plane enabled to use -* the stream with a cursor enabled. +* We require the primary plane to be enabled whenever the CRTC is, otherwise +* drm_mode_cursor_universal may end up trying to enable the cursor plane while all other +* planes are disabled, which is not supported by the hardware. And there is legacy +* userspace which stops using the HW cursor altogether in response to the resulting EINVAL. */ - if (state->enable && state->active && - does_crtc_have_active_cursor(state) && - dm_crtc_state->active_planes == 0) + if (state->enable && + !(state->plane_mask & drm_plane_mask(crtc->primary))) return -EINVAL; + /* In some use cases, like reset, no stream is attached */ + if (!dm_crtc_state->stream) + return 0; + if (dc_validate_stream(dc, dm_crtc_state->stream) == DC_OK) return 0; -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vram-helper: Fix use of top-down placement
Am 21.09.20 um 16:25 schrieb Thomas Zimmermann: Commit 7053e0eab473 ("drm/vram-helper: stop using TTM placement flags") cleared the BO placement flags if top-down placement had been selected. Hence, BOs that were supposed to go into VRAM are now placed in a default location in system memory. Trying to scanout the incorrectly pinned BO results in displayed garbage and an error message. [ 146.108127] [ cut here ] [ 146.1V08180] WARNING: CPU: 0 PID: 152 at drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60 [drm_vram_helper] ... [ 146.108591] ast_cursor_page_flip+0x3e/0x150 [ast] [ 146.108622] ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast] [ 146.108654] drm_atomic_helper_commit_planes+0x197/0x4c0 [ 146.108699] drm_atomic_helper_commit_tail_rpm+0x59/0xa0 [ 146.108718] commit_tail+0x103/0x1c0 ... [ 146.109302] ---[ end trace d901a1ba1d949036 ]--- Fix the bug by keeping the placement flags. The top-down placement flag is stored in a separate variable. Signed-off-by: Thomas Zimmermann Fixes: 7053e0eab473 ("drm/vram-helper: stop using TTM placement flags") Cc: Christian König Cc: Dave Airlie Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Reviewed-by: Christian König It's always the stupid typos, thanks for narrowing this down. And please keep an eye open I have quite a number of more patches in the pipeline. Thanks, Christian. --- drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 50cad0e4a92e..375c79e23ca5 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -140,7 +140,7 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo, unsigned int c = 0; if (pl_flag & DRM_GEM_VRAM_PL_FLAG_TOPDOWN) - pl_flag = TTM_PL_FLAG_TOPDOWN; + invariant_flags = TTM_PL_FLAG_TOPDOWN; gbo->placement.placement = gbo->placements; gbo->placement.busy_placement = gbo->placements; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: commit_work scheduling
Hello, On Mon, Sep 21, 2020 at 11:21:54AM +0200, Daniel Vetter wrote: > The part I don't like about this is that it all feels rather hacked > together, and if we add more stuff (or there's some different thing in the > system that also needs rt scheduling) then it doesn't compose. > > So question to rt/worker folks: What's the best way to let userspace set > the scheduling mode and priorities of things the kernel does on its > behalf? Surely we're not the first ones where if userspace runs with some > rt priority it'll starve out the kernel workers that it needs. Hardcoding > something behind a subsystem ioctl (which just means every time userspace > changes what it does, we need a new such flag or mode) can't be the right > thing. Maybe not first but there haven't been many. The main benefit of workqueue is that the users get to pool the worker threads automatically. I don't think the existing workqueue design is something suitable for actual RT use cases. Furthermore, there are inherent conflicts between sharing resources and RT as this this patchset is already showing w/ needing per-crtc worker thread. Maybe we can further abstract it if there are more use cases but for now kthread_worker based implementation sounds about right to me. Thanks. -- tejun ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] XDC 2020 feedback and comments
First off, I think you all did a fantastic job. I felt that things ran very smoothly and, as far as the talks themselves go, I think it went almost as smoothly as an in-person XDC. I'm really quite impressed. I do have a couple pieces of more nuanced feedback: 1. I think we were maybe a bit too scared of overloading jitsi. Having more people in the instance for questions might have made that portion go better. As it was, there was only one or two talks that had any live questions. That said, there are a few advantages to having things funneled through IRC, the most obvious of which being that people can ask their question mid-talk and have it handled at the end instead of having to remember it for 20 minutes. 2. I really miss the hallway track. On Thursday, after the conference, Bas, Connor, and I used jitsi to have a chat about ray-tracing. That was really fun and I wish I'd done something like that every day of XDC. Maybe it's my own fault for not setting up said chats but I think it could have been made more accessible (I had no idea how to fork off a jitsi instance) and/or encouraged somehow. --Jason On Mon, Sep 21, 2020 at 3:07 AM Samuel Iglesias Gonsálvez wrote: > > Hi all, > > Huge thanks again to the entire team from Intel, for their great work > organizing XDC 2020, our first virtual conference! > > As usual we're looking for feedback on both XDC itself, and the CFP > process and program selection. Both about what was great and should be > kept for next year's edition, and where there's room for improvement. > > The board does keep some notes, for those interested in what we have > already: > > - XDC notes for prospective organizers: > https://www.x.org/wiki/Events/RFP/ > > - CFP notes: https://www.x.org/wiki/Events/PapersCommittee/ > > If you want to send in your comments in private, please send them to > the X.org Foundation board: bo...@foundation.x.org > > Cheers, > > Sam > ___ > mesa-dev mailing list > mesa-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
Hi Am 17.09.20 um 14:32 schrieb Christian König: > Am 17.09.20 um 14:29 schrieb Thomas Zimmermann: >> Hi Christian >> >> Am 17.09.20 um 13:12 schrieb Christian König: >>> Hi Thomas, >>> >>> Am 17.09.20 um 12:51 schrieb Thomas Zimmermann: Hi Am 24.06.20 um 20:26 schrieb Nirmoy Das: > Calculate GEM VRAM bo's offset within vram-helper without depending on > bo->offset. > > Signed-off-by: Nirmoy Das > Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c > b/drivers/gpu/drm/drm_gem_vram_helper.c > index 0023ce1d2cf7..ad096600d89f 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct > drm_gem_vram_object *gbo) > } > EXPORT_SYMBOL(drm_gem_vram_mmap_offset); > +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo) > +{ > + /* Keep TTM behavior for now, remove when drivers are audited */ > + if (WARN_ON_ONCE(!gbo->bo.mem.mm_node)) At this line I got >>> Sounds like ast forgot to pin the cursor to VRAM. >>> >>> If you look at ast_cursor_page_flip(), you see: off = drm_gem_vram_offset(gbo); if (drm_WARN_ON_ONCE(dev, off < 0)) return; /* Bug: we didn't pin the cursor HW BO to VRAM. */ >>> The drm_WARN_ON_ONCE() just never triggered before because it checks for >>> the wrong condition (off < 0). >> GEM VRAM BOs have a pin counter. drm_gem_vram_offset() returns -ENODEV >> if the BO's pin count is 0 (i.e., the BO has not been pinned anywhere). >> That's what we're testing here. Two cursor BOs are permanently pinned to >> the top of VRAM memory by ast_cursor_init(). If perma-pinning in >> ast_cursor_init() fails, driver initialization should fail entirely. >> >> These cursor BOs do some sort of double buffering, On successful >> initialization, the actual cursor image is later blit-ed into one of the >> BOs. >> >> All this used to work from what I can tell. Is there any chance that the >> recent TTM refactoring broke this? > > Yeah, certainly possible. If you have time please bisect. Found it. https://patchwork.freedesktop.org/patch/391418/ Best regards Thomas > > Thanks, > Christian. > >> >> Best regards >> Thomas >> >>> Regards, >>> Christian. >>> [ 146.133821] [ cut here ] [ 146.133872] WARNING: CPU: 6 PID: 7 at drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60 [drm_vram_helper] [ 146.133880] Modules linked in: fuse(E) af_packet(E) ebtable_filter(E) ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E) intel_powerclamp(E) coretemp(E) kv) [ 146.134051] scsi_dh_emc(E) scsi_dh_alua(E) [ 146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted: G E 5.9.0-rc4-1-default+ #492 [ 146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN FIRE X2270 M2, BIOS 2.05 07/01/2010 [ 146.134099] Workqueue: events_unbound commit_work [ 146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60 [drm_vram_helper] [ 146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17 d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49 [ 146.134137] RSP: 0018:c9107c38 EFLAGS: 00010246 [ 146.134149] RAX: RBX: 88855000 RCX: c032323b [ 146.134158] RDX: dc00 RSI: 88810e236300 RDI: 88855278 [ 146.134168] RBP: 88810909 R08: c0323225 R09: 0002 [ 146.134177] R10: ed1020675020 R11: 0001 R12: 888109091050 [ 146.134187] R13: 88810e236300 R14: 88810909 R15: [ 146.134197] FS: () GS:88811600() knlGS: [ 146.134206] CS: 0010 DS: ES: CR0: 80050033 [ 146.134215] CR2: 7f60547d9100 CR3: 29216002 CR4: 000206e0 [ 146.134223] Call Trace: [ 146.134245] ast_cursor_page_flip+0x3e/0x150 [ast] [ 146.134272] ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast] [ 146.134300] drm_atomic_helper_commit_planes+0x197/0x4c0 [ 146.134341] drm_atomic_helper_commit_tail_rpm+0x51/0x90 [ 146.134357] commit_tail+0x103/0x1c0 [ 146.134390] process_one_work+0x56a/0xa60 [ 146.134431] ? __lock_acquired+0x1ca/0x3d0 [ 146.134447] ? pwq_dec_nr_in_flight+0x110/0x110 [ 146.134460] ? __lock_contended+0x4a0/0x4a0 [
[PATCH] drm/vram-helper: Fix use of top-down placement
Commit 7053e0eab473 ("drm/vram-helper: stop using TTM placement flags") cleared the BO placement flags if top-down placement had been selected. Hence, BOs that were supposed to go into VRAM are now placed in a default location in system memory. Trying to scanout the incorrectly pinned BO results in displayed garbage and an error message. [ 146.108127] [ cut here ] [ 146.1V08180] WARNING: CPU: 0 PID: 152 at drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60 [drm_vram_helper] ... [ 146.108591] ast_cursor_page_flip+0x3e/0x150 [ast] [ 146.108622] ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast] [ 146.108654] drm_atomic_helper_commit_planes+0x197/0x4c0 [ 146.108699] drm_atomic_helper_commit_tail_rpm+0x59/0xa0 [ 146.108718] commit_tail+0x103/0x1c0 ... [ 146.109302] ---[ end trace d901a1ba1d949036 ]--- Fix the bug by keeping the placement flags. The top-down placement flag is stored in a separate variable. Signed-off-by: Thomas Zimmermann Fixes: 7053e0eab473 ("drm/vram-helper: stop using TTM placement flags") Cc: Christian König Cc: Dave Airlie Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 50cad0e4a92e..375c79e23ca5 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -140,7 +140,7 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo, unsigned int c = 0; if (pl_flag & DRM_GEM_VRAM_PL_FLAG_TOPDOWN) - pl_flag = TTM_PL_FLAG_TOPDOWN; + invariant_flags = TTM_PL_FLAG_TOPDOWN; gbo->placement.placement = gbo->placements; gbo->placement.busy_placement = gbo->placements; -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/panfrost: simplify the return expression of cz_ih_hw_init()
On Mon, 2020-09-21 at 21:10 +0800, Qinglang Miao wrote: > Simplify the return expression. > > Signed-off-by: Qinglang Miao > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 8 +--- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index e68967338..ea8d31863 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -18,19 +18,13 @@ > > static int panfrost_reset_init(struct panfrost_device *pfdev) > { > - int err; > - > pfdev->rstc = devm_reset_control_array_get(pfdev->dev, false, true); > if (IS_ERR(pfdev->rstc)) { > dev_err(pfdev->dev, "get reset failed %ld\n", > PTR_ERR(pfdev->rstc)); > return PTR_ERR(pfdev->rstc); > } > > - err = reset_control_deassert(pfdev->rstc); > - if (err) > - return err; > - > - return 0; > + return reset_control_deassert(pfdev->rstc); > } > > static void panfrost_reset_fini(struct panfrost_device *pfdev) Reviewed-by: Philipp Zabel regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Re-send][PATCH] gpu/ipu-v3:reduce protected code area in ipu idmac get/put
Hi Bernard, On Mon, 2020-09-21 at 19:11 +0800, Bernard wrote: > This change will speed-up a bit these ipu_idmac_get & > ipu_idmac_put processing and there is no need to protect > kzalloc & kfree. I don't think that will be measurable, the channel lock is very unlikely to be contended. It might make more sense to replace the list walk with a bitfield. > Signed-off-by: Bernard Zhao > --- > drivers/gpu/ipu-v3/ipu-common.c | 24 +--- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c > index b3dae9ec1a38..8b3a57864c2e 100644 > --- a/drivers/gpu/ipu-v3/ipu-common.c > +++ b/drivers/gpu/ipu-v3/ipu-common.c > @@ -267,29 +267,30 @@ EXPORT_SYMBOL_GPL(ipu_rot_mode_to_degrees); > struct ipuv3_channel *ipu_idmac_get(struct ipu_soc *ipu, unsigned num) > { > struct ipuv3_channel *channel; > + struct ipuv3_channel *entry; > > dev_dbg(ipu->dev, "%s %d\n", __func__, num); > > if (num > 63) > return ERR_PTR(-ENODEV); > > + channel = kzalloc(sizeof(*channel), GFP_KERNEL); > + if (!channel) > + return ERR_PTR(-ENOMEM); > + > + channel->num = num; > + channel->ipu = ipu; > + > mutex_lock(>channel_lock); > > - list_for_each_entry(channel, >channels, list) { > - if (channel->num == num) { > + list_for_each_entry(entry, >channels, list) { > + if (entry->num == num) { > + kfree(channel); > channel = ERR_PTR(-EBUSY); > goto out; This leaks the channel memory allocated above. > } > } > > - channel = kzalloc(sizeof(*channel), GFP_KERNEL); > - if (!channel) { > - channel = ERR_PTR(-ENOMEM); > - goto out; > - } > - > - channel->num = num; > - channel->ipu = ipu; > list_add(>list, >channels); > > out: > @@ -308,9 +309,10 @@ void ipu_idmac_put(struct ipuv3_channel *channel) > mutex_lock(>channel_lock); > > list_del(>list); > - kfree(channel); > > mutex_unlock(>channel_lock); > + > + kfree(channel); > } > EXPORT_SYMBOL_GPL(ipu_idmac_put); regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()
On Mon, Sep 21, 2020 at 04:12:20PM +0300, Mikko Perttunen wrote: > On 9/21/20 4:10 PM, Qinglang Miao wrote: > > Simplify the return expression. > > > > Signed-off-by: Qinglang Miao > > --- > > drivers/gpu/host1x/cdma.c | 8 +--- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c > > index e8d3fda91..08a0f9e10 100644 > > --- a/drivers/gpu/host1x/cdma.c > > +++ b/drivers/gpu/host1x/cdma.c > > @@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma > > *cdma, > >*/ > > int host1x_cdma_init(struct host1x_cdma *cdma) > > { > > - int err; > > - > > mutex_init(>lock); > > init_completion(>complete); > > @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma) > > cdma->running = false; > > cdma->torndown = false; > > - err = host1x_pushbuffer_init(>push_buffer); > > - if (err) > > - return err; > > - > > - return 0; > > + return host1x_pushbuffer_init(>push_buffer); > > IMHO, this makes it less readable since now it kind of looks like > host1x_pushbuffer_init is returning some meaningful value, instead of the > code just handling error values in a sequence. Agreed. Collapsing the error handling like this also makes adding code to the bottom of this function more complicated than necessary. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()
On 9/21/20 4:10 PM, Qinglang Miao wrote: Simplify the return expression. Signed-off-by: Qinglang Miao --- drivers/gpu/host1x/cdma.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c index e8d3fda91..08a0f9e10 100644 --- a/drivers/gpu/host1x/cdma.c +++ b/drivers/gpu/host1x/cdma.c @@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma *cdma, */ int host1x_cdma_init(struct host1x_cdma *cdma) { - int err; - mutex_init(>lock); init_completion(>complete); @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma) cdma->running = false; cdma->torndown = false; - err = host1x_pushbuffer_init(>push_buffer); - if (err) - return err; - - return 0; + return host1x_pushbuffer_init(>push_buffer); IMHO, this makes it less readable since now it kind of looks like host1x_pushbuffer_init is returning some meaningful value, instead of the code just handling error values in a sequence. Mikko } /* ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/13] dt-bindings: arm: fsl: Fix matching Purism Librem5 phones
On Fri, Sep 04, 2020 at 04:53:02PM +0200, Krzysztof Kozlowski wrote: > All Purism Librem5 phones have three compatibles so they need their own > entry to fix dbts_check warnings like: > > arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml: /: > compatible: ['purism,librem5r2', 'purism,librem5', 'fsl,imx8mq'] is not > valid under any of the given schemas > > arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml: /: > compatible: ['purism,librem5r2', 'purism,librem5', 'fsl,imx8mq'] is too > long > > Signed-off-by: Krzysztof Kozlowski Applied, thanks. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] Revert "drm/radeon: handle PCIe root ports with addressing limitations"
Hi [This is an automated email] This commit has been processed because it contains a "Fixes:" tag fixing commit: 33b3ad3788ab ("drm/radeon: handle PCIe root ports with addressing limitations"). The bot has tested the following trees: v5.8.10, v5.4.66. v5.8.10: Build OK! v5.4.66: Failed to apply! Possible dependencies: 8b53e1cb2728 ("drm/radeon: switch to gem vma offset manager") 9d6f4484e81c ("drm/ttm: turn ttm_bo_device.vma_manager into a pointer") NOTE: The patch will not be queued to stable trees until it is upstream. How should we proceed with this patch? -- Thanks Sasha ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT PULL] drm/tegra: Changes for v5.10-rc1
Hi Dave, The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5: Linux 5.9-rc1 (2020-08-16 13:04:57 -0700) are available in the Git repository at: ssh://git.freedesktop.org/git/tegra/linux.git tags/drm/tegra/for-5.10-rc1 for you to fetch changes up to d9f980ebcd01d90a2a76ea2383c925abf9223cab: drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge (2020-09-17 12:31:18 +0200) Thanks, Thierry drm/tegra: Changes for v5.10-rc1 This is a handful of patches that add bridge support for Tegra devices and fix a couple of minor issues. Dmitry Osipenko (4): drm/tegra: output: Don't leak OF node on error drm/tegra: output: Support DRM bridges drm/tegra: output: rgb: Support LVDS encoder bridge drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge Thierry Reding (1): drm/tegra: Properly reference count the DDC I2C adapter drivers/gpu/drm/tegra/drm.h| 2 + drivers/gpu/drm/tegra/output.c | 24 +++--- drivers/gpu/drm/tegra/rgb.c| 102 ++--- drivers/gpu/drm/tegra/sor.c| 7 ++- 4 files changed, 80 insertions(+), 55 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel