Re: [PATCH 2/2 v4] drm/panel: ws2401: Add driver for WideChips WS2401
On Thu, Jul 15, 2021 at 4:25 AM Doug Anderson wrote: > > - Alter the logic so that we assign the backlight handle to > > panel->backlight directly and save some code. > > Officially this is disallowed according to comments. ...and I quote: > > /** > * @backlight: > * > * Backlight device, used to turn on backlight after the call > * to enable(), and to turn off backlight before the call to > * disable(). > * backlight is set by drm_panel_of_backlight() or > * drm_panel_dp_aux_backlight() and drivers shall not assign it. > */ > > I do not personally know the motivation of not letting drivers assign > it, but with the words "shall not". Yikes! I think it's a documentation bug. I trust Sam more than the docs. I'll send a patch. Yours, Linus Walleij
[PATCH] drm/panel: Document internal backlight handling
Panels with internal backlight need to assign their backlight member
directly.
Reported-by: Doug Anderson
Signed-off-by: Linus Walleij
---
include/drm/drm_panel.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 33605c3f0eba..1e63fadf1368 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -144,8 +144,9 @@ struct drm_panel {
* Backlight device, used to turn on backlight after the call
* to enable(), and to turn off backlight before the call to
* disable().
-* backlight is set by drm_panel_of_backlight() and drivers
-* shall not assign it.
+* External backlight is assigned by drm_panel_of_backlight() while
+* panel-internal backlight is assigned directly to this member by the
+* panel driver.
*/
struct backlight_device *backlight;
--
2.31.1
Re: [PATCH v3 0/2] allow simple{fb, drm} drivers to be used on non-x86 EFI platforms
Hi
Am 13.07.21 um 18:59 schrieb Javier Martinez Canillas:
On 6/25/21 3:09 PM, Javier Martinez Canillas wrote:
The simplefb and simpledrm drivers match against a "simple-framebuffer"
device, but for aarch64 this is only registered when using Device Trees
and there's a node with a "simple-framebuffer" compatible string.
There is no code to register a "simple-framebuffer" platform device when
using EFI instead. In fact, the only platform device that's registered in
this case is an "efi-framebuffer", which means that the efifb driver is
the only driver supported to have an early console with EFI on aarch64.
The x86 architecture platform has a Generic System Framebuffers (sysfb)
support, that register a system frambuffer platform device. It either
registers a "simple-framebuffer" for the simple{fb,drm} drivers or legacy
VGA/EFI FB devices for the vgafb/efifb drivers.
The sysfb is generic enough to be reused by other architectures and can be
moved out of the arch/x86 directory to drivers/firmware, allowing the EFI
logic used by non-x86 architectures to be folded into sysfb as well.
Any more comments on this series? It would be nice for this to land so the
simpledrm driver could be used on aarch64 EFI systems as well.
The patches have already been acked by x86 and DRM folks.
Time to get this merged, I'd say. People are asking for these patches
already.
Best regards
Thomas
Best regards,
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH] drm/omapdrm: Remove outdated comment
ping for review
Am 06.07.21 um 09:31 schrieb Thomas Zimmermann:
The comment refers to drm_irq_install() et al, which are not used by
omapdrm. The functions are part of the DRM IRQ midlayer and shouldn't
be used any longer. Remove the comment.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/omapdrm/omap_irq.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c
b/drivers/gpu/drm/omapdrm/omap_irq.c
index bb6e3fc18204..4aca14dab927 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -253,13 +253,6 @@ static const u32 omap_underflow_irqs[] = {
[OMAP_DSS_VIDEO3] = DISPC_IRQ_VID3_FIFO_UNDERFLOW,
};
-/*
- * We need a special version, instead of just using drm_irq_install(),
- * because we need to register the irq via omapdss. Once omapdss and
- * omapdrm are merged together we can assign the dispc hwmod data to
- * ourselves and drop these and just use drm_irq_{install,uninstall}()
- */
-
int omap_drm_irq_install(struct drm_device *dev)
{
struct omap_drm_private *priv = dev->dev_private;
--
2.32.0
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
[PULL] drm-misc-fixes
Hi Dave and Daniel, here's the PR for drm-misc-fixes. I merged drm-misc-next-fixes into it to pick up patches that were left over from the previous release cycle. The vmwgfx change comes from that. The other patches fix current errors. Best regards Thomas drm-misc-fixes-2021-07-15: Short summary of fixes pull (less than what git shortlog provides): * fbdev: Avoid use-after-free by not deleting current video mode * ttm: Avoid NULL-ptr deref in ttm_range_man_fini() * vmwgfx: Fix a merge commit The following changes since commit 1e7b5812f4890ad84058bbb6c4a5deddfb2c5b25: Merge tag 'drm-misc-fixes-2021-07-13' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2021-07-13 15:15:17 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2021-07-15 for you to fetch changes up to 9e5c772954406829e928dbe59891d08938ead04b: drm/ttm: add a check against null pointer dereference (2021-07-14 17:16:16 +0200) Short summary of fixes pull (less than what git shortlog provides): * fbdev: Avoid use-after-free by not deleting current video mode * ttm: Avoid NULL-ptr deref in ttm_range_man_fini() * vmwgfx: Fix a merge commit Christian König (1): drm/qxl: add NULL check for bo->resource Thomas Zimmermann (1): Merge remote-tracking branch 'drm-misc/drm-misc-next-fixes' into drm-misc-fixes Zack Rusin (2): drm/vmwgfx: Fix implicit declaration error drm/vmwgfx: Fix a bad merge in otable batch takedown Zhen Lei (1): fbmem: Do not delete the mode that is still in use Zheyu Ma (1): drm/ttm: add a check against null pointer dereference drivers/gpu/drm/qxl/qxl_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_range_manager.c | 3 +++ drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 1 - drivers/video/fbdev/core/fbmem.c| 12 +--- 5 files changed, 10 insertions(+), 9 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Re: [PATCH v6 06/11] drm/mediatek: Add pm runtime support for ovl and rdma
On Wed, 2021-07-14 at 10:44 +0200, Dafna Hirschfeld wrote:
>
> On 14.07.21 04:56, Yong Wu wrote:
> > From: Yongqiang Niu
> >
> > Prepare for smi cleaning up "mediatek,larb".
> >
> > Display use the dispsys device to call pm_rumtime_get_sync before.
> > This patch add pm_runtime_xx with ovl and rdma device whose nodes has
> > "iommus" property, then display could help pm_runtime_get for smi via
> > ovl or rdma device.
> >
> > CC: CK Hu
> > Signed-off-by: Yongqiang Niu
> > Signed-off-by: Yong Wu
> > (Yong: Use pm_runtime_resume_and_get instead of pm_runtime_get_sync)
> > Acked-by: Chun-Kuang Hu
> > ---
> > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 9 -
> > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 -
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 12 +++-
> > 3 files changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index fa9d79963cd3..ea5760f856ec 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -11,6 +11,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include "mtk_disp_drv.h"
> > @@ -414,15 +415,21 @@ static int mtk_disp_ovl_probe(struct platform_device
> > *pdev)
> > return ret;
> > }
> >
> > + pm_runtime_enable(dev);
> > +
> > ret = component_add(dev, &mtk_disp_ovl_component_ops);
> > - if (ret)
> > + if (ret) {
> > + pm_runtime_disable(dev);
> > dev_err(dev, "Failed to add component: %d\n", ret);
> > + }
> >
> > return ret;
> > }
> >
> > static int mtk_disp_ovl_remove(struct platform_device *pdev)
> > {
> > + pm_runtime_disable(&pdev->dev);
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > index 705f28ceb4dd..0f31d1c8e37c 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > @@ -9,6 +9,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include "mtk_disp_drv.h"
> > @@ -327,9 +328,13 @@ static int mtk_disp_rdma_probe(struct platform_device
> > *pdev)
> >
> > platform_set_drvdata(pdev, priv);
> >
> > + pm_runtime_enable(dev);
> > +
> > ret = component_add(dev, &mtk_disp_rdma_component_ops);
> > - if (ret)
> > + if (ret) {
> > + pm_runtime_disable(dev);
> > dev_err(dev, "Failed to add component: %d\n", ret);
> > + }
> >
> > return ret;
> > }
> > @@ -338,6 +343,8 @@ static int mtk_disp_rdma_remove(struct platform_device
> > *pdev)
> > {
> > component_del(&pdev->dev, &mtk_disp_rdma_component_ops);
> >
> > + pm_runtime_disable(&pdev->dev);
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 474efb844249..08e3f352377d 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -557,9 +557,15 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc
> > *crtc,
> > return;
> > }
> >
> > + ret = pm_runtime_resume_and_get(comp->dev);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(comp->dev, "Failed to enable power domain: %d\n",
> > + ret);
>
> shouldn't the code return in case of failure here?
After confirm with yongqiang, We will fix this in next version.
Thanks.
>
> Thanks,
> Dafna
>
> > +
> > ret = mtk_crtc_ddp_hw_init(mtk_crtc);
> > if (ret) {
> > mtk_smi_larb_put(comp->larb_dev);
> > + pm_runtime_put(comp->dev);
> > return;
> > }
> >
> > @@ -572,7 +578,7 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc
> > *crtc,
> > {
> > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> > - int i;
> > + int i, ret;
> >
> > DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id);
> > if (!mtk_crtc->enabled)
> > @@ -596,6 +602,10 @@ static void mtk_drm_crtc_atomic_disable(struct
> > drm_crtc *crtc,
> > drm_crtc_vblank_off(crtc);
> > mtk_crtc_ddp_hw_fini(mtk_crtc);
> > mtk_smi_larb_put(comp->larb_dev);
> > + ret = pm_runtime_put(comp->dev);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(comp->dev, "Failed to disable power domain: %d\n",
> > + ret);
> >
> > mtk_crtc->enabled = false;
> > }
> >
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH v6 00/11] Clean up "mediatek,larb"
On Wed, 2021-07-14 at 10:56 +0200, Dafna Hirschfeld wrote: > Hi > > Thanks for the patchset. > > I tested it on mt8173 (elm) with chromeos userspace. > Before that patchset, the test: > > tast -verbose run -build=false 10.42.0.175 video.DecodeAccel.h264 > > sometimes passed and sometimes failed with 'context deadline exceeded'. > With this patchset it seems that the test always passes so I added tested-by: > > Tested-by: Dafna Hirschfeld Thanks very much for your quick review and testing:) > > Thanks, > Dafna > > > > > On 14.07.21 04:56, Yong Wu wrote: > > MediaTek IOMMU block diagram always like below: > > > > M4U > > | > > smi-common > > | > >- > >| | ... > >| | > > larb1 larb2 > >| | > > vdec venc > > > > All the consumer connect with smi-larb, then connect with smi-common. > > > > When the consumer works, it should enable the smi-larb's power which also > > need enable the smi-common's power firstly. > > > > Thus, Firstly, use the device link connect the consumer and the > > smi-larbs. then add device link between the smi-larb and smi-common. > > > > After adding the device_link, then "mediatek,larb" property can be removed. > > the iommu consumer don't need call the mtk_smi_larb_get/put to enable > > the power and clock of smi-larb and smi-common. > > > > About the MM dt-binding/dtsi patches, I guess they should go together, thus > > I don't split them for each a MM module and each a SoC. > > > > Base on v5.14-rc1, and a jpeg[1] and mdp[2] patchset. > > > > [1] > > https://lore.kernel.org/linux-mediatek/20210702102304.3346429-1-hsi...@chromium.org/ > > [2] > > https://lore.kernel.org/linux-mediatek/20210709022324.1607884-1-ei...@chromium.org/ > > > > Change notes: > > v6: 1) rebase on v5.14-rc1. > > 2) Fix the issue commented in v5 from Dafna and Hsin-Yi. > > 3) Remove the patches about using pm_runtime_resume_and_get since they > > have > > already been merged by other patches. > > > > v5: > > https://lore.kernel.org/linux-mediatek/20210410091128.31823-1-yong...@mediatek.com/ > > 1) Base v5.12-rc2. > > 2) Remove changing the mtk-iommu to module_platform_driver patch, It > > have already been a > > independent patch. > > > > v4: > > https://lore.kernel.org/linux-mediatek/1590826218-23653-1-git-send-email-yong...@mediatek.com/ > > base on v5.7-rc1. > >1) Move drm PM patch before smi patchs. > >2) Change builtin_platform_driver to module_platform_driver since we may > > need > > build as module. > >3) Rebase many patchset as above. > > > > v3: > > https://lore.kernel.org/linux-iommu/1567503456-24725-1-git-send-email-yong...@mediatek.com/ > > 1) rebase on v5.3-rc1 and the latest mt8183 patchset. > > 2) Use device_is_bound to check whether the driver is ready from > > Matthias. > > 3) Add DL_FLAG_STATELESS flag when calling device_link_add and explain > > the > > reason in the commit message[3/14]. > > 4) Add a display patch[12/14] into this series. otherwise it may affect > > display HW fastlogo even though it don't happen in mt8183. > > > > v2: > > https://lore.kernel.org/linux-iommu/1560171313-28299-1-git-send-email-yong...@mediatek.com/ > > 1) rebase on v5.2-rc1. > > 2) Move adding device_link between the consumer and smi-larb into > > iommu_add_device from Robin. > > 3) add DL_FLAG_AUTOREMOVE_CONSUMER even though the smi is built-in from > > Evan. > > 4) Remove the shutdown callback in iommu. > > > > v1: > > https://lore.kernel.org/linux-iommu/1546318276-18993-1-git-send-email-yong...@mediatek.com/ > > > > Yong Wu (10): > >dt-binding: mediatek: Get rid of mediatek,larb for multimedia HW > >iommu/mediatek: Add probe_defer for smi-larb > >iommu/mediatek: Add device_link between the consumer and the larb > > devices > >media: mtk-jpeg: Get rid of mtk_smi_larb_get/put > >media: mtk-mdp: Get rid of mtk_smi_larb_get/put > >drm/mediatek: Get rid of mtk_smi_larb_get/put > >media: mtk-vcodec: Get rid of mtk_smi_larb_get/put > >memory: mtk-smi: Get rid of mtk_smi_larb_get/put > >arm: dts: mediatek: Get rid of mediatek,larb for MM nodes > >arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes > > > > Yongqiang Niu (1): > >drm/mediatek: Add pm runtime support for ovl and rdma > > > > .../display/mediatek/mediatek,disp.txt| 9 > > .../bindings/media/mediatek-jpeg-decoder.yaml | 9 > > .../bindings/media/mediatek-jpeg-encoder.yaml | 9 > > .../bindings/media/mediatek-mdp.txt | 8 > > .../bindings/media/mediatek-vcodec.txt| 4 -- > > arch/arm/boot/dts/mt2701.dtsi | 2 - > > arch/arm/boot/dts/mt7623n.dtsi| 5 -- > > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 --- > > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 6
Re: [PATCH 02/13] vfio: Introduce a vfio_uninit_group_dev() API call
On Wed, Jul 14, 2021 at 09:20:31PM -0300, Jason Gunthorpe wrote: > From: Max Gurtovoy > > This pairs with vfio_init_group_dev() and allows undoing any state that is > stored in the vfio_device unrelated to registration. Add appropriately > placed calls to all the drivers. > > The following patch will use this to add pre-registration state for the > device set. > > Signed-off-by: Max Gurtovoy > Signed-off-by: Jason Gunthorpe > --- > Documentation/driver-api/vfio.rst| 4 ++- > drivers/vfio/fsl-mc/vfio_fsl_mc.c| 6 +++-- > drivers/vfio/mdev/vfio_mdev.c| 13 +++--- > drivers/vfio/pci/vfio_pci.c | 6 +++-- > drivers/vfio/platform/vfio_platform_common.c | 7 +++-- > drivers/vfio/vfio.c | 5 > include/linux/vfio.h | 1 + > samples/vfio-mdev/mbochs.c | 2 ++ > samples/vfio-mdev/mdpy.c | 25 ++ > samples/vfio-mdev/mtty.c | 27 > 10 files changed, 64 insertions(+), 32 deletions(-) <...> > @@ -674,6 +675,7 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device > *mc_dev) > > dprc_remove_devices(mc_dev, NULL, 0); > vfio_fsl_uninit_device(vdev); > + vfio_uninit_group_dev(&vdev->vdev); This is wrong place, the _uninit_ should be after vfio_fsl_mc_reflck_put(). > vfio_fsl_mc_reflck_put(vdev->reflck); > > kfree(vdev);
Re: Aw: [PATCH v6 00/11] Clean up "mediatek,larb"
On Wed, 2021-07-14 at 10:59 +0200, Frank Wunderlich wrote: > Hi, > > sorry this (or the 2 depency-series) cause a NULL Pointer deref in > iommu_group_remove_device on mt7623/bpi-r2 Hi Frank, Thanks for your report. mt7623 use mtk_iommu_v1.c. I will try to reproduce this locally. > > i wonder why on bootup a cleanup is run, but have no hint about this. > > since "dts: mtk-mdp: remove mediatek, vpu property from primary MDP device" > all is good, i guess problem comes up while removing larb with DT > > this is backtrace > > [6.274465] PC is at iommu_group_remove_device+0x28/0x148 > [6.279877] LR is at iommu_release_device+0x4c/0x70 > > [6.674347] Backtrace: > [6.676797] [] (iommu_group_remove_device) from [] > (iomm) > [6.686221] r7: r6:c06bf04c r5:c0d7a1ac r4:c21fc010 > [6.691883] [] (iommu_release_device) from [] > (remove_io) > [6.700689] r5: r4: > [6.704265] [] (remove_iommu_group) from [] > (bus_for_eac) > [6.712725] [] (bus_for_each_dev) from [] > (bus_set_iommu) > [6.720753] r6:c331f440 r5:c1406f58 r4:ffea > [6.725370] [] (bus_set_iommu) from [] > (mtk_iommu_probe+) > [6.733484] r7:c32db0b8 r6:c21f9c00 r5:c331f1c0 r4: > [6.739145] [] (mtk_iommu_probe) from [] > (platform_probe) > [6.747176] r10:c21f9c10 r9:c2496f54 r8:c14623b8 r7:c14623b8 r6:c1405b90 > r50 > [6.755012] r4: > [6.757544] [] (platform_probe) from [] > (really_probe.pa) > [6.766006] r7:c14623b8 r6:c1405b90 r5: r4:c21f9c10 > [6.771667] [] (really_probe.part.0) from [] > (really_pro) > [6.779866] r7:c21f9c10 r6:c2549e74 r5:c1405b90 r4:c21f9c10 > [6.785527] [] (really_probe) from [] > (__driver_probe_de) > [6.793984] r5:c1405b90 r4:c21f9c10 > [6.797560] [] (__driver_probe_device) from [] > (driver_p) > [6.806543] r9:c2496f54 r8:0008 r7:c21f9c10 r6:c2549e74 r5:c14c6ec8 > r4:4 > [6.814291] [] (driver_probe_device) from [] > (__device_a) > [6.823448] r9:c2496f54 r8: r7:c21f9c10 r6:c2549e74 r5:c1405b90 > r4:1 > [6.831196] [] (__device_attach_driver) from [] > (bus_for) > [6.840007] r7:c14623b8 r6:c073635c r5:c2549e74 r4: > [6.845669] [] (bus_for_each_drv) from [] > (__device_atta) > [6.854044] r6:0001 r5:c21f9c54 r4:c21f9c10 > [6.858662] [] (__device_attach) from [] > (device_initial) > [6.867207] r6:c21f9c10 r5:c1406f58 r4:c1406ca0 > [6.871825] [] (device_initial_probe) from [] > (bus_probe) > [6.880454] [] (bus_probe_device) from [] > (deferred_prob) > > > bisect shows this commit as breaking: > > Author: Yong Wu > Date: Wed Jul 14 10:56:17 2021 +0800 > > iommu/mediatek: Add probe_defer for smi-larb > > Prepare for adding device_link. > > regards Frank
[PATCH] drivers/gpu/drm/i915/display: remove boilerplate code using LOCK_ALL macros
Replaced the boilerplate code for intel_display.c with
DRM_MODESET_LOCK_ALL macros per the TODO in gpu
(I apologize if I did something wrong, this is my first contribution of
hopefully many, so excited!!!)
Signed-off-by: Marco Kurzynski
---
drivers/gpu/drm/i915/display/intel_display.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 3bad4e00f7be..4a46d63b4d39 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13307,22 +13307,12 @@ void intel_display_resume(struct drm_device *dev)
if (state)
state->acquire_ctx = &ctx;
- drm_modeset_acquire_init(&ctx, 0);
-
- while (1) {
- ret = drm_modeset_lock_all_ctx(dev, &ctx);
- if (ret != -EDEADLK)
- break;
-
- drm_modeset_backoff(&ctx);
- }
-
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
if (!ret)
ret = __intel_display_resume(dev, state, &ctx);
intel_enable_ipc(dev_priv);
- drm_modeset_drop_locks(&ctx);
- drm_modeset_acquire_fini(&ctx);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
if (ret)
drm_err(&dev_priv->drm,
--
2.25.1
[PATCH] drm/msm: mdp4: drop vblank get/put from prepare/complete_commit
msm_atomic is doing vblank get/put's already,
currently there no need to duplicate the effort in MDP4
Fix warning:
...
WARNING: CPU: 3 PID: 79 at drivers/gpu/drm/drm_vblank.c:1194
drm_vblank_put+0x1cc/0x1d4
...
and multiple vblank time-outs:
...
msm 510.mdp: vblank time out, crtc=1
...
Tested on Nexus 7 2013 (deb), LTS 5.10.50.
Introduced by: 119ecb7fd3b5 ("drm/msm/mdp4: request vblank during modeset")
Signed-off-by: David Heidelberg
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index a129c457c372..e5d77cef64dc 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -91,13 +91,6 @@ static void mdp4_disable_commit(struct msm_kms *kms)
static void mdp4_prepare_commit(struct msm_kms *kms, struct drm_atomic_state
*state)
{
- int i;
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
-
- /* see 119ecb7fd */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i)
- drm_crtc_vblank_get(crtc);
}
static void mdp4_flush_commit(struct msm_kms *kms, unsigned crtc_mask)
@@ -116,12 +109,6 @@ static void mdp4_wait_flush(struct msm_kms *kms, unsigned
crtc_mask)
static void mdp4_complete_commit(struct msm_kms *kms, unsigned crtc_mask)
{
- struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
- struct drm_crtc *crtc;
-
- /* see 119ecb7fd */
- for_each_crtc_mask(mdp4_kms->dev, crtc, crtc_mask)
- drm_crtc_vblank_put(crtc);
}
static long mdp4_round_pixclk(struct msm_kms *kms, unsigned long rate,
--
2.30.2
[PATCH] drm/ttm: add a check against null pointer dereference
When calling ttm_range_man_fini(), 'man' may be uninitialized, which may cause a null pointer dereference bug. Fix this by checking if it is a null pointer. This log reveals it: [7.902580 ] BUG: kernel NULL pointer dereference, address: 0058 [7.905721 ] RIP: 0010:ttm_range_man_fini+0x40/0x160 [7.911826 ] Call Trace: [7.911826 ] radeon_ttm_fini+0x167/0x210 [7.911826 ] radeon_bo_fini+0x15/0x40 [7.913767 ] rs400_fini+0x55/0x80 [7.914358 ] radeon_device_fini+0x3c/0x140 [7.914358 ] radeon_driver_unload_kms+0x5c/0xe0 [7.914358 ] radeon_driver_load_kms+0x13a/0x200 [7.914358 ] ? radeon_driver_unload_kms+0xe0/0xe0 [7.914358 ] drm_dev_register+0x1db/0x290 [7.914358 ] radeon_pci_probe+0x16a/0x230 [7.914358 ] local_pci_probe+0x4a/0xb0 Signed-off-by: Zheyu Ma --- drivers/gpu/drm/ttm/ttm_range_manager.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c index 03395386e8a7..f4b08a8705b3 100644 --- a/drivers/gpu/drm/ttm/ttm_range_manager.c +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c @@ -181,6 +181,9 @@ int ttm_range_man_fini(struct ttm_device *bdev, struct drm_mm *mm = &rman->mm; int ret; + if (!man) + return 0; + ttm_resource_manager_set_used(man, false); ret = ttm_resource_manager_evict_all(bdev, man); -- 2.17.6
Re: [PATCH v6 03/11] iommu/mediatek: Add device_link between the consumer and the larb devices
On Wed, 2021-07-14 at 10:26 +0200, Dafna Hirschfeld wrote:
>
> On 14.07.21 04:56, Yong Wu wrote:
> > MediaTek IOMMU-SMI diagram is like below. all the consumer connect with
> > smi-larb, then connect with smi-common.
> >
> > M4U
> > |
> > smi-common
> > |
> >-
> >| |...
> >| |
> > larb1 larb2
> >| |
> > vdec venc
> >
> > When the consumer works, it should enable the smi-larb's power which
> > also need enable the smi-common's power firstly.
> >
> > Thus, First of all, use the device link connect the consumer and the
> > smi-larbs. then add device link between the smi-larb and smi-common.
> >
> > This patch adds device_link between the consumer and the larbs.
> >
> > When device_link_add, I add the flag DL_FLAG_STATELESS to avoid calling
> > pm_runtime_xx to keep the original status of clocks. It can avoid two
> > issues:
> > 1) Display HW show fastlogo abnormally reported in [1]. At the beggining,
> > all the clocks are enabled before entering kernel, but the clocks for
> > display HW(always in larb0) will be gated after clk_enable and clk_disable
> > called from device_link_add(->pm_runtime_resume) and rpm_idle. The clock
> > operation happened before display driver probe. At that time, the display
> > HW will be abnormal.
> >
> > 2) A deadlock issue reported in [2]. Use DL_FLAG_STATELESS to skip
> > pm_runtime_xx to avoid the deadlock.
> >
> > Corresponding, DL_FLAG_AUTOREMOVE_CONSUMER can't be added, then
> > device_link_removed should be added explicitly.
> >
> > [1]
> > https://lore.kernel.org/linux-mediatek/1564213888.22908.4.camel@mhfsdcap03/
> > [2] https://lore.kernel.org/patchwork/patch/1086569/
> >
> > Suggested-by: Tomasz Figa
> > Signed-off-by: Yong Wu
> > ---
> > drivers/iommu/mtk_iommu.c| 22 ++
> > drivers/iommu/mtk_iommu_v1.c | 20 +++-
> > 2 files changed, 41 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index a02dde094788..ee742900cf4b 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -571,22 +571,44 @@ static struct iommu_device
> > *mtk_iommu_probe_device(struct device *dev)
> > {
> > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > struct mtk_iommu_data *data;
> > + struct device_link *link;
> > + struct device *larbdev;
> > + unsigned int larbid;
> >
> > if (!fwspec || fwspec->ops != &mtk_iommu_ops)
> > return ERR_PTR(-ENODEV); /* Not a iommu client device */
> >
> > data = dev_iommu_priv_get(dev);
> >
> > + /*
> > +* Link the consumer device with the smi-larb device(supplier)
> > +* The device in each a larb is a independent HW. thus only link
> > +* one larb here.
> > +*/
> > + larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);
> > + larbdev = data->larb_imu[larbid].dev;
> > + link = device_link_add(dev, larbdev,
> > + DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
> > + if (!link)
> > + dev_err(dev, "Unable to link %s\n", dev_name(larbdev));
> shoudn't ERR_PTR be returned in case of failure?
In the previous design, this is not a fatal error. the consumer device
could probe continuously even though it fail here..Returning here may
let the issue be caught earlier, I will add this in next version.
if (!link) {
...
return ERR_PTR(EINVAL);
}
>
> Thanks,
> Dafna
>
> > return &data->iommu;
> > }
> >
> > static void mtk_iommu_release_device(struct device *dev)
> > {
> > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > + struct mtk_iommu_data *data;
> > + struct device *larbdev;
> > + unsigned int larbid;
> >
> > if (!fwspec || fwspec->ops != &mtk_iommu_ops)
> > return;
> >
> > + data = dev_iommu_priv_get(dev);
> > + larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);
> > + larbdev = data->larb_imu[larbid].dev;
> > + device_link_remove(dev, larbdev);
> > +
> > iommu_fwspec_free(dev);
> > }
> >
> > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> > index d9365a3d8dc9..d2a7c66b8239 100644
> > --- a/drivers/iommu/mtk_iommu_v1.c
> > +++ b/drivers/iommu/mtk_iommu_v1.c
> > @@ -424,7 +424,9 @@ static struct iommu_device
> > *mtk_iommu_probe_device(struct device *dev)
> > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > struct of_phandle_args iommu_spec;
> > struct mtk_iommu_data *data;
> > - int err, idx = 0;
> > + int err, idx = 0, larbid;
> > + struct device_link *link;
> > + struct device *larbdev;
> >
> > while (!of_parse_phandle_with_args(dev->of_node, "iommus",
> >"#iommu-cells",
> > @@ -445,6 +447,14 @@ static struct iommu_device
> > *mtk_iommu_probe_device(struct device *dev)
> >
> > data = dev_iommu_priv_get(dev);
> >
> > + /* Link the
Re: [PATCH] drm/omapdrm: Remove outdated comment
On Thu, Jul 15, 2021 at 10:13 AM Thomas Zimmermann wrote:
>
> ping for review
>
> Am 06.07.21 um 09:31 schrieb Thomas Zimmermann:
> > The comment refers to drm_irq_install() et al, which are not used by
> > omapdrm. The functions are part of the DRM IRQ midlayer and shouldn't
> > be used any longer. Remove the comment.
> >
> > Signed-off-by: Thomas Zimmermann
Acked-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/omapdrm/omap_irq.c | 7 ---
> > 1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c
> > b/drivers/gpu/drm/omapdrm/omap_irq.c
> > index bb6e3fc18204..4aca14dab927 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_irq.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_irq.c
> > @@ -253,13 +253,6 @@ static const u32 omap_underflow_irqs[] = {
> > [OMAP_DSS_VIDEO3] = DISPC_IRQ_VID3_FIFO_UNDERFLOW,
> > };
> >
> > -/*
> > - * We need a special version, instead of just using drm_irq_install(),
> > - * because we need to register the irq via omapdss. Once omapdss and
> > - * omapdrm are merged together we can assign the dispc hwmod data to
> > - * ourselves and drop these and just use drm_irq_{install,uninstall}()
> > - */
> > -
> > int omap_drm_irq_install(struct drm_device *dev)
> > {
> > struct omap_drm_private *priv = dev->dev_private;
> > --
> > 2.32.0
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 0/3] drm/vc4: hdmi: Interrupt fixes
On Wed, Jul 07, 2021 at 11:05:12AM +0100, Dave Stevenson wrote: > Hi Maxime > > On Wed, 7 Jul 2021 at 10:51, Maxime Ripard wrote: > > > > Hi, > > > > Those are three fixes for race conditions we currently have in the vc4 HDMI > > driver with regard to the interrupts handling. > > > > The first two are fixing an issue where the handler will be removed by devm > > after the resources it uses have been free'd already. > > > > The last one is there to deal with an interrupt coming in the window between > > the end of the driver's bind and the DRM device registration. > > > > Let me know what you think, > > Maxime > > For the series > Reviewed-by: Dave Stevenson Applied all three patches, thanks! Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/panel: Document internal backlight handling
On Thu, Jul 15, 2021 at 10:02 AM Linus Walleij wrote:
>
> Panels with internal backlight need to assign their backlight member
> directly.
>
> Reported-by: Doug Anderson
> Signed-off-by: Linus Walleij
> ---
> include/drm/drm_panel.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 33605c3f0eba..1e63fadf1368 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -144,8 +144,9 @@ struct drm_panel {
> * Backlight device, used to turn on backlight after the call
> * to enable(), and to turn off backlight before the call to
> * disable().
> -* backlight is set by drm_panel_of_backlight() and drivers
> -* shall not assign it.
> +* External backlight is assigned by drm_panel_of_backlight() while
> +* panel-internal backlight is assigned directly to this member by the
> +* panel driver.
External/internal feels a bit like imprecise wording. Maybe something like
drm_panel_of_backlight() automatically sets this, drivers which obtain
their backlight through some other means need to explicitly set this
themselves.
And then perhaps also update the kerneldoc for drm_panel_of_backlight?
Maybe in the future we'll have some similar helpers for acpi or the
backlight on dp aux or whatever might happen with hardware.
Either way: Acked-by: Daniel Vetter
Because updating and clarifying docs is always great!
-Daniel
> */
> struct backlight_device *backlight;
>
> --
> 2.31.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH] drm/vc4: hdmi: Remove drm_encoder->crtc usage
On Wed, Jul 07, 2021 at 05:35:53PM +0200, Thomas Zimmermann wrote: > > > Am 07.07.21 um 16:19 schrieb Maxime Ripard: > > The drm_encoder crtc pointer isn't really fit for an atomic driver, > > let's rely on the connector state instead. > > > > Signed-off-by: Maxime Ripard > > Acked-by: Thomas Zimmermann Applied, thanks Maxime signature.asc Description: PGP signature
Re: [PATCH v4 03/17] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property
On Wed, 14 Jul 2021 20:18:57 +0200 Werner Sembach wrote: > Am 01.07.21 um 13:30 schrieb Werner Sembach: > > Am 01.07.21 um 09:42 schrieb Pekka Paalanen: > >> On Wed, 30 Jun 2021 11:42:10 +0200 > >> Werner Sembach wrote: > >> > >>> Am 30.06.21 um 10:21 schrieb Pekka Paalanen: > On Tue, 29 Jun 2021 13:02:05 +0200 > Werner Sembach wrote: > > > Am 28.06.21 um 19:03 schrieb Werner Sembach: > >> Am 18.06.21 um 11:11 schrieb Werner Sembach: > >>> Add a new general drm property "active bpc" which can be used by > >>> graphic > >>> drivers to report the applied bit depth per pixel back to userspace. > >>> > >>> While "max bpc" can be used to change the color depth, there was no > >>> way to > >>> check which one actually got used. While in theory the driver chooses > >>> the > >>> best/highest color depth within the max bpc setting a user might not > >>> be > >>> fully aware what his hardware is or isn't capable off. This is meant > >>> as a > >>> quick way to double check the setup. > >>> > >>> In the future, automatic color calibration for screens might also > >>> depend on > >>> this information being available. > >>> > >>> Signed-off-by: Werner Sembach > >>> --- > >>>drivers/gpu/drm/drm_connector.c | 51 > >>> + > >>>include/drm/drm_connector.h | 8 ++ > >>>2 files changed, 59 insertions(+) > New idea: Instead of the "active"-properties with various if cases in > the kernel code, there could just be blob properties exposing the hdmi > infoframes, hdmi general control packages, dp misc0 and misc1 and dp vsc > sdp. > > Combined they have all the color information and it is made sure that > it's what is actually send to the monitor (I would consider sending > something differed then what is told in the infoframes a bug). > > They also have built in version numbers, if in the future they contain > more information. > > Only disadvantage: We leave parsing for human readable output to the > userspace. > > Alternatively keep the "active"-properties but fill them from the > infoframes. > > I'm not entirely sure where to do that on amd, because there the > infoframes are directly created in the dc code shortly before writing > them to the hardware registers and immediately forgotten afterwards. But > you still have access to the connector struct from that code so the > property could be updated directly there. > Hi, I'm not fundamentally against that as long as we have a common userspace library to parse those blobs. In libdrm perhaps? Or a new library? But I also don't know about the technical feasibility, is it a good idea. OTOH, that could be the best thing for testing drivers vs. KMS UAPI when you don't have a hardware HDMI/DP grabber to inspect the infoframes. So maybe kernel CI would like that? Thanks, pq pgppC7otcy0sk.pgp Description: OpenPGP digital signature
[RFC 0/8] Per client GPU stats
From: Tvrtko Ursulin Same old work but now rebased and series ending with some DRM docs proposing the common specification which should enable nice common userspace tools to be written. For the moment I only have intel_gpu_top converted to use this and that seems to work okay. v2: * Added prototype of possible amdgpu changes and spec updates to align with the common spec. Tvrtko Ursulin (8): drm/i915: Explicitly track DRM clients drm/i915: Make GEM contexts track DRM clients drm/i915: Track runtime spent in closed and unreachable GEM contexts drm/i915: Track all user contexts per client drm/i915: Track context current active time drm: Document fdinfo format specification drm/i915: Expose client engine utilisation via fdinfo drm/amdgpu: Convert to common fdinfo format Documentation/gpu/amdgpu.rst | 26 Documentation/gpu/drm-usage-stats.rst | 108 + Documentation/gpu/i915.rst| 27 Documentation/gpu/index.rst | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c| 18 ++- drivers/gpu/drm/i915/Makefile | 5 +- drivers/gpu/drm/i915/gem/i915_gem_context.c | 42 - .../gpu/drm/i915/gem/i915_gem_context_types.h | 6 + drivers/gpu/drm/i915/gt/intel_context.c | 27 +++- drivers/gpu/drm/i915/gt/intel_context.h | 15 +- drivers/gpu/drm/i915/gt/intel_context_types.h | 24 ++- .../drm/i915/gt/intel_execlists_submission.c | 23 ++- .../gpu/drm/i915/gt/intel_gt_clock_utils.c| 4 + drivers/gpu/drm/i915/gt/intel_lrc.c | 27 ++-- drivers/gpu/drm/i915/gt/intel_lrc.h | 24 +++ drivers/gpu/drm/i915/gt/selftest_lrc.c| 10 +- drivers/gpu/drm/i915/i915_drm_client.c| 143 ++ drivers/gpu/drm/i915/i915_drm_client.h| 66 drivers/gpu/drm/i915/i915_drv.c | 9 ++ drivers/gpu/drm/i915/i915_drv.h | 5 + drivers/gpu/drm/i915/i915_gem.c | 21 ++- drivers/gpu/drm/i915/i915_gpu_error.c | 9 +- drivers/gpu/drm/i915/i915_gpu_error.h | 2 +- 23 files changed, 581 insertions(+), 61 deletions(-) create mode 100644 Documentation/gpu/drm-usage-stats.rst create mode 100644 drivers/gpu/drm/i915/i915_drm_client.c create mode 100644 drivers/gpu/drm/i915/i915_drm_client.h -- 2.30.2
[RFC 1/8] drm/i915: Explicitly track DRM clients
From: Tvrtko Ursulin
Tracking DRM clients more explicitly will allow later patches to
accumulate past and current GPU usage in a centralised place and also
consolidate access to owning task pid/name.
Unique client id is also assigned for the purpose of distinguishing/
consolidating between multiple file descriptors owned by the same process.
v2:
Chris Wilson:
* Enclose new members into dedicated structs.
* Protect against failed sysfs registration.
v3:
* sysfs_attr_init.
v4:
* Fix for internal clients.
v5:
* Use cyclic ida for client id. (Chris)
* Do not leak pid reference. (Chris)
* Tidy code with some locals.
v6:
* Use xa_alloc_cyclic to simplify locking. (Chris)
* No need to unregister individial sysfs files. (Chris)
* Rebase on top of fpriv kref.
* Track client closed status and reflect in sysfs.
v7:
* Make drm_client more standalone concept.
v8:
* Simplify sysfs show. (Chris)
* Always track name and pid.
v9:
* Fix cyclic id assignment.
v10:
* No need for a mutex around xa_alloc_cyclic.
* Refactor sysfs into own function.
* Unregister sysfs before freeing pid and name.
* Move clients setup into own function.
v11:
* Call clients init directly from driver init. (Chris)
v12:
* Do not fail client add on id wrap. (Maciej)
v13 (Lucas): Rebase.
v14:
* Dropped sysfs bits.
v15:
* Dropped tracking of pid/ and name.
* Dropped RCU freeing of the client object.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Chris Wilson # v11
Reviewed-by: Aravind Iddamsetty # v11
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/Makefile | 5 +-
drivers/gpu/drm/i915/i915_drm_client.c | 68 ++
drivers/gpu/drm/i915/i915_drm_client.h | 50 +++
drivers/gpu/drm/i915/i915_drv.c| 6 +++
drivers/gpu/drm/i915/i915_drv.h| 5 ++
drivers/gpu/drm/i915/i915_gem.c| 21 ++--
6 files changed, 150 insertions(+), 5 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_drm_client.c
create mode 100644 drivers/gpu/drm/i915/i915_drm_client.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 10b3bb6207ba..784f99ca11fc 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -33,8 +33,9 @@ subdir-ccflags-y += -I$(srctree)/$(src)
# Please keep these build lists sorted!
# core driver code
-i915-y += i915_drv.o \
- i915_config.o \
+i915-y += i915_config.o \
+ i915_drm_client.o \
+ i915_drv.o \
i915_irq.o \
i915_getparam.o \
i915_mitigations.o \
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c
b/drivers/gpu/drm/i915/i915_drm_client.c
new file mode 100644
index ..e61e9ba15256
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include
+#include
+#include
+
+#include "i915_drm_client.h"
+#include "i915_gem.h"
+#include "i915_utils.h"
+
+void i915_drm_clients_init(struct i915_drm_clients *clients,
+ struct drm_i915_private *i915)
+{
+ clients->i915 = i915;
+ clients->next_id = 0;
+
+ xa_init_flags(&clients->xarray, XA_FLAGS_ALLOC | XA_FLAGS_LOCK_IRQ);
+}
+
+struct i915_drm_client *i915_drm_client_add(struct i915_drm_clients *clients)
+{
+ struct i915_drm_client *client;
+ struct xarray *xa = &clients->xarray;
+ int ret;
+
+ client = kzalloc(sizeof(*client), GFP_KERNEL);
+ if (!client)
+ return ERR_PTR(-ENOMEM);
+
+ xa_lock_irq(xa);
+ ret = __xa_alloc_cyclic(xa, &client->id, client, xa_limit_32b,
+ &clients->next_id, GFP_KERNEL);
+ xa_unlock_irq(xa);
+ if (ret < 0)
+ goto err;
+
+ kref_init(&client->kref);
+ client->clients = clients;
+
+ return client;
+
+err:
+ kfree(client);
+
+ return ERR_PTR(ret);
+}
+
+void __i915_drm_client_free(struct kref *kref)
+{
+ struct i915_drm_client *client =
+ container_of(kref, typeof(*client), kref);
+ struct xarray *xa = &client->clients->xarray;
+ unsigned long flags;
+
+ xa_lock_irqsave(xa, flags);
+ __xa_erase(xa, client->id);
+ xa_unlock_irqrestore(xa, flags);
+ kfree(client);
+}
+
+void i915_drm_clients_fini(struct i915_drm_clients *clients)
+{
+ GEM_BUG_ON(!xa_empty(&clients->xarray));
+ xa_destroy(&clients->xarray);
+}
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h
b/drivers/gpu/drm/i915/i915_drm_client.h
new file mode 100644
index ..e8986ad51176
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef __I915_DRM_CLIENT_H__
+#define __I915_DRM_CLIENT_H__
+
+#include
+#include
+
+struct drm_i915_private;
+
+struct i915_drm_clients {
+ struct drm_i915_private *i915;
+
[RFC 3/8] drm/i915: Track runtime spent in closed and unreachable GEM contexts
From: Tvrtko Ursulin
As contexts are abandoned we want to remember how much GPU time they used
(per class) so later we can used it for smarter purposes.
As GEM contexts are closed we want to have the DRM client remember how
much GPU time they used (per class) so later we can used it for smarter
purposes.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Aravind Iddamsetty
Reviewed-by: Chris Wilson
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 25 +++--
drivers/gpu/drm/i915/i915_drm_client.h | 7 ++
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 3bf409cf0214..4b93fcb11914 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -841,23 +841,44 @@ static void free_engines_rcu(struct rcu_head *rcu)
free_engines(engines);
}
+static void accumulate_runtime(struct i915_drm_client *client,
+ struct i915_gem_engines *engines)
+{
+ struct i915_gem_engines_iter it;
+ struct intel_context *ce;
+
+ if (!client)
+ return;
+
+ /* Transfer accumulated runtime to the parent GEM context. */
+ for_each_gem_engine(ce, engines, it) {
+ unsigned int class = ce->engine->uabi_class;
+
+ GEM_BUG_ON(class >= ARRAY_SIZE(client->past_runtime));
+ atomic64_add(intel_context_get_total_runtime_ns(ce),
+&client->past_runtime[class]);
+ }
+}
+
static int __i915_sw_fence_call
engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
{
struct i915_gem_engines *engines =
container_of(fence, typeof(*engines), fence);
+ struct i915_gem_context *ctx = engines->ctx;
switch (state) {
case FENCE_COMPLETE:
if (!list_empty(&engines->link)) {
- struct i915_gem_context *ctx = engines->ctx;
unsigned long flags;
spin_lock_irqsave(&ctx->stale.lock, flags);
list_del(&engines->link);
spin_unlock_irqrestore(&ctx->stale.lock, flags);
}
- i915_gem_context_put(engines->ctx);
+ accumulate_runtime(ctx->client, engines);
+ i915_gem_context_put(ctx);
+
break;
case FENCE_FREE:
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h
b/drivers/gpu/drm/i915/i915_drm_client.h
index e8986ad51176..9d80d9f715ee 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -9,6 +9,8 @@
#include
#include
+#include "gt/intel_engine_types.h"
+
struct drm_i915_private;
struct i915_drm_clients {
@@ -24,6 +26,11 @@ struct i915_drm_client {
unsigned int id;
struct i915_drm_clients *clients;
+
+ /**
+* @past_runtime: Accumulation of pphwsp runtimes from closed contexts.
+*/
+ atomic64_t past_runtime[MAX_ENGINE_CLASS + 1];
};
void i915_drm_clients_init(struct i915_drm_clients *clients,
--
2.30.2
[RFC 4/8] drm/i915: Track all user contexts per client
From: Tvrtko Ursulin
We soon want to start answering questions like how much GPU time is the
context belonging to a client which exited still using.
To enable this we start tracking all context belonging to a client on a
separate list.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Aravind Iddamsetty
Reviewed-by: Chris Wilson
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 12
drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 3 +++
drivers/gpu/drm/i915/i915_drm_client.c| 2 ++
drivers/gpu/drm/i915/i915_drm_client.h| 5 +
4 files changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 4b93fcb11914..9f32540f97bd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1215,6 +1215,7 @@ static void set_closed_name(struct i915_gem_context *ctx)
static void context_close(struct i915_gem_context *ctx)
{
+ struct i915_drm_client *client;
struct i915_address_space *vm;
/* Flush any concurrent set_engines() */
@@ -1247,6 +1248,13 @@ static void context_close(struct i915_gem_context *ctx)
list_del(&ctx->link);
spin_unlock(&ctx->i915->gem.contexts.lock);
+ client = ctx->client;
+ if (client) {
+ spin_lock(&client->ctx_lock);
+ list_del_rcu(&ctx->client_link);
+ spin_unlock(&client->ctx_lock);
+ }
+
mutex_unlock(&ctx->mutex);
/*
@@ -1469,6 +1477,10 @@ static void gem_context_register(struct i915_gem_context
*ctx,
old = xa_store(&fpriv->context_xa, id, ctx, GFP_KERNEL);
WARN_ON(old);
+ spin_lock(&ctx->client->ctx_lock);
+ list_add_tail_rcu(&ctx->client_link, &ctx->client->ctx_list);
+ spin_unlock(&ctx->client->ctx_lock);
+
spin_lock(&i915->gem.contexts.lock);
list_add_tail(&ctx->link, &i915->gem.contexts.list);
spin_unlock(&i915->gem.contexts.lock);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index e1bca913818e..4eab17591f3c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -280,6 +280,9 @@ struct i915_gem_context {
/** @client: struct i915_drm_client */
struct i915_drm_client *client;
+ /** link: &drm_client.context_list */
+ struct list_head client_link;
+
/**
* @ref: reference count
*
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c
b/drivers/gpu/drm/i915/i915_drm_client.c
index e61e9ba15256..91a8559bebf7 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -38,6 +38,8 @@ struct i915_drm_client *i915_drm_client_add(struct
i915_drm_clients *clients)
goto err;
kref_init(&client->kref);
+ spin_lock_init(&client->ctx_lock);
+ INIT_LIST_HEAD(&client->ctx_list);
client->clients = clients;
return client;
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h
b/drivers/gpu/drm/i915/i915_drm_client.h
index 9d80d9f715ee..7416e18aa33c 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -7,6 +7,8 @@
#define __I915_DRM_CLIENT_H__
#include
+#include
+#include
#include
#include "gt/intel_engine_types.h"
@@ -25,6 +27,9 @@ struct i915_drm_client {
unsigned int id;
+ spinlock_t ctx_lock; /* For add/remove from ctx_list. */
+ struct list_head ctx_list; /* List of contexts belonging to client. */
+
struct i915_drm_clients *clients;
/**
--
2.30.2
[RFC 2/8] drm/i915: Make GEM contexts track DRM clients
From: Tvrtko Ursulin
Make GEM contexts keep a reference to i915_drm_client for the whole of
of their lifetime which will come handy in following patches.
v2: Don't bother supporting selftests contexts from debugfs. (Chris)
v3 (Lucas): Finish constructing ctx before adding it to the list
v4 (Ram): Rebase.
v5: Trivial rebase for proto ctx changes.
v6: Rebase after clients no longer track name and pid.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Chris Wilson # v5
Reviewed-by: Aravind Iddamsetty # v5
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +
drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 3 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 7d6f52d8a801..3bf409cf0214 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -988,6 +988,9 @@ void i915_gem_context_release(struct kref *ref)
trace_i915_context_free(ctx);
GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
+ if (ctx->client)
+ i915_drm_client_put(ctx->client);
+
mutex_destroy(&ctx->engines_mutex);
mutex_destroy(&ctx->lut_mutex);
@@ -1436,6 +1439,8 @@ static void gem_context_register(struct i915_gem_context
*ctx,
ctx->file_priv = fpriv;
ctx->pid = get_task_pid(current, PIDTYPE_PID);
+ ctx->client = i915_drm_client_get(fpriv->client);
+
snprintf(ctx->name, sizeof(ctx->name), "%s[%d]",
current->comm, pid_nr(ctx->pid));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 94c03a97cb77..e1bca913818e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -277,6 +277,9 @@ struct i915_gem_context {
/** @link: place with &drm_i915_private.context_list */
struct list_head link;
+ /** @client: struct i915_drm_client */
+ struct i915_drm_client *client;
+
/**
* @ref: reference count
*
--
2.30.2
[RFC 5/8] drm/i915: Track context current active time
From: Tvrtko Ursulin
Track context active (on hardware) status together with the start
timestamp.
This will be used to provide better granularity of context
runtime reporting in conjunction with already tracked pphwsp accumulated
runtime.
The latter is only updated on context save so does not give us visibility
to any currently executing work.
As part of the patch the existing runtime tracking data is moved under the
new ce->stats member and updated under the seqlock. This provides the
ability to atomically read out accumulated plus active runtime.
v2:
* Rename and make __intel_context_get_active_time unlocked.
v3:
* Use GRAPHICS_VER.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Aravind Iddamsetty # v1
Reviewed-by: Chris Wilson
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gt/intel_context.c | 27 ++-
drivers/gpu/drm/i915/gt/intel_context.h | 15 ---
drivers/gpu/drm/i915/gt/intel_context_types.h | 24 +++--
.../drm/i915/gt/intel_execlists_submission.c | 23
.../gpu/drm/i915/gt/intel_gt_clock_utils.c| 4 +++
drivers/gpu/drm/i915/gt/intel_lrc.c | 27 ++-
drivers/gpu/drm/i915/gt/intel_lrc.h | 24 +
drivers/gpu/drm/i915/gt/selftest_lrc.c| 10 +++
drivers/gpu/drm/i915/i915_gpu_error.c | 9 +++
drivers/gpu/drm/i915/i915_gpu_error.h | 2 +-
10 files changed, 116 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index bd63813c8a80..06816690ffc7 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -374,7 +374,7 @@ intel_context_init(struct intel_context *ce, struct
intel_engine_cs *engine)
ce->ring = NULL;
ce->ring_size = SZ_4K;
- ewma_runtime_init(&ce->runtime.avg);
+ ewma_runtime_init(&ce->stats.runtime.avg);
ce->vm = i915_vm_get(engine->gt->vm);
@@ -500,6 +500,31 @@ struct i915_request *intel_context_create_request(struct
intel_context *ce)
return rq;
}
+u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
+{
+ u64 total, active;
+
+ total = ce->stats.runtime.total;
+ if (ce->ops->flags & COPS_RUNTIME_CYCLES)
+ total *= ce->engine->gt->clock_period_ns;
+
+ active = READ_ONCE(ce->stats.active);
+ if (active)
+ active = intel_context_clock() - active;
+
+ return total + active;
+}
+
+u64 intel_context_get_avg_runtime_ns(struct intel_context *ce)
+{
+ u64 avg = ewma_runtime_read(&ce->stats.runtime.avg);
+
+ if (ce->ops->flags & COPS_RUNTIME_CYCLES)
+ avg *= ce->engine->gt->clock_period_ns;
+
+ return avg;
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftest_context.c"
#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h
b/drivers/gpu/drm/i915/gt/intel_context.h
index b10cbe8fee99..093e2423e92b 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -245,18 +245,13 @@ intel_context_clear_nopreempt(struct intel_context *ce)
clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
}
-static inline u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
-{
- const u32 period = ce->engine->gt->clock_period_ns;
-
- return READ_ONCE(ce->runtime.total) * period;
-}
+u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
+u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
-static inline u64 intel_context_get_avg_runtime_ns(struct intel_context *ce)
+static inline u64 intel_context_clock(void)
{
- const u32 period = ce->engine->gt->clock_period_ns;
-
- return mul_u32_u32(ewma_runtime_read(&ce->runtime.avg), period);
+ /* As we mix CS cycles with CPU clocks, use the raw monotonic clock. */
+ return ktime_get_raw_fast_ns();
}
#endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 90026c177105..9c68fda36c40 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -33,6 +33,9 @@ struct intel_context_ops {
#define COPS_HAS_INFLIGHT_BIT 0
#define COPS_HAS_INFLIGHT BIT(COPS_HAS_INFLIGHT_BIT)
+#define COPS_RUNTIME_CYCLES_BIT 1
+#define COPS_RUNTIME_CYCLES BIT(COPS_RUNTIME_CYCLES_BIT)
+
int (*alloc)(struct intel_context *ce);
int (*pre_pin)(struct intel_context *ce, struct i915_gem_ww_ctx *ww,
void **vaddr);
@@ -111,14 +114,19 @@ struct intel_context {
} lrc;
u32 tag; /* cookie passed to HW to track this context on submission */
- /* Time on GPU as tracked by the hw. */
- struct {
- struct ewma_runtime avg;
- u64 total;
- u32 last;
- I915_SELFTEST_DECLARE(u32 num_underflo
[RFC 8/8] drm/amdgpu: Convert to common fdinfo format
From: Tvrtko Ursulin
Convert fdinfo format to one documented in drm-usage-stats.rst.
Opens:
* Does it work for AMD?
* What are the semantics of AMD engine utilisation reported in percents?
Can it align with what i915 does or needs to document the alternative
in the specification document?
Signed-off-by: Tvrtko Ursulin
Cc: David M Nieto
Cc: Christian König
Cc: Daniel Vetter
---
Documentation/gpu/amdgpu.rst | 26 ++
Documentation/gpu/drm-usage-stats.rst | 7 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 18 ++-
3 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/Documentation/gpu/amdgpu.rst b/Documentation/gpu/amdgpu.rst
index 364680cdad2e..b9b79c810f28 100644
--- a/Documentation/gpu/amdgpu.rst
+++ b/Documentation/gpu/amdgpu.rst
@@ -322,3 +322,29 @@ smartshift_bias
.. kernel-doc:: drivers/gpu/drm/amd/pm/amdgpu_pm.c
:doc: smartshift_bias
+
+.. _amdgpu-usage-stats:
+
+amdgpu DRM client usage stats implementation
+
+
+The amdgpu driver implements the DRM client usage stats specification as
+documented in :ref:`drm-client-usage-stats`.
+
+Example of the output showing the implemented key value pairs and entirety of
+the currenly possible format options:
+
+::
+
+ pos:0
+ flags: 012
+ mnt_id: 21
+ drm-driver: amdgpu
+ drm-pdev: :00:02.0
+ drm-client-id: 7
+ drm-engine-... TODO
+ drm-memory-... TODO
+
+Possible `drm-engine-` key names are: ``,... TODO.
+
+Possible `drm-memory-` key names are: ``,... TODO.
diff --git a/Documentation/gpu/drm-usage-stats.rst
b/Documentation/gpu/drm-usage-stats.rst
index b87505438aaa..eaaa361805c0 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -69,7 +69,7 @@ scope of each device, in which case `drm-pdev` shall be
present as well.
Userspace should make sure to not double account any usage statistics by using
the above described criteria in order to associate data to individual clients.
-- drm-engine-: ns
+- drm-engine-: [ns|%]
GPUs usually contain multiple execution engines. Each shall be given a stable
and unique name (str), with possible values documented in the driver specific
@@ -84,6 +84,9 @@ larger value within a reasonable period. Upon observing a
value lower than what
was previously read, userspace is expected to stay with that larger previous
value until a monotonic update is seen.
+Where time unit is given as a percentage...[AMD folks to fill the semantics
+and interpretation of that]...
+
- drm-memory-: [KiB|MiB]
Each possible memory type which can be used to store buffer objects by the
@@ -101,3 +104,5 @@ Driver specific implementations
===
:ref:`i915-usage-stats`
+
+:ref:`amdgpu-usage-stats`
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
index d94c5419ec25..d6b011008fe9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
@@ -76,11 +76,19 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
}
amdgpu_vm_get_memory(&fpriv->vm, &vram_mem, >t_mem, &cpu_mem);
amdgpu_bo_unreserve(fpriv->vm.root.bo);
- seq_printf(m, "pdev:\t%04x:%02x:%02x.%d\npasid:\t%u\n", domain, bus,
+
+ /*
+* **
+* For text output format description please see drm-usage-stats.rst!
+* **
+*/
+
+ seq_puts(m, "drm-driver: amdgpu\n");
+ seq_printf(m, "drm-pdev:\t%04x:%02x:%02x.%d\npasid:\t%u\n", domain, bus,
dev, fn, fpriv->vm.pasid);
- seq_printf(m, "vram mem:\t%llu kB\n", vram_mem/1024UL);
- seq_printf(m, "gtt mem:\t%llu kB\n", gtt_mem/1024UL);
- seq_printf(m, "cpu mem:\t%llu kB\n", cpu_mem/1024UL);
+ seq_printf(m, "drm-memory-vram:\t%llu KiB\n", vram_mem/1024UL);
+ seq_printf(m, "drm-memory-gtt:\t%llu KiB\n", gtt_mem/1024UL);
+ seq_printf(m, "drm-memory-cpu:\t%llu KiB\n", cpu_mem/1024UL);
for (i = 0; i < AMDGPU_HW_IP_NUM; i++) {
uint32_t count = amdgpu_ctx_num_entities[i];
int idx = 0;
@@ -96,7 +104,7 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
perc = div64_u64(1 * total, min);
frac = perc % 100;
- seq_printf(m, "%s%d:\t%d.%d%%\n",
+ seq_printf(m, "drm-engine-%s%d:\t%d.%d %%\n",
amdgpu_ip_name[i],
idx, perc/100, frac);
}
--
2.30.2
[RFC 6/8] drm: Document fdinfo format specification
From: Tvrtko Ursulin Proposal to standardise the fdinfo text format as optionally output by DRM drivers. Idea is that a simple but, well defined, spec will enable generic userspace tools to be written while at the same time avoiding a more heavy handed approach of adding a mid-layer to DRM. i915 implements a subset of the spec, everything apart from the memory stats currently, and a matching intel_gpu_top tool exists. Open is to see if AMD can migrate to using the proposed GPU utilisation key-value pairs, or if they are not workable to see whether to go vendor specific, or if a standardised alternative can be found which is workable for both drivers. Same for the memory utilisation key-value pairs proposal. v2: * Update for removal of name and pid. Signed-off-by: Tvrtko Ursulin Cc: David M Nieto Cc: Christian König Cc: Daniel Vetter --- Documentation/gpu/drm-usage-stats.rst | 97 +++ Documentation/gpu/index.rst | 1 + 2 files changed, 98 insertions(+) create mode 100644 Documentation/gpu/drm-usage-stats.rst diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst new file mode 100644 index ..78dc01c30e22 --- /dev/null +++ b/Documentation/gpu/drm-usage-stats.rst @@ -0,0 +1,97 @@ +.. _drm-client-usage-stats: + +== +DRM client usage stats +== + +DRM drivers can choose to export partly standardised text output via the +`fops->show_fdinfo()` as part of the driver specific file operations registered +in the `struct drm_driver` object registered with the DRM core. + +One purpose of this output is to enable writing as generic as practicaly +feasible `top(1)` like userspace monitoring tools. + +Given the differences between various DRM drivers the specification of the +output is split between common and driver specific parts. Having said that, +wherever possible effort should still be made to standardise as much as +possible. + +File format specification += + +- File shall contain one key value pair per one line of text. +- Colon character (`:`) must be used to delimit keys and values. +- All keys shall be prefixed with `drm-`. +- Whitespace between the delimiter and first non-whitespace character shall be + ignored when parsing. +- Neither keys or values are allowed to contain whitespace characters. +- Numerical key value pairs can end with optional unit string. +- Data type of the value is fixed as defined in the specification. + +Key types +- + +1. Mandatory, fully standardised. +2. Optional, fully standardised. +3. Driver specific. + +Data types +-- + +- - Unsigned integer without defining the maximum value. +- - String excluding any above defined reserved characters or whitespace. + +Mandatory fully standardised keys +- + +- drm-driver: + +String shall contain a fixed string uniquely identified the driver handling +the device in question. For example name of the respective kernel module. + +Optional fully standardised keys + + +- drm-pdev: + +For PCI devices this should contain the PCI slot address of the device in +question. + +- drm-client-id: + +Unique value relating to the open DRM file descriptor used to distinguish +duplicated and shared file descriptors. Conceptually the value should map 1:1 +to the in kernel representation of `struct drm_file` instances. + +Uniqueness of the value shall be either globally unique, or unique within the +scope of each device, in which case `drm-pdev` shall be present as well. + +Userspace should make sure to not double account any usage statistics by using +the above described criteria in order to associate data to individual clients. + +- drm-engine-: ns + +GPUs usually contain multiple execution engines. Each shall be given a stable +and unique name (str), with possible values documented in the driver specific +documentation. + +Value shall be in specified time units which the respective GPU engine spent +busy executing workloads belonging to this client. + +Values are not required to be constantly monotonic if it makes the driver +implementation easier, but are required to catch up with the previously reported +larger value within a reasonable period. Upon observing a value lower than what +was previously read, userspace is expected to stay with that larger previous +value until a monotonic update is seen. + +- drm-memory-: [KiB|MiB] + +Each possible memory type which can be used to store buffer objects by the +GPU in question shall be given a stable and unique name to be returned as the +string here. + +Value shall reflect the amount of storage currently consumed by the buffer +object belong to this client, in the respective memory region. + +Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' +indicating kibi- or mebi-bytes. diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst i
[RFC 7/8] drm/i915: Expose client engine utilisation via fdinfo
From: Tvrtko Ursulin
Similar to AMD commit
874442541133 ("drm/amdgpu: Add show_fdinfo() interface"), using the
infrastructure added in previous patches, we add basic client info
and GPU engine utilisation for i915.
Example of the output:
pos:0
flags: 012
mnt_id: 21
drm-driver: i915
drm-pdev: :00:02.0
drm-client-id: 7
drm-engine-render: 9288864723 ns
drm-engine-copy:2035071108 ns
drm-engine-video: 0 ns
drm-engine-video-enhance: 0 ns
v2:
* Update for removal of name and pid.
Signed-off-by: Tvrtko Ursulin
Cc: David M Nieto
Cc: Christian König
Cc: Daniel Vetter
---
Documentation/gpu/drm-usage-stats.rst | 6 +++
Documentation/gpu/i915.rst | 27 ++
drivers/gpu/drm/i915/i915_drm_client.c | 73 ++
drivers/gpu/drm/i915/i915_drm_client.h | 4 ++
drivers/gpu/drm/i915/i915_drv.c| 3 ++
5 files changed, 113 insertions(+)
diff --git a/Documentation/gpu/drm-usage-stats.rst
b/Documentation/gpu/drm-usage-stats.rst
index 78dc01c30e22..b87505438aaa 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -95,3 +95,9 @@ object belong to this client, in the respective memory region.
Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
indicating kibi- or mebi-bytes.
+
+===
+Driver specific implementations
+===
+
+:ref:`i915-usage-stats`
diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index 204ebdaadb45..b28cc316dbd9 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -701,3 +701,30 @@ The style guide for ``i915_reg.h``.
.. kernel-doc:: drivers/gpu/drm/i915/i915_reg.h
:doc: The i915 register macro definition style guide
+
+.. _i915-usage-stats:
+
+i915 DRM client usage stats implementation
+==
+
+The drm/i915 driver implements the DRM client usage stats specification as
+documented in :ref:`drm-client-usage-stats`.
+
+Example of the output showing the implemented key value pairs and entirety of
+the currenly possible format options:
+
+::
+
+ pos:0
+ flags: 012
+ mnt_id: 21
+ drm-driver: i915
+ drm-pdev: :00:02.0
+ drm-client-id: 7
+ drm-engine-render: 9288864723 ns
+ drm-engine-copy:2035071108 ns
+ drm-engine-video: 0 ns
+ drm-engine-video-enhance: 0 ns
+
+Possible `drm-engine-` key names are: `render`, `copy`, `video` and
+`video-enhance`.
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c
b/drivers/gpu/drm/i915/i915_drm_client.c
index 91a8559bebf7..8a6706e06e31 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -7,6 +7,11 @@
#include
#include
+#include
+
+#include
+
+#include "gem/i915_gem_context.h"
#include "i915_drm_client.h"
#include "i915_gem.h"
#include "i915_utils.h"
@@ -68,3 +73,71 @@ void i915_drm_clients_fini(struct i915_drm_clients *clients)
GEM_BUG_ON(!xa_empty(&clients->xarray));
xa_destroy(&clients->xarray);
}
+
+#ifdef CONFIG_PROC_FS
+static const char * const uabi_class_names[] = {
+ [I915_ENGINE_CLASS_RENDER] = "render",
+ [I915_ENGINE_CLASS_COPY] = "copy",
+ [I915_ENGINE_CLASS_VIDEO] = "video",
+ [I915_ENGINE_CLASS_VIDEO_ENHANCE] = "video-enhance",
+};
+
+static u64 busy_add(struct i915_gem_context *ctx, unsigned int class)
+{
+ struct i915_gem_engines_iter it;
+ struct intel_context *ce;
+ u64 total = 0;
+
+ for_each_gem_engine(ce, rcu_dereference(ctx->engines), it) {
+ if (ce->engine->uabi_class != class)
+ continue;
+
+ total += intel_context_get_total_runtime_ns(ce);
+ }
+
+ return total;
+}
+
+static void
+show_client_class(struct seq_file *m,
+ struct i915_drm_client *client,
+ unsigned int class)
+{
+ const struct list_head *list = &client->ctx_list;
+ u64 total = atomic64_read(&client->past_runtime[class]);
+ struct i915_gem_context *ctx;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(ctx, list, client_link)
+ total += busy_add(ctx, class);
+ rcu_read_unlock();
+
+ return seq_printf(m, "drm-engine-%s:\t%llu ns\n",
+ uabi_class_names[class], total);
+}
+
+void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
+{
+ struct drm_file *file = f->private_data;
+ struct drm_i915_file_private *file_priv = file->driver_priv;
+ struct drm_i915_private *i915 = file_priv->dev_priv;
+ struct i915_drm_client *client = file_priv->client;
+ struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+ unsigned int i;
+
+ /*
+* **
+* For text output format description please see drm-usage-stats.rs
[PATCH] drm/panel-sony-acx424akp: Modernize backlight handling
This converts the internal backlight in the Sony ACX424AKP
driver to do it the canonical way:
- Assign the panel->backlight during probe.
- Let the panel framework handle the backlight.
- Make the backlight .set_brightness() turn the backlight
off completely if blank.
- Fix some dev_err_probe() use cases along the way.
Tested on the U8500 HREF520 reference design.
Signed-off-by: Linus Walleij
---
drivers/gpu/drm/panel/panel-sony-acx424akp.c | 84 +++-
1 file changed, 28 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c
b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
index 95659a4d15e9..163f0e0cee1c 100644
--- a/drivers/gpu/drm/panel/panel-sony-acx424akp.c
+++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
@@ -40,7 +40,6 @@
struct acx424akp {
struct drm_panel panel;
struct device *dev;
- struct backlight_device *bl;
struct regulator *supply;
struct gpio_desc *reset_gpio;
bool video_mode;
@@ -102,6 +101,20 @@ static int acx424akp_set_brightness(struct
backlight_device *bl)
u8 par;
int ret;
+
+ if (backlight_is_blank(bl)) {
+ /* Disable backlight */
+ par = 0x00;
+ ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
+&par, 1);
+ if (ret) {
+ dev_err(acx->dev, "failed to disable display backlight
(%d)\n", ret);
+ return ret;
+ }
+ return 0;
+ }
+
+
/* Calculate the PWM duty cycle in n/256's */
pwm_ratio = max(((duty_ns * 256) / period_ns) - 1, 1);
pwm_div = max(1,
@@ -172,6 +185,12 @@ static const struct backlight_ops acx424akp_bl_ops = {
.update_status = acx424akp_set_brightness,
};
+static const struct backlight_properties acx424akp_bl_props = {
+ .type = BACKLIGHT_PLATFORM,
+ .brightness = 512,
+ .max_brightness = 1023,
+};
+
static int acx424akp_read_id(struct acx424akp *acx)
{
struct mipi_dsi_device *dsi = to_mipi_dsi_device(acx->dev);
@@ -310,8 +329,6 @@ static int acx424akp_prepare(struct drm_panel *panel)
}
}
- acx->bl->props.power = FB_BLANK_NORMAL;
-
return 0;
err_power_off:
@@ -323,18 +340,8 @@ static int acx424akp_unprepare(struct drm_panel *panel)
{
struct acx424akp *acx = panel_to_acx424akp(panel);
struct mipi_dsi_device *dsi = to_mipi_dsi_device(acx->dev);
- u8 par;
int ret;
- /* Disable backlight */
- par = 0x00;
- ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
-&par, 1);
- if (ret) {
- dev_err(acx->dev, "failed to disable display backlight (%d)\n",
ret);
- return ret;
- }
-
ret = mipi_dsi_dcs_set_display_off(dsi);
if (ret) {
dev_err(acx->dev, "failed to turn display off (%d)\n", ret);
@@ -350,36 +357,10 @@ static int acx424akp_unprepare(struct drm_panel *panel)
msleep(85);
acx424akp_power_off(acx);
- acx->bl->props.power = FB_BLANK_POWERDOWN;
-
- return 0;
-}
-
-static int acx424akp_enable(struct drm_panel *panel)
-{
- struct acx424akp *acx = panel_to_acx424akp(panel);
-
- /*
-* The backlight is on as long as the display is on
-* so no use to call backlight_enable() here.
-*/
- acx->bl->props.power = FB_BLANK_UNBLANK;
return 0;
}
-static int acx424akp_disable(struct drm_panel *panel)
-{
- struct acx424akp *acx = panel_to_acx424akp(panel);
-
- /*
-* The backlight is on as long as the display is on
-* so no use to call backlight_disable() here.
-*/
- acx->bl->props.power = FB_BLANK_NORMAL;
-
- return 0;
-}
static int acx424akp_get_modes(struct drm_panel *panel,
struct drm_connector *connector)
@@ -409,10 +390,8 @@ static int acx424akp_get_modes(struct drm_panel *panel,
}
static const struct drm_panel_funcs acx424akp_drm_funcs = {
- .disable = acx424akp_disable,
.unprepare = acx424akp_unprepare,
.prepare = acx424akp_prepare,
- .enable = acx424akp_enable,
.get_modes = acx424akp_get_modes,
};
@@ -458,25 +437,18 @@ static int acx424akp_probe(struct mipi_dsi_device *dsi)
/* This asserts RESET by default */
acx->reset_gpio = devm_gpiod_get_optional(dev, "reset",
GPIOD_OUT_HIGH);
- if (IS_ERR(acx->reset_gpio)) {
- ret = PTR_ERR(acx->reset_gpio);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "failed to request GPIO (%d)\n", ret);
- return ret;
- }
+ if (IS_ERR(acx->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(acx->reset_gpio),
+
Re: [PATCH -next v2] drm/bochs: Fix missing pci_disable_device() on error in bochs_pci_probe()
Hi, for the change Acked-by: Thomas Zimmermann but there are some style issues AFAICS. Am 15.07.21 um 04:05 schrieb Yang Yingliang: Replace pci_enable_device() with pcim_enable_device(), pci_disable_device() will be called in release automatically. Reported-by: Hulk Robot Signed-off-by: Yang Yingliang S-o-b line goes first --- v2: use pcim_enable_device() This changelog should rather be located between the commit description and the first S-o-b line. Best regards Thomas --- drivers/gpu/drm/tiny/bochs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c index a2cfecfa8556..73415fa9ae0f 100644 --- a/drivers/gpu/drm/tiny/bochs.c +++ b/drivers/gpu/drm/tiny/bochs.c @@ -648,7 +648,7 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent if (IS_ERR(dev)) return PTR_ERR(dev); - ret = pci_enable_device(pdev); + ret = pcim_enable_device(pdev); if (ret) goto err_free_dev; -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 1/2] drm: add crtc background color property
On Wed, 14 Jul 2021 12:13:58 -0400 Harry Wentland wrote: > On 2021-07-14 3:35 a.m., Pekka Paalanen wrote: > > On Tue, 13 Jul 2021 09:54:35 -0400 > > Harry Wentland wrote: > > > >> On 2021-07-13 3:52 a.m., Pekka Paalanen wrote: > >>> On Mon, 12 Jul 2021 12:15:59 -0400 > >>> Harry Wentland wrote: > >>> > On 2021-07-12 4:03 a.m., Pekka Paalanen wrote: > > On Fri, 9 Jul 2021 18:23:26 +0200 > > Raphael Gallais-Pou wrote: > > > >> On 7/9/21 10:04 AM, Pekka Paalanen wrote: > >>> On Wed, 7 Jul 2021 08:48:47 + > >>> Raphael GALLAIS-POU - foss wrote: > >>> > Some display controllers can be programmed to present non-black > colors > for pixels not covered by any plane (or pixels covered by the > transparent regions of higher planes). Compositors that want a UI > with > a solid color background can potentially save memory bandwidth by > setting the CRTC background property and using smaller planes to > display > the rest of the content. > > To avoid confusion between different ways of encoding RGB data, we > define a standard 64-bit format that should be used for this > property's > value. Helper functions and macros are provided to generate and > dissect > values in this standard format with varying component precision > values. > > Signed-off-by: Raphael Gallais-Pou > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 1 + > drivers/gpu/drm/drm_atomic_uapi.c | 4 +++ > drivers/gpu/drm/drm_blend.c | 34 > +-- > drivers/gpu/drm/drm_mode_config.c | 6 > include/drm/drm_blend.h | 1 + > include/drm/drm_crtc.h| 12 > include/drm/drm_mode_config.h | 5 > include/uapi/drm/drm_mode.h | 28 +++ > 8 files changed, 89 insertions(+), 2 deletions(-) > >>> > >>> ... > >>> > >>> The question about full vs. limited range seems unnecessary to me, as > >>> the background color will be used as-is in the blending stage, so > >>> userspace can just program the correct value that fits the pipeline it > >>> is setting up. > >>> > >>> One more question is, as HDR exists, could we need background colors > >>> with component values greater than 1.0? > >> > >> AR4H color format should cover that case, isn't it ? > > > > Yes, but with the inconvenience I mentioned. > > > > This is a genuine question though, would anyone actually need > > background color values > 1.0. I don't know of any case yet where it > > would be required. It would imply that plane blending happens in a > > color space where >1.0 values are meaningful. I'm not even sure if any > > hardware supporting that exists. > > > > Maybe it would be best to assume that only [0.0, 1.0] pixel value range > > is useful, and mention in the commit message that if someone really > > needs values outside of that, they should create another background > > color property. Then, you can pick a simple unsigned integer pixel > > format, too. (I didn't see any 16 bit-per-channel formats like that in > > drm_fourcc.h though.) > > > > I don't think we should artificially limit this to [0.0, 1.0]. As you > mentioned above when talking about full vs limited, the userspace > understands what's the correct value that fits the pipeline. If that > pipeline is FP16 with > 1.0 values then it would make sense that the > background color can be > 1.0. > >>> > >>> Ok. The standard FP32 format then for ease of use and guaranteed enough > >>> range and precision for far into the future? > >>> > >> > >> I don't have a strong preference for FP16 vs FP32. My understanding is > >> that FP16 is enough to represent linearly encoded data in a way that > >> looks smooth to humans. > >> > >> scRGB uses FP16 with linear encoding in a range of [-0.5, 7.4999]. > >> > >>> Or do you want to keep it in 64 bits total, so the UABI can pack > >>> everything into a u64 instead of needing to create a blob? > >>> > >>> I don't mind as long as it's clearly documented what it is and how it > >>> works, and it carries enough precision. > >>> > >>> But FP16 with its 10 bits of precision might be too little for integer > >>> 12-16 bpc pipelines and sinks? > > > > The 10 bits worries me still. > > > > If you have a pipeline that works in [0.0, 1.0] range only, then FP16 > > limits precision to 10 bits (in the upper half of the range?). > > > >>> > >>> If the values can go beyond [0.0, 1.0] range, then does the ble
Re: [Intel-gfx] [PATCH 31/47] drm/i915/guc: Reset implementation for new GuC interface
On 24/06/2021 08:05, Matthew Brost wrote:
Reset implementation for new GuC interface. This is the legacy reset
implementation which is called when the i915 owns the engine hang check.
Future patches will offload the engine hang check to GuC but we will
continue to maintain this legacy path as a fallback and this code path
is also required if the GuC dies.
With the new GuC interface it is not possible to reset individual
engines - it is only possible to reset the GPU entirely. This patch
forces an entire chip reset if any engine hangs.
No updates after my review comments on 6th of May.
At least:
1. wmb documentation
2. Spin lock cycling I either didn't understand or didn't buy the
explanation. I don't remember seeing that pattern elsewhere in the
driver - cycle a spinlock to make sure what was updated inside it is
visible you said?
3. Dropping the lock protecting the list in the middle of
list_for_each_entry_safe and just continuing to iterate like nothing
happened. (__unwind_incomplete_requests) Again, perhaps I did not
understand your explanation properly but you did appear to write:
"""
We only need the active lock for ce->guc_active.requests list. It is
indeed safe to drop the lock.
"""
+ spin_lock(&ce->guc_active.lock);
+ list_for_each_entry_safe(rq, rn,
+&ce->guc_active.requests,
+sched.link) {
+ if (i915_request_completed(rq))
+ continue;
+
+ list_del_init(&rq->sched.link);
+ spin_unlock(&ce->guc_active.lock);
...
+ spin_lock(&ce->guc_active.lock);
+ }
Safe iterator guards against list_del but dropping the lock means the
state of the overall list can change so next pointer may or may not be
valid, requests may be missed, I don't know. Needs a comment explaining
why it is safe.
Regards,
Tvrtko
Cc: John Harrison
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/intel_context.c | 3 +
drivers/gpu/drm/i915/gt/intel_context_types.h | 7 +
drivers/gpu/drm/i915/gt/intel_engine_types.h | 6 +
.../drm/i915/gt/intel_execlists_submission.c | 40 ++
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 6 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 18 +-
.../gpu/drm/i915/gt/intel_ring_submission.c | 22 +
drivers/gpu/drm/i915/gt/mock_engine.c | 31 +
drivers/gpu/drm/i915/gt/uc/intel_guc.c| 13 -
drivers/gpu/drm/i915/gt/uc/intel_guc.h| 8 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 581 ++
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 39 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.h | 3 +
drivers/gpu/drm/i915/i915_request.c | 41 +-
drivers/gpu/drm/i915/i915_request.h | 2 +
15 files changed, 649 insertions(+), 171 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index b24a1b7a3f88..2f01437056a8 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -392,6 +392,9 @@ intel_context_init(struct intel_context *ce, struct
intel_engine_cs *engine)
spin_lock_init(&ce->guc_state.lock);
INIT_LIST_HEAD(&ce->guc_state.fences);
+ spin_lock_init(&ce->guc_active.lock);
+ INIT_LIST_HEAD(&ce->guc_active.requests);
+
ce->guc_id = GUC_INVALID_LRC_ID;
INIT_LIST_HEAD(&ce->guc_id_link);
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 6945963a31ba..b63c8cf7823b 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -165,6 +165,13 @@ struct intel_context {
struct list_head fences;
} guc_state;
+ struct {
+ /** lock: protects everything in guc_active */
+ spinlock_t lock;
+ /** requests: active requests on this context */
+ struct list_head requests;
+ } guc_active;
+
/* GuC scheduling state that does not require a lock. */
atomic_t guc_sched_state_no_lock;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index e7cb6a06db9d..f9d264c008e8 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -426,6 +426,12 @@ struct intel_engine_cs {
void (*release)(struct intel_engine_cs *engine);
+ /*
+* Add / remove request from engine active tracking
+*/
+ void(*add_active_request)(struct i915_request *rq);
+ void(*remove_active_request)(struct i915_request *rq);
+
struct intel_engine_execlists execlists;
/*
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index c10ea6080752..c301a2d088b1 10
Re: Questions over DSI within DRM.
Hi Dave, On Tue, Jul 06, 2021 at 05:44:58PM +0100, Dave Stevenson wrote: > On Tue, 6 Jul 2021 at 16:13, Maxime Ripard wrote: > > > > > On a similar theme, some devices want the clock lane in HS mode early > > > > > so they can use it in place of an external oscillator, but the data > > > > > lanes still in LP-11. There appears to be no way for the > > > > > display/bridge to signal this requirement or it be achieved. > > > > > > > > You're right. A lng time ago, the omapdrm driver had an internal > > > > infrastructure that didn't use drm_bridge or drm_panel and instead > > > > required omapdrm-specific drivers for those components. It used to model > > > > the display pipeline in a different way than drm_bridge, with the sync > > > > explicitly setting the source state. A DSI sink could thus control its > > > > enable sequence, interleaving programming of the sink with control of > > > > the source. > > > > > > > > Migrating omapdrm to the drm_bridge model took a really large effort, > > > > which makes me believe that transitioning the whole subsystem to > > > > sink-controlled sources would be close to impossible. We could add > > > > DSI-specific operations, or add another enable bridge operation > > > > (post_pre_enable ? :-D). Neither would scale, but it may be enough. > > > > > > I haven't thought it through for all generic cases, but I suspect it's > > > more a pre_pre_enable that is needed to initialise the PHY etc, > > > probably from source to sink. > > > If the panel/bridge can set a flag that can be checked at this point > > > for whether an early clock is required or not, I think that allows us > > > to comply with the requirements for a large number of panels/bridges > > > (LP-11 vs HS config for clock and or data lanes before pre_enable is > > > called). > > > > > > pre_enable retains the current behaviour (initialise the chain from > > > sink to source). > > > enable then actually starts sending video and enabling outputs. > > > > Flags indeed seem like a more contained option. Another one could be to > > have a mipi_dsi_host to (for example) power up the clock lane that would > > be called by default before the bridge's enable, or at the downstream > > bridge driver discretion before that. > > Which driver will that call? The parent DSI Host > An extreme example perhaps, but Toshiba make the TC358860 eDP to DSI > bridge chip[1]. So the encoder will be eDP, but the DSI config needs > to go to that bridge. Does that happen automatically within the > framework? I guess so as the bridge will have called > mipi_dsi_host_register for the DSI sink to attach to. In that case, whatever sink would be connected to the bridge would call the bridge clock enable hook if it needs it in its pre_enable, or it would be called automatically before enable if it doesn't Would that help? > Perhaps a new mipi_dsi_host function to configure the PHY is the > easier solution. If it can allow the sink to request whatever > combination of states from clock and data lanes that it fancies, then > it can be as explicit as required for the initialisation sequence, and > the host driver does its best to comply with the requests. I don't know, I'm not really fond in general of solutions that try to cover any single case if we don't need it and / or have today an issue with this. I'd rather have something that works for the particular bridge we were discussing, see if it applies to other bridges and modify it if it doesn't until it works for all our cases. Trying to reason in all possible cases tend to lead to solutions that are difficult to maintain and usually over-engineered. > I'd have a slight query over when and how the host would drop to ULPS > or power off. It probably shouldn't be in post_disable as the sink > hasn't had a chance to finalise everything in its post_disable. > > Perhaps pm_runtime with autosuspend is the right call there? pm_runtime semantics mean that once the device is suspended, its power domains, regulators, clocks, etc. are all shut down, so it doesn't really fit the low power state expected by DSI > [1] > https://toshiba.semicon-storage.com/ap-en/semiconductor/product/interface-bridge-ics-for-mobile-peripheral-devices/display-interface-bridge-ics/detail.TC358860XBG.html > > > > When I discussed this briefly with Maxime there was a suggestion of > > > using pm_runtime to be able to power up the pipeline as a whole. If > > > the bridge driver can use pm_runtime to power up the PHY when > > > required, then that may solve the issue, however I know too little of > > > the details to say whether that is actually practical. > > > > I'm not sure it was about this topic in particular. If I remember well > > our discussion, this was about the vc4 driver that tries to circumvent > > the framework and call the pre_enable and enable hooks itself because it > > wasn't properly powered before and thus any DCS-related call by the > > downstream bridge or panel would end up creating a CPU st
Re: [PATCH v6 RESEND 1/3] gpu: drm: separate panel orientation property creating and value setting
Hi Hsin-Yi,
Thank you for your patch.
Missatge de Hsin-Yi Wang del dia dj., 24 de juny
2021 a les 12:55:
>
> drm_dev_register() sets connector->registration_state to
> DRM_CONNECTOR_REGISTERED and dev->registered to true. If
> drm_connector_set_panel_orientation() is first called after
> drm_dev_register(), it will fail several checks and results in following
> warning.
>
In fact, results with the following warning
> Add a function to create panel orientation property and set default value
> to UNKNOWN, so drivers can call this function to init the property earlier
> , and let the panel set the real value later.
>
> [4.480976] [ cut here ]
> [4.485603] WARNING: CPU: 5 PID: 369 at
> drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xb4/0xbc
>
> [4.609772] Call trace:
> [4.612208] __drm_mode_object_add+0xb4/0xbc
> [4.616466] drm_mode_object_add+0x20/0x2c
> [4.620552] drm_property_create+0xdc/0x174
> [4.624723] drm_property_create_enum+0x34/0x98
> [4.629241] drm_connector_set_panel_orientation+0x64/0xa0
> [4.634716] boe_panel_get_modes+0x88/0xd8
> [4.638802] drm_panel_get_modes+0x2c/0x48
> [4.642887] panel_bridge_get_modes+0x1c/0x28
> [4.647233] drm_bridge_connector_get_modes+0xa0/0xd4
> [4.652273] drm_helper_probe_single_connector_modes+0x218/0x700
> [4.658266] drm_mode_getconnector+0x1b4/0x45c
> [4.662699] drm_ioctl_kernel+0xac/0x128
> [4.11] drm_ioctl+0x268/0x410
> [4.670002] drm_compat_ioctl+0xdc/0xf0
> [4.673829] __arm64_compat_sys_ioctl+0xc8/0x100
> [4.678436] el0_svc_common+0xf4/0x1c0
> [4.682174] do_el0_svc_compat+0x28/0x3c
> [4.686088] el0_svc_compat+0x10/0x1c
> [4.689738] el0_sync_compat_handler+0xa8/0xcc
> [4.694171] el0_sync_compat+0x178/0x180
> [4.698082] ---[ end trace b4f2db9d9c88610b ]---
> [4.702721] [ cut here ]
> [4.707329] WARNING: CPU: 5 PID: 369 at
> drivers/gpu/drm/drm_mode_object.c:243 drm_object_attach_property+0x48/0xb8
>
> [4.833830] Call trace:
> [4.836266] drm_object_attach_property+0x48/0xb8
> [4.840958] drm_connector_set_panel_orientation+0x84/0xa0
> [4.846432] boe_panel_get_modes+0x88/0xd8
> [4.850516] drm_panel_get_modes+0x2c/0x48
> [4.854600] panel_bridge_get_modes+0x1c/0x28
> [4.858946] drm_bridge_connector_get_modes+0xa0/0xd4
> [4.863984] drm_helper_probe_single_connector_modes+0x218/0x700
> [4.869978] drm_mode_getconnector+0x1b4/0x45c
> [4.874410] drm_ioctl_kernel+0xac/0x128
> [4.878320] drm_ioctl+0x268/0x410
> [4.881711] drm_compat_ioctl+0xdc/0xf0
> [4.885536] __arm64_compat_sys_ioctl+0xc8/0x100
> [4.890142] el0_svc_common+0xf4/0x1c0
> [4.893879] do_el0_svc_compat+0x28/0x3c
> [4.897791] el0_svc_compat+0x10/0x1c
> [4.901441] el0_sync_compat_handler+0xa8/0xcc
> [4.905873] el0_sync_compat+0x178/0x180
> [4.909783] ---[ end trace b4f2db9d9c88610c ]---
>
> Signed-off-by: Hsin-Yi Wang
> Reviewed-by: Sean Paul
And that patch fixes the problem, apart from, not being an expert, but
the change looks reasonable to me, there is already some reviewed tags
and I'm not sure I am the appropriate to review it but I can
Tested-by: Enric Balletbo i Serra
As together with the other two patches works and I don't see any
problem on the Lenovo IdeaPad Duet.
> ---
> drivers/gpu/drm/drm_connector.c | 58 ++---
> drivers/gpu/drm/i915/display/icl_dsi.c | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 1 +
> drivers/gpu/drm/i915/display/vlv_dsi.c | 1 +
> include/drm/drm_connector.h | 2 +
> 5 files changed, 47 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 7631f76e7f345..7189baaabf416 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1210,7 +1210,7 @@ static const struct drm_prop_enum_list dp_colorspaces[]
> = {
> * INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
> * coordinates, so if userspace rotates the picture to adjust for
> * the orientation it must also apply the same transformation to the
> - * touchscreen input coordinates. This property is initialized by calling
> + * touchscreen input coordinates. This property value is set by calling
> * drm_connector_set_panel_orientation() or
> * drm_connector_set_panel_orientation_with_quirk()
> *
> @@ -2173,8 +2173,8 @@ EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
> * @connector: connector for which to set the panel-orientation property.
> * @panel_orientation: drm_panel_orientation value to set
> *
> - * This function sets the connector's panel_orientation and attaches
> - * a "panel orientation" property to the connector.
> + * This function sets the connector's panel_orientation value. If the
> property
> + * doesn't exis
[PATCH v2] drm/shmobile: Convert to Linux IRQ interfaces
Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.
v2:
* handle errors in platform_get_irq() (Geert, Sergei)
* store IRQ number in struct shmob_drm_device (Laurent)
Signed-off-by: Thomas Zimmermann
Acked-by: Sam Ravnborg
---
drivers/gpu/drm/shmobile/shmob_drm_drv.c | 16 +++-
drivers/gpu/drm/shmobile/shmob_drm_drv.h | 1 +
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 0a02b7092c04..07878f4ef23e 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include
@@ -130,7 +129,6 @@ DEFINE_DRM_GEM_CMA_FOPS(shmob_drm_fops);
static const struct drm_driver shmob_drm_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET,
- .irq_handler= shmob_drm_irq,
DRM_GEM_CMA_DRIVER_OPS,
.fops = &shmob_drm_fops,
.name = "shmob-drm",
@@ -183,7 +181,7 @@ static int shmob_drm_remove(struct platform_device *pdev)
drm_dev_unregister(ddev);
drm_kms_helper_poll_fini(ddev);
- drm_irq_uninstall(ddev);
+ free_irq(sdev->irq, ddev);
drm_dev_put(ddev);
return 0;
@@ -258,7 +256,15 @@ static int shmob_drm_probe(struct platform_device *pdev)
goto err_modeset_cleanup;
}
- ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
+ ret = platform_get_irq(pdev, 0);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to get IRQ number\n");
+ goto err_modeset_cleanup;
+ }
+ sdev->irq = ret;
+
+ ret = request_irq(sdev->irq, shmob_drm_irq, 0, ddev->driver->name,
+ ddev);
if (ret < 0) {
dev_err(&pdev->dev, "failed to install IRQ handler\n");
goto err_modeset_cleanup;
@@ -275,7 +281,7 @@ static int shmob_drm_probe(struct platform_device *pdev)
return 0;
err_irq_uninstall:
- drm_irq_uninstall(ddev);
+ free_irq(sdev->irq, ddev);
err_modeset_cleanup:
drm_kms_helper_poll_fini(ddev);
err_free_drm_dev:
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.h
b/drivers/gpu/drm/shmobile/shmob_drm_drv.h
index 80dc4b1020aa..4964ddd5ab74 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.h
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.h
@@ -29,6 +29,7 @@ struct shmob_drm_device {
u32 lddckr;
u32 ldmt1r;
+ unsigned int irq;
spinlock_t irq_lock;/* Protects hardware LDINTR register */
struct drm_device *ddev;
--
2.32.0
[PATCH v2] drm/ingenic: Convert to Linux IRQ interfaces
Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.
This patch also fixes a bug where the driver didn't release the
IRQ.
v2:
* automatically release IRQ via devm_request_irq() (Paul)
* mention the bugfix (Sam)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Sam Ravnborg
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index c296472164d9..857ed070b21b 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -33,7 +33,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -799,8 +798,6 @@ static const struct drm_driver ingenic_drm_driver_data = {
.fops = &ingenic_drm_fops,
.gem_create_object = ingenic_drm_gem_create_object,
DRM_GEM_CMA_DRIVER_OPS,
-
- .irq_handler= ingenic_drm_irq_handler,
};
static const struct drm_plane_funcs ingenic_drm_primary_plane_funcs = {
@@ -1098,7 +1095,7 @@ static int ingenic_drm_bind(struct device *dev, bool
has_components)
encoder->possible_clones = clone_mask;
}
- ret = drm_irq_install(drm, irq);
+ ret = devm_request_irq(dev, irq, ingenic_drm_irq_handler, 0,
drm->driver->name, drm);
if (ret) {
dev_err(dev, "Unable to install IRQ handler\n");
return ret;
--
2.32.0
[PATCH 0/4] Some DG1 uAPI cleanup
Test-with: 20210715100932.2605648-1-matthew.a...@intel.com Chris Wilson (1): drm/i915/userptr: Probe existence of backing struct pages upon creation Matthew Auld (3): drm/i915/uapi: reject caching ioctls for discrete drm/i915/uapi: convert drm_i915_gem_userptr to kernel doc drm/i915/uapi: reject set_domain for discrete drivers/gpu/drm/i915/gem/i915_gem_domain.c | 9 ++ drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 40 +++- drivers/gpu/drm/i915/i915_getparam.c| 3 + include/uapi/drm/i915_drm.h | 106 +++- 4 files changed, 156 insertions(+), 2 deletions(-) -- 2.26.3
[PATCH 1/4] drm/i915/uapi: reject caching ioctls for discrete
It's a noop on DG1, and in the future when need to support other devices
which let us control the coherency, then it should be an immutable
creation time property for the BO. This will likely be controlled
through a new gem_create_ext extension.
v2: add some kernel doc for the discrete changes, and document the
implicit rules
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Maarten Lankhorst
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Ramalingam C
Reviewed-by: Ramalingam C
Reviewed-by: Kenneth Graunke
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 6 +
include/uapi/drm/i915_drm.h| 29 ++
2 files changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 7d1400b13429..43004bef55cb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -268,6 +268,9 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void
*data,
struct drm_i915_gem_object *obj;
int err = 0;
+ if (IS_DGFX(to_i915(dev)))
+ return -ENODEV;
+
rcu_read_lock();
obj = i915_gem_object_lookup_rcu(file, args->handle);
if (!obj) {
@@ -303,6 +306,9 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void
*data,
enum i915_cache_level level;
int ret = 0;
+ if (IS_DGFX(i915))
+ return -ENODEV;
+
switch (args->caching) {
case I915_CACHING_NONE:
level = I915_CACHE_NONE;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e54f9efaead0..868c2ee7be60 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1395,6 +1395,35 @@ struct drm_i915_gem_busy {
* ppGTT support, or if the object is used for scanout). Note that this might
* require unbinding the object from the GTT first, if its current caching
value
* doesn't match.
+ *
+ * Note that this all changes on discrete platforms, starting from DG1, the
+ * set/get caching is no longer supported, and is now rejected. Instead the
CPU
+ * caching attributes(WB vs WC) will become an immutable creation time property
+ * for the object, along with the GTT caching level. For now we don't expose
any
+ * new uAPI for this, instead on DG1 this is all implicit, although this
largely
+ * shouldn't matter since DG1 is coherent by default(without any way of
+ * controlling it).
+ *
+ * Implicit caching rules, starting from DG1:
+ *
+ * - If any of the object placements (see
&drm_i915_gem_create_ext_memory_regions)
+ * contain I915_MEMORY_CLASS_DEVICE then the object will be allocated and
+ * mapped as write-combined only.
+ *
+ * - Everything else is always allocated and mapped as write-back, with the
+ * guarantee that everything is also coherent with the GPU.
+ *
+ * Note that this is likely to change in the future again, where we might need
+ * more flexibility on future devices, so making this all explicit as part of a
+ * new &drm_i915_gem_create_ext extension is probable.
+ *
+ * Side note: Part of the reason for this is that changing the
at-allocation-time CPU
+ * caching attributes for the pages might be required(and is expensive) if we
+ * need to then CPU map the pages later with different caching attributes. This
+ * inconsistent caching behaviour, while supported on x86, is not universally
+ * supported on other architectures. So for simplicity we opt for setting
+ * everything at creation time, whilst also making it immutable, on discrete
+ * platforms.
*/
struct drm_i915_gem_caching {
/**
--
2.26.3
[PATCH 2/4] drm/i915/uapi: convert drm_i915_gem_userptr to kernel doc
Add the missing kernel-doc.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Maarten Lankhorst
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Ramalingam C
---
include/uapi/drm/i915_drm.h | 40 -
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 868c2ee7be60..e20eeeca7a1c 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2141,14 +2141,52 @@ struct drm_i915_reset_stats {
__u32 pad;
};
+/**
+ * struct drm_i915_gem_userptr - Create GEM object from user allocated memory.
+ *
+ * Userptr objects have several restrictions on what ioctls can be used with
the
+ * object handle.
+ */
struct drm_i915_gem_userptr {
+ /**
+* @user_ptr: The pointer to the allocated memory.
+*
+* Needs to be aligned to PAGE_SIZE.
+*/
__u64 user_ptr;
+
+ /**
+* @user_size:
+*
+* The size in bytes for the allocated memory. This will also become the
+* object size.
+*
+* Needs to be aligned to PAGE_SIZE, and should be at least PAGE_SIZE,
+* or larger.
+*/
__u64 user_size;
+
+ /**
+* @flags:
+*
+* Supported flags:
+*
+* I915_USERPTR_READ_ONLY:
+*
+* Mark the object as readonly, this also means GPU access can only be
+* readonly. This is only supported on HW which supports readonly access
+* through the GTT. If the HW can't support readonly access, an error is
+* returned.
+*
+* I915_USERPTR_UNSYNCHRONIZED:
+*
+* NOT USED. Setting this flag will result in an error.
+*/
__u32 flags;
#define I915_USERPTR_READ_ONLY 0x1
#define I915_USERPTR_UNSYNCHRONIZED 0x8000
/**
-* Returned handle for the object.
+* @handle: Returned handle for the object.
*
* Object handles are nonzero.
*/
--
2.26.3
[PATCH 3/4] drm/i915/userptr: Probe existence of backing struct pages upon creation
From: Chris Wilson
Jason Ekstrand requested a more efficient method than userptr+set-domain
to determine if the userptr object was backed by a complete set of pages
upon creation. To be more efficient than simply populating the userptr
using get_user_pages() (as done by the call to set-domain or execbuf),
we can walk the tree of vm_area_struct and check for gaps or vma not
backed by struct page (VM_PFNMAP). The question is how to handle
VM_MIXEDMAP which may be either struct page or pfn backed...
With discrete are going to drop support for set_domain(), so offering a
way to probe the pages, without having to resort to dummy batches has
been requested.
v2:
- add new query param for the PROPBE flag, so userspace can easily
check if the kernel supports it(Jason).
- use mmap_read_{lock, unlock}.
- add some kernel-doc.
Testcase: igt/gem_userptr_blits/probe
Signed-off-by: Chris Wilson
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Maarten Lankhorst
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Ramalingam C
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 40 -
drivers/gpu/drm/i915/i915_getparam.c| 3 ++
include/uapi/drm/i915_drm.h | 18 ++
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 56edfeff8c02..fd6880328596 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -422,6 +422,33 @@ static const struct drm_i915_gem_object_ops
i915_gem_userptr_ops = {
#endif
+static int
+probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
+{
+ const unsigned long end = addr + len;
+ struct vm_area_struct *vma;
+ int ret = -EFAULT;
+
+ mmap_read_lock(mm);
+ for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
+ if (vma->vm_start > addr)
+ break;
+
+ if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
+ break;
+
+ if (vma->vm_end >= end) {
+ ret = 0;
+ break;
+ }
+
+ addr = vma->vm_end;
+ }
+ mmap_read_unlock(mm);
+
+ return ret;
+}
+
/*
* Creates a new mm object that wraps some normal memory from the process
* context - user memory.
@@ -477,7 +504,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
}
if (args->flags & ~(I915_USERPTR_READ_ONLY |
- I915_USERPTR_UNSYNCHRONIZED))
+ I915_USERPTR_UNSYNCHRONIZED |
+ I915_USERPTR_PROBE))
return -EINVAL;
if (i915_gem_object_size_2big(args->user_size))
@@ -504,6 +532,16 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
return -ENODEV;
}
+ if (args->flags & I915_USERPTR_PROBE) {
+ /*
+* Check that the range pointed to represents real struct
+* pages and not iomappings (at this moment in time!)
+*/
+ ret = probe_range(current->mm, args->user_ptr, args->user_size);
+ if (ret)
+ return ret;
+ }
+
#ifdef CONFIG_MMU_NOTIFIER
obj = i915_gem_object_alloc();
if (obj == NULL)
diff --git a/drivers/gpu/drm/i915/i915_getparam.c
b/drivers/gpu/drm/i915/i915_getparam.c
index 24e18219eb50..d6d2e1a10d14 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -163,6 +163,9 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
case I915_PARAM_PERF_REVISION:
value = i915_perf_ioctl_version();
break;
+ case I915_PARAM_HAS_USERPTR_PROBE:
+ value = true;
+ break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
return -EINVAL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e20eeeca7a1c..2e4112bf4d38 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -674,6 +674,9 @@ typedef struct drm_i915_irq_wait {
*/
#define I915_PARAM_HAS_EXEC_TIMELINE_FENCES 55
+/* Query if the kernel supports the I915_USERPTR_PROBE flag. */
+#define I915_PARAM_HAS_USERPTR_PROBE 56
+
/* Must be kept compact -- no holes and well documented */
typedef struct drm_i915_getparam {
@@ -2178,12 +2181,27 @@ struct drm_i915_gem_userptr {
* through the GTT. If the HW can't support readonly access, an error is
* returned.
*
+* I915_USERPTR_PROBE:
+*
+* Probe the provided @user_ptr range and validate that the @user_ptr is
+* indeed pointing to normal memory and that the range is also valid.
+* For example if some garbage address is gi
[PATCH 4/4] drm/i915/uapi: reject set_domain for discrete
The CPU domain should be static for discrete, and on DG1 we don't need
any flushing since everything is already coherent, so really all this
does is an object wait, for which we have an ioctl. Longer term the
desired caching should be an immutable creation time property for the
BO, which can be set with something like gem_create_ext.
One other user is iris + userptr, which uses the set_domain to probe all
the pages to check if the GUP succeeds, however we now have a PROBE
flag for this purpose.
v2: add some more kernel doc, also add the implicit rules with caching
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Maarten Lankhorst
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Ramalingam C
Reviewed-by: Ramalingam C
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
include/uapi/drm/i915_drm.h| 19 +++
2 files changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 43004bef55cb..b684a62bf3b0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -490,6 +490,9 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void
*data,
u32 write_domain = args->write_domain;
int err;
+ if (IS_DGFX(to_i915(dev)))
+ return -ENODEV;
+
/* Only handle setting domains to types used by the CPU. */
if ((write_domain | read_domains) & I915_GEM_GPU_DOMAINS)
return -EINVAL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 2e4112bf4d38..04ce310e7ee6 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -901,6 +901,25 @@ struct drm_i915_gem_mmap_offset {
* - I915_GEM_DOMAIN_GTT: Mappable aperture domain
*
* All other domains are rejected.
+ *
+ * Note that for discrete, starting from DG1, this is no longer supported, and
+ * is instead rejected. On such platforms the CPU domain is effectively static,
+ * where we also only support a single &drm_i915_gem_mmap_offset cache mode,
+ * which can't be set explicitly and instead depends on the object placements,
+ * as per the below.
+ *
+ * Implicit caching rules, starting from DG1:
+ *
+ * - If any of the object placements (see
&drm_i915_gem_create_ext_memory_regions)
+ * contain I915_MEMORY_CLASS_DEVICE then the object will be allocated and
+ * mapped as write-combined only.
+ *
+ * - Everything else is always allocated and mapped as write-back, with the
+ * guarantee that everything is also coherent with the GPU.
+ *
+ * Note that this is likely to change in the future again, where we might need
+ * more flexibility on future devices, so making this all explicit as part of a
+ * new &drm_i915_gem_create_ext extension is probable.
*/
struct drm_i915_gem_set_domain {
/** @handle: Handle for the object. */
--
2.26.3
Re: [PATCH v2] drm/shmobile: Convert to Linux IRQ interfaces
Hi Thomas,
On Thu, Jul 15, 2021 at 11:57 AM Thomas Zimmermann wrote:
> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
> don't benefit from using it.
>
> v2:
> * handle errors in platform_get_irq() (Geert, Sergei)
> * store IRQ number in struct shmob_drm_device (Laurent)
>
> Signed-off-by: Thomas Zimmermann
Thanks for the update!
> --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> @@ -258,7 +256,15 @@ static int shmob_drm_probe(struct platform_device *pdev)
> goto err_modeset_cleanup;
> }
>
> - ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
> + ret = platform_get_irq(pdev, 0);
> + if (ret) {
if (ret < 0) {
> + dev_err(&pdev->dev, "failed to get IRQ number\n");
platform_get_irq() already prints an error message, so no need to
repeat it.
> + goto err_modeset_cleanup;
> + }
> + sdev->irq = ret;
> +
> + ret = request_irq(sdev->irq, shmob_drm_irq, 0, ddev->driver->name,
> + ddev);
> if (ret < 0) {
> dev_err(&pdev->dev, "failed to install IRQ handler\n");
> goto err_modeset_cleanup;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v4 02/18] drm/sched: Barriers are needed for entity->last_scheduled
On Wed, Jul 14, 2021 at 06:12:54PM -0400, Andrey Grodzovsky wrote: > > On 2021-07-13 12:45 p.m., Daniel Vetter wrote: > > On Tue, Jul 13, 2021 at 6:11 PM Andrey Grodzovsky > > wrote: > > > On 2021-07-13 5:10 a.m., Daniel Vetter wrote: > > > > On Tue, Jul 13, 2021 at 9:25 AM Christian König > > > > wrote: > > > > > Am 13.07.21 um 08:50 schrieb Daniel Vetter: > > > > > > On Tue, Jul 13, 2021 at 8:35 AM Christian König > > > > > > wrote: > > > > > > > Am 12.07.21 um 19:53 schrieb Daniel Vetter: > > > > > > > > It might be good enough on x86 with just READ_ONCE, but the > > > > > > > > write side > > > > > > > > should then at least be WRITE_ONCE because x86 has total store > > > > > > > > order. > > > > > > > > > > > > > > > > It's definitely not enough on arm. > > > > > > > > > > > > > > > > Fix this proplery, which means > > > > > > > > - explain the need for the barrier in both places > > > > > > > > - point at the other side in each comment > > > > > > > > > > > > > > > > Also pull out the !sched_list case as the first check, so that > > > > > > > > the > > > > > > > > code flow is clearer. > > > > > > > > > > > > > > > > While at it sprinkle some comments around because it was very > > > > > > > > non-obvious to me what's actually going on here and why. > > > > > > > > > > > > > > > > Note that we really need full barriers here, at first I thought > > > > > > > > store-release and load-acquire on ->last_scheduled would be > > > > > > > > enough, > > > > > > > > but we actually requiring ordering between that and the queue > > > > > > > > state. > > > > > > > > > > > > > > > > v2: Put smp_rmp() in the right place and fix up comment (Andrey) > > > > > > > > > > > > > > > > Signed-off-by: Daniel Vetter > > > > > > > > Cc: "Christian König" > > > > > > > > Cc: Steven Price > > > > > > > > Cc: Daniel Vetter > > > > > > > > Cc: Andrey Grodzovsky > > > > > > > > Cc: Lee Jones > > > > > > > > Cc: Boris Brezillon > > > > > > > > --- > > > > > > > > drivers/gpu/drm/scheduler/sched_entity.c | 27 > > > > > > > > ++-- > > > > > > > > 1 file changed, 25 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/scheduler/sched_entity.c > > > > > > > > b/drivers/gpu/drm/scheduler/sched_entity.c > > > > > > > > index f7347c284886..89e3f6eaf519 100644 > > > > > > > > --- a/drivers/gpu/drm/scheduler/sched_entity.c > > > > > > > > +++ b/drivers/gpu/drm/scheduler/sched_entity.c > > > > > > > > @@ -439,8 +439,16 @@ struct drm_sched_job > > > > > > > > *drm_sched_entity_pop_job(struct drm_sched_entity *entity) > > > > > > > > > > > > > > > > dma_fence_set_error(&sched_job->s_fence->finished, -ECANCELED); > > > > > > > > > > > > > > > > dma_fence_put(entity->last_scheduled); > > > > > > > > + > > > > > > > > entity->last_scheduled = > > > > > > > > dma_fence_get(&sched_job->s_fence->finished); > > > > > > > > > > > > > > > > + /* > > > > > > > > + * If the queue is empty we allow > > > > > > > > drm_sched_entity_select_rq() to > > > > > > > > + * locklessly access ->last_scheduled. This only works if > > > > > > > > we set the > > > > > > > > + * pointer before we dequeue and if we a write barrier > > > > > > > > here. > > > > > > > > + */ > > > > > > > > + smp_wmb(); > > > > > > > > + > > > > > > > Again, conceptual those barriers should be part of the spsc_queue > > > > > > > container and not externally. > > > > > > That would be extremely unusual api. Let's assume that your queue is > > > > > > very dumb, and protected by a simple lock. That's about the maximum > > > > > > any user could expect. > > > > > > > > > > > > But then you still need barriers here, because linux locks > > > > > > (spinlock, > > > > > > mutex) are defined to be one-way barriers: Stuff that's inside is > > > > > > guaranteed to be done insinde, but stuff outside of the locked > > > > > > region > > > > > > can leak in. They're load-acquire/store-release barriers. So not > > > > > > good > > > > > > enough. > > > > > > > > > > > > You really need to have barriers here, and they really all need to > > > > > > be > > > > > > documented properly. And yes that's a shit-ton of work in drm/sched, > > > > > > because it's full of yolo lockless stuff. > > > > > > > > > > > > The other case you could make is that this works like a wakeup > > > > > > queue, > > > > > > or similar. The rules there are: > > > > > > - wake_up (i.e. pushing something into the queue) is a > > > > > > store-release barrier > > > > > > - the waked up (i.e. popping an entry) is a load acquire barrier > > > > > > Which is obviuosly needed because otherwise you don't have coherency > > > > > > for the data queued up. And again not the barriers you're locking > > > > > > for > > > > > > here. > > > > > Exactly that was the idea, yes. > > > > > > > > > > > Either way, we'd still need the comments, because it's still > >
Re: [PATCH] drm/mediatek: dpi: fix NULL dereference in mtk_dpi_bridge_atomic_check
On Mon, Jul 12, 2021 at 4:08 PM Frank Wunderlich wrote:
>
> From: Frank Wunderlich
>
> bridge->driver_private is not set (NULL) so use bridge_to_dpi(bridge)
> like it's done in bridge_atomic_get_output_bus_fmts
>
> Fixes: ec8747c52434 ("drm/mediatek: dpi: Add bus format negotiation")
> Signed-off-by: Frank Wunderlich
Tested-by: Hsin-Yi Wang
Tested on a mt8183 device.
> ---
> drivers/gpu/drm/mediatek/mtk_dpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index bced555648b0..a2eca1f66984 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -605,7 +605,7 @@ static int mtk_dpi_bridge_atomic_check(struct drm_bridge
> *bridge,
>struct drm_crtc_state *crtc_state,
>struct drm_connector_state *conn_state)
> {
> - struct mtk_dpi *dpi = bridge->driver_private;
> + struct mtk_dpi *dpi = bridge_to_dpi(bridge);
> unsigned int out_bus_format;
>
> out_bus_format = bridge_state->output_bus_cfg.format;
> --
> 2.25.1
>
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH v2] drm/of: free the iterator object on failure
On 14/07/2021 16:26, Laurent Pinchart wrote:
> Hi Steven,
>
> Thank you for the patch.
>
> On Wed, Jul 14, 2021 at 03:33:00PM +0100, Steven Price wrote:
>> When bailing out due to the sanity check the iterator value needs to be
>> freed because the early return prevents for_each_child_of_node() from
>> doing the dereference itself.
>>
>> Fixes: 6529007522de ("drm: of: Add drm_of_lvds_get_dual_link_pixel_order")
>> Signed-off-by: Steven Price
>
> Reviewed-by: Laurent Pinchart
Thanks! Applied to drm-misc-next.
Steve
>> ---
>> drivers/gpu/drm/drm_of.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> v2: Fixes now refers to the original commit as suggested by Laurent, rather
>> than 4ee48cc5586b ("drm: of: Fix double-free bug") which only fixed part of
>> the problem. Note that 4ee48cc5586b is a dependency for this patch to
>> cleanly apply.
>>
>> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
>> index 197c57477344..997b8827fed2 100644
>> --- a/drivers/gpu/drm/drm_of.c
>> +++ b/drivers/gpu/drm/drm_of.c
>> @@ -331,8 +331,10 @@ static int drm_of_lvds_get_remote_pixels_type(
>> * configurations by passing the endpoints explicitly to
>> * drm_of_lvds_get_dual_link_pixel_order().
>> */
>> -if (!current_pt || pixels_type != current_pt)
>> +if (!current_pt || pixels_type != current_pt) {
>> +of_node_put(endpoint);
>> return -EINVAL;
>> +}
>> }
>>
>> return pixels_type;
>
Re: [PATCH 3/4] drm/i915/userptr: Probe existence of backing struct pages upon creation
On 15/07/2021 11:15, Matthew Auld wrote:
From: Chris Wilson
Jason Ekstrand requested a more efficient method than userptr+set-domain
to determine if the userptr object was backed by a complete set of pages
upon creation. To be more efficient than simply populating the userptr
using get_user_pages() (as done by the call to set-domain or execbuf),
we can walk the tree of vm_area_struct and check for gaps or vma not
backed by struct page (VM_PFNMAP). The question is how to handle
VM_MIXEDMAP which may be either struct page or pfn backed...
With discrete are going to drop support for set_domain(), so offering a
way to probe the pages, without having to resort to dummy batches has
been requested.
v2:
- add new query param for the PROPBE flag, so userspace can easily
check if the kernel supports it(Jason).
- use mmap_read_{lock, unlock}.
- add some kernel-doc.
1)
I think probing is too weak to be offered as part of the uapi. What
probes successfully at create time might not be there anymore at usage
time. So if the pointer is not trusted at one point, why should it be at
a later stage?
Only thing which works for me is populate (so get_pages) at create time.
But again with no guarantees they are still there at use time clearly
documented.
2)
I am also not a fan of getparam for individual ioctl flags since I don't
think it scales nicely. How about add a param which returns all
supported flags like I915_PARAM_USERPTR_SUPPORTED_FLAGS?
Downside is it only works for 32-bit flag fields with getparam. Or it
could be a query to solve that as well.
Regards,
Tvrtko
Testcase: igt/gem_userptr_blits/probe
Signed-off-by: Chris Wilson
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Maarten Lankhorst
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Ramalingam C
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 40 -
drivers/gpu/drm/i915/i915_getparam.c| 3 ++
include/uapi/drm/i915_drm.h | 18 ++
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 56edfeff8c02..fd6880328596 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -422,6 +422,33 @@ static const struct drm_i915_gem_object_ops
i915_gem_userptr_ops = {
#endif
+static int
+probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
+{
+ const unsigned long end = addr + len;
+ struct vm_area_struct *vma;
+ int ret = -EFAULT;
+
+ mmap_read_lock(mm);
+ for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
+ if (vma->vm_start > addr)
+ break;
+
+ if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
+ break;
+
+ if (vma->vm_end >= end) {
+ ret = 0;
+ break;
+ }
+
+ addr = vma->vm_end;
+ }
+ mmap_read_unlock(mm);
+
+ return ret;
+}
+
/*
* Creates a new mm object that wraps some normal memory from the process
* context - user memory.
@@ -477,7 +504,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
}
if (args->flags & ~(I915_USERPTR_READ_ONLY |
- I915_USERPTR_UNSYNCHRONIZED))
+ I915_USERPTR_UNSYNCHRONIZED |
+ I915_USERPTR_PROBE))
return -EINVAL;
if (i915_gem_object_size_2big(args->user_size))
@@ -504,6 +532,16 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
return -ENODEV;
}
+ if (args->flags & I915_USERPTR_PROBE) {
+ /*
+* Check that the range pointed to represents real struct
+* pages and not iomappings (at this moment in time!)
+*/
+ ret = probe_range(current->mm, args->user_ptr, args->user_size);
+ if (ret)
+ return ret;
+ }
+
#ifdef CONFIG_MMU_NOTIFIER
obj = i915_gem_object_alloc();
if (obj == NULL)
diff --git a/drivers/gpu/drm/i915/i915_getparam.c
b/drivers/gpu/drm/i915/i915_getparam.c
index 24e18219eb50..d6d2e1a10d14 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -163,6 +163,9 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
case I915_PARAM_PERF_REVISION:
value = i915_perf_ioctl_version();
break;
+ case I915_PARAM_HAS_USERPTR_PROBE:
+ value = true;
+ break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
return -EINVAL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e20eeeca7a1c..2e4112bf4d38 100644
--- a/include/uapi/drm/i91
Re: [PATCH v2] drm/shmobile: Convert to Linux IRQ interfaces
Am 15.07.21 um 12:16 schrieb Geert Uytterhoeven:
Hi Thomas,
On Thu, Jul 15, 2021 at 11:57 AM Thomas Zimmermann wrote:
Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.
v2:
* handle errors in platform_get_irq() (Geert, Sergei)
* store IRQ number in struct shmob_drm_device (Laurent)
Signed-off-by: Thomas Zimmermann
Thanks for the update!
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -258,7 +256,15 @@ static int shmob_drm_probe(struct platform_device *pdev)
goto err_modeset_cleanup;
}
- ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
+ ret = platform_get_irq(pdev, 0);
+ if (ret) {
if (ret < 0) {
Indeed :/
+ dev_err(&pdev->dev, "failed to get IRQ number\n");
platform_get_irq() already prints an error message, so no need to
repeat it.
+ goto err_modeset_cleanup;
+ }
+ sdev->irq = ret;
+
+ ret = request_irq(sdev->irq, shmob_drm_irq, 0, ddev->driver->name,
+ ddev);
if (ret < 0) {
dev_err(&pdev->dev, "failed to install IRQ handler\n");
goto err_modeset_cleanup;
Gr{oetje,eeting}s,
Geert
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v2] drm/ingenic: Convert to Linux IRQ interfaces
On 2021-07-15 12:02, Thomas Zimmermann wrote:
Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.
This patch also fixes a bug where the driver didn't release the
IRQ.
v2:
* automatically release IRQ via devm_request_irq() (Paul)
* mention the bugfix (Sam)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Sam Ravnborg
Reviewed-by: Paul Cercueil
Cheers,
-Paul
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index c296472164d9..857ed070b21b 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -33,7 +33,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -799,8 +798,6 @@ static const struct drm_driver
ingenic_drm_driver_data = {
.fops = &ingenic_drm_fops,
.gem_create_object = ingenic_drm_gem_create_object,
DRM_GEM_CMA_DRIVER_OPS,
-
- .irq_handler= ingenic_drm_irq_handler,
};
static const struct drm_plane_funcs ingenic_drm_primary_plane_funcs =
{
@@ -1098,7 +1095,7 @@ static int ingenic_drm_bind(struct device *dev,
bool has_components)
encoder->possible_clones = clone_mask;
}
- ret = drm_irq_install(drm, irq);
+ ret = devm_request_irq(dev, irq, ingenic_drm_irq_handler, 0,
drm->driver->name, drm);
if (ret) {
dev_err(dev, "Unable to install IRQ handler\n");
return ret;
Re: [PATCH -next v2] drm/bochs: Fix missing pci_disable_device() on error in bochs_pci_probe()
On 2021/7/15 17:30, Thomas Zimmermann wrote: Hi, for the change Acked-by: Thomas Zimmermann but there are some style issues AFAICS. OK, need I resend it with the style changes and your ack ? Am 15.07.21 um 04:05 schrieb Yang Yingliang: Replace pci_enable_device() with pcim_enable_device(), pci_disable_device() will be called in release automatically. Reported-by: Hulk Robot Signed-off-by: Yang Yingliang S-o-b line goes first --- v2: use pcim_enable_device() This changelog should rather be located between the commit description and the first S-o-b line. Best regards Thomas --- drivers/gpu/drm/tiny/bochs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c index a2cfecfa8556..73415fa9ae0f 100644 --- a/drivers/gpu/drm/tiny/bochs.c +++ b/drivers/gpu/drm/tiny/bochs.c @@ -648,7 +648,7 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent if (IS_ERR(dev)) return PTR_ERR(dev); - ret = pci_enable_device(pdev); + ret = pcim_enable_device(pdev); if (ret) goto err_free_dev;
Re: [PATCH 3/4] drm/i915/userptr: Probe existence of backing struct pages upon creation
On Thu, Jul 15, 2021 at 11:33:10AM +0100, Tvrtko Ursulin wrote:
>
> On 15/07/2021 11:15, Matthew Auld wrote:
> > From: Chris Wilson
> >
> > Jason Ekstrand requested a more efficient method than userptr+set-domain
> > to determine if the userptr object was backed by a complete set of pages
> > upon creation. To be more efficient than simply populating the userptr
> > using get_user_pages() (as done by the call to set-domain or execbuf),
> > we can walk the tree of vm_area_struct and check for gaps or vma not
> > backed by struct page (VM_PFNMAP). The question is how to handle
> > VM_MIXEDMAP which may be either struct page or pfn backed...
> >
> > With discrete are going to drop support for set_domain(), so offering a
> > way to probe the pages, without having to resort to dummy batches has
> > been requested.
> >
> > v2:
> > - add new query param for the PROPBE flag, so userspace can easily
> >check if the kernel supports it(Jason).
> > - use mmap_read_{lock, unlock}.
> > - add some kernel-doc.
>
> 1)
>
> I think probing is too weak to be offered as part of the uapi. What probes
> successfully at create time might not be there anymore at usage time. So if
> the pointer is not trusted at one point, why should it be at a later stage?
>
> Only thing which works for me is populate (so get_pages) at create time. But
> again with no guarantees they are still there at use time clearly
> documented.
Populate is exactly as racy as probe. We don't support pinned userptr
anymore.
> 2)
>
> I am also not a fan of getparam for individual ioctl flags since I don't
> think it scales nicely. How about add a param which returns all supported
> flags like I915_PARAM_USERPTR_SUPPORTED_FLAGS?
>
> Downside is it only works for 32-bit flag fields with getparam. Or it could
> be a query to solve that as well.
I expect the actual userspace code will simply try with the probe flag
first, and then without + set_domain. So strictly speaking we might not
even have a need for the getparam.
Otoh, let's not overthink/engineer this, whatever works for userspace is
ok imo. A new query that lists all flags is the kind of fake-generic stuff
that will like mis-estimate where the future actually lands, e.g. how do
you encode if we add extensions to userptr to this? Which is something we
absolutely can, by extending the struct at the end, which doesn't even
need a new flag.
Let's solve the immediate problem at hand, and not try to guess what more
problems we might have in the future.
-Daniel
> Regards,
>
> Tvrtko
>
> > Testcase: igt/gem_userptr_blits/probe
> > Signed-off-by: Chris Wilson
> > Signed-off-by: Matthew Auld
> > Cc: Thomas Hellström
> > Cc: Maarten Lankhorst
> > Cc: Tvrtko Ursulin
> > Cc: Jordan Justen
> > Cc: Kenneth Graunke
> > Cc: Jason Ekstrand
> > Cc: Daniel Vetter
> > Cc: Ramalingam C
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 40 -
> > drivers/gpu/drm/i915/i915_getparam.c| 3 ++
> > include/uapi/drm/i915_drm.h | 18 ++
> > 3 files changed, 60 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > index 56edfeff8c02..fd6880328596 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > @@ -422,6 +422,33 @@ static const struct drm_i915_gem_object_ops
> > i915_gem_userptr_ops = {
> > #endif
> > +static int
> > +probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
> > +{
> > + const unsigned long end = addr + len;
> > + struct vm_area_struct *vma;
> > + int ret = -EFAULT;
> > +
> > + mmap_read_lock(mm);
> > + for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
> > + if (vma->vm_start > addr)
> > + break;
> > +
> > + if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
> > + break;
> > +
> > + if (vma->vm_end >= end) {
> > + ret = 0;
> > + break;
> > + }
> > +
> > + addr = vma->vm_end;
> > + }
> > + mmap_read_unlock(mm);
> > +
> > + return ret;
> > +}
> > +
> > /*
> >* Creates a new mm object that wraps some normal memory from the process
> >* context - user memory.
> > @@ -477,7 +504,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
> > }
> > if (args->flags & ~(I915_USERPTR_READ_ONLY |
> > - I915_USERPTR_UNSYNCHRONIZED))
> > + I915_USERPTR_UNSYNCHRONIZED |
> > + I915_USERPTR_PROBE))
> > return -EINVAL;
> > if (i915_gem_object_size_2big(args->user_size))
> > @@ -504,6 +532,16 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
> > return -ENODEV;
> > }
> > + if (args->flags & I915_USERPTR_PROBE) {
> > + /*
> > +* Check that the range pointed to represents real struct
> >
Re: [PATCH v6 RESEND 2/3] drm/mediatek: init panel orientation property
Hi Hsin-Yi,
Thank you for your patch.
Missatge de Hsin-Yi Wang del dia dj., 24 de juny
2021 a les 12:55:
>
> Init panel orientation property after connector is initialized. Let the
> panel driver decides the orientation value later.
>
> Signed-off-by: Hsin-Yi Wang
> Acked-by: Chun-Kuang Hu
Tested-by: Enric Balletbo i Serra
As together with the other two patches works and I don't see any
problem on the Lenovo IdeaPad Duet, and the panel has the proper
orientation
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index ae403c67cbd92..9da1fd6491319 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -964,6 +964,13 @@ static int mtk_dsi_encoder_init(struct drm_device *drm,
> struct mtk_dsi *dsi)
> ret = PTR_ERR(dsi->connector);
> goto err_cleanup_encoder;
> }
> +
> + ret = drm_connector_init_panel_orientation_property(dsi->connector);
> + if (ret) {
> + DRM_ERROR("Unable to init panel orientation\n");
> + goto err_cleanup_encoder;
> + }
> +
> drm_connector_attach_encoder(dsi->connector, &dsi->encoder);
>
> return 0;
> --
> 2.32.0.288.g62a8d224e6-goog
>
Re: [Intel-gfx] [PATCH 3/4] drm/i915/userptr: Probe existence of backing struct pages upon creation
On Thu, 15 Jul 2021 at 11:33, Tvrtko Ursulin
wrote:
>
>
> On 15/07/2021 11:15, Matthew Auld wrote:
> > From: Chris Wilson
> >
> > Jason Ekstrand requested a more efficient method than userptr+set-domain
> > to determine if the userptr object was backed by a complete set of pages
> > upon creation. To be more efficient than simply populating the userptr
> > using get_user_pages() (as done by the call to set-domain or execbuf),
> > we can walk the tree of vm_area_struct and check for gaps or vma not
> > backed by struct page (VM_PFNMAP). The question is how to handle
> > VM_MIXEDMAP which may be either struct page or pfn backed...
> >
> > With discrete are going to drop support for set_domain(), so offering a
> > way to probe the pages, without having to resort to dummy batches has
> > been requested.
> >
> > v2:
> > - add new query param for the PROPBE flag, so userspace can easily
> >check if the kernel supports it(Jason).
> > - use mmap_read_{lock, unlock}.
> > - add some kernel-doc.
>
> 1)
>
> I think probing is too weak to be offered as part of the uapi. What
> probes successfully at create time might not be there anymore at usage
> time. So if the pointer is not trusted at one point, why should it be at
> a later stage?
>
> Only thing which works for me is populate (so get_pages) at create time.
> But again with no guarantees they are still there at use time clearly
> documented.
>
> 2)
>
> I am also not a fan of getparam for individual ioctl flags since I don't
> think it scales nicely. How about add a param which returns all
> supported flags like I915_PARAM_USERPTR_SUPPORTED_FLAGS?
>
> Downside is it only works for 32-bit flag fields with getparam. Or it
> could be a query to solve that as well.
I guess. You don't think it's a little iffy though, since there were
other flags which were added before this? So effectively userspace
queries SUPPORTED_FLAGS and might get -EINVAL on older kernels, even
though the flag is supported on that kernel(like READONLY)?
Maybe a versioning scheme instead? I915_PARAM_USERPTR_VERSION? Seems
quite common for other params.
>
> Regards,
>
> Tvrtko
>
> > Testcase: igt/gem_userptr_blits/probe
> > Signed-off-by: Chris Wilson
> > Signed-off-by: Matthew Auld
> > Cc: Thomas Hellström
> > Cc: Maarten Lankhorst
> > Cc: Tvrtko Ursulin
> > Cc: Jordan Justen
> > Cc: Kenneth Graunke
> > Cc: Jason Ekstrand
> > Cc: Daniel Vetter
> > Cc: Ramalingam C
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 40 -
> > drivers/gpu/drm/i915/i915_getparam.c| 3 ++
> > include/uapi/drm/i915_drm.h | 18 ++
> > 3 files changed, 60 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > index 56edfeff8c02..fd6880328596 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > @@ -422,6 +422,33 @@ static const struct drm_i915_gem_object_ops
> > i915_gem_userptr_ops = {
> >
> > #endif
> >
> > +static int
> > +probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
> > +{
> > + const unsigned long end = addr + len;
> > + struct vm_area_struct *vma;
> > + int ret = -EFAULT;
> > +
> > + mmap_read_lock(mm);
> > + for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
> > + if (vma->vm_start > addr)
> > + break;
> > +
> > + if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
> > + break;
> > +
> > + if (vma->vm_end >= end) {
> > + ret = 0;
> > + break;
> > + }
> > +
> > + addr = vma->vm_end;
> > + }
> > + mmap_read_unlock(mm);
> > +
> > + return ret;
> > +}
> > +
> > /*
> >* Creates a new mm object that wraps some normal memory from the process
> >* context - user memory.
> > @@ -477,7 +504,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
> > }
> >
> > if (args->flags & ~(I915_USERPTR_READ_ONLY |
> > - I915_USERPTR_UNSYNCHRONIZED))
> > + I915_USERPTR_UNSYNCHRONIZED |
> > + I915_USERPTR_PROBE))
> > return -EINVAL;
> >
> > if (i915_gem_object_size_2big(args->user_size))
> > @@ -504,6 +532,16 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
> > return -ENODEV;
> > }
> >
> > + if (args->flags & I915_USERPTR_PROBE) {
> > + /*
> > + * Check that the range pointed to represents real struct
> > + * pages and not iomappings (at this moment in time!)
> > + */
> > + ret = probe_range(current->mm, args->user_ptr,
> > args->user_size);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > #ifdef CONFIG_MMU_NOTIFIER
> > obj = i915_gem_object_alloc();
Re: [PATCH v6 RESEND 3/3] arm64: dts: mt8183: Add panel rotation
Hi Hsin-Yi,
Thank you for the patch.
Missatge de Hsin-Yi Wang del dia dj., 27 de maig
2021 a les 9:42:
>
> krane, kakadu, and kodama boards have a default panel rotation.
>
> Signed-off-by: Hsin-Yi Wang
It looks good to me, so
Reviewed-by: Enric Balletbo i Serra
and, on a Lenovo IdeaPad Duet. The display appears well rotated now.
Tested-by: Enric Balletbo i Serra
Thanks,
Enric
> ---
> arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index ff56bcfa3370..793cc9501337 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> @@ -263,6 +263,7 @@ panel: panel@0 {
> avee-supply = <&ppvarp_lcd>;
> pp1800-supply = <&pp1800_lcd>;
> backlight = <&backlight_lcd0>;
> + rotation = <270>;
> port {
> panel_in: endpoint {
> remote-endpoint = <&dsi_out>;
> --
> 2.31.1.818.g46aad6cb9e-goog
>
Re: [PATCH v3] backlight: ktd253: Stabilize backlight
On Tue, 13 Jul 2021, Linus Walleij wrote:
> On Fri, Jun 4, 2021 at 8:34 AM Linus Walleij wrote:
>
> > Remove interrupt disablement during backlight setting. It is
> > way to dangerous and makes platforms instable by having it
> > miss vblank IRQs leading to the graphics derailing.
> >
> > The code is using ndelay() which is not available on
> > platforms such as ARM and will result in 32 * udelay(1)
> > which is substantial.
> >
> > Add some code to detect if an interrupt occurs during the
> > tight loop and in that case just redo it from the top.
> >
> > Fixes: 5317f37e48b9 ("backlight: Add Kinetic KTD253 backlight driver")
> > Cc: Stephan Gerhold
> > Reported-by: newb...@disroot.org
> > Signed-off-by: Linus Walleij
>
> Hm it seems this patch did not make it into v5.14-rc1, could it be applied
> as a fix for the -rc:s?
Ah, it was sent late in the cycle, so I postponed it.
> Shall I resend it with Daniel's ACK?
Yes please.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 3/4] drm/i915/userptr: Probe existence of backing struct pages upon creation
On 15/07/2021 12:07, Daniel Vetter wrote:
On Thu, Jul 15, 2021 at 11:33:10AM +0100, Tvrtko Ursulin wrote:
On 15/07/2021 11:15, Matthew Auld wrote:
From: Chris Wilson
Jason Ekstrand requested a more efficient method than userptr+set-domain
to determine if the userptr object was backed by a complete set of pages
upon creation. To be more efficient than simply populating the userptr
using get_user_pages() (as done by the call to set-domain or execbuf),
we can walk the tree of vm_area_struct and check for gaps or vma not
backed by struct page (VM_PFNMAP). The question is how to handle
VM_MIXEDMAP which may be either struct page or pfn backed...
With discrete are going to drop support for set_domain(), so offering a
way to probe the pages, without having to resort to dummy batches has
been requested.
v2:
- add new query param for the PROPBE flag, so userspace can easily
check if the kernel supports it(Jason).
- use mmap_read_{lock, unlock}.
- add some kernel-doc.
1)
I think probing is too weak to be offered as part of the uapi. What probes
successfully at create time might not be there anymore at usage time. So if
the pointer is not trusted at one point, why should it be at a later stage?
Only thing which works for me is populate (so get_pages) at create time. But
again with no guarantees they are still there at use time clearly
documented.
Populate is exactly as racy as probe. We don't support pinned userptr
anymore.
Yes, wrote so myself - "..again with no guarantees they are still there
at use time..".
Perhaps I don't understand what problem is probe supposed to solve. It
doesn't deal 1:1 with set_domain removal since that one actually did
get_pages so that would be populate. But fact remains regardless that if
userspace is given a pointer it doesn't trust, _and_ wants the check it
for this reason or that, then probe solves nothing. Unless there is
actually at minimum some protocol to reply to whoever sent the pointer
like "not that pointer please".
2)
I am also not a fan of getparam for individual ioctl flags since I don't
think it scales nicely. How about add a param which returns all supported
flags like I915_PARAM_USERPTR_SUPPORTED_FLAGS?
Downside is it only works for 32-bit flag fields with getparam. Or it could
be a query to solve that as well.
I expect the actual userspace code will simply try with the probe flag
first, and then without + set_domain. So strictly speaking we might not
even have a need for the getparam.
Otoh, let's not overthink/engineer this, whatever works for userspace is
ok imo. A new query that lists all flags is the kind of fake-generic stuff
that will like mis-estimate where the future actually lands, e.g. how do
you encode if we add extensions to userptr to this? Which is something we
absolutely can, by extending the struct at the end, which doesn't even
need a new flag.
Let's solve the immediate problem at hand, and not try to guess what more
problems we might have in the future.
Yeah I am guessing if anyone is using some of the other userptr flags
they don't have a getparam for that either. At least that would be
consolidated with I915_PARAM_USERPTR_SUPPORTED_FLAGS. There is no
requirement that getparam needs to keep supporting future extensions to
the struct itself, equally as I915_PARAM_HAS_USERPTR_PROBE is directly
tied to the same flags field, only a subset of it.
Regards,
Tvrtko
-Daniel
Regards,
Tvrtko
Testcase: igt/gem_userptr_blits/probe
Signed-off-by: Chris Wilson
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Maarten Lankhorst
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Ramalingam C
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 40 -
drivers/gpu/drm/i915/i915_getparam.c| 3 ++
include/uapi/drm/i915_drm.h | 18 ++
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 56edfeff8c02..fd6880328596 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -422,6 +422,33 @@ static const struct drm_i915_gem_object_ops
i915_gem_userptr_ops = {
#endif
+static int
+probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
+{
+ const unsigned long end = addr + len;
+ struct vm_area_struct *vma;
+ int ret = -EFAULT;
+
+ mmap_read_lock(mm);
+ for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
+ if (vma->vm_start > addr)
+ break;
+
+ if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
+ break;
+
+ if (vma->vm_end >= end) {
+ ret = 0;
+ break;
+ }
+
+ addr = vma->vm_end;
+ }
+ mmap_read_unlock(mm);
+
+ return ret;
+}
Re: [Intel-gfx] [PATCH 3/4] drm/i915/userptr: Probe existence of backing struct pages upon creation
On 15/07/2021 12:09, Matthew Auld wrote:
On Thu, 15 Jul 2021 at 11:33, Tvrtko Ursulin
wrote:
On 15/07/2021 11:15, Matthew Auld wrote:
From: Chris Wilson
Jason Ekstrand requested a more efficient method than userptr+set-domain
to determine if the userptr object was backed by a complete set of pages
upon creation. To be more efficient than simply populating the userptr
using get_user_pages() (as done by the call to set-domain or execbuf),
we can walk the tree of vm_area_struct and check for gaps or vma not
backed by struct page (VM_PFNMAP). The question is how to handle
VM_MIXEDMAP which may be either struct page or pfn backed...
With discrete are going to drop support for set_domain(), so offering a
way to probe the pages, without having to resort to dummy batches has
been requested.
v2:
- add new query param for the PROPBE flag, so userspace can easily
check if the kernel supports it(Jason).
- use mmap_read_{lock, unlock}.
- add some kernel-doc.
1)
I think probing is too weak to be offered as part of the uapi. What
probes successfully at create time might not be there anymore at usage
time. So if the pointer is not trusted at one point, why should it be at
a later stage?
Only thing which works for me is populate (so get_pages) at create time.
But again with no guarantees they are still there at use time clearly
documented.
2)
I am also not a fan of getparam for individual ioctl flags since I don't
think it scales nicely. How about add a param which returns all
supported flags like I915_PARAM_USERPTR_SUPPORTED_FLAGS?
Downside is it only works for 32-bit flag fields with getparam. Or it
could be a query to solve that as well.
I guess. You don't think it's a little iffy though, since there were
other flags which were added before this? So effectively userspace
queries SUPPORTED_FLAGS and might get -EINVAL on older kernels, even
though the flag is supported on that kernel(like READONLY)?
For me it is probably passable since unsupported getparam fundamentally
doesn't imply unsupported features predating that getparam. Same as for
example unsupported engine query does not mean there are no engines. But
anyway, seems question will be resolved by Daniel so don't pay attention
to me.
Regards,
Tvrtko
Maybe a versioning scheme instead? I915_PARAM_USERPTR_VERSION? Seems
quite common for other params.
Regards,
Tvrtko
Testcase: igt/gem_userptr_blits/probe
Signed-off-by: Chris Wilson
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Maarten Lankhorst
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Ramalingam C
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 40 -
drivers/gpu/drm/i915/i915_getparam.c| 3 ++
include/uapi/drm/i915_drm.h | 18 ++
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 56edfeff8c02..fd6880328596 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -422,6 +422,33 @@ static const struct drm_i915_gem_object_ops
i915_gem_userptr_ops = {
#endif
+static int
+probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
+{
+ const unsigned long end = addr + len;
+ struct vm_area_struct *vma;
+ int ret = -EFAULT;
+
+ mmap_read_lock(mm);
+ for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
+ if (vma->vm_start > addr)
+ break;
+
+ if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
+ break;
+
+ if (vma->vm_end >= end) {
+ ret = 0;
+ break;
+ }
+
+ addr = vma->vm_end;
+ }
+ mmap_read_unlock(mm);
+
+ return ret;
+}
+
/*
* Creates a new mm object that wraps some normal memory from the process
* context - user memory.
@@ -477,7 +504,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
}
if (args->flags & ~(I915_USERPTR_READ_ONLY |
- I915_USERPTR_UNSYNCHRONIZED))
+ I915_USERPTR_UNSYNCHRONIZED |
+ I915_USERPTR_PROBE))
return -EINVAL;
if (i915_gem_object_size_2big(args->user_size))
@@ -504,6 +532,16 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
return -ENODEV;
}
+ if (args->flags & I915_USERPTR_PROBE) {
+ /*
+ * Check that the range pointed to represents real struct
+ * pages and not iomappings (at this moment in time!)
+ */
+ ret = probe_range(current->mm, args->user_ptr, args->user_size);
+ if (ret)
+ return ret;
+ }
+
#ifdef CONFIG_MMU_NOTIFIER
obj = i915_gem_object_alloc();
if (obj == NULL)
diff --git a/drivers/gpu/drm/i9
[PATCH v4] backlight: ktd253: Stabilize backlight
Remove interrupt disablement during backlight setting. It is
way to dangerous and makes platforms instable by having it
miss vblank IRQs leading to the graphics derailing.
The code is using ndelay() which is not available on
platforms such as ARM and will result in 32 * udelay(1)
which is substantial.
Add some code to detect if an interrupt occurs during the
tight loop and in that case just redo it from the top.
Fixes: 5317f37e48b9 ("backlight: Add Kinetic KTD253 backlight driver")
Cc: Stephan Gerhold
Reported-by: newb...@disroot.org
Reviewed-by: Daniel Thompson
Signed-off-by: Linus Walleij
---
ChangeLog v3->v4:
- Collect Daniel's Reviewed-by.
ChangeLog v2->v3:
- Read my own patch and realized a bug: when we get a timeout
we bounce back to max period, but still count down the pwm
with one leading to an off-by-one error. Fix it by extending
some else clauses.
ChangeLog v1->v2:
- Alter the dimming code to check for how many ns the pulse
is low, and if it gets to ~100 us then redo from start.
This is to account for the advent that an IRQ arrives while
setting backlight and hits the low pulse making it way
too long.
---
drivers/video/backlight/ktd253-backlight.c | 75 --
1 file changed, 55 insertions(+), 20 deletions(-)
diff --git a/drivers/video/backlight/ktd253-backlight.c
b/drivers/video/backlight/ktd253-backlight.c
index a7df5bcca9da..37aa5a669530 100644
--- a/drivers/video/backlight/ktd253-backlight.c
+++ b/drivers/video/backlight/ktd253-backlight.c
@@ -25,6 +25,7 @@
#define KTD253_T_LOW_NS (200 + 10) /* Additional 10ns as safety factor */
#define KTD253_T_HIGH_NS (200 + 10) /* Additional 10ns as safety factor */
+#define KTD253_T_OFF_CRIT_NS 10 /* 100 us, now it doesn't look good */
#define KTD253_T_OFF_MS 3
struct ktd253_backlight {
@@ -34,13 +35,50 @@ struct ktd253_backlight {
u16 ratio;
};
+static void ktd253_backlight_set_max_ratio(struct ktd253_backlight *ktd253)
+{
+ gpiod_set_value_cansleep(ktd253->gpiod, 1);
+ ndelay(KTD253_T_HIGH_NS);
+ /* We always fall back to this when we power on */
+}
+
+static int ktd253_backlight_stepdown(struct ktd253_backlight *ktd253)
+{
+ /*
+* These GPIO operations absolutely can NOT sleep so no _cansleep
+* suffixes, and no using GPIO expanders on slow buses for this!
+*
+* The maximum number of cycles of the loop is 32 so the time taken
+* should nominally be:
+* (T_LOW_NS + T_HIGH_NS + loop_time) * 32
+*
+* Architectures do not always support ndelay() and we will get a few us
+* instead. If we get to a critical time limit an interrupt has likely
+* occured in the low part of the loop and we need to restart from the
+* top so we have the backlight in a known state.
+*/
+ u64 ns;
+
+ ns = ktime_get_ns();
+ gpiod_set_value(ktd253->gpiod, 0);
+ ndelay(KTD253_T_LOW_NS);
+ gpiod_set_value(ktd253->gpiod, 1);
+ ns = ktime_get_ns() - ns;
+ if (ns >= KTD253_T_OFF_CRIT_NS) {
+ dev_err(ktd253->dev, "PCM on backlight took too long (%llu
ns)\n", ns);
+ return -EAGAIN;
+ }
+ ndelay(KTD253_T_HIGH_NS);
+ return 0;
+}
+
static int ktd253_backlight_update_status(struct backlight_device *bl)
{
struct ktd253_backlight *ktd253 = bl_get_data(bl);
int brightness = backlight_get_brightness(bl);
u16 target_ratio;
u16 current_ratio = ktd253->ratio;
- unsigned long flags;
+ int ret;
dev_dbg(ktd253->dev, "new brightness/ratio: %d/32\n", brightness);
@@ -62,37 +100,34 @@ static int ktd253_backlight_update_status(struct
backlight_device *bl)
}
if (current_ratio == 0) {
- gpiod_set_value_cansleep(ktd253->gpiod, 1);
- ndelay(KTD253_T_HIGH_NS);
- /* We always fall back to this when we power on */
+ ktd253_backlight_set_max_ratio(ktd253);
current_ratio = KTD253_MAX_RATIO;
}
- /*
-* WARNING:
-* The loop to set the correct current level is performed
-* with interrupts disabled as it is timing critical.
-* The maximum number of cycles of the loop is 32
-* so the time taken will be (T_LOW_NS + T_HIGH_NS + loop_time) * 32,
-*/
- local_irq_save(flags);
while (current_ratio != target_ratio) {
/*
* These GPIO operations absolutely can NOT sleep so no
* _cansleep suffixes, and no using GPIO expanders on
* slow buses for this!
*/
- gpiod_set_value(ktd253->gpiod, 0);
- ndelay(KTD253_T_LOW_NS);
- gpiod_set_value(ktd253->gpiod, 1);
- ndelay(KTD253_T_HIGH_NS);
- /* After 1/32 we loop back to 32/32 */
- if (current_ratio == KTD253_MIN_RATIO)
+
Re: [PATCH -next v2] drm/bochs: Fix missing pci_disable_device() on error in bochs_pci_probe()
Hi Am 15.07.21 um 13:03 schrieb Yang Yingliang: On 2021/7/15 17:30, Thomas Zimmermann wrote: Hi, for the change Acked-by: Thomas Zimmermann but there are some style issues AFAICS. OK, need I resend it with the style changes and your ack ? Please do. I'll merge it a few days later if no further comments come in. Best regards Thomas Am 15.07.21 um 04:05 schrieb Yang Yingliang: Replace pci_enable_device() with pcim_enable_device(), pci_disable_device() will be called in release automatically. Reported-by: Hulk Robot Signed-off-by: Yang Yingliang S-o-b line goes first --- v2: use pcim_enable_device() This changelog should rather be located between the commit description and the first S-o-b line. Best regards Thomas --- drivers/gpu/drm/tiny/bochs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c index a2cfecfa8556..73415fa9ae0f 100644 --- a/drivers/gpu/drm/tiny/bochs.c +++ b/drivers/gpu/drm/tiny/bochs.c @@ -648,7 +648,7 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent if (IS_ERR(dev)) return PTR_ERR(dev); - ret = pci_enable_device(pdev); + ret = pcim_enable_device(pdev); if (ret) goto err_free_dev; -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 2/4] drm/i915/uapi: convert drm_i915_gem_userptr to kernel doc
Op 15-07-2021 om 12:15 schreef Matthew Auld:
> Add the missing kernel-doc.
>
> Signed-off-by: Matthew Auld
> Cc: Thomas Hellström
> Cc: Maarten Lankhorst
> Cc: Tvrtko Ursulin
> Cc: Jordan Justen
> Cc: Kenneth Graunke
> Cc: Jason Ekstrand
> Cc: Daniel Vetter
> Cc: Ramalingam C
> ---
> include/uapi/drm/i915_drm.h | 40 -
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 868c2ee7be60..e20eeeca7a1c 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -2141,14 +2141,52 @@ struct drm_i915_reset_stats {
> __u32 pad;
> };
>
> +/**
> + * struct drm_i915_gem_userptr - Create GEM object from user allocated
> memory.
> + *
> + * Userptr objects have several restrictions on what ioctls can be used with
> the
> + * object handle.
> + */
> struct drm_i915_gem_userptr {
> + /**
> + * @user_ptr: The pointer to the allocated memory.
> + *
> + * Needs to be aligned to PAGE_SIZE.
> + */
> __u64 user_ptr;
> +
> + /**
> + * @user_size:
> + *
> + * The size in bytes for the allocated memory. This will also become the
> + * object size.
> + *
> + * Needs to be aligned to PAGE_SIZE, and should be at least PAGE_SIZE,
> + * or larger.
> + */
> __u64 user_size;
> +
> + /**
> + * @flags:
> + *
> + * Supported flags:
> + *
> + * I915_USERPTR_READ_ONLY:
> + *
> + * Mark the object as readonly, this also means GPU access can only be
> + * readonly. This is only supported on HW which supports readonly access
> + * through the GTT. If the HW can't support readonly access, an error is
> + * returned.
> + *
> + * I915_USERPTR_UNSYNCHRONIZED:
> + *
> + * NOT USED. Setting this flag will result in an error.
> + */
> __u32 flags;
> #define I915_USERPTR_READ_ONLY 0x1
> #define I915_USERPTR_UNSYNCHRONIZED 0x8000
> /**
> - * Returned handle for the object.
> + * @handle: Returned handle for the object.
>*
>* Object handles are nonzero.
>*/
Reviewed-by: Maarten Lankhorst
Can you review
https://patchwork.freedesktop.org/patch/444147/?series=92359&rev=2
?
I noticed some parts of i915_drm.h missing.
Patch "drm/dp_mst: Do not set proposed vcpi directly" has been added to the 5.10-stable tree
This is a note to let you know that I've just added the patch titled
drm/dp_mst: Do not set proposed vcpi directly
to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-dp_mst-do-not-set-proposed-vcpi-directly.patch
and it can be found in the queue-5.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 35d3e8cb35e75450f87f87e3d314e2d418b6954b Mon Sep 17 00:00:00 2001
From: Wayne Lin
Date: Wed, 16 Jun 2021 11:55:00 +0800
Subject: drm/dp_mst: Do not set proposed vcpi directly
From: Wayne Lin
commit 35d3e8cb35e75450f87f87e3d314e2d418b6954b upstream.
[Why]
When we receive CSN message to notify one port is disconnected, we will
implicitly set its corresponding num_slots to 0. Later on, we will
eventually call drm_dp_update_payload_part1() to arrange down streams.
In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[]
to do the update. Not specific to a target sink only. For example, if we
light up 2 monitors, Monitor_A and Monitor_B, and then we unplug
Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try
to update payload for Monitor_A, we'll also implicitly clean payload for
Monitor_B at the same time. And finally, when we try to call
drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do
nothing at this time since payload for Monitor_B has been cleaned up
previously.
For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload
ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail
and this polling will last for 3 seconds.
Therefore, guess the best way is we don't set the proposed_vcpi[]
diretly. Let user of these herlper functions to set the proposed_vcpi
directly.
[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports
invalid")
2. Tackle the issue in previous commit by skipping those trasient
proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by
user later on.
Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Fixed & Cc tags
Signed-off-by: Wayne Lin
Fixes: 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
Cc: Lyude Paul
Cc: Wayne Lin
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.5+
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616035501.3776-2-wayne@amd.com
Reviewed-by: Lyude Paul
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_dp_mst_topology.c | 36 +-
1 file changed, 10 insertions(+), 26 deletions(-)
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2499,7 +2499,7 @@ drm_dp_mst_handle_conn_stat(struct drm_d
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, old_input, ret, i;
+ int old_ddps, ret;
u8 new_pdt;
bool new_mcs;
bool dowork = false, create_connector = false;
@@ -2531,7 +2531,6 @@ drm_dp_mst_handle_conn_stat(struct drm_d
}
old_ddps = port->ddps;
- old_input = port->input;
port->input = conn_stat->input_port;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2554,28 +2553,6 @@ drm_dp_mst_handle_conn_stat(struct drm_d
dowork = false;
}
- if (!old_input && old_ddps != port->ddps && !port->ddps) {
- for (i = 0; i < mgr->max_payloads; i++) {
- struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
- struct drm_dp_mst_port *port_validated;
-
- if (!vcpi)
- continue;
-
- port_validated =
- container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port_validated =
- drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
- if (!port_validated) {
- mutex_lock(&mgr->payload_lock);
- vcpi->num_slots = 0;
- mutex_unlock(&mgr->payload_lock);
- } else {
- drm_dp_mst_topology_put_port(port_validated);
- }
- }
- }
-
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
@@ -3406,8 +3383,15 @@ int drm_dp_update_payload_part1(struct d
port = drm_dp_mst_topology_get_port_validated(
mgr, port);
if (!port) {
-
Patch "drm/dp_mst: Do not set proposed vcpi directly" has been added to the 5.12-stable tree
This is a note to let you know that I've just added the patch titled
drm/dp_mst: Do not set proposed vcpi directly
to the 5.12-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-dp_mst-do-not-set-proposed-vcpi-directly.patch
and it can be found in the queue-5.12 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 35d3e8cb35e75450f87f87e3d314e2d418b6954b Mon Sep 17 00:00:00 2001
From: Wayne Lin
Date: Wed, 16 Jun 2021 11:55:00 +0800
Subject: drm/dp_mst: Do not set proposed vcpi directly
From: Wayne Lin
commit 35d3e8cb35e75450f87f87e3d314e2d418b6954b upstream.
[Why]
When we receive CSN message to notify one port is disconnected, we will
implicitly set its corresponding num_slots to 0. Later on, we will
eventually call drm_dp_update_payload_part1() to arrange down streams.
In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[]
to do the update. Not specific to a target sink only. For example, if we
light up 2 monitors, Monitor_A and Monitor_B, and then we unplug
Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try
to update payload for Monitor_A, we'll also implicitly clean payload for
Monitor_B at the same time. And finally, when we try to call
drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do
nothing at this time since payload for Monitor_B has been cleaned up
previously.
For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload
ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail
and this polling will last for 3 seconds.
Therefore, guess the best way is we don't set the proposed_vcpi[]
diretly. Let user of these herlper functions to set the proposed_vcpi
directly.
[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports
invalid")
2. Tackle the issue in previous commit by skipping those trasient
proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by
user later on.
Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Fixed & Cc tags
Signed-off-by: Wayne Lin
Fixes: 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
Cc: Lyude Paul
Cc: Wayne Lin
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.5+
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616035501.3776-2-wayne@amd.com
Reviewed-by: Lyude Paul
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_dp_mst_topology.c | 36 +-
1 file changed, 10 insertions(+), 26 deletions(-)
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2499,7 +2499,7 @@ drm_dp_mst_handle_conn_stat(struct drm_d
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, old_input, ret, i;
+ int old_ddps, ret;
u8 new_pdt;
bool new_mcs;
bool dowork = false, create_connector = false;
@@ -2531,7 +2531,6 @@ drm_dp_mst_handle_conn_stat(struct drm_d
}
old_ddps = port->ddps;
- old_input = port->input;
port->input = conn_stat->input_port;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2554,28 +2553,6 @@ drm_dp_mst_handle_conn_stat(struct drm_d
dowork = false;
}
- if (!old_input && old_ddps != port->ddps && !port->ddps) {
- for (i = 0; i < mgr->max_payloads; i++) {
- struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
- struct drm_dp_mst_port *port_validated;
-
- if (!vcpi)
- continue;
-
- port_validated =
- container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port_validated =
- drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
- if (!port_validated) {
- mutex_lock(&mgr->payload_lock);
- vcpi->num_slots = 0;
- mutex_unlock(&mgr->payload_lock);
- } else {
- drm_dp_mst_topology_put_port(port_validated);
- }
- }
- }
-
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
@@ -3406,8 +3383,15 @@ int drm_dp_update_payload_part1(struct d
port = drm_dp_mst_topology_get_port_validated(
mgr, port);
if (!port) {
-
Patch "drm/dp_mst: Do not set proposed vcpi directly" has been added to the 5.13-stable tree
This is a note to let you know that I've just added the patch titled
drm/dp_mst: Do not set proposed vcpi directly
to the 5.13-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-dp_mst-do-not-set-proposed-vcpi-directly.patch
and it can be found in the queue-5.13 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 35d3e8cb35e75450f87f87e3d314e2d418b6954b Mon Sep 17 00:00:00 2001
From: Wayne Lin
Date: Wed, 16 Jun 2021 11:55:00 +0800
Subject: drm/dp_mst: Do not set proposed vcpi directly
From: Wayne Lin
commit 35d3e8cb35e75450f87f87e3d314e2d418b6954b upstream.
[Why]
When we receive CSN message to notify one port is disconnected, we will
implicitly set its corresponding num_slots to 0. Later on, we will
eventually call drm_dp_update_payload_part1() to arrange down streams.
In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[]
to do the update. Not specific to a target sink only. For example, if we
light up 2 monitors, Monitor_A and Monitor_B, and then we unplug
Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try
to update payload for Monitor_A, we'll also implicitly clean payload for
Monitor_B at the same time. And finally, when we try to call
drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do
nothing at this time since payload for Monitor_B has been cleaned up
previously.
For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload
ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail
and this polling will last for 3 seconds.
Therefore, guess the best way is we don't set the proposed_vcpi[]
diretly. Let user of these herlper functions to set the proposed_vcpi
directly.
[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports
invalid")
2. Tackle the issue in previous commit by skipping those trasient
proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by
user later on.
Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Fixed & Cc tags
Signed-off-by: Wayne Lin
Fixes: 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
Cc: Lyude Paul
Cc: Wayne Lin
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.5+
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616035501.3776-2-wayne@amd.com
Reviewed-by: Lyude Paul
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_dp_mst_topology.c | 36 +-
1 file changed, 10 insertions(+), 26 deletions(-)
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2497,7 +2497,7 @@ drm_dp_mst_handle_conn_stat(struct drm_d
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, old_input, ret, i;
+ int old_ddps, ret;
u8 new_pdt;
bool new_mcs;
bool dowork = false, create_connector = false;
@@ -2529,7 +2529,6 @@ drm_dp_mst_handle_conn_stat(struct drm_d
}
old_ddps = port->ddps;
- old_input = port->input;
port->input = conn_stat->input_port;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2552,28 +2551,6 @@ drm_dp_mst_handle_conn_stat(struct drm_d
dowork = false;
}
- if (!old_input && old_ddps != port->ddps && !port->ddps) {
- for (i = 0; i < mgr->max_payloads; i++) {
- struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
- struct drm_dp_mst_port *port_validated;
-
- if (!vcpi)
- continue;
-
- port_validated =
- container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port_validated =
- drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
- if (!port_validated) {
- mutex_lock(&mgr->payload_lock);
- vcpi->num_slots = 0;
- mutex_unlock(&mgr->payload_lock);
- } else {
- drm_dp_mst_topology_put_port(port_validated);
- }
- }
- }
-
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
@@ -3404,8 +3381,15 @@ int drm_dp_update_payload_part1(struct d
port = drm_dp_mst_topology_get_port_validated(
mgr, port);
if (!port) {
-
Re: [PATCH 02/13] vfio: Introduce a vfio_uninit_group_dev() API call
On Thu, Jul 15, 2021 at 06:49:05AM +0300, Leon Romanovsky wrote: > On Wed, Jul 14, 2021 at 09:20:31PM -0300, Jason Gunthorpe wrote: > > From: Max Gurtovoy > > > > This pairs with vfio_init_group_dev() and allows undoing any state that is > > stored in the vfio_device unrelated to registration. Add appropriately > > placed calls to all the drivers. > > > > The following patch will use this to add pre-registration state for the > > device set. > > > > Signed-off-by: Max Gurtovoy > > Signed-off-by: Jason Gunthorpe > > Documentation/driver-api/vfio.rst| 4 ++- > > drivers/vfio/fsl-mc/vfio_fsl_mc.c| 6 +++-- > > drivers/vfio/mdev/vfio_mdev.c| 13 +++--- > > drivers/vfio/pci/vfio_pci.c | 6 +++-- > > drivers/vfio/platform/vfio_platform_common.c | 7 +++-- > > drivers/vfio/vfio.c | 5 > > include/linux/vfio.h | 1 + > > samples/vfio-mdev/mbochs.c | 2 ++ > > samples/vfio-mdev/mdpy.c | 25 ++ > > samples/vfio-mdev/mtty.c | 27 > > 10 files changed, 64 insertions(+), 32 deletions(-) > > <...> > > > @@ -674,6 +675,7 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device > > *mc_dev) > > > > dprc_remove_devices(mc_dev, NULL, 0); > > vfio_fsl_uninit_device(vdev); > > + vfio_uninit_group_dev(&vdev->vdev); > > This is wrong place, the _uninit_ should be after vfio_fsl_mc_reflck_put(). Well, maybe, but it doesn't matter, the uninit doesn't effect the reflck and the next fsl patch deletes the line below. I can switch it if there is a v2 > > vfio_fsl_mc_reflck_put(vdev->reflck); Jason
Re: [Intel-gfx] [PATCH] drm/i915: Check object_can_migrate from object_migrate
On Wed, 14 Jul 2021 at 21:45, Jason Ekstrand wrote: > > We don't roll them together entirely because there are still a couple > cases where we want a separate can_migrate check. For instance, the > display code checks that you can migrate a buffer to LMEM before it > accepts it in fb_create. The dma-buf import code also uses it to do an > early check and return a different error code if someone tries to attach > a LMEM-only dma-buf to another driver. > > However, no one actually wants to call object_migrate when can_migrate > has failed. The stated intention is for self-tests but none of those > actually take advantage of this unsafe migration. > > Signed-off-by: Jason Ekstrand > Cc: Daniel Vetter > Cc: Matthew Auld Reviewed-by: Matthew Auld
[PATCH -next v2 resend] drm/bochs: Fix missing pci_disable_device() on error in bochs_pci_probe()
Replace pci_enable_device() with pcim_enable_device(), pci_disable_device() will be called in release automatically. v2: use pcim_enable_device() Signed-off-by: Yang Yingliang Reported-by: Hulk Robot Acked-by: Thomas Zimmermann --- drivers/gpu/drm/bochs/bochs_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bochs/bochs_drv.c b/drivers/gpu/drm/bochs/bochs_drv.c index c828cadbabff..8065c9537237 100644 --- a/drivers/gpu/drm/bochs/bochs_drv.c +++ b/drivers/gpu/drm/bochs/bochs_drv.c @@ -118,7 +118,7 @@ static int bochs_pci_probe(struct pci_dev *pdev, if (IS_ERR(dev)) return PTR_ERR(dev); - ret = pci_enable_device(pdev); + ret = pcim_enable_device(pdev); if (ret) goto err_free_dev; -- 2.25.1
Re: [PATCH 00/13] Provide core infrastructure for managing open/release
On 7/15/2021 5:50 AM, Jason Gunthorpe wrote: Prologue: This is the first series of three to send the "mlx5_vfio_pci" driver that has been discussed on the list for a while now. - Reorganize reflck to support splitting vfio_pci - Split vfio_pci into vfio_pci/vfio_pci_core and provide infrastructure for non-generic VFIO PCI drivers - The new driver mlx5_vfio_pci that is a full implementation of suspend/resume functionality for mlx5 devices. A preview of all the patches can be seen here: https://github.com/jgunthorpe/linux/commits/mlx5_vfio_pci === This is in support of Max's series to split vfio-pci. For that to work the reflck concept embedded in vfio-pci needs to be sharable across all of the new VFIO PCI drivers which motivated re-examining how this is implemented. Another significant issue is how the VFIO PCI core includes code like: if (pci_dev_driver(pdev) != &vfio_pci_driver) Which is not scalable if there are going to be multiple different driver types. This series takes the approach of moving the "reflck" mechanism into the core code as a "device set". Each vfio_device driver can specify how vfio_devices are grouped into the set using a key and the set comes along with a set-global mutex. The core code manages creating per-device set memory and associating it with each vfio_device. In turn this allows the core code to provide an open/close_device() operation that is called only for the first/last FD, and is called under the global device set lock. Review of all the drivers show that they are either already open coding the first/last semantic or are buggy and missing it. All drivers are migrated/fixed to the new open/close_device ops and the unused per-FD open()/release() ops are deleted. Why can't open()/release() ops be reused instead of adding open_device()/close_device(). Thanks, Kirti
[Bug 213391] AMDGPU retries page fault with some specific processes amdgpu and sometimes followed [gfxhub0] retry page fault until *ERROR* ring gfx timeout, but soft recovered
https://bugzilla.kernel.org/show_bug.cgi?id=213391 --- Comment #33 from Leandro Jacques (ls...@yahoo.com) --- Created attachment 297881 --> https://bugzilla.kernel.org/attachment.cgi?id=297881&action=edit Kernel crash log for linux firmware version 20210511.7685cf4 amdgpu kernel crash log when the problem ocurred, with the exact same message telling about page fault. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 213391] AMDGPU retries page fault with some specific processes amdgpu and sometimes followed [gfxhub0] retry page fault until *ERROR* ring gfx timeout, but soft recovered
https://bugzilla.kernel.org/show_bug.cgi?id=213391 --- Comment #34 from Leandro Jacques (ls...@yahoo.com) --- Created attachment 297883 --> https://bugzilla.kernel.org/attachment.cgi?id=297883&action=edit Linux Firmware version info 20210511.7685cf4 Firmware info as of the moment when the system crashed -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 209457] AMDGPU resume fail with RX 580 GPU
https://bugzilla.kernel.org/show_bug.cgi?id=209457 --- Comment #39 from Leandro Jacques (ls...@yahoo.com) --- (In reply to Alex Deucher from comment #38) > > You have a Picasso system. The original bug was about an RX 580. I don't > think this is the same issue. Sounds like you are seeing this issue: > https://lists.freedesktop.org/archives/amd-gfx/2021-July/066452.html No, the error message is exactly the same of this one https://bugzilla.kernel.org/show_bug.cgi?id=213391 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PULL] drm-intel-fixes
Hi Dave and Daniel, I was unsure about the -EDEADLK one based on Daniel's comment on dri-devel, but Ville's response and the patch looks reasonable to me. Also they are in sync with the test cases. So if anything needs to still change on that area I believe it can be a follow-up work. Here goes drm-intel-fixes-2021-07-15: Two regression fixes targeting stable: - Fix -EDEADLK handling regression (Ville) - Drop the page table optimisation (Matt) Thanks, Rodrigo. The following changes since commit e73f0f0ee7541171d89f2e2491130c7771ba58d3: Linux 5.14-rc1 (2021-07-11 15:07:40 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2021-07-15 for you to fetch changes up to 0abb33bfca0fb74df76aac03e90ce685016ef7be: drm/i915/gtt: drop the page table optimisation (2021-07-14 08:46:18 -0400) Two regression fixes targeting stable: - Fix -EDEADLK handling regression (Ville) - Drop the page table optimisation (Matt) Matthew Auld (1): drm/i915/gtt: drop the page table optimisation Ville Syrjälä (1): drm/i915/gt: Fix -EDEADLK handling regression drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 5 + drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-)
Re: [PATCH] drm/panel: Document internal backlight handling
Hi,
On Thu, Jul 15, 2021 at 1:02 AM Linus Walleij wrote:
>
> Panels with internal backlight need to assign their backlight member
> directly.
>
> Reported-by: Doug Anderson
> Signed-off-by: Linus Walleij
> ---
> include/drm/drm_panel.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 33605c3f0eba..1e63fadf1368 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -144,8 +144,9 @@ struct drm_panel {
> * Backlight device, used to turn on backlight after the call
> * to enable(), and to turn off backlight before the call to
> * disable().
> -* backlight is set by drm_panel_of_backlight() and drivers
> -* shall not assign it.
> +* External backlight is assigned by drm_panel_of_backlight() while
> +* panel-internal backlight is assigned directly to this member by the
> +* panel driver.
Can you rebase/post against drm-misc-next? If not then you'll conflict
with commit 10f7b40e4f30 ("drm/panel: add basic DP AUX backlight
support"). :-)
-Doug
Re: [PATCH v8 00/14] drm/tegra: Introduce a modern UABI
14.07.2021 18:26, Mikko Perttunen пишет: ... > While my goal of course is to enable proper use of Host1x on the newer > SoCs, there is absolutely no intention to forget about the older SoCs. > Observably, to me at least, GR2D and GR3D are working -- the test suites > are passing (though I did not port/try mesa). We are also not regressing > anything, and I do not think after this series we are worse (at least in > any fundamental manner) than the downstream software stack that these > chips were originally productized with. > As such I have a hard time > understanding the doom and gloom about the driver's state and needing > grand overarching re-architectures. You need to work with a real consumer device and use it everyday to feel all those problems. But problems always exist, we will fix them eventually and then have new problems. We also have an alternative experimental driver that solves the problems for the time being, so it's not really that bad. I may sound a bit too negative, but that's because you're are already doing this work and I want to get your attention to a core problems that in my opinion should be solved first to avoid a need to re-do the work later on. ... > I have a long TODO list of improvements to work on. Admittedly, I won't > have as much time to work on it as I would have before since I need to > start working on other projects in parallel as well. And things will > need to be agreed on A slow pace parallel work is a normal thing, that's exactly what me and everyone else are doing in regards to having fun with Tegra development. We already managed to achieve a lot this way and it's only getting better every day. > (e.g. as alluded to earlier I still don't know of > any concrete reason why we would need to add a software scheduler. I can > only make guesses. It probably makes sense for the old SoCs, but I don't > know why we now need one and downstream never did) Scheduler is needed for running jobs in a correct order. You don't feel like it's needed because you use only a single h/w engine and haven't seen places where it's needed. DRM scheduler also has a much cleaner code, has more features, it's optimized, well thought, uses modern upstream concepts and successfully used by multiple drivers in comparison to what we have in the Host1x driver. We will discuss it in a more details.
Re: [pull] amdgpu, amdkfd drm-fixes-5.14
On Thu, Jul 15, 2021 at 12:52 AM Sam Ravnborg wrote: > > Hi Alex, > > On Wed, Jul 14, 2021 at 06:08:58PM -0400, Alex Deucher wrote: > > Hi Dave, Daniel, > > > > Fixes for 5.14. The big change here is unifying the SMU13 code. This was > > new code added in 5.14 to support Yellow Carp, but we've since cleaned it > > up and removed a lot of duplication, so better to merge it now to facilitate > > any bug fixes in the future that need to go back to this kernel via stable. > > Only affects Yellow Carp which is new for 5.14 anyway so not much chance for > > regressions. The rest is just standard bug fixes. > > This pull seems not to include any fixes for the W=1 warnings that > has crept in again. It would be nice if the amdgpu could be warning free > again, this would maybe motivate the others to fix theirs too so we > could keep most/all of drivers/gpu/ free of W=1 warnings. We haven't really been monitoring the W=1 stuff that closely. I'll see what we can do going forward. Alex
Re: [PATCH] drm/fb-helper: Try to protect cleanup against delayed setup
Am 13.07.21 um 15:59 schrieb Daniel Vetter:
Some vague evidences suggests this can go wrong. Try to prevent it by
holding the right mutex and clearing ->deferred_setup to make sure we
later on don't accidentally try to re-register the fbdev when the
driver thought it had it all cleaned up already.
v2: I realized that this is fundamentally butchered, and CI complained
about lockdep splats. So limit the critical section again and just add
a few notes what the proper fix is.
References:
https://intel-gfx-ci.01.org/tree/linux-next/next-20201215/fi-byt-j1900/igt@i915_pm_...@module-reload.html
Signed-off-by: Daniel Vetter
Cc: Ville Syrjälä
Cc: Chris Wilson
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_fb_helper.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 9d82fda274eb..8f11e5abb222 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -598,6 +598,9 @@ EXPORT_SYMBOL(drm_fb_helper_alloc_fbi);
* A wrapper around unregister_framebuffer, to release the fb_info
* framebuffer device. This must be called before releasing all resources for
* @fb_helper by calling drm_fb_helper_fini().
+ *
+ * Note that this is fundamentally racy on hotunload because it doen't handle
+ * open fbdev file descriptors at all. Use drm_fbdev_generic_setup() instead.
*/
void drm_fb_helper_unregister_fbi(struct drm_fb_helper *fb_helper)
{
@@ -611,6 +614,9 @@ EXPORT_SYMBOL(drm_fb_helper_unregister_fbi);
* @fb_helper: driver-allocated fbdev helper, can be NULL
*
* This cleans up all remaining resources associated with @fb_helper.
+ *
+ * Note that this is fundamentally racy on hotunload because it doen't handle
+ * open fbdev file descriptors at all. Use drm_fbdev_generic_setup() instead.
*/
void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
{
@@ -2382,6 +2388,10 @@ static void drm_fbdev_client_unregister(struct
drm_client_dev *client)
{
struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+ mutex_lock(&fb_helper->lock);
+ fb_helper->deferred_setup = false;
+ mutex_unlock(&fb_helper->lock);
+
if (fb_helper->fbdev)
/* drm_fbdev_fb_destroy() takes care of cleanup */
drm_fb_helper_unregister_fbi(fb_helper);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v2 resent] drm/i915: Add TTM offset argument to mmap.
On Wed, 14 Jul 2021 at 13:28, Maarten Lankhorst
wrote:
>
> The FIXED mapping is only used for ttm, and tells userspace that the
> mapping type is pre-defined. This disables the other type of mmap
> offsets when discrete memory is used, so fix the selftests as well.
>
> Document the struct as well, so it shows up in docbook.
>
> Cc: Jason Ekstrand
> Reviewed-by: Daniel Vetter
> Signed-off-by: Maarten Lankhorst
> ---
> Resent, forgot to cc dri-devel
>
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 17 ++-
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 1 +
> .../drm/i915/gem/selftests/i915_gem_mman.c| 27 ++-
> include/uapi/drm/i915_drm.h | 46 ++-
> 4 files changed, 77 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index a90f796e85c0..31c4021bb6be 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -679,10 +679,16 @@ __assign_mmap_offset(struct drm_i915_gem_object *obj,
> return -ENODEV;
>
> if (obj->ops->mmap_offset) {
> + if (mmap_type != I915_MMAP_TYPE_FIXED)
> + return -ENODEV;
> +
> *offset = obj->ops->mmap_offset(obj);
> return 0;
> }
>
> + if (mmap_type == I915_MMAP_TYPE_FIXED)
> + return -ENODEV;
> +
> if (mmap_type != I915_MMAP_TYPE_GTT &&
> !i915_gem_object_has_struct_page(obj) &&
> !i915_gem_object_has_iomem(obj))
> @@ -727,7 +733,9 @@ i915_gem_dumb_mmap_offset(struct drm_file *file,
> {
> enum i915_mmap_type mmap_type;
>
> - if (boot_cpu_has(X86_FEATURE_PAT))
> + if (HAS_LMEM(to_i915(dev)))
> + mmap_type = I915_MMAP_TYPE_FIXED;
> + else if (boot_cpu_has(X86_FEATURE_PAT))
> mmap_type = I915_MMAP_TYPE_WC;
> else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
> return -ENODEV;
> @@ -798,6 +806,10 @@ i915_gem_mmap_offset_ioctl(struct drm_device *dev, void
> *data,
> type = I915_MMAP_TYPE_UC;
> break;
>
> + case I915_MMAP_OFFSET_FIXED:
> + type = I915_MMAP_TYPE_FIXED;
> + break;
> +
> default:
> return -EINVAL;
> }
> @@ -968,6 +980,9 @@ int i915_gem_mmap(struct file *filp, struct
> vm_area_struct *vma)
> vma->vm_ops = &vm_ops_cpu;
> break;
>
> + case I915_MMAP_TYPE_FIXED:
> + GEM_WARN_ON(1);
> + /* fall-through */
> case I915_MMAP_TYPE_WB:
> vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> vma->vm_ops = &vm_ops_cpu;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> index ef3de2ae9723..afbadfc5516b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -105,6 +105,7 @@ enum i915_mmap_type {
> I915_MMAP_TYPE_WC,
> I915_MMAP_TYPE_WB,
> I915_MMAP_TYPE_UC,
> + I915_MMAP_TYPE_FIXED,
> };
>
> struct i915_mmap_offset {
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> index 1da8bd675e54..52789c8ad337 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -573,6 +573,14 @@ static int make_obj_busy(struct drm_i915_gem_object *obj)
> return 0;
> }
>
> +static enum i915_mmap_type default_mapping(struct drm_i915_private *i915)
> +{
> + if (HAS_LMEM(i915))
> + return I915_MMAP_TYPE_FIXED;
> +
> + return I915_MMAP_TYPE_GTT;
> +}
> +
> static bool assert_mmap_offset(struct drm_i915_private *i915,
>unsigned long size,
>int expected)
> @@ -585,7 +593,7 @@ static bool assert_mmap_offset(struct drm_i915_private
> *i915,
> if (IS_ERR(obj))
> return expected && expected == PTR_ERR(obj);
>
> - ret = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL);
> + ret = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL);
> i915_gem_object_put(obj);
>
> return ret == expected;
> @@ -689,7 +697,7 @@ static int igt_mmap_offset_exhaustion(void *arg)
> goto out;
> }
>
> - err = __assign_mmap_offset(obj, I915_MMAP_TYPE_GTT, &offset, NULL);
> + err = __assign_mmap_offset(obj, default_mapping(i915), &offset, NULL);
> if (err) {
> pr_err("Unable to insert object into reclaimed hole\n");
> goto err_obj;
> @@ -831,8 +839,14 @@ static int wc_check(struct drm_i915_gem_object *obj)
>
> static bool can_mmap(struct drm_i915_gem_object *ob
[PATCH] drm/amdgpu/display: make a const array common_rates static, makes object smaller
From: Colin Ian King
Don't populate the const array common_rates on the stack but instead it
static. Makes the object code smaller by 80 bytes:
Before:
textdata bss dec hex filename
268019 98322 256 366597 59805 ../display/amdgpu_dm/amdgpu_dm.o
After:
textdata bss dec hex filename
267843 98418 256 366517 597b5 ../display/amdgpu_dm/amdgpu_dm.o
Reduction of 80 bytes
(gcc version 10.3.0)
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 --
1 file changed, 4 insertions(+), 2 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 2d48bb09645f..b196bb6eafc0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7580,8 +7580,10 @@ static uint add_fs_modes(struct amdgpu_dm_connector
*aconnector)
* 60 - Commonly used
* 48,72,96 - Multiples of 24
*/
- const uint32_t common_rates[] = { 23976, 24000, 25000, 29970, 3,
-48000, 5, 6, 72000, 96000 };
+ static const uint32_t common_rates[] = {
+ 23976, 24000, 25000, 29970, 3,
+ 48000, 5, 6, 72000, 96000
+ };
/*
* Find mode with highest refresh rate with the same resolution
--
2.31.1
[PATCH] drm: bridge: make const array frs_limits static, makes object smaller
From: Colin Ian King
Don't populate the const array frs_limits on the stack but instead it
static. Makes the object code smaller by 128 bytes:
Before:
textdata bss dec hex filename
251557440 64 326597f93 ./drivers/gpu/drm/bridge/tc358768.o
After:
textdata bss dec hex filename
250597408 64 325317f13 ./drivers/gpu/drm/bridge/tc358768.o
(gcc version 10.3.0)
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/bridge/tc358768.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/tc358768.c
b/drivers/gpu/drm/bridge/tc358768.c
index 8ed8302d6bbb..2495ea46b091 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -291,7 +291,7 @@ static int tc358768_calc_pll(struct tc358768_priv *priv,
const struct drm_display_mode *mode,
bool verify_only)
{
- const u32 frs_limits[] = {
+ static const u32 frs_limits[] = {
10,
5,
25000,
--
2.31.1
Re: [PATCH 00/13] Provide core infrastructure for managing open/release
On Thu, Jul 15, 2021 at 06:58:31PM +0530, Kirti Wankhede wrote: > > Review of all the drivers show that they are either already open coding > > the first/last semantic or are buggy and missing it. All drivers are > > migrated/fixed to the new open/close_device ops and the unused per-FD > > open()/release() ops are deleted. > > Why can't open()/release() ops be reused instead of adding > open_device()/close_device(). It could be done but it would ruin the structure of the patch series, obfuscate the naming of the ops, and complicate backporting as this is a significant semantic difference. Overall when funtionality changes significantly it is better to change the name along with it Jason
Re: [PATCH] drm/panel-simple: Power the panel when probing DP AUX backlight
Hi,
On Wed, Jul 14, 2021 at 9:34 AM Douglas Anderson wrote:
>
> When I tried booting up a device that needed the DP AUX backlight, I
> found an error in the logs:
> panel-simple-dp-aux: probe of aux-ti_sn65dsi86.aux.0 failed with error -110
>
> The aux transfers were failing because the panel wasn't powered. Just
> like when reading the EDID we need to power the panel when trying to
> talk to it. Add the needed pm_runtime calls.
>
> After I do this I can successfully probe the panel and adjust the
> backlight on my board.
>
> Fixes: bfd451403d70 ("drm/panel-simple: Support DP AUX backlight")
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/panel/panel-simple.c | 3 +++
> 1 file changed, 3 insertions(+)
Pushed with Lyude's review to drm-misc-next:
5ead9b5b1575 drm/panel-simple: Power the panel when probing DP AUX backlight
Re: [PATCH] drm/dp: For drm_panel_dp_aux_backlight(), init backlight as disabled
Hi,
On Wed, Jul 14, 2021 at 10:17 AM Douglas Anderson wrote:
>
> Even after the DP AUX backlight on my board worked OK after applying
> the patch ("drm/panel-simple: Power the panel when probing DP AUX
> backlight") [1], I still noticed some strange timeouts being reported
> by ti_sn_aux_transfer(). Digging, I realized the problem was this:
> * Even though `enabled` in `struct dp_aux_backlight` was false, the
> base backlight structure (`base` in that structure) thought that the
> backlight was powered on.
> * If userspace wrote to sysfs in this state then we'd try to enable
> the backlight.
> * Unfortunatley, enabling the backlight didn't work because the panel
> itself wasn't powered.
>
> We can only use the backlight if the panel is on and the panel is not
> officially on when we probe (it's temporarily just on enough for us to
> talk to it).
>
> The important thing we want here is to get `BL_CORE_FBBLANK` set since
> userspace can't mess with that. This will keep us disabled until
> drm_panel enables us, which means that the panel is enabled
> first. Ideally we'd just set this in our `props` before calling
> devm_backlight_device_register() but the comments in the header file
> are pretty explicit that we're not supposed to much with the `state`
> ourselves. Because of this, there may be a small window where the
> backlight device is registered and someone could try to tweak with the
> backlight. This isn't likely to happen and even if it did, I don't
> believe this causes any huge problem.
>
> [1]
> https://lore.kernel.org/lkml/20210714093334.1.Idb41f87e5abae4aee0705db7458b0097fc50e7ab@changeid/
>
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/drm_dp_helper.c | 2 ++
> 1 file changed, 2 insertions(+)
Pushed with Lyude's review to drm-misc-next:
17a1837d07be drm/dp: For drm_panel_dp_aux_backlight(), init backlight
as disabled
Re: [PATCH v8 01/14] gpu: host1x: Add DMA fence implementation
09.07.2021 22:31, Thierry Reding пишет: > diff --git a/drivers/gpu/host1x/fence.c b/drivers/gpu/host1x/fence.c > new file mode 100644 > index ..06c6b86237bd > --- /dev/null > +++ b/drivers/gpu/host1x/fence.c > @@ -0,0 +1,209 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Syncpoint dma_fence implementation > + * > + * Copyright (c) 2020, NVIDIA Corporation. > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include "fence.h" > +#include "intr.h" > +#include "syncpt.h" > + > +DEFINE_SPINLOCK(lock); *static*
Re: [PATCH v8 09/14] drm/tegra: Implement new UAPI
09.07.2021 22:31, Thierry Reding пишет: > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c > index cddee6425461..6ee08e49ec57 100644 > --- a/drivers/gpu/drm/tegra/drm.c > +++ b/drivers/gpu/drm/tegra/drm.c > @@ -21,6 +21,7 @@ > #include > #include > > +#include "uapi.h" > #include "drm.h" > #include "gem.h" alphabet order
Re: [pull] amdgpu, amdkfd drm-fixes-5.14
On 2021-07-15 4:07 p.m., Alex Deucher wrote: > On Thu, Jul 15, 2021 at 12:52 AM Sam Ravnborg wrote: >> On Wed, Jul 14, 2021 at 06:08:58PM -0400, Alex Deucher wrote: >>> Hi Dave, Daniel, >>> >>> Fixes for 5.14. The big change here is unifying the SMU13 code. This was >>> new code added in 5.14 to support Yellow Carp, but we've since cleaned it >>> up and removed a lot of duplication, so better to merge it now to facilitate >>> any bug fixes in the future that need to go back to this kernel via stable. >>> Only affects Yellow Carp which is new for 5.14 anyway so not much chance for >>> regressions. The rest is just standard bug fixes. >> >> This pull seems not to include any fixes for the W=1 warnings that >> has crept in again. It would be nice if the amdgpu could be warning free >> again, this would maybe motivate the others to fix theirs too so we >> could keep most/all of drivers/gpu/ free of W=1 warnings. > > We haven't really been monitoring the W=1 stuff that closely. I'll > see what we can do going forward. IMHO keeping the W=1 build clean isn't realistic without enforcing it in CI. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/amdgpu/display: make a const array common_rates static, makes object smaller
On Thu, Jul 15, 2021 at 10:37 AM Colin King wrote:
>
> From: Colin Ian King
>
> Don't populate the const array common_rates on the stack but instead it
> static. Makes the object code smaller by 80 bytes:
>
> Before:
>textdata bss dec hex filename
> 268019 98322 256 366597 59805 ../display/amdgpu_dm/amdgpu_dm.o
>
> After:
>textdata bss dec hex filename
> 267843 98418 256 366517 597b5 ../display/amdgpu_dm/amdgpu_dm.o
>
> Reduction of 80 bytes
>
> (gcc version 10.3.0)
>
> Signed-off-by: Colin Ian King
Applied. Thanks!
Alex
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 --
> 1 file changed, 4 insertions(+), 2 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 2d48bb09645f..b196bb6eafc0 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7580,8 +7580,10 @@ static uint add_fs_modes(struct amdgpu_dm_connector
> *aconnector)
> * 60 - Commonly used
> * 48,72,96 - Multiples of 24
> */
> - const uint32_t common_rates[] = { 23976, 24000, 25000, 29970, 3,
> -48000, 5, 6, 72000, 96000 };
> + static const uint32_t common_rates[] = {
> + 23976, 24000, 25000, 29970, 3,
> + 48000, 5, 6, 72000, 96000
> + };
>
> /*
> * Find mode with highest refresh rate with the same resolution
> --
> 2.31.1
>
Re: [PATCH] drm/amd/display: Fix 10bit 4K display on CIK GPUs
On Wed, Jul 14, 2021 at 4:15 AM Liviu Dudau wrote:
>
> Commit 72a7cf0aec0c ("drm/amd/display: Keep linebuffer pixel depth at
> 30bpp for DCE-11.0.") doesn't seems to have fixed 10bit 4K rendering over
> DisplayPort for CIK GPUs. On my machine with a HAWAII GPU I get a broken
> image that looks like it has an effective resolution of 1920x1080 but
> scaled up in an irregular way. Reverting the commit or applying this
> patch fixes the problem on v5.14-rc1.
>
> Fixes: 72a7cf0aec0c ("drm/amd/display: Keep linebuffer pixel depth at 30bpp
> for DCE-11.0.")
> Signed-off-by: Liviu Dudau
Harry or Mario any ideas? Maybe we need finer grained DCE version
checking? I don't remember all of the caveats of this stuff. DCE11
and older is getting to be pretty old at this point. I can just apply
this if you don't have any insights.
Alex
> ---
> drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index a6a67244a322e..1596f6b7fed7c 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -1062,7 +1062,7 @@ bool resource_build_scaling_params(struct pipe_ctx
> *pipe_ctx)
> * so use only 30 bpp on DCE_VERSION_11_0. Testing with DCE 11.2 and
> 8.3
> * did not show such problems, so this seems to be the exception.
> */
> - if (plane_state->ctx->dce_version != DCE_VERSION_11_0)
> + if (plane_state->ctx->dce_version > DCE_VERSION_11_0)
> pipe_ctx->plane_res.scl_data.lb_params.depth =
> LB_PIXEL_DEPTH_36BPP;
> else
> pipe_ctx->plane_res.scl_data.lb_params.depth =
> LB_PIXEL_DEPTH_30BPP;
> --
> 2.32.0
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 1/8] dt-bindings: display: msm: dsi-controller-main: restore assigned-clocks
On Sat, 10 Jul 2021 00:07:22 +0300, Dmitry Baryshkov wrote: > Restore the assgined-clocks and assigned-clock-parents properties that > were lost during the txt -> YAML conversion. > > Signed-off-by: Dmitry Baryshkov > --- > .../display/msm/dsi-controller-main.yaml| 17 + > 1 file changed, 17 insertions(+) > Reviewed-by: Rob Herring
[drm-intel:drm-intel-gt-next 29/30] drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25: sparse: sparse: incorrect type in assignment (different address spaces)
tree: git://anongit.freedesktop.org/drm-intel drm-intel-gt-next
head: ca06f93638362bf83584cdf33897822bf1578cf9
commit: 0eee9977f9d3d8f1e40175dada55b3d00121ac79 [29/30] drm/i915/gem: Roll all
of context creation together
config: x86_64-randconfig-s022-20210715 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
git remote add drm-intel git://anongit.freedesktop.org/drm-intel
git fetch --no-tags drm-intel drm-intel-gt-next
git checkout 0eee9977f9d3d8f1e40175dada55b3d00121ac79
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
sparse warnings: (new ones prefixed by >>)
drivers/gpu/drm/i915/gem/i915_gem_context.c:1412:34: sparse: sparse:
incorrect type in argument 1 (different address spaces) @@ expected struct
i915_address_space *vm @@ got struct i915_address_space [noderef] __rcu *vm
@@
drivers/gpu/drm/i915/gem/i915_gem_context.c:1412:34: sparse: expected
struct i915_address_space *vm
drivers/gpu/drm/i915/gem/i915_gem_context.c:1412:34: sparse: got struct
i915_address_space [noderef] __rcu *vm
drivers/gpu/drm/i915/gem/i915_gem_context.c: note: in included file:
>> drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25: sparse: sparse:
>> incorrect type in assignment (different address spaces) @@ expected
>> struct i915_address_space [noderef] __rcu *vm @@ got struct
>> i915_address_space * @@
drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25: sparse:
expected struct i915_address_space [noderef] __rcu *vm
drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25: sparse: got
struct i915_address_space *
>> drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34: sparse: sparse:
>> incorrect type in argument 1 (different address spaces) @@ expected
>> struct i915_address_space *vm @@ got struct i915_address_space [noderef]
>> __rcu *vm @@
drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34: sparse:
expected struct i915_address_space *vm
drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34: sparse: got
struct i915_address_space [noderef] __rcu *vm
vim +43 drivers/gpu/drm/i915/gem/selftests/mock_context.c
10
11 struct i915_gem_context *
12 mock_context(struct drm_i915_private *i915,
13 const char *name)
14 {
15 struct i915_gem_context *ctx;
16 struct i915_gem_engines *e;
17 struct intel_sseu null_sseu = {};
18
19 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
20 if (!ctx)
21 return NULL;
22
23 kref_init(&ctx->ref);
24 INIT_LIST_HEAD(&ctx->link);
25 ctx->i915 = i915;
26
27 mutex_init(&ctx->mutex);
28
29 spin_lock_init(&ctx->stale.lock);
30 INIT_LIST_HEAD(&ctx->stale.engines);
31
32 i915_gem_context_set_persistence(ctx);
33
34 if (name) {
35 struct i915_ppgtt *ppgtt;
36
37 strncpy(ctx->name, name, sizeof(ctx->name) - 1);
38
39 ppgtt = mock_ppgtt(i915, name);
40 if (!ppgtt)
41 goto err_free;
42
> 43 ctx->vm = i915_vm_open(&ppgtt->vm);
44 i915_vm_put(&ppgtt->vm);
45 }
46
47 mutex_init(&ctx->engines_mutex);
48 e = default_engines(ctx, null_sseu);
49 if (IS_ERR(e))
50 goto err_vm;
51 RCU_INIT_POINTER(ctx->engines, e);
52
53 INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
54 mutex_init(&ctx->lut_mutex);
55
56 return ctx;
57
58 err_vm:
59 if (ctx->vm)
> 60 i915_vm_close(ctx->vm);
61 err_free:
62 kfree(ctx);
63 return NULL;
64 }
65
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[Regression] No framebuffer console on Rpi since 5.14-rc1
Hi guys, starting with Linux 5.14-rc1 the framebuffer console on Raspberry Pi 3/4 (no U-Boot, multi_v7_defconfig) isn't available anymore. The display shows the rainbow screen from the bootloader and than the HDMI screen goes black instead of kernel messages. I bisected the issue: 62fb9874f5da54fdb243003b386128037319b219 good e73f0f0ee7541171d89f2e2491130c7771ba58d3 bad e058a84bfddc42ba356a2316f2cf1141974625c9 bad a6eaf3850cb171c328a8b0db6d3c79286a1eba9d good 007b312c6f294770de01fbc0643610145012d244 good 18703923a66aecf6f7ded0e16d22eb412ddae72f good 334200bf52f0637a5ab8331c557dfcecbb9c30fa bad c707b73f0cfb1acc94a20389aecde65e6385349b bad caa18dd6dd9305d52943a6b59f410cbc960ad0a0 good 691cf8cd7a531dbfcc29d09a23c509a86fd9b24f bad 2fdcb55dfc86835e4845e3f422180b5596d23cb4 bad 6c3f953381e526a1623d4575660afae8b19ffa20 bad 5ea4dba68305d9648b9dba30036cc36d4e877bca bad 4a888ba03fd97d1cb0253581973533965bf348c4 good c5ef15ae09637fb51ae43e1d1d98329d67dd4fd6 good f611b1e7624ccdbd495c19e9805629e22265aa16 bad ff323d6d72e1e4971c8ba9e2f3cf8afc48f22383 good f611b1e7624ccdbd495c19e9805629e22265aa16 is the first bad commit commit f611b1e7624ccdbd495c19e9805629e22265aa16 Author: Kees Cook Date: Wed Jun 2 14:52:50 2021 -0700 drm: Avoid circular dependencies for CONFIG_FB When cleaning up other drm config dependencies, it is too easy to create larger problems. Instead, mark CONFIG_FB as a "depends": drivers/gpu/drm/Kconfig:74:error: recursive dependency detected! I compared the changes to the config (based on multi_v7_defconfig) with and without this patch and it shows a lot of changes. Is this intended? Best regards Stefan
[PATCH 1/5] drm: Define DRM_FORMAT_MAX_PLANES
DRM uses a magic number of 4 for the maximum number of planes per color
format. Declare this constant via DRM_FORMAT_MAX_PLANES and update the
related code.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 14 --
include/drm/drm_fourcc.h | 13 +
include/drm/drm_framebuffer.h| 8
include/drm/drm_gem_atomic_helper.h | 2 +-
4 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index e2c68822e05c..975a3df0561e 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -48,7 +48,7 @@
struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
unsigned int plane)
{
- if (plane >= 4)
+ if (plane >= ARRAY_SIZE(fb->obj))
return NULL;
return fb->obj[plane];
@@ -62,7 +62,8 @@ drm_gem_fb_init(struct drm_device *dev,
struct drm_gem_object **obj, unsigned int num_planes,
const struct drm_framebuffer_funcs *funcs)
{
- int ret, i;
+ unsigned int i;
+ int ret;
drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
@@ -86,9 +87,9 @@ drm_gem_fb_init(struct drm_device *dev,
*/
void drm_gem_fb_destroy(struct drm_framebuffer *fb)
{
- int i;
+ size_t i;
- for (i = 0; i < 4; i++)
+ for (i = 0; i < ARRAY_SIZE(fb->obj); i++)
drm_gem_object_put(fb->obj[i]);
drm_framebuffer_cleanup(fb);
@@ -145,8 +146,9 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
const struct drm_framebuffer_funcs *funcs)
{
const struct drm_format_info *info;
- struct drm_gem_object *objs[4];
- int ret, i;
+ struct drm_gem_object *objs[DRM_FORMAT_MAX_PLANES];
+ unsigned int i;
+ int ret;
info = drm_get_format_info(dev, mode_cmd);
if (!info) {
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 3b138d4ae67e..22aa64d07c79 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -25,6 +25,11 @@
#include
#include
+/**
+ * DRM_FORMAT_MAX_PLANES - maximum number of planes a DRM format can have
+ */
+#define DRM_FORMAT_MAX_PLANES 4u
+
/*
* DRM formats are little endian. Define host endian variants for the
* most common formats here, to reduce the #ifdefs needed in drivers.
@@ -78,7 +83,7 @@ struct drm_format_info {
* triplet @char_per_block, @block_w, @block_h for better
* describing the pixel format.
*/
- u8 cpp[4];
+ u8 cpp[DRM_FORMAT_MAX_PLANES];
/**
* @char_per_block:
@@ -104,7 +109,7 @@ struct drm_format_info {
* information from their drm_mode_config.get_format_info hook
* if they want the core to be validating the pitch.
*/
- u8 char_per_block[4];
+ u8 char_per_block[DRM_FORMAT_MAX_PLANES];
};
/**
@@ -113,7 +118,7 @@ struct drm_format_info {
* Block width in pixels, this is intended to be accessed through
* drm_format_info_block_width()
*/
- u8 block_w[4];
+ u8 block_w[DRM_FORMAT_MAX_PLANES];
/**
* @block_h:
@@ -121,7 +126,7 @@ struct drm_format_info {
* Block height in pixels, this is intended to be accessed through
* drm_format_info_block_height()
*/
- u8 block_h[4];
+ u8 block_h[DRM_FORMAT_MAX_PLANES];
/** @hsub: Horizontal chroma subsampling factor */
u8 hsub;
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index be658ebbec72..f67c5b7bcb68 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -27,12 +27,12 @@
#include
#include
+#include
#include
struct drm_clip_rect;
struct drm_device;
struct drm_file;
-struct drm_format_info;
struct drm_framebuffer;
struct drm_gem_object;
@@ -147,7 +147,7 @@ struct drm_framebuffer {
* @pitches: Line stride per buffer. For userspace created object this
* is copied from drm_mode_fb_cmd2.
*/
- unsigned int pitches[4];
+ unsigned int pitches[DRM_FORMAT_MAX_PLANES];
/**
* @offsets: Offset from buffer start to the actual pixel data in bytes,
* per buffer. For userspace created object this is copied from
@@ -165,7 +165,7 @@ struct drm_framebuffer {
* data (even for linear buffers). Specifying an x/y pixel offset is
* instead done through the source rectangle in &struct drm_plane_state.
*/
- unsigned int offsets[4];
+ unsigned int offsets[DRM_FORMAT_MAX_PLANES];
/**
* @modifier: Data layout modifier. This
[PATCH 2/5] drm/gem: Provide drm_gem_fb_{vmap,vunmap}()
Move framebuffer vmap code from shadow-buffered plane state into the new
interfaces drm_gem_fb_vmap() and drm_gem_fb_vunmap(). These functions
provide mappings of a framebuffer's BOs into kernel address space. No
functional changes.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_atomic_helper.c | 37 +--
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 70
include/drm/drm_gem_atomic_helper.h | 1 +
include/drm/drm_gem_framebuffer_helper.h | 5 ++
4 files changed, 79 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c
b/drivers/gpu/drm/drm_gem_atomic_helper.c
index 26af09b959d4..b1cc19e47165 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -330,10 +330,7 @@ int drm_gem_prepare_shadow_fb(struct drm_plane *plane,
struct drm_plane_state *p
{
struct drm_shadow_plane_state *shadow_plane_state =
to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
- struct drm_gem_object *obj;
- struct dma_buf_map map;
int ret;
- size_t i;
if (!fb)
return 0;
@@ -342,27 +339,7 @@ int drm_gem_prepare_shadow_fb(struct drm_plane *plane,
struct drm_plane_state *p
if (ret)
return ret;
- for (i = 0; i < ARRAY_SIZE(shadow_plane_state->map); ++i) {
- obj = drm_gem_fb_get_obj(fb, i);
- if (!obj)
- continue;
- ret = drm_gem_vmap(obj, &map);
- if (ret)
- goto err_drm_gem_vunmap;
- shadow_plane_state->map[i] = map;
- }
-
- return 0;
-
-err_drm_gem_vunmap:
- while (i) {
- --i;
- obj = drm_gem_fb_get_obj(fb, i);
- if (!obj)
- continue;
- drm_gem_vunmap(obj, &shadow_plane_state->map[i]);
- }
- return ret;
+ return drm_gem_fb_vmap(fb, shadow_plane_state->map);
}
EXPORT_SYMBOL(drm_gem_prepare_shadow_fb);
@@ -374,25 +351,17 @@ EXPORT_SYMBOL(drm_gem_prepare_shadow_fb);
* This function implements struct &drm_plane_helper_funcs.cleanup_fb.
* This function unmaps all buffer objects of the plane's framebuffer.
*
- * See drm_gem_prepare_shadow_fb() for more inforamtion.
+ * See drm_gem_prepare_shadow_fb() for more information.
*/
void drm_gem_cleanup_shadow_fb(struct drm_plane *plane, struct drm_plane_state
*plane_state)
{
struct drm_shadow_plane_state *shadow_plane_state =
to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
- size_t i = ARRAY_SIZE(shadow_plane_state->map);
- struct drm_gem_object *obj;
if (!fb)
return;
- while (i) {
- --i;
- obj = drm_gem_fb_get_obj(fb, i);
- if (!obj)
- continue;
- drm_gem_vunmap(obj, &shadow_plane_state->map[i]);
- }
+ drm_gem_fb_vunmap(fb, shadow_plane_state->map);
}
EXPORT_SYMBOL(drm_gem_cleanup_shadow_fb);
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 975a3df0561e..cc4465100cc2 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -15,6 +15,8 @@
#include
#include
+#include "drm_internal.h"
+
#define AFBC_HEADER_SIZE 16
#define AFBC_TH_LAYOUT_ALIGNMENT 8
#define AFBC_HDR_ALIGN 64
@@ -308,6 +310,74 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev,
struct drm_file *file,
}
EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
+/**
+ * drm_gem_fb_vmap - maps all framebuffer BOs into kernel address space
+ * @fb: the framebuffer
+ * @map: returns the mapping's address for each BO
+ *
+ * This function maps all buffer objects of the given framebuffer into
+ * kernel address space and stores them in struct dma_buf_map. If the
+ * mapping operation fails for one of the BOs, the function unmaps the
+ * already established mappings automatically.
+ *
+ * See drm_gem_fb_vunmap() for unmapping.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct dma_buf_map
map[DRM_FORMAT_MAX_PLANES])
+{
+ struct drm_gem_object *obj;
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < DRM_FORMAT_MAX_PLANES; ++i) {
+ obj = drm_gem_fb_get_obj(fb, i);
+ if (!obj)
+ continue;
+ ret = drm_gem_vmap(obj, &map[i]);
+ if (ret)
+ goto err_drm_gem_vunmap;
+ }
+
+ return 0;
+
+err_drm_gem_vunmap:
+ while (i) {
+ --i;
+ obj = drm_gem_fb_get_obj(fb, i);
+ if (!obj)
+ continue;
+ drm_ge
[PATCH 0/5] drm: Provide framebuffer vmap helpers
Add the new helpers drm_gem_fb_vmap() and drm_gem_fb_vunmap(), which
provide vmap/vunmap for all BOs of a framebuffer. Convert shadow-
plane helpers, gud and vkms.
Callers of GEM vmap and vunmap functions used to do the minumum work
or get some detail wrong. Therefore shadow-plane helpers were intro-
duced to implement the details for all callers. The vmapping code in
the shadow-plane helpers is also useful for gud and vkms. So it makes
sense to provide rsp helpers. Simply call drm_gem_fb_vmap() to
retrieve mappings of all of a framebuffer's BOs.
Future work: besides the mapping's addresses, drm_gem_fb_vmap() should
also return the mappings with the frambuffer data offset added. These
are the addresses were the actual image data is located. A follow-up
set of patches will implement this feature.
Thomas Zimmermann (5):
drm: Define DRM_FORMAT_MAX_PLANES
drm/gem: Provide drm_gem_fb_{vmap,vunmap}()
drm/gem: Clear mapping addresses for unused framebuffer planes
drm/gud: Map framebuffer BOs with drm_gem_fb_vmap()
drm/vkms: Map output framebuffer BOs with drm_gem_fb_vmap()
drivers/gpu/drm/drm_gem_atomic_helper.c | 37 +---
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 88 ++--
drivers/gpu/drm/gud/gud_pipe.c | 11 +--
drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
drivers/gpu/drm/vkms/vkms_writeback.c| 21 +++--
include/drm/drm_fourcc.h | 13 ++-
include/drm/drm_framebuffer.h| 8 +-
include/drm/drm_gem_atomic_helper.h | 3 +-
include/drm/drm_gem_framebuffer_helper.h | 5 ++
10 files changed, 127 insertions(+), 67 deletions(-)
base-commit: 4d00e2309398147acdbfefbe1deb4b0e78868466
--
2.32.0
[PATCH 3/5] drm/gem: Clear mapping addresses for unused framebuffer planes
Set the returned mapping address to NULL if a framebuffer plane does
not have a BO associated with it. Likewise, ignore mappings of NULL
during framebuffer unmap operations. Allows users of the functions to
perform unmap operations of certain BOs by themselfes.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index cc4465100cc2..15b0fe3ee7b6 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -333,8 +333,10 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct
dma_buf_map map[DRM_FORMA
for (i = 0; i < DRM_FORMAT_MAX_PLANES; ++i) {
obj = drm_gem_fb_get_obj(fb, i);
- if (!obj)
+ if (!obj) {
+ dma_buf_map_clear(&map[i]);
continue;
+ }
ret = drm_gem_vmap(obj, &map[i]);
if (ret)
goto err_drm_gem_vunmap;
@@ -373,6 +375,8 @@ void drm_gem_fb_vunmap(struct drm_framebuffer *fb, struct
dma_buf_map map[DRM_FO
obj = drm_gem_fb_get_obj(fb, i);
if (!obj)
continue;
+ if (dma_buf_map_is_null(&map[i]))
+ continue;
drm_gem_vunmap(obj, &map[i]);
}
}
--
2.32.0
[PATCH 4/5] drm/gud: Map framebuffer BOs with drm_gem_fb_vmap()
Abstract the framebuffer details by mapping its BOs with a call
to drm_gem_fb_vmap(). Unmap with drm_gem_fb_vunmap().
The call to drm_gem_fb_vmap() ensures that all BOs are mapped
correctly. Gud still only supports single-plane formats.
No functional changes.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/gud/gud_pipe.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index 8f56bf618ac2..8243c8682366 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -15,7 +15,8 @@
#include
#include
#include
-#include
+#include
+#include
#include
#include
#include
@@ -152,7 +153,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct
drm_framebuffer *fb,
{
struct dma_buf_attachment *import_attach = fb->obj[0]->import_attach;
u8 compression = gdrm->compression;
- struct dma_buf_map map;
+ struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
void *vaddr, *buf;
size_t pitch, len;
int ret = 0;
@@ -162,11 +163,11 @@ static int gud_prep_flush(struct gud_device *gdrm, struct
drm_framebuffer *fb,
if (len > gdrm->bulk_len)
return -E2BIG;
- ret = drm_gem_shmem_vmap(fb->obj[0], &map);
+ ret = drm_gem_fb_vmap(fb, map);
if (ret)
return ret;
- vaddr = map.vaddr + fb->offsets[0];
+ vaddr = map[0].vaddr + fb->offsets[0];
if (import_attach) {
ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
DMA_FROM_DEVICE);
@@ -228,7 +229,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct
drm_framebuffer *fb,
if (import_attach)
dma_buf_end_cpu_access(import_attach->dmabuf, DMA_FROM_DEVICE);
vunmap:
- drm_gem_shmem_vunmap(fb->obj[0], &map);
+ drm_gem_fb_vunmap(fb, map);
return ret;
}
--
2.32.0
[PATCH 5/5] drm/vkms: Map output framebuffer BOs with drm_gem_fb_vmap()
Abstract the framebuffer details by mappings its BOs with a call
to drm_gem_fb_vmap(). Unmap with drm_gem_fb_vunamp().
Before, the output address with stored as raw pointer in the priv
field of struct drm_writeback_job. Introduce the new type
struct vkms_writeback_job, which holds the output mappings addresses
while the writeback job is active.
The patchset also cleans up some internal casting an setup of the
output addresses. No functional changes.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
drivers/gpu/drm/vkms/vkms_writeback.c | 21 ++---
3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index ead8fff81f30..49f109c3a2b3 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -257,7 +257,7 @@ void vkms_composer_worker(struct work_struct *work)
return;
if (wb_pending)
- vaddr_out = crtc_state->active_writeback;
+ vaddr_out = crtc_state->active_writeback->map[0].vaddr;
ret = compose_active_planes(&vaddr_out, primary_composer,
crtc_state);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 8c731b6dcba7..8bc9e3f52e1f 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -20,6 +20,10 @@
#define XRES_MAX 8192
#define YRES_MAX 8192
+struct vkms_writeback_job {
+ struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
+};
+
struct vkms_composer {
struct drm_framebuffer fb;
struct drm_rect src, dst;
@@ -57,7 +61,7 @@ struct vkms_crtc_state {
int num_active_planes;
/* stack of active planes for crc computation, should be in z order */
struct vkms_plane_state **active_planes;
- void *active_writeback;
+ struct vkms_writeback_job *active_writeback;
/* below four are protected by vkms_output.composer_lock */
bool crc_pending;
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c
b/drivers/gpu/drm/vkms/vkms_writeback.c
index 0935686475a0..765bb85ba76c 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -65,21 +65,23 @@ static int vkms_wb_connector_get_modes(struct drm_connector
*connector)
static int vkms_wb_prepare_job(struct drm_writeback_connector *wb_connector,
struct drm_writeback_job *job)
{
- struct drm_gem_object *gem_obj;
- struct dma_buf_map map;
+ struct vkms_writeback_job *vkmsjob;
int ret;
if (!job->fb)
return 0;
- gem_obj = drm_gem_fb_get_obj(job->fb, 0);
- ret = drm_gem_shmem_vmap(gem_obj, &map);
+ vkmsjob = kzalloc(sizeof(*vkmsjob), GFP_KERNEL);
+ if (!vkmsjob)
+ return -ENOMEM;
+
+ ret = drm_gem_fb_vmap(job->fb, vkmsjob->map);
if (ret) {
DRM_ERROR("vmap failed: %d\n", ret);
return ret;
}
- job->priv = map.vaddr;
+ job->priv = vkmsjob;
return 0;
}
@@ -87,18 +89,15 @@ static int vkms_wb_prepare_job(struct
drm_writeback_connector *wb_connector,
static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector,
struct drm_writeback_job *job)
{
- struct drm_gem_object *gem_obj;
+ struct vkms_writeback_job *vkmsjob = job->priv;
struct vkms_device *vkmsdev;
- struct dma_buf_map map;
if (!job->fb)
return;
- gem_obj = drm_gem_fb_get_obj(job->fb, 0);
- dma_buf_map_set_vaddr(&map, job->priv);
- drm_gem_shmem_vunmap(gem_obj, &map);
+ drm_gem_fb_vunmap(job->fb, vkmsjob->map);
- vkmsdev = drm_device_to_vkms_device(gem_obj->dev);
+ vkmsdev = drm_device_to_vkms_device(job->fb->dev);
vkms_set_composer(&vkmsdev->output, false);
}
--
2.32.0
Re: [PATCH 1/2] drm: add crtc background color property
On 2021-07-15 5:34 a.m., Pekka Paalanen wrote: > On Wed, 14 Jul 2021 12:13:58 -0400 > Harry Wentland wrote: > >> On 2021-07-14 3:35 a.m., Pekka Paalanen wrote: >>> On Tue, 13 Jul 2021 09:54:35 -0400 >>> Harry Wentland wrote: >>> On 2021-07-13 3:52 a.m., Pekka Paalanen wrote: > On Mon, 12 Jul 2021 12:15:59 -0400 > Harry Wentland wrote: > >> On 2021-07-12 4:03 a.m., Pekka Paalanen wrote: >>> On Fri, 9 Jul 2021 18:23:26 +0200 >>> Raphael Gallais-Pou wrote: >>> On 7/9/21 10:04 AM, Pekka Paalanen wrote: > On Wed, 7 Jul 2021 08:48:47 + > Raphael GALLAIS-POU - foss wrote: > >> Some display controllers can be programmed to present non-black >> colors >> for pixels not covered by any plane (or pixels covered by the >> transparent regions of higher planes). Compositors that want a UI >> with >> a solid color background can potentially save memory bandwidth by >> setting the CRTC background property and using smaller planes to >> display >> the rest of the content. >> >> To avoid confusion between different ways of encoding RGB data, we >> define a standard 64-bit format that should be used for this >> property's >> value. Helper functions and macros are provided to generate and >> dissect >> values in this standard format with varying component precision >> values. >> >> Signed-off-by: Raphael Gallais-Pou >> Signed-off-by: Matt Roper >> --- >> drivers/gpu/drm/drm_atomic_state_helper.c | 1 + >> drivers/gpu/drm/drm_atomic_uapi.c | 4 +++ >> drivers/gpu/drm/drm_blend.c | 34 >> +-- >> drivers/gpu/drm/drm_mode_config.c | 6 >> include/drm/drm_blend.h | 1 + >> include/drm/drm_crtc.h| 12 >> include/drm/drm_mode_config.h | 5 >> include/uapi/drm/drm_mode.h | 28 +++ >> 8 files changed, 89 insertions(+), 2 deletions(-) > > ... > > The question about full vs. limited range seems unnecessary to me, as > the background color will be used as-is in the blending stage, so > userspace can just program the correct value that fits the pipeline it > is setting up. > > One more question is, as HDR exists, could we need background colors > with component values greater than 1.0? AR4H color format should cover that case, isn't it ? >>> >>> Yes, but with the inconvenience I mentioned. >>> >>> This is a genuine question though, would anyone actually need >>> background color values > 1.0. I don't know of any case yet where it >>> would be required. It would imply that plane blending happens in a >>> color space where >1.0 values are meaningful. I'm not even sure if any >>> hardware supporting that exists. >>> >>> Maybe it would be best to assume that only [0.0, 1.0] pixel value range >>> is useful, and mention in the commit message that if someone really >>> needs values outside of that, they should create another background >>> color property. Then, you can pick a simple unsigned integer pixel >>> format, too. (I didn't see any 16 bit-per-channel formats like that in >>> drm_fourcc.h though.) >>> >> >> I don't think we should artificially limit this to [0.0, 1.0]. As you >> mentioned above when talking about full vs limited, the userspace >> understands what's the correct value that fits the pipeline. If that >> pipeline is FP16 with > 1.0 values then it would make sense that the >> background color can be > 1.0. > > Ok. The standard FP32 format then for ease of use and guaranteed enough > range and precision for far into the future? > I don't have a strong preference for FP16 vs FP32. My understanding is that FP16 is enough to represent linearly encoded data in a way that looks smooth to humans. scRGB uses FP16 with linear encoding in a range of [-0.5, 7.4999]. > Or do you want to keep it in 64 bits total, so the UABI can pack > everything into a u64 instead of needing to create a blob? > > I don't mind as long as it's clearly documented what it is and how it > works, and it carries enough precision. > > But FP16 with its 10 bits of precision might be too little for integer > 12-16 bpc pipelines and sinks? >>> >>> The 10 bits worries me still. >>> >>> If you have a pipeline that works in [0.0, 1.0] range only, then FP16 >>> limits precision to 10 bits (in the upper half of the range?). >>> > > If the v
Patch "drm/ast: Remove reference to struct drm_device.pdev" has been added to the 5.10-stable tree
This is a note to let you know that I've just added the patch titled
drm/ast: Remove reference to struct drm_device.pdev
to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-ast-remove-reference-to-struct-drm_device.pdev.patch
and it can be found in the queue-5.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 0ecb51824e838372e01330752503ddf9c0430ef7 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann
Date: Thu, 29 Apr 2021 12:50:57 +0200
Subject: drm/ast: Remove reference to struct drm_device.pdev
From: Thomas Zimmermann
commit 0ecb51824e838372e01330752503ddf9c0430ef7 upstream.
Using struct drm_device.pdev is deprecated. Upcast with to_pci_dev()
from struct drm_device.dev to get the PCI device structure.
v9:
* fix remaining pdev references
Signed-off-by: Thomas Zimmermann
Reviewed-by: Michael J. Ruhl
Fixes: ba4e0339a6a3 ("drm/ast: Fixed CVE for DP501")
Cc: KuoHsiang Chou
Cc: kernel test robot
Cc: Thomas Zimmermann
Cc: Dave Airlie
Cc: dri-devel@lists.freedesktop.org
Link:
https://patchwork.freedesktop.org/patch/msgid/20210429105101.25667-2-tzimmerm...@suse.de
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/ast/ast_main.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -406,7 +406,6 @@ struct ast_private *ast_device_create(st
return ast;
dev = &ast->base;
- dev->pdev = pdev;
pci_set_drvdata(pdev, dev);
ast->regs = pcim_iomap(pdev, 1, 0);
@@ -448,8 +447,8 @@ struct ast_private *ast_device_create(st
/* map reserved buffer */
ast->dp501_fw_buf = NULL;
- if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
- ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0,
dev->vram_mm->vram_size, 0);
+ if (dev->vram_mm->vram_size < pci_resource_len(pdev, 0)) {
+ ast->dp501_fw_buf = pci_iomap_range(pdev, 0,
dev->vram_mm->vram_size, 0);
if (!ast->dp501_fw_buf)
drm_info(dev, "failed to map reserved buffer!\n");
}
Patches currently in stable-queue which might be from tzimmerm...@suse.de are
queue-5.10/drm-mxsfb-don-t-select-drm_kms_fb_helper.patch
queue-5.10/drm-ast-remove-reference-to-struct-drm_device.pdev.patch
queue-5.10/drm-vc4-crtc-skip-the-txp.patch
queue-5.10/drm-zte-don-t-select-drm_kms_fb_helper.patch
queue-5.10/drm-ast-fixed-cve-for-dp501.patch
queue-5.10/drm-vc4-txp-properly-set-the-possible_crtcs-mask.patch
Patch "drm/ast: Remove reference to struct drm_device.pdev" has been added to the 5.12-stable tree
This is a note to let you know that I've just added the patch titled
drm/ast: Remove reference to struct drm_device.pdev
to the 5.12-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-ast-remove-reference-to-struct-drm_device.pdev.patch
and it can be found in the queue-5.12 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 0ecb51824e838372e01330752503ddf9c0430ef7 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann
Date: Thu, 29 Apr 2021 12:50:57 +0200
Subject: drm/ast: Remove reference to struct drm_device.pdev
From: Thomas Zimmermann
commit 0ecb51824e838372e01330752503ddf9c0430ef7 upstream.
Using struct drm_device.pdev is deprecated. Upcast with to_pci_dev()
from struct drm_device.dev to get the PCI device structure.
v9:
* fix remaining pdev references
Signed-off-by: Thomas Zimmermann
Reviewed-by: Michael J. Ruhl
Fixes: ba4e0339a6a3 ("drm/ast: Fixed CVE for DP501")
Cc: KuoHsiang Chou
Cc: kernel test robot
Cc: Thomas Zimmermann
Cc: Dave Airlie
Cc: dri-devel@lists.freedesktop.org
Link:
https://patchwork.freedesktop.org/patch/msgid/20210429105101.25667-2-tzimmerm...@suse.de
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/ast/ast_main.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -411,7 +411,6 @@ struct ast_private *ast_device_create(co
return ast;
dev = &ast->base;
- dev->pdev = pdev;
pci_set_drvdata(pdev, dev);
ast->regs = pcim_iomap(pdev, 1, 0);
@@ -453,8 +452,8 @@ struct ast_private *ast_device_create(co
/* map reserved buffer */
ast->dp501_fw_buf = NULL;
- if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
- ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0,
dev->vram_mm->vram_size, 0);
+ if (dev->vram_mm->vram_size < pci_resource_len(pdev, 0)) {
+ ast->dp501_fw_buf = pci_iomap_range(pdev, 0,
dev->vram_mm->vram_size, 0);
if (!ast->dp501_fw_buf)
drm_info(dev, "failed to map reserved buffer!\n");
}
Patches currently in stable-queue which might be from tzimmerm...@suse.de are
queue-5.12/drm-ingenic-fix-pixclock-rate-for-24-bit-serial-panels.patch
queue-5.12/drm-mxsfb-don-t-select-drm_kms_fb_helper.patch
queue-5.12/drm-ast-remove-reference-to-struct-drm_device.pdev.patch
queue-5.12/drm-vc4-crtc-skip-the-txp.patch
queue-5.12/drm-zte-don-t-select-drm_kms_fb_helper.patch
queue-5.12/drm-ast-fixed-cve-for-dp501.patch
queue-5.12/drm-vc4-txp-properly-set-the-possible_crtcs-mask.patch
Patch "drm/ast: Remove reference to struct drm_device.pdev" has been added to the 5.13-stable tree
This is a note to let you know that I've just added the patch titled
drm/ast: Remove reference to struct drm_device.pdev
to the 5.13-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-ast-remove-reference-to-struct-drm_device.pdev.patch
and it can be found in the queue-5.13 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 0ecb51824e838372e01330752503ddf9c0430ef7 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann
Date: Thu, 29 Apr 2021 12:50:57 +0200
Subject: drm/ast: Remove reference to struct drm_device.pdev
From: Thomas Zimmermann
commit 0ecb51824e838372e01330752503ddf9c0430ef7 upstream.
Using struct drm_device.pdev is deprecated. Upcast with to_pci_dev()
from struct drm_device.dev to get the PCI device structure.
v9:
* fix remaining pdev references
Signed-off-by: Thomas Zimmermann
Reviewed-by: Michael J. Ruhl
Fixes: ba4e0339a6a3 ("drm/ast: Fixed CVE for DP501")
Cc: KuoHsiang Chou
Cc: kernel test robot
Cc: Thomas Zimmermann
Cc: Dave Airlie
Cc: dri-devel@lists.freedesktop.org
Link:
https://patchwork.freedesktop.org/patch/msgid/20210429105101.25667-2-tzimmerm...@suse.de
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/ast/ast_main.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -411,7 +411,6 @@ struct ast_private *ast_device_create(co
return ast;
dev = &ast->base;
- dev->pdev = pdev;
pci_set_drvdata(pdev, dev);
ast->regs = pcim_iomap(pdev, 1, 0);
@@ -453,8 +452,8 @@ struct ast_private *ast_device_create(co
/* map reserved buffer */
ast->dp501_fw_buf = NULL;
- if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
- ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0,
dev->vram_mm->vram_size, 0);
+ if (dev->vram_mm->vram_size < pci_resource_len(pdev, 0)) {
+ ast->dp501_fw_buf = pci_iomap_range(pdev, 0,
dev->vram_mm->vram_size, 0);
if (!ast->dp501_fw_buf)
drm_info(dev, "failed to map reserved buffer!\n");
}
Patches currently in stable-queue which might be from tzimmerm...@suse.de are
queue-5.13/drm-ingenic-fix-pixclock-rate-for-24-bit-serial-panels.patch
queue-5.13/drm-mxsfb-don-t-select-drm_kms_fb_helper.patch
queue-5.13/drm-ast-remove-reference-to-struct-drm_device.pdev.patch
queue-5.13/drm-vc4-crtc-skip-the-txp.patch
queue-5.13/drm-zte-don-t-select-drm_kms_fb_helper.patch
queue-5.13/drm-ast-fixed-cve-for-dp501.patch
queue-5.13/drm-vc4-txp-properly-set-the-possible_crtcs-mask.patch
Re: [PATCH 3/4] drm/i915/userptr: Probe existence of backing struct pages upon creation
On Thursday, July 15, 2021 4:27:44 AM PDT Tvrtko Ursulin wrote:
>
> On 15/07/2021 12:07, Daniel Vetter wrote:
> > On Thu, Jul 15, 2021 at 11:33:10AM +0100, Tvrtko Ursulin wrote:
> >>
> >> On 15/07/2021 11:15, Matthew Auld wrote:
> >>> From: Chris Wilson
> >>>
> >>> Jason Ekstrand requested a more efficient method than userptr+set-domain
> >>> to determine if the userptr object was backed by a complete set of pages
> >>> upon creation. To be more efficient than simply populating the userptr
> >>> using get_user_pages() (as done by the call to set-domain or execbuf),
> >>> we can walk the tree of vm_area_struct and check for gaps or vma not
> >>> backed by struct page (VM_PFNMAP). The question is how to handle
> >>> VM_MIXEDMAP which may be either struct page or pfn backed...
> >>>
> >>> With discrete are going to drop support for set_domain(), so offering a
> >>> way to probe the pages, without having to resort to dummy batches has
> >>> been requested.
> >>>
> >>> v2:
> >>> - add new query param for the PROPBE flag, so userspace can easily
> >>> check if the kernel supports it(Jason).
> >>> - use mmap_read_{lock, unlock}.
> >>> - add some kernel-doc.
> >>
> >> 1)
> >>
> >> I think probing is too weak to be offered as part of the uapi. What probes
> >> successfully at create time might not be there anymore at usage time. So if
> >> the pointer is not trusted at one point, why should it be at a later stage?
> >>
> >> Only thing which works for me is populate (so get_pages) at create time.
> >> But
> >> again with no guarantees they are still there at use time clearly
> >> documented.
> >
> > Populate is exactly as racy as probe. We don't support pinned userptr
> > anymore.
>
> Yes, wrote so myself - "..again with no guarantees they are still there
> at use time..".
>
> Perhaps I don't understand what problem is probe supposed to solve. It
> doesn't deal 1:1 with set_domain removal since that one actually did
> get_pages so that would be populate. But fact remains regardless that if
> userspace is given a pointer it doesn't trust, _and_ wants the check it
> for this reason or that, then probe solves nothing. Unless there is
> actually at minimum some protocol to reply to whoever sent the pointer
> like "not that pointer please".
That's exactly the point. GL_AMD_pinned_memory requires us the OpenGL
implementation to return an error for "not that pointer, please", at the
time when said pointer is supplied - not at first use.
Sure, there can be reasons why it might seem fine up front, and not work
later. But an early check of "just no, you're doing it totally wrong"
at the right moment can be helpful for application developers. While it
shouldn't really happen, if it ever did, it would be a lot more obvious
to debug than "much later on, when something randomly flushed the GPU
commands we were building, something went wrong, and we don't know why."
--Ken
signature.asc
Description: This is a digitally signed message part.
[PATCH 0/4] drm: Make modeset locking easier
From: Ville Syrjälä
While staring at some DRM_MODESET_LOCK_ALL_{BEGIN,END}() conversions
I decided I don't really like what those macros do.
The main problems that I see:
- the magic goto inside limits their usefulness to baically
a single statement, unless you're willing to look inside and
find out the name of the magic label
- can't help at all in the "we don't want to lock everything"
cases, which are quite numerous
- not at all obvious that there's a loop in there
So I figured I'd try to come up with something more universally useful.
Cc: Sean Paul
Cc: Daniel Vetter
Ville Syrjälä (4):
drm: Introduce drm_modeset_lock_ctx_retry()
drm: Introduce drm_modeset_lock_all_ctx_retry()
drm/i915: Extract intel_crtc_initial_commit()
drm/i915: Use drm_modeset_lock_ctx_retry() & co.
drivers/gpu/drm/drm_modeset_lock.c| 44
drivers/gpu/drm/i915/display/g4x_dp.c | 17 +-
drivers/gpu/drm/i915/display/intel_audio.c| 17 +-
drivers/gpu/drm/i915/display/intel_ddi.c | 16 +-
drivers/gpu/drm/i915/display/intel_display.c | 192 --
.../drm/i915/display/intel_display_debugfs.c | 45 ++--
drivers/gpu/drm/i915/display/intel_pipe_crc.c | 38 ++--
include/drm/drm_modeset_lock.h| 26 +++
8 files changed, 188 insertions(+), 207 deletions(-)
--
2.31.1
[PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
From: Ville Syrjälä
Quite a few places are hand rolling the modeset lock backoff dance.
Let's suck that into a helper macro that is easier to use without
forgetting some steps.
The main downside is probably that the implementation of
drm_with_modeset_lock_ctx() is a bit harder to read than a hand
rolled version on account of being split across three functions,
but the actual code using it ends up being much simpler.
Cc: Sean Paul
Cc: Daniel Vetter
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_modeset_lock.c | 44 ++
include/drm/drm_modeset_lock.h | 20 ++
2 files changed, 64 insertions(+)
diff --git a/drivers/gpu/drm/drm_modeset_lock.c
b/drivers/gpu/drm/drm_modeset_lock.c
index fcfe1a03c4a1..083df96632e8 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev,
return 0;
}
EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
+
+void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
+struct drm_atomic_state *state,
+unsigned int flags, int *ret)
+{
+ drm_modeset_acquire_init(ctx, flags);
+
+ if (state)
+ state->acquire_ctx = ctx;
+
+ *ret = -EDEADLK;
+}
+EXPORT_SYMBOL(_drm_modeset_lock_begin);
+
+bool _drm_modeset_lock_loop(int *ret)
+{
+ if (*ret == -EDEADLK) {
+ *ret = 0;
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(_drm_modeset_lock_loop);
+
+void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
+ struct drm_atomic_state *state,
+ int *ret)
+{
+ if (*ret == -EDEADLK) {
+ if (state)
+ drm_atomic_state_clear(state);
+
+ *ret = drm_modeset_backoff(ctx);
+ if (*ret == 0) {
+ *ret = -EDEADLK;
+ return;
+ }
+ }
+
+ drm_modeset_drop_locks(ctx);
+ drm_modeset_acquire_fini(ctx);
+}
+EXPORT_SYMBOL(_drm_modeset_lock_end);
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index aafd07388eb7..5eaad2533de5 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -26,6 +26,7 @@
#include
+struct drm_atomic_state;
struct drm_modeset_lock;
/**
@@ -203,4 +204,23 @@ modeset_lock_fail:
\
if (!drm_drv_uses_atomic_modeset(dev)) \
mutex_unlock(&dev->mode_config.mutex);
+void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx,
+struct drm_atomic_state *state,
+unsigned int flags,
+int *ret);
+bool _drm_modeset_lock_loop(int *ret);
+void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx,
+ struct drm_atomic_state *state,
+ int *ret);
+
+/*
+ * Note that one must always use "continue" rather than
+ * "break" or "return" to handle errors within the
+ * drm_modeset_lock_ctx_retry() block.
+ */
+#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \
+ for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \
+_drm_modeset_lock_loop(&(ret)); \
+_drm_modeset_lock_end((ctx), (state), &(ret)))
+
#endif /* DRM_MODESET_LOCK_H_ */
--
2.31.1
