Re: [PATCH v6 3/4] fbcon: Prevent that screen size is smaller than font size
Hi Helge,
On Tue, Jun 28, 2022 at 10:52 PM Helge Deller wrote:
> On 6/28/22 10:39, Geert Uytterhoeven wrote:
> > On Sun, Jun 26, 2022 at 12:33 PM Helge Deller wrote:
> >> We need to prevent that users configure a screen size which is smaller
> >> than the
> >> currently selected font size. Otherwise rendering chars on the screen will
> >> access memory outside the graphics memory region.
> >>
> >> This patch adds a new function fbcon_modechange_possible() which
> >> implements this check and which later may be extended with other checks
> >> if necessary. The new function is called from the FBIOPUT_VSCREENINFO
> >> ioctl handler in fbmem.c, which will return -EINVAL if userspace asked
> >> for a too small screen size.
> >>
> >> Signed-off-by: Helge Deller
> >> Reviewed-by: Daniel Vetter
> >> Cc: sta...@vger.kernel.org # v5.4+
> >
> > Thanks for your patch, which is now commit f0b6a66d33ca6e7e ("fbcon:
> > Prevent that screen size is smaller than font size") in fbdev/for-next
> >> --- a/drivers/video/fbdev/core/fbmem.c
> >> +++ b/drivers/video/fbdev/core/fbmem.c
> >> @@ -1112,7 +1112,9 @@ static long do_fb_ioctl(struct fb_info *info,
> >> unsigned int cmd,
> >> return -EFAULT;
> >> console_lock();
> >> lock_fb_info(info);
> >> - ret = fb_set_var(info, &var);
> >> + ret = fbcon_modechange_possible(info, &var);
> >
> > Again, this should be done (if done at all) after the call to
> > fb_ops.check_var(), as it breaks the FBIOPUT_VSCREENINFO rounding rule.
> >
> > What if the user just wants to display graphics, not text?
>
> Yes, I need to go back to an older version here too and check that
> the test is only run on text consoles.
> That check was dropped, due feedback that you could switch
> back from graphics (e.g. X11) to text console at any timeso the
> check for text-only is not correct.
>
> > Can't the text console be disabled instead?
>
> I think the solution is to return failure if switching back to text mode
> isn't possible if
> fonts are bigger than the screen resolution. That will be another patch.
Isn't the font a per-VC setting? Hence can't you change resolution,
switch to a different VC, and run into this?
I think the only real solution is to set the number of text columns
and/or rows to zero, and make sure that is handled correctly.
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
[Bug 216188] New: nvidiafb: unknown NV_ARCH
https://bugzilla.kernel.org/show_bug.cgi?id=216188 Bug ID: 216188 Summary: nvidiafb: unknown NV_ARCH Product: Drivers Version: 2.5 Kernel Version: 5.18.6 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: ionut_n2...@yahoo.com Regression: No Hi Kernel Team, I notice this with 5.18.6 vanilla kernel: dmesg | grep nvidiafb [3.540222] nvidiafb :01:00.0: enabling device ( -> 0003) [3.540378] nvidiafb: Device ID: 10de2520 [3.540379] nvidiafb: unknown NV_ARCH lspci -s 01:00.0 -v 01:00.0 VGA compatible controller: NVIDIA Corporation GA106M [GeForce RTX 3060 Mobile / Max-Q] (rev a1) (prog-if 00 [VGA controller]) Subsystem: ASUSTeK Computer Inc. GA106M [GeForce RTX 3060 Mobile / Max-Q] Physical Slot: 0 Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Capabilities: [100 v1] Virtual Channel Caps: LPEVC=0 RefClk=100ns PATEntryBits=1 Arb:Fixed- WRR32- WRR64- WRR128- Ctrl: ArbSelect=Fixed Status: InProgress- VC0:Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans- Arb:Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256- Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=01 Status: NegoPending- InProgress- Capabilities: [250 v1] Latency Tolerance Reporting Max snoop latency: 0ns Max no snoop latency: 0ns Capabilities: [258 v1] L1 PM Substates L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ PortCommonModeRestoreTime=255us PortTPowerOnTime=10us L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2- ASPM_L1.1- T_CommonMode=0us LTR1.2_Threshold=0ns L1SubCtl2: T_PwrOn=10us Capabilities: [128 v1] Power Budgeting Capabilities: [420 v2] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr+ CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr+ AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: Capabilities: [600 v1] Vendor Specific Information: ID=0001 Rev=1 Len=024 Capabilities: [900 v1] Secondary PCI Express LnkCtl3: LnkEquIntrruptEn- PerformEqu- LaneErrStat: 0 Capabilities: [bb0 v1] Physical Resizable BAR BAR 0: current size: 16MB, supported: 16MB BAR 1: current size: 8GB, supported: 64MB 128MB 256MB 512MB 1GB 2GB 4GB 8GB BAR 3: current size: 32MB, supported: 32MB Capabilities: [c1c v1] Physical Layer 16.0 GT/s Capabilities: [d00 v1] Lane Margining at the Receiver Capabilities: [e00 v1] Data Link Feature Kernel modules: nvidiafb, nouveau lspci 00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Renoir/Cezanne Root Complex 00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Renoir/Cezanne IOMMU 00:01.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Renoir PCIe Dummy Host Bridge 00:01.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Renoir PCIe GPP Bridge 00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Renoir PCIe Dummy Host Bridge 00:02.2 PCI bridge: Advanced Micro Devices, Inc. [AMD] Renoir/Cezanne PCIe GPP Bridge 00:02.4 PCI bridge: Advanced Micro Devices, Inc. [AMD] Renoir/Cezanne PCIe GPP Bridge 00:08.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Renoir PCIe Dummy Host Bridge 00:08.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Renoir Internal PCIe GPP Bridge to Bus 00:14.0 SMBus: Advanced Micro Devices, Inc. [AMD] FCH SMBus Controller (rev 51) 00:14.3 ISA bridge: Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge (rev 51) 00:18.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Cezanne Data Fabric; Function 0 00:18.1 Host bridge: Advanced Micro Devices, Inc. [AMD] Cezanne Data Fabric; Function 1 00:18.2 Host bridge: Advanced Micro Devices, Inc. [AMD] Cezanne Data Fabric; Functi
Re: [PATCH 6/6] i2c: Make remove callback return void
Hello,
[I dropped nearly all individuals from the Cc: list because various
bounces reported to be unhappy about the long (logical) line.]
On Wed, Jun 29, 2022 at 03:03:54PM +0800, Jeremy Kerr wrote:
> Looks good - just one minor change for the mctp-i2c driver, but only
> worthwhile if you end up re-rolling this series for other reasons:
>
> > -static int mctp_i2c_remove(struct i2c_client *client)
> > +static void mctp_i2c_remove(struct i2c_client *client)
> > {
> > struct mctp_i2c_client *mcli = i2c_get_clientdata(client);
> > struct mctp_i2c_dev *midev = NULL, *tmp = NULL;
> > @@ -1000,7 +1000,6 @@ static int mctp_i2c_remove(struct i2c_client *client)
> > mctp_i2c_free_client(mcli);
> > mutex_unlock(&driver_clients_lock);
> > /* Callers ignore return code */
> > - return 0;
> > }
>
> The comment there no longer makes much sense, I'd suggest removing that
> too.
Yeah, that was already pointed out to me in a private reply. It's
already fixed in
https://git.pengutronix.de/cgit/ukl/linux/log/?h=i2c-remove-void
> Either way:
>
> Reviewed-by: Jeremy Kerr
Added to my tree, too.
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [PATCH 00/22] Fix kernel-doc warnings at linux-next
On Tue, Jun 28, 2022 at 10:46:04AM +0100, Mauro Carvalho Chehab wrote: > As we're currently discussing about making kernel-doc issues fatal when > CONFIG_WERROR is enable, let's fix all 60 kernel-doc warnings > inside linux-next: > To be fair, besides triggering error on kernel-doc warnings, Sphinx warnings should also be errors on CONFIG_WERROR. -- An old man doll... just what I always wanted! - Clara
Re: [PATCH v7 01/14] mm: rename is_pinnable_pages to is_pinnable_longterm_pages
On 29.06.22 05:54, Alex Sierra wrote: > is_pinnable_page() and folio_is_pinnable() were renamed to > is_longterm_pinnable_page() and folio_is_longterm_pinnable() > respectively. These functions are used in the FOLL_LONGTERM flag > context. Subject talks about "*_pages" Can you elaborate why the move from mm.h to memremap.h is justified? I'd have called it "is_longterm_pinnable_page", but I am not a native speaker, so no strong opinion :) -- Thanks, David / dhildenb
Re: [PATCH 4/4] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb565()
Hi
Am 27.06.22 um 18:11 schrieb José Expósito:
Extend the existing test cases to test the conversion from XRGB to
RGB565.
The documentation and the color picker available on [1] are useful
resources to understand this patch and validate the values returned by
the conversion function.
[1] http://www.barth-dev.de/online/rgb565-color-picker/
URLs in commit messages are usually given as Link tag like this:
Link: http://www.barth-dev.de/online/rgb565-color-picker/ # [1]
Put this somewhere below your signed-off-by tag.
Best regards
Thomas
Signed-off-by: José Expósito
---
.../gpu/drm/tests/drm_format_helper_test.c| 100 +-
1 file changed, 99 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 52dc41cc7c60..3fbe8026bccc 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -48,7 +48,7 @@ struct convert_xrgb_case {
unsigned int pitch;
struct drm_rect clip;
const u32 xrgb[TEST_BUF_SIZE];
- struct convert_xrgb_result results[1];
+ struct convert_xrgb_result results[3];
};
static struct convert_xrgb_case convert_xrgb_cases[] = {
@@ -64,6 +64,26 @@ static struct convert_xrgb_case convert_xrgb_cases[]
= {
.dst_pitch = 0,
.expected = { 0xE0 },
},
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb_to_rgb565,
+ .swab = false,
+ },
+ .dst_pitch = 0,
+ .expected = { 0x00, 0xF8 },
+ },
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb_to_rgb565,
+ .swab = true,
+ },
+ .dst_pitch = 0,
+ .expected = { 0xF8, 0x00 },
+ },
},
},
{
@@ -81,6 +101,26 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = 0,
.expected = { 0xE0 },
},
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb_to_rgb565,
+ .swab = false,
+ },
+ .dst_pitch = 0,
+ .expected = { 0x00, 0xF8 },
+ },
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb_to_rgb565,
+ .swab = true,
+ },
+ .dst_pitch = 0,
+ .expected = { 0xF8, 0x00 },
+ },
},
},
{
@@ -110,6 +150,36 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0xFC, 0x1F,
},
},
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb_to_rgb565,
+ .swab = false,
+ },
+ .dst_pitch = 0,
+ .expected = {
+ 0xFF, 0xFF, 0x00, 0x00,
+ 0x00, 0xF8, 0xE0, 0x07,
+ 0x1F, 0x00, 0x1F, 0xF8,
+ 0xE0, 0xFF, 0xFF, 0x07,
+ },
+ },
+ {
+ .dst_format = DRM_FORMAT_RGB565,
+ .has_swab = true,
+ .conv_swab = {
+ .func = drm_fb_xrgb_to_rgb565,
+
Re: [PATCH 6/6] i2c: Make remove callback return void
On 6/29/22 09:23, Uwe Kleine-König wrote:
> Hello,
>
> [I dropped nearly all individuals from the Cc: list because various
> bounces reported to be unhappy about the long (logical) line.]
>
Yes, it also bounced for me when I tried to reply earlier today.
> diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c
> b/drivers/gpu/drm/solomon/ssd130x-i2c.c
> index 1e0fcec7be47..ddfa0bb5d9c9 100644
> --- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
> +++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
> @@ -39,13 +39,11 @@ static int ssd130x_i2c_probe(struct i2c_client *client)
> return 0;
> }
>
> -static int ssd130x_i2c_remove(struct i2c_client *client)
> +static void ssd130x_i2c_remove(struct i2c_client *client)
> {
> struct ssd130x_device *ssd130x = i2c_get_clientdata(client);
>
> ssd130x_remove(ssd130x);
> -
> - return 0;
> }
>
> static void ssd130x_i2c_shutdown(struct i2c_client *client)
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH 0/4] KUnit tests for RGB565 conversion
Hi Am 27.06.22 um 18:11 schrieb José Expósito: Hello everyone, This series is a follow up of the XRGB to RGB332 conversion KUnit tests. The first 3 patches refactor the existing test to make them agnostic of the target format and add support for "swab". The last patch adds the RGB565 conversion values, and shows how more formats will be easily added in the future. Thanks for your patches. I had one comment for the final one. With this fixed, you can add Acked-by: Thomas Zimmermann to the series. Best regards Thomas Thank you very much in advance for your feedback, José Expósito José Expósito (4): drm/format-helper: Rename test cases to make them more generic drm/format-helper: Transform tests to be agnostic of target format drm/format-helper: Add support for conversion functions with swab drm/format-helper: Add KUnit tests for drm_fb_xrgb_to_rgb565() .../gpu/drm/tests/drm_format_helper_test.c| 231 +++--- 1 file changed, 196 insertions(+), 35 deletions(-) base-commit: 6fde8eec71796f3534f0c274066862829813b21f prerequisite-patch-id: 8a16f4c8004d6161035eaea275c8eafaa0ac927e prerequisite-patch-id: 53fded2a49e6212b546db76ec52563a683752e65 prerequisite-patch-id: 294b0ca27a6ee57096c8f097c0572336b8a2d583 prerequisite-patch-id: 5e05bfc5287d16c207bfc616b2776ad72eb4ab29 prerequisite-patch-id: e94560be85dffb62a5b3cf58d1f0fc3d278ad806 prerequisite-patch-id: a471df39c7b32c69dd2b138a7d0af015ea42e00a -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 6/6] i2c: Make remove callback return void
Le 29/06/2022 à 09:23, Uwe Kleine-König a écrit :
> Hello,
>
> [I dropped nearly all individuals from the Cc: list because various
> bounces reported to be unhappy about the long (logical) line.]
Good idea, even patchwork made a mess of it, see
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220628140313.74984-7-u.kleine-koe...@pengutronix.de/
>
> On Wed, Jun 29, 2022 at 03:03:54PM +0800, Jeremy Kerr wrote:
>> Looks good - just one minor change for the mctp-i2c driver, but only
>> worthwhile if you end up re-rolling this series for other reasons:
>>
>>> -static int mctp_i2c_remove(struct i2c_client *client)
>>> +static void mctp_i2c_remove(struct i2c_client *client)
>>> {
>>> struct mctp_i2c_client *mcli = i2c_get_clientdata(client);
>>> struct mctp_i2c_dev *midev = NULL, *tmp = NULL;
>>> @@ -1000,7 +1000,6 @@ static int mctp_i2c_remove(struct i2c_client *client)
>>> mctp_i2c_free_client(mcli);
>>> mutex_unlock(&driver_clients_lock);
>>> /* Callers ignore return code */
>>> - return 0;
>>> }
>>
>> The comment there no longer makes much sense, I'd suggest removing that
>> too.
>
> Yeah, that was already pointed out to me in a private reply. It's
> already fixed in
>
> https://git.pengutronix.de/cgit/ukl/linux/log/?h=i2c-remove-void
>
>> Either way:
>>
>> Reviewed-by: Jeremy Kerr
>
> Added to my tree, too.
>
> Thanks
> Uwe
>
Re: [PATCH 6/6] i2c: Make remove callback return void
On 6/29/22 09:55, Christophe Leroy wrote: > > > Le 29/06/2022 à 09:23, Uwe Kleine-König a écrit : >> Hello, >> >> [I dropped nearly all individuals from the Cc: list because various >> bounces reported to be unhappy about the long (logical) line.] > > Good idea, even patchwork made a mess of it, see > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220628140313.74984-7-u.kleine-koe...@pengutronix.de/ > FYI, for patches like these what I usually use is: ./scripts/get_maintainer.pl --nogit-fallback --no-m --no-r -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: Annoying AMDGPU boot-time warning due to simplefb / amdgpu resource clash
On 6/28/22 14:41, Jocelyn Falempe wrote:
> On 28/06/2022 10:43, Thomas Zimmermann wrote:
[snip]
>>
>> Unfortunately, this currently only works if a driver is bound to the
>> platform device. Without simpledrm or simplefb, amdgpu won't find the
>> platform device to remove.
>>
>> I guess, what happens on your system is that sysfb create a device for
>> the EFI framebuffer and then amdgpu comes and doesn't find it for
>> removal. And later you see these warnings because BOOTFB is still around.
>>
>> Javier already provided patches for this scenario, which are in the DRM
>> tree. From drm-next, please cherry-pick
>>
>> 0949ee75da6c ("firmware: sysfb: Make sysfb_create_simplefb() return a
>> pdev pointer")
>>
>> bc824922b264 ("firmware: sysfb: Add sysfb_disable() helper function")
>>
>> 873eb3b11860 ("fbdev: Disable sysfb device registration when removing
>> conflicting FBs")
>>
>> for testing. With these patches, amdgpu will find the sysfb device and
>> unregister it.
>>
>> The patches are queued up for the next merge window. If they resolve the
>> issue, we'll already send with the next round of fixes.
>
> I was able to reproduce the warning with kernel v5.19-rc4, a radeon GPU
> and the following config:
>
> CONFIG_SYSFB=y
> CONFIG_SYSFB_SIMPLEFB=y
> # CONFIG_DRM_SIMPLEDRM is not set
> # CONFIG_FB_SIMPLE is not set
>
Yes, and this could happen even if both drivers (e.g: simplefb and amdgpu)
are enabled but built-in. For example, if amdgpu probes before simplefb.
> After applying the 3 patches you mentioned, the issue is resolved. (at
> least on my setup).
>
Thanks for testing Jocelyn! I've pushed the patches to drm-misc-fixes now.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v6 02/22] drm/gem: Move mapping of imported dma-bufs to drm_gem_mmap_obj()
On 6/29/22 09:40, Thomas Hellström (Intel) wrote: > > On 5/27/22 01:50, Dmitry Osipenko wrote: >> Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers don't >> handle imported dma-bufs properly, which results in mapping of something >> else than the imported dma-buf. For example, on NVIDIA Tegra we get a >> hard >> lockup when userspace writes to the memory mapping of a dma-buf that was >> imported into Tegra's DRM GEM. >> >> To fix this bug, move mapping of imported dma-bufs to drm_gem_mmap_obj(). >> Now mmaping of imported dma-bufs works properly for all DRM drivers. > Same comment about Fixes: as in patch 1, >> >> Cc: sta...@vger.kernel.org >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/gpu/drm/drm_gem.c | 3 +++ >> drivers/gpu/drm/drm_gem_shmem_helper.c | 9 - >> drivers/gpu/drm/tegra/gem.c | 4 >> 3 files changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >> index 86d670c71286..7c0b025508e4 100644 >> --- a/drivers/gpu/drm/drm_gem.c >> +++ b/drivers/gpu/drm/drm_gem.c >> @@ -1038,6 +1038,9 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, >> unsigned long obj_size, >> if (obj_size < vma->vm_end - vma->vm_start) >> return -EINVAL; >> + if (obj->import_attach) >> + return dma_buf_mmap(obj->dma_buf, vma, 0); > > If we start enabling mmaping of imported dma-bufs on a majority of > drivers in this way, how do we ensure that user-space is not blindly > using the object mmap without calling the needed DMA_BUF_IOCTL_SYNC > which is needed before and after cpu access of mmap'ed dma-bufs? > > I was under the impression (admittedly without looking) that the few > drivers that actually called into dma_buf_mmap() had some private > user-mode driver code in place that ensured this happened. Since it's a userspace who does the mapping, then it should be a responsibility of userspace to do all the necessary syncing. I'm not sure whether anyone in userspace really needs to map imported dma-bufs in practice. Nevertheless, this use-case is broken and should be fixed by either allowing to do the mapping or prohibiting it. -- Best regards, Dmitry
Re: [Intel-gfx] [PATCH 2/2] drm/i915/fbdev: suspend HPD before fbdev unregistration
Thanks for comments,
On 29.06.2022 07:38, Murthy, Arun R wrote:
void intel_fbdev_unregister(struct drm_i915_private *dev_priv) {
struct intel_fbdev *ifbdev = dev_priv->fbdev; @@ -573,6 +594,8 @@
void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
if (!ifbdev)
return;
+ intel_fbdev_hpd_set_suspend(dev_priv,
FBINFO_STATE_SUSPENDED);
+
Instead of intel_fbdev_hpd_set_suspend(), will intel_fbdev_set_suspend() make
more sense?
If intel_fbdev_set_suspend() is called, then the below cancel_work_sync() may
not be required.
It does more than I needed (calls drm_fb_helper_set_suspend), but it
shouldn't hurt. I will try this approach.
Regards
Andrzej
cancel_work_sync(&dev_priv->fbdev_suspend_work);
if (!current_is_async())
intel_fbdev_sync(ifbdev);
Thanks and Regards,
Arun R Murthy
Re: [PATCH v6 01/22] drm/gem: Properly annotate WW context on drm_gem_lock_reservations() error
On 6/28/22 23:12, Thomas Hellström (Intel) wrote: > Hi, > > On 5/27/22 01:50, Dmitry Osipenko wrote: >> Use ww_acquire_fini() in the error code paths. Otherwise lockdep >> thinks that lock is held when lock's memory is freed after the >> drm_gem_lock_reservations() error. The WW needs to be annotated >> as "freed" > > s /WW/ww_acquire_context/ ? > s /"freed"/"released"/ ? > > >> , which fixes the noisy "WARNING: held lock freed!" splat >> of VirtIO-GPU driver with CONFIG_DEBUG_MUTEXES=y and enabled lockdep. >> >> Cc: sta...@vger.kernel.org > > Can you dig up the commit in error and add a Fixes: Tag? > > Using that and "dim fixes" will also make the Cc: stable tag a bit more > verbose. > > With that fixed, > > Reviewed-by: Thomas Hellström I'll update this patch, thank you for taking a look. -- Best regards, Dmitry
Re: [PATCH v6 02/22] drm/gem: Move mapping of imported dma-bufs to drm_gem_mmap_obj()
On 6/29/22 10:22, Dmitry Osipenko wrote: On 6/29/22 09:40, Thomas Hellström (Intel) wrote: On 5/27/22 01:50, Dmitry Osipenko wrote: Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers don't handle imported dma-bufs properly, which results in mapping of something else than the imported dma-buf. For example, on NVIDIA Tegra we get a hard lockup when userspace writes to the memory mapping of a dma-buf that was imported into Tegra's DRM GEM. To fix this bug, move mapping of imported dma-bufs to drm_gem_mmap_obj(). Now mmaping of imported dma-bufs works properly for all DRM drivers. Same comment about Fixes: as in patch 1, Cc: sta...@vger.kernel.org Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/drm_gem.c | 3 +++ drivers/gpu/drm/drm_gem_shmem_helper.c | 9 - drivers/gpu/drm/tegra/gem.c | 4 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 86d670c71286..7c0b025508e4 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1038,6 +1038,9 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL; + if (obj->import_attach) + return dma_buf_mmap(obj->dma_buf, vma, 0); If we start enabling mmaping of imported dma-bufs on a majority of drivers in this way, how do we ensure that user-space is not blindly using the object mmap without calling the needed DMA_BUF_IOCTL_SYNC which is needed before and after cpu access of mmap'ed dma-bufs? I was under the impression (admittedly without looking) that the few drivers that actually called into dma_buf_mmap() had some private user-mode driver code in place that ensured this happened. Since it's a userspace who does the mapping, then it should be a responsibility of userspace to do all the necessary syncing. Sure, but nothing prohibits user-space to ignore the syncing thinking "It works anyway", testing those drivers where the syncing is a NOP. And when a driver that finally needs syncing is tested it's too late to fix all broken user-space. I'm not sure whether anyone in userspace really needs to map imported dma-bufs in practice. Nevertheless, this use-case is broken and should be fixed by either allowing to do the mapping or prohibiting it. Then I'd vote for prohibiting it, at least for now. And for the future moving forward we could perhaps revisit the dma-buf need for syncing, requiring those drivers that actually need it to implement emulated coherent memory which can be done not too inefficiently (vmwgfx being one example). /Thomas
Re: [PATCH v1] Fix: SYNCOBJ TIMELINE Test failed.
Am 29.06.22 um 08:02 schrieb jie1zhan: The issue cause by the commit : 721255b527(drm/syncobj: flatten dma_fence_chains on transfer). Because it use the point of dma_fence incorrectly Correct the point of dma_fence by fence array Well that patch is just utterly nonsense as far as I can see. Signed-off-by: jie1zhan Reviewed-by: Christian König Reviewed-by: Nirmoy Das I have strong doubts that Nirmoy has reviewed this and I certainly haven't reviewed it. Christian. --- drivers/gpu/drm/drm_syncobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 7e48dcd1bee4..d5db818f1c76 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -887,7 +887,7 @@ static int drm_syncobj_flatten_chain(struct dma_fence **f) goto free_fences; dma_fence_put(*f); - *f = &array->base; + *f = array->fences[0]; return 0; free_fences:
[CI RESEND 00/10] drm/edid: expand on struct drm_edid usage
Resend of [1] for CI, also sending without the i915 changes, to be merged on top. BR, Jani. [1] https://patchwork.freedesktop.org/series/104309/ Jani Nikula (10): drm/edid: move drm_connector_update_edid_property() to drm_edid.c drm/edid: convert drm_connector_update_edid_property() to struct drm_edid drm/edid: clean up connector update error handling and debug logging drm/edid: abstract debugfs override EDID set/reset drm/edid: add drm_edid_connector_update() drm/probe-helper: add drm_connector_helper_get_modes() drm/edid: add drm_edid_raw() to access the raw EDID data drm/edid: do invalid block filtering in-place drm/edid: add HF-EEODB support to EDID read and allocation drm/edid: take HF-EEODB extension count into account drivers/gpu/drm/drm_connector.c | 74 -- drivers/gpu/drm/drm_crtc_internal.h | 5 +- drivers/gpu/drm/drm_debugfs.c | 21 +- drivers/gpu/drm/drm_edid.c | 376 drivers/gpu/drm/drm_probe_helper.c | 34 +++ include/drm/drm_connector.h | 6 +- include/drm/drm_edid.h | 3 + include/drm/drm_probe_helper.h | 1 + 8 files changed, 381 insertions(+), 139 deletions(-) -- 2.30.2
[CI RESEND 01/10] drm/edid: move drm_connector_update_edid_property() to drm_edid.c
The function needs access to drm_edid.c internals more than
drm_connector.c. We can make drm_reset_display_info(),
drm_add_display_info() and drm_update_tile_info() static. There will be
more benefits with follow-up struct drm_edid refactoring.
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_connector.c | 74 -
drivers/gpu/drm/drm_crtc_internal.h | 3 -
drivers/gpu/drm/drm_edid.c | 86 +++--
3 files changed, 81 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index c7ca435ceb95..7b1b61183747 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2077,80 +2077,6 @@ int drm_connector_set_tile_property(struct drm_connector
*connector)
}
EXPORT_SYMBOL(drm_connector_set_tile_property);
-/**
- * drm_connector_update_edid_property - update the edid property of a connector
- * @connector: drm connector
- * @edid: new value of the edid property
- *
- * This function creates a new blob modeset object and assigns its id to the
- * connector's edid property.
- * Since we also parse tile information from EDID's displayID block, we also
- * set the connector's tile property here. See
drm_connector_set_tile_property()
- * for more details.
- *
- * Returns:
- * Zero on success, negative errno on failure.
- */
-int drm_connector_update_edid_property(struct drm_connector *connector,
- const struct edid *edid)
-{
- struct drm_device *dev = connector->dev;
- size_t size = 0;
- int ret;
- const struct edid *old_edid;
-
- /* ignore requests to set edid when overridden */
- if (connector->override_edid)
- return 0;
-
- if (edid)
- size = EDID_LENGTH * (1 + edid->extensions);
-
- /* Set the display info, using edid if available, otherwise
-* resetting the values to defaults. This duplicates the work
-* done in drm_add_edid_modes, but that function is not
-* consistently called before this one in all drivers and the
-* computation is cheap enough that it seems better to
-* duplicate it rather than attempt to ensure some arbitrary
-* ordering of calls.
-*/
- if (edid)
- drm_add_display_info(connector, edid);
- else
- drm_reset_display_info(connector);
-
- drm_update_tile_info(connector, edid);
-
- if (connector->edid_blob_ptr) {
- old_edid = (const struct edid *)connector->edid_blob_ptr->data;
- if (old_edid) {
- if (!drm_edid_are_equal(edid, old_edid)) {
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was
changed.\n",
- connector->base.id,
connector->name);
-
- connector->epoch_counter += 1;
- DRM_DEBUG_KMS("Updating change counter to
%llu\n",
- connector->epoch_counter);
- }
- }
- }
-
- drm_object_property_set_value(&connector->base,
- dev->mode_config.non_desktop_property,
- connector->display_info.non_desktop);
-
- ret = drm_property_replace_global_blob(dev,
- &connector->edid_blob_ptr,
- size,
- edid,
- &connector->base,
- dev->mode_config.edid_property);
- if (ret)
- return ret;
- return drm_connector_set_tile_property(connector);
-}
-EXPORT_SYMBOL(drm_connector_update_edid_property);
-
/**
* drm_connector_set_link_status_property - Set link status property of a
connector
* @connector: drm connector
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index 63279e984342..aecab5308bae 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -286,6 +286,3 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
/* drm_edid.c */
void drm_mode_fixup_1366x768(struct drm_display_mode *mode);
-void drm_reset_display_info(struct drm_connector *connector);
-u32 drm_add_display_info(struct drm_connector *connector, const struct edid
*edid);
-void drm_update_tile_info(struct drm_connector *connector, const struct edid
*edid);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 2bdaf1e34a9d..36bf7b0fe8d9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5928,8 +5928,7 @@ static void drm_update_mso(struct drm_connector
*connector,
/* A connector has no EDID information, so we've got no EDID to compute quirks
[CI RESEND 02/10] drm/edid: convert drm_connector_update_edid_property() to struct drm_edid
Make drm_connector_update_edid_property() a thin wrapper around a struct
drm_edid based version of the same.
This lets us remove the legacy drm_update_tile_info() and
drm_add_display_info() functions altogether.
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 81 --
1 file changed, 35 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 36bf7b0fe8d9..62967db78139 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6042,14 +6042,6 @@ static u32 update_display_info(struct drm_connector
*connector,
return quirks;
}
-static u32 drm_add_display_info(struct drm_connector *connector, const struct
edid *edid)
-{
- struct drm_edid drm_edid;
-
- return update_display_info(connector,
- drm_edid_legacy_init(&drm_edid, edid));
-}
-
static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device
*dev,
struct
displayid_detailed_timings_1 *timings,
bool type_7)
@@ -6206,38 +6198,19 @@ static int drm_edid_connector_update(struct
drm_connector *connector,
return num_modes;
}
-static void drm_update_tile_info(struct drm_connector *connector,
-const struct edid *edid);
+static void _drm_update_tile_info(struct drm_connector *connector,
+ const struct drm_edid *drm_edid);
-/**
- * drm_connector_update_edid_property - update the edid property of a connector
- * @connector: drm connector
- * @edid: new value of the edid property
- *
- * This function creates a new blob modeset object and assigns its id to the
- * connector's edid property.
- * Since we also parse tile information from EDID's displayID block, we also
- * set the connector's tile property here. See
drm_connector_set_tile_property()
- * for more details.
- *
- * Returns:
- * Zero on success, negative errno on failure.
- */
-int drm_connector_update_edid_property(struct drm_connector *connector,
- const struct edid *edid)
+static int _drm_connector_update_edid_property(struct drm_connector *connector,
+ const struct drm_edid *drm_edid)
{
struct drm_device *dev = connector->dev;
- size_t size = 0;
int ret;
- const struct edid *old_edid;
/* ignore requests to set edid when overridden */
if (connector->override_edid)
return 0;
- if (edid)
- size = EDID_LENGTH * (1 + edid->extensions);
-
/*
* Set the display info, using edid if available, otherwise resetting
* the values to defaults. This duplicates the work done in
@@ -6246,17 +6219,18 @@ int drm_connector_update_edid_property(struct
drm_connector *connector,
* that it seems better to duplicate it rather than attempt to ensure
* some arbitrary ordering of calls.
*/
- if (edid)
- drm_add_display_info(connector, edid);
+ if (drm_edid)
+ update_display_info(connector, drm_edid);
else
drm_reset_display_info(connector);
- drm_update_tile_info(connector, edid);
+ _drm_update_tile_info(connector, drm_edid);
if (connector->edid_blob_ptr) {
- old_edid = (const struct edid *)connector->edid_blob_ptr->data;
+ const struct edid *old_edid = connector->edid_blob_ptr->data;
+
if (old_edid) {
- if (!drm_edid_are_equal(edid, old_edid)) {
+ if (!drm_edid_are_equal(drm_edid ? drm_edid->edid :
NULL, old_edid)) {
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was
changed.\n",
connector->base.id,
connector->name);
@@ -6273,14 +6247,37 @@ int drm_connector_update_edid_property(struct
drm_connector *connector,
ret = drm_property_replace_global_blob(dev,
&connector->edid_blob_ptr,
- size,
- edid,
+ drm_edid ? drm_edid->size : 0,
+ drm_edid ? drm_edid->edid : NULL,
&connector->base,
dev->mode_config.edid_property);
if (ret)
return ret;
return drm_connector_set_tile_property(connector);
}
+
+/**
+ * drm_connector_update_edid_property - update the edid property of a connector
+ * @connector: drm connector
+ * @edid: new value of the edid property
+ *
+ * This function creates a new blob modeset object
[CI RESEND 03/10] drm/edid: clean up connector update error handling and debug logging
Bail out on all errors, debug log all errors, and convert to drm device
based debug logging.
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 41 ++
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 62967db78139..e360e1a269f4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6231,29 +6231,44 @@ static int _drm_connector_update_edid_property(struct
drm_connector *connector,
if (old_edid) {
if (!drm_edid_are_equal(drm_edid ? drm_edid->edid :
NULL, old_edid)) {
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was
changed.\n",
- connector->base.id,
connector->name);
-
- connector->epoch_counter += 1;
- DRM_DEBUG_KMS("Updating change counter to
%llu\n",
- connector->epoch_counter);
+ connector->epoch_counter++;
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID
changed, epoch counter %llu\n",
+ connector->base.id, connector->name,
+ connector->epoch_counter);
}
}
}
- drm_object_property_set_value(&connector->base,
- dev->mode_config.non_desktop_property,
- connector->display_info.non_desktop);
-
ret = drm_property_replace_global_blob(dev,
&connector->edid_blob_ptr,
drm_edid ? drm_edid->size : 0,
drm_edid ? drm_edid->edid : NULL,
&connector->base,
dev->mode_config.edid_property);
- if (ret)
- return ret;
- return drm_connector_set_tile_property(connector);
+ if (ret) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID property update failed
(%d)\n",
+ connector->base.id, connector->name, ret);
+ goto out;
+ }
+
+ ret = drm_object_property_set_value(&connector->base,
+
dev->mode_config.non_desktop_property,
+
connector->display_info.non_desktop);
+ if (ret) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Non-desktop property update
failed (%d)\n",
+ connector->base.id, connector->name, ret);
+ goto out;
+ }
+
+ ret = drm_connector_set_tile_property(connector);
+ if (ret) {
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Tile property update failed
(%d)\n",
+ connector->base.id, connector->name, ret);
+ goto out;
+ }
+
+out:
+ return ret;
}
/**
--
2.30.2
[CI RESEND 04/10] drm/edid: abstract debugfs override EDID set/reset
Add functions drm_edid_override_set() and drm_edid_override_reset() to
support "edid_override" connector debugfs, and to hide the details about
it in drm_edid.c. No functional changes at this time.
Also note in the connector.override_edid flag kernel-doc that this is
only supposed to be modified by the code doing debugfs EDID override
handling. Currently, it is still being modified by amdgpu in
create_eml_sink() and handle_edid_mgmt() for reasons unknown. This was
added in commit 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
and later moved to amdgpu_dm.c in commit e7b07ceef2a6 ("drm/amd/display:
Merge amdgpu_dm_types and amdgpu_dm").
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_crtc_internal.h | 2 ++
drivers/gpu/drm/drm_debugfs.c | 21 +
drivers/gpu/drm/drm_edid.c | 26 ++
include/drm/drm_connector.h | 6 +-
4 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index aecab5308bae..56041b604881 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -286,3 +286,5 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
/* drm_edid.c */
void drm_mode_fixup_1366x768(struct drm_display_mode *mode);
+int drm_edid_override_set(struct drm_connector *connector, const void *edid,
size_t size);
+int drm_edid_override_reset(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index fb04b7a984de..493922069c90 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -350,31 +350,20 @@ static ssize_t edid_write(struct file *file, const char
__user *ubuf,
struct seq_file *m = file->private_data;
struct drm_connector *connector = m->private;
char *buf;
- struct edid *edid;
int ret;
buf = memdup_user(ubuf, len);
if (IS_ERR(buf))
return PTR_ERR(buf);
- edid = (struct edid *) buf;
-
- if (len == 5 && !strncmp(buf, "reset", 5)) {
- connector->override_edid = false;
- ret = drm_connector_update_edid_property(connector, NULL);
- } else if (len < EDID_LENGTH ||
- EDID_LENGTH * (1 + edid->extensions) > len)
- ret = -EINVAL;
- else {
- connector->override_edid = false;
- ret = drm_connector_update_edid_property(connector, edid);
- if (!ret)
- connector->override_edid = true;
- }
+ if (len == 5 && !strncmp(buf, "reset", 5))
+ ret = drm_edid_override_reset(connector);
+ else
+ ret = drm_edid_override_set(connector, buf, len);
kfree(buf);
- return (ret) ? ret : len;
+ return ret ? ret : len;
}
/*
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e360e1a269f4..c3f0f0a5a8a9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2161,6 +2161,32 @@ static struct edid *drm_get_override_edid(struct
drm_connector *connector,
return IS_ERR(override) ? NULL : override;
}
+/* For debugfs edid_override implementation */
+int drm_edid_override_set(struct drm_connector *connector, const void *edid,
+ size_t size)
+{
+ int ret;
+
+ if (size < EDID_LENGTH || edid_size(edid) > size)
+ return -EINVAL;
+
+ connector->override_edid = false;
+
+ ret = drm_connector_update_edid_property(connector, edid);
+ if (!ret)
+ connector->override_edid = true;
+
+ return ret;
+}
+
+/* For debugfs edid_override implementation */
+int drm_edid_override_reset(struct drm_connector *connector)
+{
+ connector->override_edid = false;
+
+ return drm_connector_update_edid_property(connector, NULL);
+}
+
/**
* drm_add_override_edid_modes - add modes from override/firmware EDID
* @connector: connector we're probing
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 94b422b55cc1..a1705d6b3fba 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1527,7 +1527,11 @@ struct drm_connector {
struct drm_cmdline_mode cmdline_mode;
/** @force: a DRM_FORCE_ state for forced mode sets */
enum drm_connector_force force;
- /** @override_edid: has the EDID been overwritten through debugfs for
testing? */
+ /**
+* @override_edid: has the EDID been overwritten through debugfs for
+* testing? Do not modify outside of drm_edid_override_set() and
+* drm_edid_override_reset().
+*/
bool override_edid;
/** @epoch_counter: used to detect any other changes in connector,
besides status */
u64 epoch_counter;
--
2.30.2
[CI RESEND 05/10] drm/edid: add drm_edid_connector_update()
Add a new function drm_edid_connector_update() to replace the
combination of calls drm_connector_update_edid_property() and
drm_add_edid_modes(). Usually they are called in the drivers in this
order, however the former needs information from the latter.
Since the new drm_edid_read*() functions no longer call the connector
updates directly, and the read and update are separated, we'll need this
new function for the connector update.
This is all in drm_edid.c simply to keep struct drm_edid opaque.
v2:
- Share code with drm_connector_update_edid_property() (Ville)
- Add comment about override EDID handling
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 103 -
include/drm/drm_edid.h | 2 +
2 files changed, 81 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index c3f0f0a5a8a9..41b3de52b8f1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6160,8 +6160,8 @@ static int add_displayid_detailed_modes(struct
drm_connector *connector,
return num_modes;
}
-static int drm_edid_connector_update(struct drm_connector *connector,
-const struct drm_edid *drm_edid)
+static int _drm_edid_connector_update(struct drm_connector *connector,
+ const struct drm_edid *drm_edid)
{
int num_modes = 0;
u32 quirks;
@@ -6227,31 +6227,12 @@ static int drm_edid_connector_update(struct
drm_connector *connector,
static void _drm_update_tile_info(struct drm_connector *connector,
const struct drm_edid *drm_edid);
-static int _drm_connector_update_edid_property(struct drm_connector *connector,
+static int _drm_edid_connector_property_update(struct drm_connector *connector,
const struct drm_edid *drm_edid)
{
struct drm_device *dev = connector->dev;
int ret;
- /* ignore requests to set edid when overridden */
- if (connector->override_edid)
- return 0;
-
- /*
-* Set the display info, using edid if available, otherwise resetting
-* the values to defaults. This duplicates the work done in
-* drm_add_edid_modes, but that function is not consistently called
-* before this one in all drivers and the computation is cheap enough
-* that it seems better to duplicate it rather than attempt to ensure
-* some arbitrary ordering of calls.
-*/
- if (drm_edid)
- update_display_info(connector, drm_edid);
- else
- drm_reset_display_info(connector);
-
- _drm_update_tile_info(connector, drm_edid);
-
if (connector->edid_blob_ptr) {
const struct edid *old_edid = connector->edid_blob_ptr->data;
@@ -6297,6 +6278,76 @@ static int _drm_connector_update_edid_property(struct
drm_connector *connector,
return ret;
}
+/**
+ * drm_edid_connector_update - Update connector information from EDID
+ * @connector: Connector
+ * @drm_edid: EDID
+ *
+ * Update the connector mode list, display info, ELD, HDR metadata, relevant
+ * properties, etc. from the passed in EDID.
+ *
+ * If EDID is NULL, reset the information.
+ *
+ * Return: The number of modes added or 0 if we couldn't find any.
+ */
+int drm_edid_connector_update(struct drm_connector *connector,
+ const struct drm_edid *drm_edid)
+{
+ int count;
+
+ /*
+* FIXME: Reconcile the differences in override_edid handling between
+* this and drm_connector_update_edid_property().
+*
+* If override_edid is set, and the EDID passed in here originates from
+* drm_edid_read() and friends, it will be the override EDID, and there
+* are no issues. drm_connector_update_edid_property() ignoring requests
+* to set the EDID dates back to a time when override EDID was not
+* handled at the low level EDID read.
+*
+* The only way the EDID passed in here can be different from the
+* override EDID is when a driver passes in an EDID that does *not*
+* originate from drm_edid_read() and friends, or passes in a stale
+* cached version. This, in turn, is a question of when an override EDID
+* set via debugfs should take effect.
+*/
+
+ count = _drm_edid_connector_update(connector, drm_edid);
+
+ _drm_update_tile_info(connector, drm_edid);
+
+ /* Note: Ignore errors for now. */
+ _drm_edid_connector_property_update(connector, drm_edid);
+
+ return count;
+}
+EXPORT_SYMBOL(drm_edid_connector_update);
+
+static int _drm_connector_update_edid_property(struct drm_connector *connector,
+ const struct drm_edid *drm_edid)
+{
+ /* ignore requests to set edid when overridden */
+ if (
[CI RESEND 07/10] drm/edid: add drm_edid_raw() to access the raw EDID data
Unfortunately, there are still plenty of interfaces around that require
a struct edid pointer, and it's impossible to change them all at
once. Add an accessor to the raw EDID data to help the transition.
While there are no such cases now, be defensive against raw EDID
extension count indicating bigger EDID than is actually allocated.
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 26 ++
include/drm/drm_edid.h | 1 +
2 files changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 41b3de52b8f1..1c761e12820e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2359,6 +2359,32 @@ struct edid *drm_do_get_edid(struct drm_connector
*connector,
}
EXPORT_SYMBOL_GPL(drm_do_get_edid);
+/**
+ * drm_edid_raw - Get a pointer to the raw EDID data.
+ * @drm_edid: drm_edid container
+ *
+ * Get a pointer to the raw EDID data.
+ *
+ * This is for transition only. Avoid using this like the plague.
+ *
+ * Return: Pointer to raw EDID data.
+ */
+const struct edid *drm_edid_raw(const struct drm_edid *drm_edid)
+{
+ if (!drm_edid || !drm_edid->size)
+ return NULL;
+
+ /*
+* Do not return pointers where relying on EDID extension count would
+* lead to buffer overflow.
+*/
+ if (WARN_ON(edid_size(drm_edid->edid) > drm_edid->size))
+ return NULL;
+
+ return drm_edid->edid;
+}
+EXPORT_SYMBOL(drm_edid_raw);
+
/* Allocate struct drm_edid container *without* duplicating the edid data */
static const struct drm_edid *_drm_edid_alloc(const void *edid, size_t size)
{
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index aeb2fa95bc04..2181977ae683 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -597,6 +597,7 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
const struct drm_edid *drm_edid_alloc(const void *edid, size_t size);
const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid);
void drm_edid_free(const struct drm_edid *drm_edid);
+const struct edid *drm_edid_raw(const struct drm_edid *drm_edid);
const struct drm_edid *drm_edid_read(struct drm_connector *connector);
const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,
struct i2c_adapter *adapter);
--
2.30.2
[CI RESEND 06/10] drm/probe-helper: add drm_connector_helper_get_modes()
Add a helper function to be used as the "default" .get_modes()
hook. This also works as an example of what the driver .get_modes()
hooks are supposed to do regarding the new drm_edid_read*() and
drm_edid_connector_update() calls.
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_probe_helper.c | 34 ++
include/drm/drm_probe_helper.h | 1 +
2 files changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index a8d26b29bfa0..bb427c5a4f1f 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -1049,3 +1049,37 @@ int drm_connector_helper_get_modes_from_ddc(struct
drm_connector *connector)
return count;
}
EXPORT_SYMBOL(drm_connector_helper_get_modes_from_ddc);
+
+/**
+ * drm_connector_helper_get_modes - Read EDID and update connector.
+ * @connector: The connector
+ *
+ * Read the EDID using drm_edid_read() (which requires that connector->ddc is
+ * set), and update the connector using the EDID.
+ *
+ * This can be used as the "default" connector helper .get_modes() hook if the
+ * driver does not need any special processing. This is sets the example what
+ * custom .get_modes() hooks should do regarding EDID read and connector
update.
+ *
+ * Returns: Number of modes.
+ */
+int drm_connector_helper_get_modes(struct drm_connector *connector)
+{
+ const struct drm_edid *drm_edid;
+ int count;
+
+ drm_edid = drm_edid_read(connector);
+
+ /*
+* Unconditionally update the connector. If the EDID was read
+* successfully, fill in the connector information derived from the
+* EDID. Otherwise, if the EDID is NULL, clear the connector
+* information.
+*/
+ count = drm_edid_connector_update(connector, drm_edid);
+
+ drm_edid_free(drm_edid);
+
+ return count;
+}
+EXPORT_SYMBOL(drm_connector_helper_get_modes);
diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h
index c80cab7a53b7..8075e02aa865 100644
--- a/include/drm/drm_probe_helper.h
+++ b/include/drm/drm_probe_helper.h
@@ -27,5 +27,6 @@ void drm_kms_helper_poll_enable(struct drm_device *dev);
bool drm_kms_helper_is_poll_worker(void);
int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector);
+int drm_connector_helper_get_modes(struct drm_connector *connector);
#endif
--
2.30.2
[CI RESEND 08/10] drm/edid: do invalid block filtering in-place
Rewrite edid_filter_invalid_blocks() to filter invalid blocks
in-place. The main motivation is to not rely on passed in information on
invalid block count or the allocation size, which will be helpful in
follow-up work on HF-EEODB.
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 43 --
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1c761e12820e..a80ea0aa7b32 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2020,33 +2020,37 @@ bool drm_edid_is_valid(struct edid *edid)
}
EXPORT_SYMBOL(drm_edid_is_valid);
-static struct edid *edid_filter_invalid_blocks(const struct edid *edid,
- int invalid_blocks,
+static struct edid *edid_filter_invalid_blocks(struct edid *edid,
size_t *alloc_size)
{
- struct edid *new, *dest_block;
- int valid_extensions = edid->extensions - invalid_blocks;
- int i;
+ struct edid *new;
+ int i, valid_blocks = 0;
- *alloc_size = edid_size_by_blocks(valid_extensions + 1);
+ for (i = 0; i < edid_block_count(edid); i++) {
+ const void *src_block = edid_block_data(edid, i);
- new = kmalloc(*alloc_size, GFP_KERNEL);
- if (!new)
- goto out;
+ if (edid_block_valid(src_block, i == 0)) {
+ void *dst_block = (void *)edid_block_data(edid,
valid_blocks);
- dest_block = new;
- for (i = 0; i < edid_block_count(edid); i++) {
- const void *block = edid_block_data(edid, i);
+ memmove(dst_block, src_block, EDID_LENGTH);
+ valid_blocks++;
+ }
+ }
- if (edid_block_valid(block, i == 0))
- memcpy(dest_block++, block, EDID_LENGTH);
+ /* We already trusted the base block to be valid here... */
+ if (WARN_ON(!valid_blocks)) {
+ kfree(edid);
+ return NULL;
}
- new->extensions = valid_extensions;
- new->checksum = edid_block_compute_checksum(new);
+ edid->extensions = valid_blocks - 1;
+ edid->checksum = edid_block_compute_checksum(edid);
-out:
- kfree(edid);
+ *alloc_size = edid_size_by_blocks(valid_blocks);
+
+ new = krealloc(edid, *alloc_size, GFP_KERNEL);
+ if (!new)
+ kfree(edid);
return new;
}
@@ -2316,8 +2320,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector
*connector,
if (invalid_blocks) {
connector_bad_edid(connector, edid, edid_block_count(edid));
- edid = edid_filter_invalid_blocks(edid, invalid_blocks,
- &alloc_size);
+ edid = edid_filter_invalid_blocks(edid, &alloc_size);
}
ok:
--
2.30.2
[CI RESEND 09/10] drm/edid: add HF-EEODB support to EDID read and allocation
HDMI 2.1 section 10.3.6 defines an HDMI Forum EDID Extension Override
Data Block, which may contain a different extension count than the base
block claims. Add support for reading more EDID data if available. The
extra blocks aren't parsed yet, though.
Hard-coding the EEODB parsing instead of using the iterators we have is
a bit of a bummer, but we have to be able to do this on a partially
allocated EDID while reading it.
v2:
- Check for CEA Data Block Collection size (Ville)
- Amend commit message and comment about hard-coded parsing
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 89 --
1 file changed, 86 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a80ea0aa7b32..fa3a3e294560 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1581,6 +1581,15 @@ static bool version_greater(const struct drm_edid
*drm_edid,
(edid->version == version && edid->revision > revision);
}
+static int edid_hfeeodb_extension_block_count(const struct edid *edid);
+
+static int edid_hfeeodb_block_count(const struct edid *edid)
+{
+ int eeodb = edid_hfeeodb_extension_block_count(edid);
+
+ return eeodb ? eeodb + 1 : 0;
+}
+
static int edid_extension_block_count(const struct edid *edid)
{
return edid->extensions;
@@ -2026,6 +2035,11 @@ static struct edid *edid_filter_invalid_blocks(struct
edid *edid,
struct edid *new;
int i, valid_blocks = 0;
+ /*
+* Note: If the EDID uses HF-EEODB, but has invalid blocks, we'll revert
+* back to regular extension count here. We don't want to start
+* modifying the HF-EEODB extension too.
+*/
for (i = 0; i < edid_block_count(edid); i++) {
const void *src_block = edid_block_data(edid, i);
@@ -2261,7 +2275,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector
*connector,
size_t *size)
{
enum edid_block_status status;
- int i, invalid_blocks = 0;
+ int i, num_blocks, invalid_blocks = 0;
struct edid *edid, *new;
size_t alloc_size = EDID_LENGTH;
@@ -2303,7 +2317,8 @@ static struct edid *_drm_do_get_edid(struct drm_connector
*connector,
goto fail;
edid = new;
- for (i = 1; i < edid_block_count(edid); i++) {
+ num_blocks = edid_block_count(edid);
+ for (i = 1; i < num_blocks; i++) {
void *block = (void *)edid_block_data(edid, i);
status = edid_block_read(block, i, read_block, context);
@@ -2314,11 +2329,31 @@ static struct edid *_drm_do_get_edid(struct
drm_connector *connector,
if (status == EDID_BLOCK_READ_FAIL)
goto fail;
invalid_blocks++;
+ } else if (i == 1) {
+ /*
+* If the first EDID extension is a CTA extension, and
+* the first Data Block is HF-EEODB, override the
+* extension block count.
+*
+* Note: HF-EEODB could specify a smaller extension
+* count too, but we can't risk allocating a smaller
+* amount.
+*/
+ int eeodb = edid_hfeeodb_block_count(edid);
+
+ if (eeodb > num_blocks) {
+ num_blocks = eeodb;
+ alloc_size = edid_size_by_blocks(num_blocks);
+ new = krealloc(edid, alloc_size, GFP_KERNEL);
+ if (!new)
+ goto fail;
+ edid = new;
+ }
}
}
if (invalid_blocks) {
- connector_bad_edid(connector, edid, edid_block_count(edid));
+ connector_bad_edid(connector, edid, num_blocks);
edid = edid_filter_invalid_blocks(edid, &alloc_size);
}
@@ -3851,6 +3886,7 @@ static int add_detailed_modes(struct drm_connector
*connector,
#define CTA_EXT_DB_HDR_STATIC_METADATA 6
#define CTA_EXT_DB_420_VIDEO_DATA 14
#define CTA_EXT_DB_420_VIDEO_CAP_MAP 15
+#define CTA_EXT_DB_HF_EEODB0x78
#define CTA_EXT_DB_HF_SCDB 0x79
#define EDID_BASIC_AUDIO (1 << 6)
@@ -4910,6 +4946,12 @@ static bool cea_db_is_hdmi_forum_vsdb(const struct
cea_db *db)
cea_db_payload_len(db) >= 7;
}
+static bool cea_db_is_hdmi_forum_eeodb(const void *db)
+{
+ return cea_db_is_extended_tag(db, CTA_EXT_DB_HF_EEODB) &&
+ cea_db_payload_len(db) >= 2;
+}
+
static bool cea_db_is_microsoft_vsdb(const struct cea_db *db)
{
return cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) &&
@@ -4944,6
[CI RESEND 10/10] drm/edid: take HF-EEODB extension count into account
Take the HF-EEODB extension count override into account.
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fa3a3e294560..bbc25e3b7220 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1629,6 +1629,19 @@ static int drm_edid_block_count(const struct drm_edid
*drm_edid)
/* Starting point */
num_blocks = edid_block_count(drm_edid->edid);
+ /* HF-EEODB override */
+ if (drm_edid->size >= edid_size_by_blocks(2)) {
+ int eeodb;
+
+ /*
+* Note: HF-EEODB may specify a smaller extension count than the
+* regular one. Unlike in buffer allocation, here we can use it.
+*/
+ eeodb = edid_hfeeodb_block_count(drm_edid->edid);
+ if (eeodb)
+ num_blocks = eeodb;
+ }
+
/* Limit by allocated size */
num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);
--
2.30.2
Re: [PATCH v7 03/14] mm: handling Non-LRU pages returned by vm_normal_pages
On 29.06.22 05:54, Alex Sierra wrote: > With DEVICE_COHERENT, we'll soon have vm_normal_pages() return > device-managed anonymous pages that are not LRU pages. Although they > behave like normal pages for purposes of mapping in CPU page, and for > COW. They do not support LRU lists, NUMA migration or THP. > > Callers to follow_page that expect LRU pages, are also checked for > device zone pages due to DEVICE_COHERENT type. Can we rephrase that to (because zeropage) "Callers to follow_page() currently don't expect ZONE_DEVICE pages, however, with DEVICE_COHERENT we might now return ZONE_DEVICE. Check for ZONE_DEVICE pages in applicable users of follow_page() as well." [...] > /* > diff --git a/mm/memory.c b/mm/memory.c > index 7a089145cad4..e18555af9024 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -624,6 +624,13 @@ struct page *vm_normal_page(struct vm_area_struct *vma, > unsigned long addr, > if (is_zero_pfn(pfn)) > return NULL; > if (pte_devmap(pte)) > +/* > + * NOTE: New uers of ZONE_DEVICE will not set pte_devmap() and will have s/uers/users/ > + * refcounts incremented on their struct pages when they are inserted into > + * PTEs, thus they are safe to return here. Legacy ZONE_DEVICE pages that set > + * pte_devmap() do not have refcounts. Example of legacy ZONE_DEVICE is > + * MEMORY_DEVICE_FS_DAX type in pmem or virtio_fs drivers. > + */ [...] > diff --git a/mm/mprotect.c b/mm/mprotect.c > index ba5592655ee3..e034aae2a98b 100644 > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -95,7 +95,7 @@ static unsigned long change_pte_range(struct mmu_gather > *tlb, > continue; > > page = vm_normal_page(vma, addr, oldpte); > - if (!page || PageKsm(page)) > + if (!page || is_zone_device_page(page) || > PageKsm(page)) > continue; > > /* Also skip shared copy-on-write pages */ In -next/-mm there is now an additional can_change_pte_writable() that calls vm_normal_page() -- added by me. I assume that that is indeed fine because we can simply map device coherent pages writable. Besides the nits, LGTM Acked-by: David Hildenbrand -- Thanks, David / dhildenb
[Bug 216175] PowerColor Radeon Rx 6400 ITX does not work.
https://bugzilla.kernel.org/show_bug.cgi?id=216175 --- Comment #11 from Nobiuki (nobutarounos...@gmail.com) --- Created attachment 301305 --> https://bugzilla.kernel.org/attachment.cgi?id=301305&action=edit journal with amdgpu.aspm=0 Adding amdgpu.aspm=0 makes my Rx 6400 work fine. Thank you. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v3 1/2] dt-bindings: display: ti, am65x-dss: Add am625 dss compatible
On 27/06/2022 17:11, Aradhya Bhatia wrote: > Add ti,am625-dss compatible string. > The DSS IP on TI's AM625 SoC is an update from the DSS on TI's AM65X > SoC. The former has an additional OLDI TX to enable a 2K resolution on > OLDI displays or enable 2 duplicated displays with a smaller resolution. > > Signed-off-by: Aradhya Bhatia > Reviewed-by: Rahul T R > --- Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 6/6] i2c: Make remove callback return void
On Tue, Jun 28, 2022 at 04:03:12PM +0200, Uwe Kleine-König wrote: > From: Uwe Kleine-König > > The value returned by an i2c driver's remove function is mostly ignored. > (Only an error message is printed if the value is non-zero that the > error is ignored.) > > So change the prototype of the remove function to return no value. This > way driver authors are not tempted to assume that passing an error to > the upper layer is a good idea. All drivers are adapted accordingly. > There is no intended change of behaviour, all callbacks were prepared to > return 0 before. > > Signed-off-by: Uwe Kleine-König > --- > Documentation/i2c/writing-clients.rst | 2 +- > arch/arm/mach-davinci/board-dm644x-evm.c | 3 +-- > arch/arm/mach-davinci/board-dm646x-evm.c | 3 +-- > arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c| 3 +-- > drivers/auxdisplay/ht16k33.c | 4 +--- > drivers/auxdisplay/lcd2s.c| 3 +-- > drivers/char/ipmi/ipmb_dev_int.c | 4 +--- > drivers/char/ipmi/ipmi_ipmb.c | 4 +--- > drivers/char/ipmi/ipmi_ssif.c | 6 ++ > drivers/char/tpm/st33zp24/i2c.c | 4 +--- > drivers/char/tpm/tpm_i2c_atmel.c | 3 +-- > drivers/char/tpm/tpm_i2c_infineon.c | 4 +--- > drivers/char/tpm/tpm_i2c_nuvoton.c| 3 +-- > drivers/char/tpm/tpm_tis_i2c_cr50.c | 6 ++ > drivers/clk/clk-cdce706.c | 3 +-- > drivers/clk/clk-cs2000-cp.c | 4 +--- > drivers/clk/clk-si514.c | 3 +-- > drivers/clk/clk-si5341.c | 4 +--- > drivers/clk/clk-si5351.c | 4 +--- > drivers/clk/clk-si570.c | 3 +-- > drivers/clk/clk-versaclock5.c | 4 +--- > drivers/crypto/atmel-ecc.c| 6 ++ > drivers/crypto/atmel-sha204a.c| 6 ++ > drivers/extcon/extcon-rt8973a.c | 4 +--- > drivers/gpio/gpio-adp5588.c | 4 +--- > drivers/gpio/gpio-max7300.c | 4 +--- > drivers/gpio/gpio-pca953x.c | 4 +--- > drivers/gpio/gpio-pcf857x.c | 4 +--- > drivers/gpio/gpio-tpic2810.c | 4 +--- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 4 +--- > drivers/gpu/drm/bridge/analogix/analogix-anx6345.c| 4 +--- > drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c| 4 +--- > drivers/gpu/drm/bridge/analogix/anx7625.c | 4 +--- > drivers/gpu/drm/bridge/chrontel-ch7033.c | 4 +--- > drivers/gpu/drm/bridge/cros-ec-anx7688.c | 4 +--- > drivers/gpu/drm/bridge/ite-it6505.c | 4 +--- > drivers/gpu/drm/bridge/ite-it66121.c | 4 +--- > drivers/gpu/drm/bridge/lontium-lt8912b.c | 3 +-- > drivers/gpu/drm/bridge/lontium-lt9211.c | 4 +--- > drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +--- > drivers/gpu/drm/bridge/lontium-lt9611uxc.c| 4 +--- > drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 8 ++-- > drivers/gpu/drm/bridge/nxp-ptn3460.c | 4 +--- > drivers/gpu/drm/bridge/parade-ps8622.c| 4 +--- > drivers/gpu/drm/bridge/parade-ps8640.c| 4 +--- > drivers/gpu/drm/bridge/sii902x.c | 4 +--- > drivers/gpu/drm/bridge/sii9234.c | 4 +--- > drivers/gpu/drm/bridge/sil-sii8620.c | 4 +--- > drivers/gpu/drm/bridge/tc358767.c | 4 +--- > drivers/gpu/drm/bridge/tc358768.c | 4 +--- > drivers/gpu/drm/bridge/tc358775.c | 4 +--- > drivers/gpu/drm/bridge/ti-sn65dsi83.c | 4 +--- > drivers/gpu/drm/bridge/ti-tfp410.c| 4 +--- > drivers/gpu/drm/i2c/ch7006_drv.c | 4 +--- > drivers/gpu/drm/i2c/tda9950.c | 4 +--- > drivers/gpu/drm/i2c/tda998x_drv.c | 3 +-- > drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c| 4 +--- > drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c | 4 +--- > drivers/gpu/drm/solomon/ssd130x-i2c.c | 4 +--- > drivers/hid/i2c-hid/i2c-hid-core.c| 4 +--- > drivers/hid/i2c-hid/i2c-hid.h | 2 +- > drivers/hwmon/adc128d818.c| 4 +--- > drivers/hwmon/adt7470.c
[PULL] drm-intel-gt-next
Hi Dave, Daniel, This is the first pull request for 5.20 merge window. A lot of fixes across the board, a few improvements and quite a lot of driver refactoring to prepare for Ponte Vecchio, and then a bunch of Ponte Vecchio early enablement on top. In terms of improvements, we now enable huge pages on Icelake and above regardless of the IOMMU status. This is after confirming with hardware architects and measuring the performance regression which we had on older platforms is gone. Resume speed on modern integrated platforms has also been improved with around 100ms improvement measured. DG2 has gained HuC support and some new workarounds. In terms of fixes, the headline ones are: * Driver load on DG2 small BAR configurations has been fixed to not crash. * GuC backend has fixed the EU scheduling priority for DG2 and in general fixed reset handling, improved error capture and optimised performance of querying engine busyness. * Rendering corruption on Haswell and older platforms caused by some recent refactoring has also been fixed, as was the dma_resv handling in the relatively new multi-batch execbuf code. * Temporary and unintended relaxation of the persistent contexts handling in the protocontext code has been fixed up before it hopefully "escaped" into the wild. * Reset handling on Icelake and above has been improved with a new workaround which should improve the stability in some corner cases. * Processes which use non-persistent context and abruptly exit will now be allowed to exit gracefuly without triggering resets and error capture in the logs. * A bunch more as mentioned in the tag summary. On the uapi front we have deprecation of some legacy get params on Xe_HP+ and media block frequency control exposed in per gt sysfs. Outside i915 we have a rename in the intel-gtt module and one cross-merge from drm-intel-next. Regards, Tvrtko drm-intel-gt-next-2022-06-29: UAPI Changes: - Expose per tile media freq factor in sysfs (Ashutosh Dixit, Dale B Stimson) - Document memory residency and Flat-CCS capability of obj (Ramalingam C) - Disable GETPARAM lookups of I915_PARAM_[SUB]SLICE_MASK on Xe_HP+ (Matt Roper) Cross-subsystem Changes: - Rename intel-gtt symbols (Lucas De Marchi) Core Changes: Driver Changes: - Support programming the EU priority in the GuC descriptor (DG2) (Matthew Brost) - DG2 HuC loading support (Daniele Ceraolo Spurio) - Fix build error without CONFIG_PM (YueHaibing) - Enable THP on Icelake and beyond (Tvrtko Ursulin) - Only setup private tmpfs mount when needed and fix logging (Tvrtko Ursulin) - Make __guc_reset_context aware of guilty engines (Umesh Nerlige Ramappa) - DG2 small bar memory probing fixes (Nirmoy Das) - Remove unnecessary GuC err capture noise (Alan Previn) - Fix i915_gem_object_ggtt_pin_ww regression on old platforms (Maarten Lankhorst) - Fix undefined behavior in GuC backend due to shift overflowing the constant (Borislav Petkov) - New DG2 workarounds (Swathi Dhanavanthri, Anshuman Gupta) - Report no hwconfig support on ADL-N (Balasubramani Vivekanandan) - Fix error_state_read ptr + offset use (Alan Previn) - Expose per tile media freq factor in sysfs (Ashutosh Dixit, Dale B Stimson) - Fix memory leaks in per-gt sysfs (Ashutosh Dixit) - Fix dma_resv fence handling in multi-batch execbuf (Nirmoy Das) - Add extra registers to GPU error dump on Gen11+ (Stuart Summers) - More PVC+DG2 workarounds (Matt Roper) - Improve user experience and driver robustness under SIGINT or similar (Tvrtko Ursulin) - Don't show engine classes not present (Tvrtko Ursulin) - Improve on suspend / resume time with VT-d enabled (Thomas Hellström) - Add missing else (katrinzhou) - Don't leak lmem mapping in vma_evict (Juha-Pekka Heikkila) - Add smem fallback allocation for dpt (Juha-Pekka Heikkila) - Tweak the ordering in cpu_write_needs_clflush (Matthew Auld) - Do not access rq->engine without a reference (Niranjana Vishwanathapura) - Revert "drm/i915: Hold reference to intel_context over life of i915_request" (Niranjana Vishwanathapura) - Don't update engine busyness stats too frequently (Alan Previn) - Add additional steps for Wa_22011802037 for execlist backend (Umesh Nerlige Ramappa) - Fix a lockdep warning at error capture (Nirmoy Das) - Ponte Vecchio prep work and new blitter engines (Matt Roper, John Harrison, Lucas De Marchi) - Read correct RP_STATE_CAP register (PVC) (Matt Roper) - Define MOCS table for PVC (Ayaz A Siddiqui) - Driver refactor and support Ponte Vecchio forcewake handling (Matt Roper) - Remove additional 3D flags from PIPE_CONTROL (Ponte Vecchio) (Stuart Summers) - XEHPSDV and PVC do not use HuC (Daniele Ceraolo Spurio) - Extract stepping information from PCI revid (Ponte Vecchio) (Matt Roper) - Add initial PVC workarounds (Stuart Summers) - SSEU handling driver refactor and Ponte Vecchio support (Matt Roper) - GuC depriv applies to PVC (Matt Roper) - Add register steering (Ponte Vecchio) (Matt Roper) - Add recommended MMIO se
[PULL] drm-intel-fixes
Hi Dave & Daniel - drm-intel-fixes-2022-06-29: drm/i915 fixes for v5.19-rc5: - Fix ioctl argument error return - Fix d3cold disable to allow PCI upstream bridge D3 transition - Fix setting cache_dirty for dma-buf objects on discrete Rodrigo will cover the remaining fixes until v5.19 final. BR, Jani. The following changes since commit 03c765b0e3b4cb5063276b086c76f7a612856a9a: Linux 5.19-rc4 (2022-06-26 14:22:10 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2022-06-29 for you to fetch changes up to 79538490fd7ade244dba400923e792519a2bdfea: drm/i915: tweak the ordering in cpu_write_needs_clflush (2022-06-27 18:12:10 +0300) drm/i915 fixes for v5.19-rc5: - Fix ioctl argument error return - Fix d3cold disable to allow PCI upstream bridge D3 transition - Fix setting cache_dirty for dma-buf objects on discrete Anshuman Gupta (1): drm/i915/dgfx: Disable d3cold at gfx root port Matthew Auld (1): drm/i915: tweak the ordering in cpu_write_needs_clflush katrinzhou (1): drm/i915/gem: add missing else drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +++-- drivers/gpu/drm/i915/gem/i915_gem_domain.c | 6 ++--- drivers/gpu/drm/i915/i915_driver.c | 34 + 3 files changed, 21 insertions(+), 24 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center
[PATCH] drm/fb-helper: Remove helpers to change frame buffer config
The DRM fbdev emulation layer does not support pushing back
changes to fb_var_screeninfo to KMS.
However, drm_fb_helper still implements the fb_ops.fb_check_var() and
fb_ops.fb_set_par() callbacks, but the former fails to validate various
parameters passed from userspace. Hence unsanitized and possible
invaled values are passed up through the fbdev, fbcon, and console
stack, which has become an endless source of security issues reported
against fbdev.
Fix this by not populating the fb_ops.fb_check_var() and
fb_ops.fb_set_par() callbacks, as there is no point in providing them if
the frame buffer config cannot be changed anyway. This makes the fbdev
core aware that making changes to the frame buffer config is not
supported, so it will always return the current config.
Signed-off-by: Geert Uytterhoeven
---
The only remaining DRM driver that implements fb_ops.fb_check_var() is
also broken, as it fails to validate various parameters passed from
userspace. So vmw_fb_check_var() should either be fixed, or removed.
---
drivers/gpu/drm/drm_fb_helper.c| 180 ++---
drivers/gpu/drm/i915/display/intel_fbdev.c | 15 --
drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 -
include/drm/drm_fb_helper.h| 16 --
4 files changed, 13 insertions(+), 200 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 2d4cee6a10e7..1041a11c410d7967 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -228,9 +228,18 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
}
EXPORT_SYMBOL(drm_fb_helper_debug_leave);
-static int
-__drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
- bool force)
+/**
+ * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
+ *
+ * This should be called from driver's drm &drm_driver.lastclose callback
+ * when implementing an fbcon on top of kms using this helper. This ensures
that
+ * the user isn't greeted with a black screen when e.g. X dies.
+ *
+ * RETURNS:
+ * Zero if everything went ok, negative error code otherwise.
+ */
+int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
{
bool do_delayed;
int ret;
@@ -242,16 +251,7 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct
drm_fb_helper *fb_helper,
return 0;
mutex_lock(&fb_helper->lock);
- if (force) {
- /*
-* Yes this is the _locked version which expects the master lock
-* to be held. But for forced restores we're intentionally
-* racing here, see drm_fb_helper_set_par().
-*/
- ret = drm_client_modeset_commit_locked(&fb_helper->client);
- } else {
- ret = drm_client_modeset_commit(&fb_helper->client);
- }
+ ret = drm_client_modeset_commit(&fb_helper->client);
do_delayed = fb_helper->delayed_hotplug;
if (do_delayed)
@@ -263,22 +263,6 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct
drm_fb_helper *fb_helper,
return ret;
}
-
-/**
- * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
- * @fb_helper: driver-allocated fbdev helper, can be NULL
- *
- * This should be called from driver's drm &drm_driver.lastclose callback
- * when implementing an fbcon on top of kms using this helper. This ensures
that
- * the user isn't greeted with a black screen when e.g. X dies.
- *
- * RETURNS:
- * Zero if everything went ok, negative error code otherwise.
- */
-int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
-{
- return __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, false);
-}
EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
#ifdef CONFIG_MAGIC_SYSRQ
@@ -1254,25 +1238,6 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned
int cmd,
}
EXPORT_SYMBOL(drm_fb_helper_ioctl);
-static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1,
- const struct fb_var_screeninfo *var_2)
-{
- return var_1->bits_per_pixel == var_2->bits_per_pixel &&
- var_1->grayscale == var_2->grayscale &&
- var_1->red.offset == var_2->red.offset &&
- var_1->red.length == var_2->red.length &&
- var_1->red.msb_right == var_2->red.msb_right &&
- var_1->green.offset == var_2->green.offset &&
- var_1->green.length == var_2->green.length &&
- var_1->green.msb_right == var_2->green.msb_right &&
- var_1->blue.offset == var_2->blue.offset &&
- var_1->blue.length == var_2->blue.length &&
- var_1->blue.msb_right == var_2->blue.msb_right &&
- var_1->transp.offset == var_2->transp.offset &&
- var_1->transp.leng
[Bug 216188] nvidiafb: unknown NV_ARCH
https://bugzilla.kernel.org/show_bug.cgi?id=216188 Artem S. Tashkinov (a...@gmx.com) changed: What|Removed |Added Status|NEW |RESOLVED CC||a...@gmx.com Resolution|--- |ANSWERED --- Comment #1 from Artem S. Tashkinov (a...@gmx.com) --- nvidiafb shouldn't be used nowadays, specially with your unsupported hardware. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v3 00/13] small BAR uapi bits
IGT: https://patchwork.freedesktop.org/series/104368/#rev2 Mesa: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16739 -- 2.36.1
[PATCH v3 01/13] drm/doc: add rfc section for small BAR uapi
Add an entry for the new uapi needed for small BAR on DG2+.
v2:
- Some spelling fixes and other small tweaks. (Akeem & Thomas)
- Rework error capture interactions, including no longer needing
NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
- Add probed_cpu_visible_size. (Lionel)
v3:
- Drop the vma query for now.
- Add unallocated_cpu_visible_size as part of the region query.
- Improve the docs some more, including documenting the expected
behaviour on older kernels, since this came up in some offline
discussion.
v4:
- Various improvements all over. (Tvrtko)
v5:
- Include newer integrated platforms when applying the non-recoverable
context and error capture restriction. (Thomas)
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Cc: mesa-...@lists.freedesktop.org
Acked-by: Tvrtko Ursulin
Acked-by: Akeem G Abodunrin
Reviewed-by: Thomas Hellström
Acked-by: Lionel Landwerlin
Acked-by: Jordan Justen
---
Documentation/gpu/rfc/i915_small_bar.h | 189 +++
Documentation/gpu/rfc/i915_small_bar.rst | 47 ++
Documentation/gpu/rfc/index.rst | 4 +
3 files changed, 240 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
diff --git a/Documentation/gpu/rfc/i915_small_bar.h
b/Documentation/gpu/rfc/i915_small_bar.h
new file mode 100644
index ..6003c81d5aa4
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,189 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as known to the
+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct
drm_i915_query.
+ * For this new query we are adding the new query id
DRM_I915_QUERY_MEMORY_REGIONS
+ * at &drm_i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+ /** @region: The class:instance pair encoding */
+ struct drm_i915_gem_memory_class_instance region;
+
+ /** @rsvd0: MBZ */
+ __u32 rsvd0;
+
+ /**
+* @probed_size: Memory probed by the driver
+*
+* Note that it should not be possible to ever encounter a zero value
+* here, also note that no current region type will ever return -1 here.
+* Although for future region types, this might be a possibility. The
+* same applies to the other size fields.
+*/
+ __u64 probed_size;
+
+ /**
+* @unallocated_size: Estimate of memory remaining
+*
+* Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable accounting.
+* Without this (or if this is an older kernel) the value here will
+* always equal the @probed_size. Note this is only currently tracked
+* for I915_MEMORY_CLASS_DEVICE regions (for other types the value here
+* will always equal the @probed_size).
+*/
+ __u64 unallocated_size;
+
+ union {
+ /** @rsvd1: MBZ */
+ __u64 rsvd1[8];
+ struct {
+ /**
+* @probed_cpu_visible_size: Memory probed by the driver
+* that is CPU accessible.
+*
+* This will be always be <= @probed_size, and the
+* remainder (if there is any) will not be CPU
+* accessible.
+*
+* On systems without small BAR, the @probed_size will
+* always equal the @probed_cpu_visible_size, since all
+* of it will be CPU accessible.
+*
+* Note this is only tracked for
+* I915_MEMORY_CLASS_DEVICE regions (for other types the
+* value here will always equal the @probed_size).
+*
+* Note that if the value returned here is zero, then
+* this must be an old kernel which lacks the relevant
+* small-bar uAPI support (including
+* I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
+* such systems we should never actually end up with a
+* small BAR configuration, assuming we are able to load
+* the kernel module. Hence it should be safe to treat
+* this the same as when @probed_cpu_visible_size ==
+* @probed_size.
+*/
+ __u64 probed_cpu_visible_size;
+
+ /**
+* @unallocated_cpu_visible_size: Estimate of CPU
+* visible memory remaining
+*
+
[PATCH v3 02/13] drm/i915/uapi: add probed_cpu_visible_size
Userspace wants to know the size of CPU visible portion of device
local-memory, and on small BAR devices the probed_size is no longer
enough. In Vulkan, for example, it would like to know the size in bytes
for CPU visible VkMemoryHeap. We already track the io_size for each
region, so plumb that through to the region query.
v2: Drop the ( -1 = unknown ) stuff, which is confusing since nothing
can currently ever return such a value.
Testcase: igt@i915_query@query-regions-sanity-check
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Acked-by: Nirmoy Das
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/i915_query.c | 6 +++
include/uapi/drm/i915_drm.h | 76 +--
2 files changed, 48 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 0094f67c63f2..9894add651dd 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -498,6 +498,12 @@ static int query_memregion_info(struct drm_i915_private
*i915,
info.region.memory_class = mr->type;
info.region.memory_instance = mr->instance;
info.probed_size = mr->total;
+
+ if (mr->type == INTEL_MEMORY_LOCAL)
+ info.probed_cpu_visible_size = mr->io_size;
+ else
+ info.probed_cpu_visible_size = mr->total;
+
info.unallocated_size = mr->avail;
if (__copy_to_user(info_ptr, &info, sizeof(info)))
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index de49b68b4fc8..7eacacb00373 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3207,36 +3207,6 @@ struct drm_i915_gem_memory_class_instance {
* struct drm_i915_memory_region_info - Describes one region as known to the
* driver.
*
- * Note that we reserve some stuff here for potential future work. As an
example
- * we might want expose the capabilities for a given region, which could
include
- * things like if the region is CPU mappable/accessible, what are the supported
- * mapping types etc.
- *
- * Note that to extend struct drm_i915_memory_region_info and struct
- * drm_i915_query_memory_regions in the future the plan is to do the following:
- *
- * .. code-block:: C
- *
- * struct drm_i915_memory_region_info {
- * struct drm_i915_gem_memory_class_instance region;
- * union {
- * __u32 rsvd0;
- * __u32 new_thing1;
- * };
- * ...
- * union {
- * __u64 rsvd1[8];
- * struct {
- * __u64 new_thing2;
- * __u64 new_thing3;
- * ...
- * };
- * };
- * };
- *
- * With this things should remain source compatible between versions for
- * userspace, even as we add new fields.
- *
* Note this is using both struct drm_i915_query_item and struct
drm_i915_query.
* For this new query we are adding the new query id
DRM_I915_QUERY_MEMORY_REGIONS
* at &drm_i915_query_item.query_id.
@@ -3248,14 +3218,52 @@ struct drm_i915_memory_region_info {
/** @rsvd0: MBZ */
__u32 rsvd0;
- /** @probed_size: Memory probed by the driver (-1 = unknown) */
+ /**
+* @probed_size: Memory probed by the driver
+*
+* Note that it should not be possible to ever encounter a zero value
+* here, also note that no current region type will ever return -1 here.
+* Although for future region types, this might be a possibility. The
+* same applies to the other size fields.
+*/
__u64 probed_size;
- /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
+ /** @unallocated_size: Estimate of memory remaining */
__u64 unallocated_size;
- /** @rsvd1: MBZ */
- __u64 rsvd1[8];
+ union {
+ /** @rsvd1: MBZ */
+ __u64 rsvd1[8];
+ struct {
+ /**
+* @probed_cpu_visible_size: Memory probed by the driver
+* that is CPU accessible.
+*
+* This will be always be <= @probed_size, and the
+* remainder (if there is any) will not be CPU
+* accessible.
+*
+* On systems without small BAR, the @probed_size will
+* always equal the @probed_cpu_visible_size, since all
+* of it will be CPU accessible.
+*
+* Note this is only tracked for
+
[PATCH v3 05/13] drm/i915/uapi: apply ALLOC_GPU_ONLY by default
On small BAR configurations, when dealing with I915_MEMORY_CLASS_DEVICE allocations, we assume that by default, all userspace allocations should be placed in the non-CPU visible portion. Note that dumb buffers are not included here, since these are not "GPU accelerated" and likely need CPU access. We choose to just always set GPU_ONLY, and let the backend figure out if that should be ignored or not, for example on full BAR systems. In a later patch userspace will be able to provide a hint if CPU access to the buffer is needed. v2(Thomas) - Apply GPU_ONLY on all discrete devices, but only if the BO can be placed in LMEM. Down in the depths this should be turned into a noop, where required, and as an annotation it still make some sense. If we apply it regardless of the placements then we end up needing to check the placements during exec capture. Also it's slightly inconsistent since the NEEDS_CPU_ACCESS can only be applied on objects that can be placed in LMEM. The other annoyance would be gem_create_ext vs plain gem_create, if we were to always apply GPU_ONLY. Testcase: igt@gem-create@create-ext-cpu-access-sanity-check Testcase: igt@gem-create@create-ext-cpu-access-big Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Lionel Landwerlin Cc: Tvrtko Ursulin Cc: Jon Bloomfield Cc: Daniel Vetter Cc: Jordan Justen Cc: Kenneth Graunke Cc: Akeem G Abodunrin Reviewed-by: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 5802692ea604..d094cae0ddf1 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -427,6 +427,14 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, ext_data.n_placements = 1; } + /* +* TODO: add a userspace hint to force CPU_ACCESS for the object, which +* can override this. +*/ + if (ext_data.n_placements > 1 || + ext_data.placements[0]->type != INTEL_MEMORY_SYSTEM) + ext_data.flags |= I915_BO_ALLOC_GPU_ONLY; + obj = __i915_gem_object_create_user_ext(i915, args->size, ext_data.placements, ext_data.n_placements, -- 2.36.1
[PATCH v3 04/13] drm/i915: remove intel_memory_region avail
No longer used.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/intel_memory_region.c | 4 +---
drivers/gpu/drm/i915/intel_memory_region.h | 1 -
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c
b/drivers/gpu/drm/i915/intel_memory_region.c
index 94ee26e99549..9a4a7fb55582 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -198,8 +198,7 @@ void intel_memory_region_debug(struct intel_memory_region
*mr,
if (mr->region_private)
ttm_resource_manager_debug(mr->region_private, printer);
else
- drm_printf(printer, "total:%pa, available:%pa bytes\n",
- &mr->total, &mr->avail);
+ drm_printf(printer, "total:%pa bytes\n", &mr->total);
}
static int intel_memory_region_memtest(struct intel_memory_region *mem,
@@ -242,7 +241,6 @@ intel_memory_region_create(struct drm_i915_private *i915,
mem->min_page_size = min_page_size;
mem->ops = ops;
mem->total = size;
- mem->avail = mem->total;
mem->type = type;
mem->instance = instance;
diff --git a/drivers/gpu/drm/i915/intel_memory_region.h
b/drivers/gpu/drm/i915/intel_memory_region.h
index 2214f251bec3..2953ed5c3248 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.h
+++ b/drivers/gpu/drm/i915/intel_memory_region.h
@@ -75,7 +75,6 @@ struct intel_memory_region {
resource_size_t io_size;
resource_size_t min_page_size;
resource_size_t total;
- resource_size_t avail;
u16 type;
u16 instance;
--
2.36.1
[PATCH v3 06/13] drm/i915/uapi: add NEEDS_CPU_ACCESS hint
If set, force the allocation to be placed in the mappable portion of
I915_MEMORY_CLASS_DEVICE. One big restriction here is that system memory
(i.e I915_MEMORY_CLASS_SYSTEM) must be given as a potential placement for the
object, that way we can always spill the object into system memory if we
can't make space.
Testcase: igt@gem-create@create-ext-cpu-access-sanity-check
Testcase: igt@gem-create@create-ext-cpu-access-big
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Reviewed-by: Nirmoy Das
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_create.c | 26 ++---
include/uapi/drm/i915_drm.h| 61 +++---
2 files changed, 71 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index d094cae0ddf1..33673fe7ee0a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -241,6 +241,7 @@ struct create_ext {
struct drm_i915_private *i915;
struct intel_memory_region *placements[INTEL_REGION_UNKNOWN];
unsigned int n_placements;
+ unsigned int placement_mask;
unsigned long flags;
};
@@ -337,6 +338,7 @@ static int set_placements(struct
drm_i915_gem_create_ext_memory_regions *args,
for (i = 0; i < args->num_regions; i++)
ext_data->placements[i] = placements[i];
+ ext_data->placement_mask = mask;
return 0;
out_dump:
@@ -411,7 +413,7 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void
*data,
struct drm_i915_gem_object *obj;
int ret;
- if (args->flags)
+ if (args->flags & ~I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS)
return -EINVAL;
ret = i915_user_extensions(u64_to_user_ptr(args->extensions),
@@ -427,13 +429,21 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void
*data,
ext_data.n_placements = 1;
}
- /*
-* TODO: add a userspace hint to force CPU_ACCESS for the object, which
-* can override this.
-*/
- if (ext_data.n_placements > 1 ||
- ext_data.placements[0]->type != INTEL_MEMORY_SYSTEM)
- ext_data.flags |= I915_BO_ALLOC_GPU_ONLY;
+ if (args->flags & I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS) {
+ if (ext_data.n_placements == 1)
+ return -EINVAL;
+
+ /*
+* We always need to be able to spill to system memory, if we
+* can't place in the mappable part of LMEM.
+*/
+ if (!(ext_data.placement_mask & BIT(INTEL_REGION_SMEM)))
+ return -EINVAL;
+ } else {
+ if (ext_data.n_placements > 1 ||
+ ext_data.placements[0]->type != INTEL_MEMORY_SYSTEM)
+ ext_data.flags |= I915_BO_ALLOC_GPU_ONLY;
+ }
obj = __i915_gem_object_create_user_ext(i915, args->size,
ext_data.placements,
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e4847436bab8..3e78a00220ea 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3366,11 +3366,11 @@ struct drm_i915_query_memory_regions {
* struct drm_i915_gem_create_ext - Existing gem_create behaviour, with added
* extension support using struct i915_user_extension.
*
- * Note that in the future we want to have our buffer flags here, at least for
- * the stuff that is immutable. Previously we would have two ioctls, one to
- * create the object with gem_create, and another to apply various parameters,
- * however this creates some ambiguity for the params which are considered
- * immutable. Also in general we're phasing out the various SET/GET ioctls.
+ * Note that new buffer flags should be added here, at least for the stuff that
+ * is immutable. Previously we would have two ioctls, one to create the object
+ * with gem_create, and another to apply various parameters, however this
+ * creates some ambiguity for the params which are considered immutable. Also
in
+ * general we're phasing out the various SET/GET ioctls.
*/
struct drm_i915_gem_create_ext {
/**
@@ -3378,7 +3378,6 @@ struct drm_i915_gem_create_ext {
*
* The (page-aligned) allocated size for the object will be returned.
*
-*
* DG2 64K min page size implications:
*
* On discrete platforms, starting from DG2, we have to contend with GTT
@@ -3390,7 +3389,9 @@ struct drm_i915_gem_create_ext {
*
* Note that the returned size here will always reflect any required
* rounding up done by the kernel, i.e 4K will now become 64K on devices
-* such as DG2.
+* such as DG2. The kernel will always
[PATCH v3 03/13] drm/i915/uapi: expose the avail tracking
Vulkan would like to have a rough measure of how much device memory can
in theory be allocated. Also add unallocated_cpu_visible_size to track
the visible portion, in case the device is using small BAR. Also tweak
the locking so we nice consistent values for both the mm->avail and the
visible tracking.
v2: tweak the locking slightly so we update the mm->avail and visible
tracking as one atomic operation, such that userspace doesn't get
strange values when sampling the values.
Testcase: igt@i915_query@query-regions-unallocated
Testcase: igt@i915_query@query-regions-sanity-check
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/i915_query.c | 10 +-
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 31 ++-
drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 3 ++
drivers/gpu/drm/i915/intel_memory_region.c| 14 +
drivers/gpu/drm/i915/intel_memory_region.h| 3 ++
include/uapi/drm/i915_drm.h | 31 ++-
6 files changed, 82 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 9894add651dd..6ec9c9fb7b0d 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -504,7 +504,15 @@ static int query_memregion_info(struct drm_i915_private
*i915,
else
info.probed_cpu_visible_size = mr->total;
- info.unallocated_size = mr->avail;
+ if (perfmon_capable()) {
+ intel_memory_region_avail(mr,
+ &info.unallocated_size,
+
&info.unallocated_cpu_visible_size);
+ } else {
+ info.unallocated_size = info.probed_size;
+ info.unallocated_cpu_visible_size =
+ info.probed_cpu_visible_size;
+ }
if (__copy_to_user(info_ptr, &info, sizeof(info)))
return -EFAULT;
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index a5109548abc0..427de1aaab36 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -104,18 +104,15 @@ static int i915_ttm_buddy_man_alloc(struct
ttm_resource_manager *man,
min_page_size,
&bman_res->blocks,
bman_res->flags);
- mutex_unlock(&bman->lock);
if (unlikely(err))
goto err_free_blocks;
if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT;
- mutex_lock(&bman->lock);
drm_buddy_block_trim(mm,
original_size,
&bman_res->blocks);
- mutex_unlock(&bman->lock);
}
if (lpfn <= bman->visible_size) {
@@ -137,11 +134,10 @@ static int i915_ttm_buddy_man_alloc(struct
ttm_resource_manager *man,
}
}
- if (bman_res->used_visible_size) {
- mutex_lock(&bman->lock);
+ if (bman_res->used_visible_size)
bman->visible_avail -= bman_res->used_visible_size;
- mutex_unlock(&bman->lock);
- }
+
+ mutex_unlock(&bman->lock);
if (place->lpfn - place->fpfn == n_pages)
bman_res->base.start = place->fpfn;
@@ -154,7 +150,6 @@ static int i915_ttm_buddy_man_alloc(struct
ttm_resource_manager *man,
return 0;
err_free_blocks:
- mutex_lock(&bman->lock);
drm_buddy_free_list(mm, &bman_res->blocks);
mutex_unlock(&bman->lock);
err_free_res:
@@ -365,6 +360,26 @@ u64 i915_ttm_buddy_man_visible_size(struct
ttm_resource_manager *man)
return bman->visible_size;
}
+/**
+ * i915_ttm_buddy_man_avail - Query the avail tracking for the manager.
+ *
+ * @man: The buddy allocator ttm manager
+ * @avail: The total available memory in pages for the entire manager.
+ * @visible_avail: The total available memory in pages for the CPU visible
+ * portion. Note that this will always give the same value as @avail on
+ * configurations that don't have a small BAR.
+ */
+void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
+ u64 *avail, u64 *visible_avail)
+{
+ struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
+
+ mutex_lock(&bman->lock);
+ *avail = bman->mm.avail >> PAGE_SHIFT;
+ *visible_avail = bman->visible_avail;
+ mutex_unlock(&bman->lock);
+}
+
#if IS_ENABLED(CONFIG_
[PATCH v3 07/13] drm/i915/error: skip non-mappable pages
Skip capturing any lmem pages that can't be copied using the CPU. This
in now only best effort on platforms that have small BAR.
Testcase: igt@gem-exec-capture@capture-invisible
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Reviewed-by: Nirmoy Das
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/i915_gpu_error.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index f9b1969ed7ed..52ea13fee015 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1129,11 +1129,15 @@ i915_vma_coredump_create(const struct intel_gt *gt,
dma_addr_t dma;
for_each_sgt_daddr(dma, iter, vma_res->bi.pages) {
+ dma_addr_t offset = dma - mem->region.start;
void __iomem *s;
- s = io_mapping_map_wc(&mem->iomap,
- dma - mem->region.start,
- PAGE_SIZE);
+ if (offset + PAGE_SIZE > mem->io_size) {
+ ret = -EINVAL;
+ break;
+ }
+
+ s = io_mapping_map_wc(&mem->iomap, offset, PAGE_SIZE);
ret = compress_page(compress,
(void __force *)s, dst,
true);
--
2.36.1
[PATCH v3 09/13] drm/i915/selftests: skip the mman tests for stolen
It's not supported, and just skips later anyway. With small-BAR things
get more complicated since all of stolen is likely not even CPU
accessible, hence not passing I915_BO_ALLOC_GPU_ONLY just results in the
object create failing.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 12
1 file changed, 12 insertions(+)
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 5bc93a1ce3e3..388c85b0f764 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -979,6 +979,9 @@ static int igt_mmap(void *arg)
};
int i;
+ if (mr->private)
+ continue;
+
for (i = 0; i < ARRAY_SIZE(sizes); i++) {
struct drm_i915_gem_object *obj;
int err;
@@ -1435,6 +1438,9 @@ static int igt_mmap_access(void *arg)
struct drm_i915_gem_object *obj;
int err;
+ if (mr->private)
+ continue;
+
obj = __i915_gem_object_create_user(i915, PAGE_SIZE, &mr, 1);
if (obj == ERR_PTR(-ENODEV))
continue;
@@ -1580,6 +1586,9 @@ static int igt_mmap_gpu(void *arg)
struct drm_i915_gem_object *obj;
int err;
+ if (mr->private)
+ continue;
+
obj = __i915_gem_object_create_user(i915, PAGE_SIZE, &mr, 1);
if (obj == ERR_PTR(-ENODEV))
continue;
@@ -1727,6 +1736,9 @@ static int igt_mmap_revoke(void *arg)
struct drm_i915_gem_object *obj;
int err;
+ if (mr->private)
+ continue;
+
obj = __i915_gem_object_create_user(i915, PAGE_SIZE, &mr, 1);
if (obj == ERR_PTR(-ENODEV))
continue;
--
2.36.1
[PATCH v3 13/13] drm/i915: turn on small BAR support
With the uAPI in place we should now have enough in place to ensure a
working system on small BAR configurations.
v2: (Nirmoy & Thomas):
- s/full BAR/Resizable BAR/ which is hopefully more easily
understood by users.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
---
drivers/gpu/drm/i915/gt/intel_region_lmem.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index d09b996a9759..fa7b86f83e7b 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -112,12 +112,6 @@ static struct intel_memory_region *setup_lmem(struct
intel_gt *gt)
flat_ccs_base = intel_gt_mcr_read_any(gt,
XEHP_FLAT_CCS_BASE_ADDR);
flat_ccs_base = (flat_ccs_base >> XEHP_CCS_BASE_SHIFT) * SZ_64K;
- /* FIXME: Remove this when we have small-bar enabled */
- if (pci_resource_len(pdev, 2) < lmem_size) {
- drm_err(&i915->drm, "System requires small-BAR support,
which is currently unsupported on this kernel\n");
- return ERR_PTR(-EINVAL);
- }
-
if (GEM_WARN_ON(lmem_size < flat_ccs_base))
return ERR_PTR(-EIO);
@@ -170,6 +164,10 @@ static struct intel_memory_region *setup_lmem(struct
intel_gt *gt)
drm_info(&i915->drm, "Local memory available: %pa\n",
&lmem_size);
+ if (io_size < lmem_size)
+ drm_info(&i915->drm, "Using a reduced BAR size of %lluMiB.
Consider enabling 'Resizable BAR' or similar, if available in the BIOS.\n",
+(u64)io_size >> 20);
+
return mem;
err_region_put:
--
2.36.1
[PATCH v3 12/13] drm/i915/ttm: disallow CPU fallback mode for ccs pages
Falling back to memcpy/memset shouldn't be allowed if we know we have
CCS state to manage using the blitter. Otherwise we are potentially
leaving the aux CCS state in an unknown state, which smells like an info
leak.
Fixes: 48760ffe923a ("drm/i915/gt: Clear compress metadata for Flat-ccs
objects")
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Cc: Ramalingam C
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 26
drivers/gpu/drm/i915/gem/i915_gem_object.h | 2 ++
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 18 --
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 3 +++
4 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 642a5d59ce26..ccec4055fde3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -717,6 +717,32 @@ bool i915_gem_object_placement_possible(struct
drm_i915_gem_object *obj,
return false;
}
+/**
+ * i915_gem_object_needs_ccs_pages - Check whether the object requires extra
+ * pages when placed in system-memory, in order to save and later restore the
+ * flat-CCS aux state when the object is moved between local-memory and
+ * system-memory
+ * @obj: Pointer to the object
+ *
+ * Return: True if the object needs extra ccs pages. False otherwise.
+ */
+bool i915_gem_object_needs_ccs_pages(struct drm_i915_gem_object *obj)
+{
+ bool lmem_placement = false;
+ int i;
+
+ for (i = 0; i < obj->mm.n_placements; i++) {
+ /* Compression is not allowed for the objects with smem
placement */
+ if (obj->mm.placements[i]->type == INTEL_MEMORY_SYSTEM)
+ return false;
+ if (!lmem_placement &&
+ obj->mm.placements[i]->type == INTEL_MEMORY_LOCAL)
+ lmem_placement = true;
+ }
+
+ return lmem_placement;
+}
+
void i915_gem_init__objects(struct drm_i915_private *i915)
{
INIT_DELAYED_WORK(&i915->mm.free_work, __i915_gem_free_work);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 0bf3ee27a2a8..6f0a3ce35567 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -618,6 +618,8 @@ int i915_gem_object_wait_migration(struct
drm_i915_gem_object *obj,
bool i915_gem_object_placement_possible(struct drm_i915_gem_object *obj,
enum intel_memory_type type);
+bool i915_gem_object_needs_ccs_pages(struct drm_i915_gem_object *obj);
+
int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
size_t size, struct intel_memory_region *mr,
struct address_space *mapping,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 098409a33e10..7e1f8b83077f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -266,24 +266,6 @@ static const struct i915_refct_sgt_ops tt_rsgt_ops = {
.release = i915_ttm_tt_release
};
-static inline bool
-i915_gem_object_needs_ccs_pages(struct drm_i915_gem_object *obj)
-{
- bool lmem_placement = false;
- int i;
-
- for (i = 0; i < obj->mm.n_placements; i++) {
- /* Compression is not allowed for the objects with smem
placement */
- if (obj->mm.placements[i]->type == INTEL_MEMORY_SYSTEM)
- return false;
- if (!lmem_placement &&
- obj->mm.placements[i]->type == INTEL_MEMORY_LOCAL)
- lmem_placement = true;
- }
-
- return lmem_placement;
-}
-
static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo,
uint32_t page_flags)
{
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 364e7fe8efb1..d22e38aad6b9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -429,6 +429,9 @@ i915_ttm_memcpy_work_arm(struct i915_ttm_memcpy_work *work,
static bool i915_ttm_memcpy_allowed(struct ttm_buffer_object *bo,
struct ttm_resource *dst_mem)
{
+ if (i915_gem_object_needs_ccs_pages(i915_ttm_to_gem(bo)))
+ return false;
+
if (!(i915_ttm_resource_mappable(bo->resource) &&
i915_ttm_resource_mappable(dst_mem)))
return false;
--
2.36.1
[PATCH v3 10/13] drm/i915/selftests: ensure we reserve a fence slot
We should always be explicit and allocate a fence slot before adding a
new fence.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
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 388c85b0f764..da28acb78a88 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -1224,8 +1224,10 @@ static int __igt_mmap_migrate(struct intel_memory_region
**placements,
expand32(POISON_INUSE), &rq);
i915_gem_object_unpin_pages(obj);
if (rq) {
- dma_resv_add_fence(obj->base.resv, &rq->fence,
- DMA_RESV_USAGE_KERNEL);
+ err = dma_resv_reserve_fences(obj->base.resv, 1);
+ if (!err)
+ dma_resv_add_fence(obj->base.resv, &rq->fence,
+ DMA_RESV_USAGE_KERNEL);
i915_request_put(rq);
}
i915_gem_object_unlock(obj);
--
2.36.1
[PATCH v3 11/13] drm/i915/ttm: handle blitter failure on DG2
If the move or clear operation somehow fails, and the memory underneath
is not cleared, like when moving to lmem, then we currently fallback to
memcpy or memset. However with small-BAR systems this fallback might no
longer be possible. For now we use the set_wedged sledgehammer if we
ever encounter such a scenario, and mark the object as borked to plug
any holes where access to the memory underneath can happen. Add some
basic selftests to exercise this.
v2:
- In the selftests make sure we grab the runtime pm around the reset.
Also make sure we grab the reset lock before checking if the device
is wedged, since the wedge might still be in-progress and hence the
bit might not be set yet.
- Don't wedge or put the object into an unknown state, if the request
construction fails (or similar). Just returning an error and
skipping the fallback should be safe here.
- Make sure we wedge each gt. (Thomas)
- Peek at the unknown_state in io_reserve, that way we don't have to
export or hand roll the fault_wait_for_idle. (Thomas)
- Add the missing read-side barriers for the unknown_state. (Thomas)
- Some kernel-doc fixes. (Thomas)
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
---
drivers/gpu/drm/i915/gem/i915_gem_object.c| 21 +++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 1 +
.../gpu/drm/i915/gem/i915_gem_object_types.h | 18 +++
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 26 +++-
drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 3 +
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 88 +--
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 1 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 141 +++---
.../drm/i915/gem/selftests/i915_gem_mman.c| 69 +
drivers/gpu/drm/i915/i915_vma.c | 25 ++--
10 files changed, 346 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 06b1b188ce5a..642a5d59ce26 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -783,10 +783,31 @@ int i915_gem_object_wait_moving_fence(struct
drm_i915_gem_object *obj,
intr, MAX_SCHEDULE_TIMEOUT);
if (!ret)
ret = -ETIME;
+ else if (ret > 0 && i915_gem_object_has_unknown_state(obj))
+ ret = -EIO;
return ret < 0 ? ret : 0;
}
+/**
+ * i915_gem_object_has_unknown_state - Return true if the object backing pages
are
+ * in an unknown_state. This means that userspace must NEVER be allowed to
touch
+ * the pages, with either the GPU or CPU.
+ *
+ * ONLY valid to be called after ensuring that all kernel fences have signalled
+ * (in particular the fence for moving/clearing the object).
+ */
+bool i915_gem_object_has_unknown_state(struct drm_i915_gem_object *obj)
+{
+ /*
+* The below barrier pairs with the dma_fence_signal() in
+* __memcpy_work(). We should only sample the unknown_state after all
+* the kernel fences have signalled.
+*/
+ smp_rmb();
+ return obj->mm.unknown_state;
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftests/huge_gem_object.c"
#include "selftests/huge_pages.c"
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e11d82a9f7c3..0bf3ee27a2a8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -524,6 +524,7 @@ int i915_gem_object_get_moving_fence(struct
drm_i915_gem_object *obj,
struct dma_fence **fence);
int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
bool intr);
+bool i915_gem_object_has_unknown_state(struct drm_i915_gem_object *obj);
void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
unsigned int cache_level);
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 2c88bdb8ff7c..5cf36a130061 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -547,6 +547,24 @@ struct drm_i915_gem_object {
*/
bool ttm_shrinkable;
+ /**
+* @unknown_state: Indicate that the object is effectively
+* borked. This is write-once and set if we somehow encounter a
+* fatal error when moving/clearing the pages, and we are not
+* able to fallback to memcpy/memset, like on small-BAR systems.
+* The GPU should also be wedged (or in the process) at this
+* point.
+
[PATCH v3 08/13] drm/i915/uapi: tweak error capture on recoverable contexts
A non-recoverable context must be used if the user wants proper error
capture on discrete platforms. In the future the kernel may want to blit
the contents of some objects when later doing the capture stage. Also
extend to newer integrated platforms.
v2(Thomas):
- Also extend to newer integrated platforms, for capture buffer memory
allocation purposes.
v3 (Reported-by: kernel test robot ):
- Fix build on !CONFIG_DRM_I915_CAPTURE_ERROR
Testcase: igt@gem_exec_capture@capture-recoverable
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Lionel Landwerlin
Cc: Tvrtko Ursulin
Cc: Jon Bloomfield
Cc: Daniel Vetter
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: Akeem G Abodunrin
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 30fe847c6664..b7b2c14fd9e1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1951,7 +1951,7 @@ eb_find_first_request_added(struct i915_execbuffer *eb)
#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
/* Stage with GFP_KERNEL allocations before we enter the signaling critical
path */
-static void eb_capture_stage(struct i915_execbuffer *eb)
+static int eb_capture_stage(struct i915_execbuffer *eb)
{
const unsigned int count = eb->buffer_count;
unsigned int i = count, j;
@@ -1964,6 +1964,10 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
if (!(flags & EXEC_OBJECT_CAPTURE))
continue;
+ if (i915_gem_context_is_recoverable(eb->gem_context) &&
+ (IS_DGFX(eb->i915) || GRAPHICS_VER_FULL(eb->i915) >
IP_VER(12, 0)))
+ return -EINVAL;
+
for_each_batch_create_order(eb, j) {
struct i915_capture_list *capture;
@@ -1976,6 +1980,8 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
eb->capture_lists[j] = capture;
}
}
+
+ return 0;
}
/* Commit once we're in the critical path */
@@ -2017,8 +2023,9 @@ static void eb_capture_list_clear(struct i915_execbuffer
*eb)
#else
-static void eb_capture_stage(struct i915_execbuffer *eb)
+static int eb_capture_stage(struct i915_execbuffer *eb)
{
+ return 0;
}
static void eb_capture_commit(struct i915_execbuffer *eb)
@@ -3410,7 +3417,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
}
ww_acquire_done(&eb.ww.ctx);
- eb_capture_stage(&eb);
+ err = eb_capture_stage(&eb);
+ if (err)
+ goto err_vma;
out_fence = eb_requests_create(&eb, in_fence, out_fence_fd);
if (IS_ERR(out_fence)) {
--
2.36.1
Re: [PATCH v2 1/2] procfs: Add 'size' to /proc//fdinfo/
On Tue, Jun 28, 2022 at 03:38:02PM -0700, Kalesh Singh wrote: > On Tue, Jun 28, 2022 at 4:54 AM Brian Foster wrote: > > > > On Thu, Jun 23, 2022 at 03:06:06PM -0700, Kalesh Singh wrote: > > > To be able to account the amount of memory a process is keeping pinned > > > by open file descriptors add a 'size' field to fdinfo output. > > > > > > dmabufs fds already expose a 'size' field for this reason, remove this > > > and make it a common field for all fds. This allows tracking of > > > other types of memory (e.g. memfd and ashmem in Android). > > > > > > Signed-off-by: Kalesh Singh > > > Reviewed-by: Christian König > > > --- > > > > > > Changes in v2: > > > - Add Christian's Reviewed-by > > > > > > Changes from rfc: > > > - Split adding 'size' and 'path' into a separate patches, per Christian > > > - Split fdinfo seq_printf into separate lines, per Christian > > > - Fix indentation (use tabs) in documentaion, per Randy > > > > > > Documentation/filesystems/proc.rst | 12 ++-- > > > drivers/dma-buf/dma-buf.c | 1 - > > > fs/proc/fd.c | 9 + > > > 3 files changed, 15 insertions(+), 7 deletions(-) > > > ... > > > > Also not sure if it matters that much for your use case, but something > > worth noting at least with shmem is that one can do something like: > > > > # cat /proc/meminfo | grep Shmem: > > Shmem: 764 kB > > # xfs_io -fc "falloc -k 0 10m" ./file > > # ls -alh file > > -rw---. 1 root root 0 Jun 28 07:22 file > > # stat file > > File: file > > Size: 0 Blocks: 20480 IO Block: 4096 regular empty > > file > > # cat /proc/meminfo | grep Shmem: > > Shmem: 11004 kB > > > > ... where the resulting memory usage isn't reflected in i_size (but is > > is in i_blocks/bytes). > > I tried a similar experiment a few times, but I don't see the same > results. In my case, there is not any change in shmem. IIUC the > fallocate is allocating the disk space not shared memory. > Sorry, it was implied in my previous test was that I was running against tmpfs. So regardless of fs, the fallocate keep_size semantics shown in both cases is as expected: the underlying blocks are allocated and the inode size is unchanged. What wasn't totally clear to me when I read this patch was 1. whether tmpfs refers to Shmem and 2. whether tmpfs allowed this sort of operation. The test above seems to confirm both, however, right? E.g., a more detailed example: # mount | grep /tmp tmpfs on /tmp type tmpfs (rw,nosuid,nodev,seclabel,nr_inodes=1048576,inode64) # cat /proc/meminfo | grep Shmem: Shmem: 5300 kB # xfs_io -fc "falloc -k 0 1g" /tmp/file # stat /tmp/file File: /tmp/file Size: 0 Blocks: 2097152IO Block: 4096 regular empty file Device: 22h/34d Inode: 45 Links: 1 Access: (0600/-rw---) Uid: (0/root) Gid: (0/root) Context: unconfined_u:object_r:user_tmp_t:s0 Access: 2022-06-29 08:04:01.301307154 -0400 Modify: 2022-06-29 08:04:01.301307154 -0400 Change: 2022-06-29 08:04:01.451312834 -0400 Birth: 2022-06-29 08:04:01.301307154 -0400 # cat /proc/meminfo | grep Shmem: Shmem: 1053876 kB # rm -f /tmp/file # cat /proc/meminfo | grep Shmem: Shmem: 5300 kB So clearly this impacts Shmem.. was your test run against tmpfs or some other (disk based) fs? FWIW, I don't have any objection to exposing inode size if it's commonly useful information. My feedback was more just an fyi that i_size doesn't necessarily reflect underlying space consumption (whether it's memory or disk space) in more generic cases, because it sounds like that is really what you're after here. The opposite example to the above would be something like an 'xfs_io -fc "truncate 1t" /tmp/file', which shows a 1TB inode size with zero additional shmem usage. Brian > cat /proc/meminfo > meminfo.start > xfs_io -fc "falloc -k 0 50m" ./xfs_file > cat /proc/meminfo > meminfo.stop > tail -n +1 meminfo.st* | grep -i '==\|Shmem:' > > ==> meminfo.start <== > Shmem: 484 kB > ==> meminfo.stop <== > Shmem: 484 kB > > ls -lh xfs_file > -rw--- 1 root root 0 Jun 28 15:12 xfs_file > > stat xfs_file > File: xfs_file > Size: 0 Blocks: 102400 IO Block: 4096 regular empty file > > Thanks, > Kalesh > > > > > Brian > > > > > > > > /* show_fd_locks() never deferences files so a stale value is safe > > > */ > > > show_fd_locks(m, file, files); > > > -- > > > 2.37.0.rc0.161.g10f37bed90-goog > > > > > >
[PATCH v3 00/71] drm/vc4: Fix hotplug for vc4
Hi, Here is a series that address multiple issues when trying to unbind/rebind vc4-related devices to their drivers. Most of these issues involve either use-after-free, improper resource liberation or similar. It has been tested on the Pi3 and Pi4, with X and glxgears running and KASAN enabled to properly validate our memory accesses. Pi3 isn't functional after a rebind though, with vblank timeouts occuring. I'm not quite sure why at this point, but at least the kernel doesn't completely crash now. Let me know what you think, Maxime --- Changes from v2: - Rebased on top of next-20220629 - Fix va arguments on the crtc and encoder init - Removed drmm_connector_init_with_ddc and consolidated drm_connector_init* - Reworked the doc for drmm_of_get_bridge Changes from v1: - Rebased on top on next-20220622 - Consolidated the new drmm init helpers with their alloc counterparts - Dropped the drmm writeback and simple encoder helpers - Clarified the documentation of drm_connector_unregister - Removed the drm_connector_unregister usage - Fixed a vblank timeout when unbinding - Renamed the kref functions in vc4_dsi - Collected the tags Maxime Ripard (71): drm/mipi-dsi: Detach devices when removing the host drm/crtc: Introduce drmm_crtc_init_with_planes drm/encoder: Introduce drmm_encoder_init drm/connector: Reorder headers drm/connector: Mention the cleanup after drm_connector_init drm/connector: Clarify when drm_connector_unregister is needed drm/connector: Consolidate Connector Initialization drm/connector: Check for destroy implementation drm/connector: Introduce drmm_connector_init drm/bridge: panel: Introduce drmm_panel_bridge_add drm/bridge: panel: Introduce drmm_of_get_bridge drm/vc4: drv: Call component_unbind_all() drm/vc4: drv: Use drm_dev_unplug drm/vc4: crtc: Create vblank reporting function drm/vc4: hvs: Protect device resources after removal drm/vc4: hvs: Remove planes currently allocated before taking down drm/vc4: plane: Take possible_crtcs as an argument drm/vc4: crtc: Remove manual plane removal on error drm/vc4: plane: Switch to drmm_universal_plane_alloc() drm/vc4: crtc: Move debugfs_name to crtc_data drm/vc4: crtc: Switch to drmm_kzalloc drm/vc4: crtc: Switch to DRM-managed CRTC initialization drm/vc4: dpi: Remove vc4_dev dpi pointer drm/vc4: dpi: Embed DRM structures into the private structure drm/vc4: dpi: Switch to drmm_kzalloc drm/vc4: dpi: Return an error if we can't enable our clock drm/vc4: dpi: Remove unnecessary drm_of_panel_bridge_remove call drm/vc4: dpi: Add action to disable the clock drm/vc4: dpi: Switch to DRM-managed encoder initialization drm/vc4: dpi: Switch to drmm_of_get_bridge drm/vc4: dpi: Protect device resources drm/vc4: dsi: Embed DRM structures into the private structure drm/vc4: dsi: Switch to DRM-managed encoder initialization drm/vc4: dsi: Switch to drmm_of_get_bridge drm/vc4: dsi: Fix the driver structure lifetime drm/vc4: dsi: Switch to devm_pm_runtime_enable drm/vc4: hdmi: Depends on CONFIG_PM drm/vc4: hdmi: Rework power up drm/vc4: hdmi: Switch to drmm_kzalloc drm/vc4: hdmi: Remove call to drm_connector_unregister() drm/vc4: hdmi: Switch to DRM-managed encoder initialization drm/vc4: hdmi: Switch to DRM-managed connector initialization drm/vc4: hdmi: Switch to device-managed ALSA initialization drm/vc4: hdmi: Switch to device-managed CEC initialization drm/vc4: hdmi: Use a device-managed action for DDC drm/vc4: hdmi: Switch to DRM-managed kfree to build regsets drm/vc4: hdmi: Use devm to register hotplug interrupts drm/vc4: hdmi: Move audio structure offset checks drm/vc4: hdmi: Protect device resources after removal drm/vc4: hdmi: Switch to devm_pm_runtime_enable drm/vc4: txp: Remove vc4_dev txp pointer drm/vc4: txp: Remove duplicate regset drm/vc4: txp: Switch to drmm_kzalloc drm/vc4: txp: Remove call to drm_connector_unregister() drm/vc4: txp: Protect device resources drm/vc4: vec: Remove vc4_dev vec pointer drm/vc4: vec: Embed DRM structures into the private structure drm/vc4: vec: Switch to drmm_kzalloc drm/vc4: vec: Remove call to drm_connector_unregister() drm/vc4: vec: Switch to DRM-managed encoder initialization drm/vc4: vec: Switch to DRM-managed connector initialization drm/vc4: vec: Protect device resources after removal drm/vc4: vec: Switch to devm_pm_runtime_enable drm/vc4: debugfs: Protect device resources drm/vc4: debugfs: Return an error on failure drm/vc4: debugfs: Simplify debugfs registration drm/vc4: Switch to drmm_mutex_init drm/vc4: perfmon: Add missing mutex_destroy drm/vc4: v3d: Stop disabling interrupts drm/vc4: v3d: Rework the runtime_pm setup drm/vc4: v3d: Switch to devm_pm_runtime_enable drivers/gpu/drm/bridge/panel.c| 74 drivers/gpu/drm/drm_connector.c | 143 +-- drivers/gpu/drm/drm_crtc.c| 93 +++- drivers/g
[PATCH v3 02/71] drm/crtc: Introduce drmm_crtc_init_with_planes
The DRM-managed function to register a CRTC is
drmm_crtc_alloc_with_planes(), which will allocate the underlying
structure and initialisation the CRTC.
However, we might want to separate the structure creation and the CRTC
initialisation, for example if the structure is shared across multiple
DRM entities, for example an encoder and a connector.
Let's create an helper to only initialise a CRTC that would be passed as
an argument.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_crtc.c | 93 +-
include/drm/drm_crtc.h | 9
2 files changed, 90 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index cad2a7e5166f..fa2ff2819c10 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -343,9 +343,10 @@ static int __drm_crtc_init_with_planes(struct drm_device
*dev, struct drm_crtc *
* The @primary and @cursor planes are only relevant for legacy uAPI, see
* &drm_crtc.primary and &drm_crtc.cursor.
*
- * Note: consider using drmm_crtc_alloc_with_planes() instead of
- * drm_crtc_init_with_planes() to let the DRM managed resource infrastructure
- * take care of cleanup and deallocation.
+ * Note: consider using drmm_crtc_alloc_with_planes() or
+ * drmm_crtc_init_with_planes() instead of drm_crtc_init_with_planes()
+ * to let the DRM managed resource infrastructure take care of cleanup
+ * and deallocation.
*
* Returns:
* Zero on success, error code on failure.
@@ -370,14 +371,87 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
struct drm_crtc *crtc,
}
EXPORT_SYMBOL(drm_crtc_init_with_planes);
-static void drmm_crtc_alloc_with_planes_cleanup(struct drm_device *dev,
- void *ptr)
+static void drmm_crtc_init_with_planes_cleanup(struct drm_device *dev,
+ void *ptr)
{
struct drm_crtc *crtc = ptr;
drm_crtc_cleanup(crtc);
}
+static int __drmm_crtc_init_with_planes(struct drm_device *dev,
+ struct drm_crtc *crtc,
+ struct drm_plane *primary,
+ struct drm_plane *cursor,
+ const struct drm_crtc_funcs *funcs,
+ const char *name,
+ va_list args)
+{
+ int ret;
+
+ WARN_ON(funcs && funcs->destroy);
+
+ ret = __drm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs,
+ name, args);
+ if (ret)
+ return ret;
+
+ ret = drmm_add_action_or_reset(dev, drmm_crtc_init_with_planes_cleanup,
+ crtc);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+/**
+ * drmm_crtc_init_with_planes - Initialise a new CRTC object with
+ *specified primary and cursor planes.
+ * @dev: DRM device
+ * @crtc: CRTC object to init
+ * @primary: Primary plane for CRTC
+ * @cursor: Cursor plane for CRTC
+ * @funcs: callbacks for the new CRTC
+ * @name: printf style format string for the CRTC name, or NULL for default
name
+ *
+ * Inits a new object created as base part of a driver crtc object. Drivers
+ * should use this function instead of drm_crtc_init(), which is only provided
+ * for backwards compatibility with drivers which do not yet support universal
+ * planes). For really simple hardware which has only 1 plane look at
+ * drm_simple_display_pipe_init() instead.
+ *
+ * Cleanup is automatically handled through registering
+ * drmm_crtc_cleanup() with drmm_add_action(). The crtc structure should
+ * be allocated with drmm_kzalloc().
+ *
+ * The @drm_crtc_funcs.destroy hook must be NULL.
+ *
+ * The @primary and @cursor planes are only relevant for legacy uAPI, see
+ * &drm_crtc.primary and &drm_crtc.cursor.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drmm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
+ struct drm_plane *primary,
+ struct drm_plane *cursor,
+ const struct drm_crtc_funcs *funcs,
+ const char *name, ...)
+{
+ va_list ap;
+ int ret;
+
+ va_start(ap, name);
+ ret = __drmm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs,
+ name, ap);
+ va_end(ap);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(drmm_crtc_init_with_planes);
+
void *__drmm_crtc_alloc_with_planes(struct drm_device *dev,
size_t size, size_t offset,
struct drm_plane *primary,
@@ -400,17 +474,12 @@ void *__drmm_crtc_alloc_with_planes(struct drm_device
*dev,
crtc = container + offset;
va_start(a
[PATCH v3 01/71] drm/mipi-dsi: Detach devices when removing the host
Whenever the MIPI-DSI host is unregistered, the code of
mipi_dsi_host_unregister() loops over every device currently found on that
bus and will unregister it.
However, it doesn't detach it from the bus first, which leads to all kind
of resource leaks if the host wants to perform some clean up whenever a
device is detached.
Fixes: 068a00233969 ("drm: Add MIPI DSI bus support")
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_mipi_dsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index c40bde96cfdf..c317ee9fa445 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -346,6 +346,7 @@ static int mipi_dsi_remove_device_fn(struct device *dev,
void *priv)
{
struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
+ mipi_dsi_detach(dsi);
mipi_dsi_device_unregister(dsi);
return 0;
--
2.36.1
[PATCH v3 04/71] drm/connector: Reorder headers
Unlike most of the other files in DRM, and Linux in general, the headers in drm_connector.c aren't sorted alphabetically. Let's fix that. Acked-by: Sam Ravnborg Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_connector.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index c7ca435ceb95..77c33662b436 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -22,15 +22,15 @@ #include #include +#include #include #include +#include #include -#include #include -#include -#include #include #include +#include #include -- 2.36.1
[PATCH v3 06/71] drm/connector: Clarify when drm_connector_unregister is needed
The current documentation for drm_connector_unregister() mentions that
it's needed for connectors that have been registered through
drm_dev_register().
However, this was a typo and was meant to be drm_connector_register(),
which only applies to connectors registered after drm_dev_register() has
been called.
In addition, it was also mentioning that connectors are unregistered
automatically when drm_dev_unregister() is called. This part is a bit
misleading, since it might make it appear that
drm_connector_unregister() applies either to all connectors, or none of
them.
After discussing it with Daniel, it appears that we always need to call
drm_connector_unregister() on connectors that have been registered with
drm_connector_register(), but only those.
drm_connector_init() already mentions that it only needs
drm_connector_cleanup(), so let's clarify the drm_connector_register()
and drm_connector_unregister() documentation to point at each other, and
remove the misleading part about drm_dev_unregister().
Acked-by: Sam Ravnborg
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_connector.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index c8f23a3d9b03..327e8cc076ad 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -524,6 +524,9 @@ EXPORT_SYMBOL(drm_connector_cleanup);
* e.g. DP MST connectors. All other connectors will be registered
automatically
* when calling drm_dev_register().
*
+ * When the connector is no longer available, callers must call
+ * drm_connector_unregister().
+ *
* Returns:
* Zero on success, error code on failure.
*/
@@ -580,9 +583,8 @@ EXPORT_SYMBOL(drm_connector_register);
* @connector: the connector to unregister
*
* Unregister userspace interfaces for a connector. Only call this for
- * connectors which have registered explicitly by calling drm_dev_register(),
- * since connectors are unregistered automatically when drm_dev_unregister() is
- * called.
+ * connectors which have been registered explicitly by calling
+ * drm_connector_register().
*/
void drm_connector_unregister(struct drm_connector *connector)
{
--
2.36.1
[PATCH v3 03/71] drm/encoder: Introduce drmm_encoder_init
The DRM-managed function to register an encoder is
drmm_encoder_alloc() and its variants, which will allocate the underlying
structure and initialisation the encoder.
However, we might want to separate the structure creation and the encoder
initialisation, for example if the structure is shared across multiple DRM
entities, for example an encoder and a connector.
Let's create an helper to only initialise an encoder that would be passed
as an argument.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_encoder.c | 76 ++-
include/drm/drm_encoder.h | 6 +++
2 files changed, 71 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index a940024c8087..ae2da4be080b 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -148,9 +148,9 @@ static int __drm_encoder_init(struct drm_device *dev,
* the encoder structure. The encoder structure should not be allocated with
* devm_kzalloc().
*
- * Note: consider using drmm_encoder_alloc() instead of drm_encoder_init() to
- * let the DRM managed resource infrastructure take care of cleanup and
- * deallocation.
+ * Note: consider using drmm_encoder_alloc() or drmm_encoder_init()
+ * instead of drm_encoder_init() to let the DRM managed resource
+ * infrastructure take care of cleanup and deallocation.
*
* Returns:
* Zero on success, error code on failure.
@@ -212,6 +212,29 @@ static void drmm_encoder_alloc_release(struct drm_device
*dev, void *ptr)
drm_encoder_cleanup(encoder);
}
+static int __drmm_encoder_init(struct drm_device *dev,
+ struct drm_encoder *encoder,
+ const struct drm_encoder_funcs *funcs,
+ int encoder_type,
+ const char *name,
+ va_list args)
+{
+ int ret;
+
+ if (WARN_ON(funcs && funcs->destroy))
+ return -EINVAL;
+
+ ret = __drm_encoder_init(dev, encoder, funcs, encoder_type, name, args);
+ if (ret)
+ return ret;
+
+ ret = drmm_add_action_or_reset(dev, drmm_encoder_alloc_release,
encoder);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
void *__drmm_encoder_alloc(struct drm_device *dev, size_t size, size_t offset,
const struct drm_encoder_funcs *funcs,
int encoder_type, const char *name, ...)
@@ -221,9 +244,6 @@ void *__drmm_encoder_alloc(struct drm_device *dev, size_t
size, size_t offset,
va_list ap;
int ret;
- if (WARN_ON(funcs && funcs->destroy))
- return ERR_PTR(-EINVAL);
-
container = drmm_kzalloc(dev, size, GFP_KERNEL);
if (!container)
return ERR_PTR(-ENOMEM);
@@ -231,19 +251,53 @@ void *__drmm_encoder_alloc(struct drm_device *dev, size_t
size, size_t offset,
encoder = container + offset;
va_start(ap, name);
- ret = __drm_encoder_init(dev, encoder, funcs, encoder_type, name, ap);
+ ret = __drmm_encoder_init(dev, encoder, funcs, encoder_type, name, ap);
va_end(ap);
if (ret)
return ERR_PTR(ret);
- ret = drmm_add_action_or_reset(dev, drmm_encoder_alloc_release,
encoder);
- if (ret)
- return ERR_PTR(ret);
-
return container;
}
EXPORT_SYMBOL(__drmm_encoder_alloc);
+/**
+ * drmm_encoder_init - Initialize a preallocated encoder
+ * @dev: drm device
+ * @encoder: the encoder to init
+ * @funcs: callbacks for this encoder (optional)
+ * @encoder_type: user visible type of the encoder
+ * @name: printf style format string for the encoder name, or NULL for default
name
+ *
+ * Initializes a preallocated encoder. Encoder should be subclassed as
+ * part of driver encoder objects. Cleanup is automatically handled
+ * through registering drm_encoder_cleanup() with drmm_add_action(). The
+ * encoder structure should be allocated with drmm_kzalloc().
+ *
+ * The @drm_encoder_funcs.destroy hook must be NULL.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drmm_encoder_init(struct drm_device *dev, struct drm_encoder *encoder,
+ const struct drm_encoder_funcs *funcs,
+ int encoder_type, const char *name, ...)
+{
+ va_list ap;
+ int ret;
+
+ if (WARN_ON(funcs && funcs->destroy))
+ return -EINVAL;
+
+ va_start(ap, name);
+ ret = __drmm_encoder_init(dev, encoder, funcs, encoder_type, name, ap);
+ va_end(ap);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(drmm_encoder_init);
+
static struct drm_crtc *drm_encoder_get_crtc(struct drm_encoder *encoder)
{
struct drm_connector *connector;
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 6e91a0280f31..3a09682af685 100644
--- a/include/drm/drm_encoder.h
+
[PATCH v3 05/71] drm/connector: Mention the cleanup after drm_connector_init
Unlike encoders and CRTCs, the drm_connector_init() and drm_connector_init_with_ddc() don't mention how the cleanup is supposed to be done. Let's add it. Acked-by: Sam Ravnborg Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_connector.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 77c33662b436..c8f23a3d9b03 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -223,6 +223,10 @@ void drm_connector_free_work_fn(struct work_struct *work) * Initialises a preallocated connector. Connectors should be * subclassed as part of driver connector objects. * + * At driver unload time the driver's &drm_connector_funcs.destroy hook + * should call drm_connector_cleanup() and free the connector structure. + * The connector structure should not be allocated with devm_kzalloc(). + * * Returns: * Zero on success, error code on failure. */ @@ -346,6 +350,10 @@ EXPORT_SYMBOL(drm_connector_init); * Initialises a preallocated connector. Connectors should be * subclassed as part of driver connector objects. * + * At driver unload time the driver's &drm_connector_funcs.destroy hook + * should call drm_connector_cleanup() and free the connector structure. + * The connector structure should not be allocated with devm_kzalloc(). + * * Ensures that the ddc field of the connector is correctly set. * * Returns: -- 2.36.1
[PATCH v3 09/71] drm/connector: Introduce drmm_connector_init
Unlike other DRM entities, there's no helper to create a DRM-managed
initialisation of a connector.
Let's create an helper to initialise a connector that would be passed as an
argument, and handle the cleanup through a DRM-managed action.
Acked-by: Sam Ravnborg
Acked-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_connector.c | 60 +
include/drm/drm_connector.h | 5 +++
2 files changed, 65 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 855267116e68..79bce5a3504b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -339,6 +340,10 @@ static int __drm_connector_init(struct drm_device *dev,
* should call drm_connector_cleanup() and free the connector structure.
* The connector structure should not be allocated with devm_kzalloc().
*
+ * Note: consider using drmm_connector_init() instead of
+ * drm_connector_init() to let the DRM managed resource infrastructure
+ * take care of cleanup and deallocation.
+ *
* Returns:
* Zero on success, error code on failure.
*/
@@ -371,6 +376,10 @@ EXPORT_SYMBOL(drm_connector_init);
*
* Ensures that the ddc field of the connector is correctly set.
*
+ * Note: consider using drmm_connector_init() instead of
+ * drm_connector_init_with_ddc() to let the DRM managed resource
+ * infrastructure take care of cleanup and deallocation.
+ *
* Returns:
* Zero on success, error code on failure.
*/
@@ -387,6 +396,57 @@ int drm_connector_init_with_ddc(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_connector_init_with_ddc);
+static void drm_connector_cleanup_action(struct drm_device *dev,
+void *ptr)
+{
+ struct drm_connector *connector = ptr;
+
+ drm_connector_cleanup(connector);
+}
+
+/**
+ * drmm_connector_init - Init a preallocated connector
+ * @dev: DRM device
+ * @connector: the connector to init
+ * @funcs: callbacks for this connector
+ * @connector_type: user visible type of the connector
+ * @ddc: optional pointer to the associated ddc adapter
+ *
+ * Initialises a preallocated connector. Connectors should be
+ * subclassed as part of driver connector objects.
+ *
+ * Cleanup is automatically handled with a call to
+ * drm_connector_cleanup() in a DRM-managed action.
+ *
+ * The connector structure should be allocated with drmm_kzalloc().
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drmm_connector_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ int connector_type,
+ struct i2c_adapter *ddc)
+{
+ int ret;
+
+ if (WARN_ON(funcs && funcs->destroy))
+ return -EINVAL;
+
+ ret = __drm_connector_init(dev, connector, funcs, connector_type, NULL);
+ if (ret)
+ return ret;
+
+ ret = drmm_add_action_or_reset(dev, drm_connector_cleanup_action,
+ connector);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(drmm_connector_init);
+
/**
* drm_connector_attach_edid_property - attach edid property.
* @connector: the connector
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 94b422b55cc1..7a999e901c7b 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1673,6 +1673,11 @@ int drm_connector_init_with_ddc(struct drm_device *dev,
const struct drm_connector_funcs *funcs,
int connector_type,
struct i2c_adapter *ddc);
+int drmm_connector_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ int connector_type,
+ struct i2c_adapter *ddc);
void drm_connector_attach_edid_property(struct drm_connector *connector);
int drm_connector_register(struct drm_connector *connector);
void drm_connector_unregister(struct drm_connector *connector);
--
2.36.1
[PATCH v3 10/71] drm/bridge: panel: Introduce drmm_panel_bridge_add
Unlike what can be found for other entities, there's no DRM-managed
function to create a panel_bridge instance from a panel.
Let's introduce one.
Acked-by: Sam Ravnborg
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/panel.c | 39 ++
include/drm/drm_bridge.h | 2 ++
2 files changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 4277bf4f032b..56ac51b257b9 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -367,6 +368,44 @@ struct drm_bridge *devm_drm_panel_bridge_add_typed(struct
device *dev,
}
EXPORT_SYMBOL(devm_drm_panel_bridge_add_typed);
+static void drmm_drm_panel_bridge_release(struct drm_device *drm, void *ptr)
+{
+ struct drm_bridge *bridge = ptr;
+
+ drm_panel_bridge_remove(bridge);
+}
+
+/**
+ * drmm_panel_bridge_add - Creates a DRM-managed &drm_bridge and
+ * &drm_connector that just calls the
+ * appropriate functions from &drm_panel.
+ *
+ * @dev: DRM device to tie the bridge lifetime to
+ * @panel: The drm_panel being wrapped. Must be non-NULL.
+ *
+ * This is the DRM-managed version of drm_panel_bridge_add() which
+ * automatically calls drm_panel_bridge_remove() when @dev is cleaned
+ * up.
+ */
+struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm,
+struct drm_panel *panel)
+{
+ struct drm_bridge *bridge;
+ int ret;
+
+ bridge = drm_panel_bridge_add_typed(panel, panel->connector_type);
+ if (IS_ERR(bridge))
+ return bridge;
+
+ ret = drmm_add_action_or_reset(drm, drmm_drm_panel_bridge_release,
+ bridge);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return bridge;
+}
+EXPORT_SYMBOL(drmm_panel_bridge_add);
+
/**
* drm_panel_bridge_connector - return the connector for the panel bridge
* @bridge: The drm_bridge.
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index d434ab416ad4..e37a419ac2b4 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -930,6 +930,8 @@ struct drm_bridge *devm_drm_panel_bridge_add(struct device
*dev,
struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
struct drm_panel *panel,
u32 connector_type);
+struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm,
+struct drm_panel *panel);
struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge);
#else
static inline bool drm_bridge_is_panel(const struct drm_bridge *bridge)
--
2.36.1
[PATCH v3 07/71] drm/connector: Consolidate Connector Initialization
We're going to add a DRM-managed connector initialization function.
Since we'll need both the with and without the DDC pointer, having a
single function that takes an optional pointer is easier to maintain.
Let's create a static function that will back both existing variants,
and will be reused by the DRM-managed variant.
Suggested-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_connector.c | 65 +
1 file changed, 34 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 327e8cc076ad..d7dab4242290 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -213,27 +213,11 @@ void drm_connector_free_work_fn(struct work_struct *work)
}
}
-/**
- * drm_connector_init - Init a preallocated connector
- * @dev: DRM device
- * @connector: the connector to init
- * @funcs: callbacks for this connector
- * @connector_type: user visible type of the connector
- *
- * Initialises a preallocated connector. Connectors should be
- * subclassed as part of driver connector objects.
- *
- * At driver unload time the driver's &drm_connector_funcs.destroy hook
- * should call drm_connector_cleanup() and free the connector structure.
- * The connector structure should not be allocated with devm_kzalloc().
- *
- * Returns:
- * Zero on success, error code on failure.
- */
-int drm_connector_init(struct drm_device *dev,
- struct drm_connector *connector,
- const struct drm_connector_funcs *funcs,
- int connector_type)
+static int __drm_connector_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ int connector_type,
+ struct i2c_adapter *ddc)
{
struct drm_mode_config *config = &dev->mode_config;
int ret;
@@ -281,6 +265,9 @@ int drm_connector_init(struct drm_device *dev,
goto out_put_type_id;
}
+ /* provide ddc symlink in sysfs */
+ connector->ddc = ddc;
+
INIT_LIST_HEAD(&connector->global_connector_list_entry);
INIT_LIST_HEAD(&connector->probed_modes);
INIT_LIST_HEAD(&connector->modes);
@@ -337,6 +324,31 @@ int drm_connector_init(struct drm_device *dev,
return ret;
}
+
+/**
+ * drm_connector_init - Init a preallocated connector
+ * @dev: DRM device
+ * @connector: the connector to init
+ * @funcs: callbacks for this connector
+ * @connector_type: user visible type of the connector
+ *
+ * Initialises a preallocated connector. Connectors should be
+ * subclassed as part of driver connector objects.
+ *
+ * At driver unload time the driver's &drm_connector_funcs.destroy hook
+ * should call drm_connector_cleanup() and free the connector structure.
+ * The connector structure should not be allocated with devm_kzalloc().
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_connector_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ int connector_type)
+{
+ return __drm_connector_init(dev, connector, funcs, connector_type,
NULL);
+}
EXPORT_SYMBOL(drm_connector_init);
/**
@@ -365,16 +377,7 @@ int drm_connector_init_with_ddc(struct drm_device *dev,
int connector_type,
struct i2c_adapter *ddc)
{
- int ret;
-
- ret = drm_connector_init(dev, connector, funcs, connector_type);
- if (ret)
- return ret;
-
- /* provide ddc symlink in sysfs */
- connector->ddc = ddc;
-
- return ret;
+ return __drm_connector_init(dev, connector, funcs, connector_type, ddc);
}
EXPORT_SYMBOL(drm_connector_init_with_ddc);
--
2.36.1
[PATCH v3 11/71] drm/bridge: panel: Introduce drmm_of_get_bridge
Unlike what can be found for other DRM entities, we don't have a
DRM-managed function equivalent to devm_drm_of_get_bridge().
Let's create it.
Acked-by: Sam Ravnborg
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/panel.c | 35 ++
include/drm/drm_bridge.h | 2 ++
2 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 56ac51b257b9..ec5304e9fb17 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -459,4 +459,39 @@ struct drm_bridge *devm_drm_of_get_bridge(struct device
*dev,
return bridge;
}
EXPORT_SYMBOL(devm_drm_of_get_bridge);
+
+/**
+ * drmm_of_get_bridge - Return next bridge in the chain
+ * @dev: device to tie the bridge lifetime to
+ * @np: device tree node containing encoder output ports
+ * @port: port in the device tree node
+ * @endpoint: endpoint in the device tree node
+ *
+ * Given a DT node's port and endpoint number, finds the connected node
+ * and returns the associated bridge if any, or creates and returns a
+ * drm panel bridge instance if a panel is connected.
+ *
+ * Returns a drmm managed pointer to the bridge if successful, or an error
+ * pointer otherwise.
+ */
+struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
+ struct device_node *np,
+ u32 port, u32 endpoint)
+{
+ struct drm_bridge *bridge;
+ struct drm_panel *panel;
+ int ret;
+
+ ret = drm_of_find_panel_or_bridge(np, port, endpoint,
+ &panel, &bridge);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (panel)
+ bridge = drmm_panel_bridge_add(drm, panel);
+
+ return bridge;
+}
+EXPORT_SYMBOL(drmm_of_get_bridge);
+
#endif
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index e37a419ac2b4..dba5d81e3b4a 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -949,6 +949,8 @@ static inline int drm_panel_bridge_set_orientation(struct
drm_connector *connect
#if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL_BRIDGE)
struct drm_bridge *devm_drm_of_get_bridge(struct device *dev, struct
device_node *node,
u32 port, u32 endpoint);
+struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm, struct
device_node *node,
+ u32 port, u32 endpoint);
#else
static inline struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
struct device_node
*node,
--
2.36.1
[PATCH v3 12/71] drm/vc4: drv: Call component_unbind_all()
While we were using the component framework to deal with all the DRM
subdevices, we were not calling component_unbind_all().
This leads to none of the subdevices freeing up their resources as part of
their unbind() or device managed hooks.
Fixes: c8b75bca92cb ("drm/vc4: Add KMS support for Raspberry Pi.")
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_drv.c | 14 --
drivers/gpu/drm/vc4/vc4_drv.h | 1 +
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 2b014b6332a6..072c65aa1d98 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -267,6 +267,13 @@ static void vc4_match_add_drivers(struct device *dev,
}
}
+static void vc4_component_unbind_all(void *ptr)
+{
+ struct vc4_dev *vc4 = ptr;
+
+ component_unbind_all(vc4->dev, &vc4->base);
+}
+
const struct of_device_id vc4_dma_range_matches[] = {
{ .compatible = "brcm,bcm2711-hvs" },
{ .compatible = "brcm,bcm2835-hvs" },
@@ -310,6 +317,7 @@ static int vc4_drm_bind(struct device *dev)
if (IS_ERR(vc4))
return PTR_ERR(vc4);
vc4->is_vc5 = is_vc5;
+ vc4->dev = dev;
drm = &vc4->base;
platform_set_drvdata(pdev, drm);
@@ -360,6 +368,10 @@ static int vc4_drm_bind(struct device *dev)
if (ret)
return ret;
+ ret = devm_add_action_or_reset(dev, vc4_component_unbind_all, vc4);
+ if (ret)
+ return ret;
+
ret = vc4_plane_create_additional_planes(drm);
if (ret)
goto unbind_all;
@@ -380,8 +392,6 @@ static int vc4_drm_bind(struct device *dev)
return 0;
unbind_all:
- component_unbind_all(dev, drm);
-
return ret;
}
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 93fd55b9e99e..c48a73914200 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -75,6 +75,7 @@ struct vc4_perfmon {
struct vc4_dev {
struct drm_device base;
+ struct device *dev;
bool is_vc5;
--
2.36.1
[PATCH v3 13/71] drm/vc4: drv: Use drm_dev_unplug
When our KMS driver is unbound, the device is no longer there but we might
still have users with an opened fd to the KMS device.
To avoid any issue in such a situation, every device access needs to be
protected by calls to drm_dev_enter() and drm_dev_exit(), and the driver
needs to call drm_dev_unplug().
We'll add calls to drm_dev_enter()/drm_dev_exit() in subsequent patches
changing the relevant drivers, but let's start by calling drm_dev_unplug().
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 072c65aa1d98..942b5442b843 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -399,8 +399,7 @@ static void vc4_drm_unbind(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
- drm_dev_unregister(drm);
-
+ drm_dev_unplug(drm);
drm_atomic_helper_shutdown(drm);
}
--
2.36.1
[PATCH v3 15/71] drm/vc4: hvs: Protect device resources after removal
Whenever the device and driver are unbound, the main device and all the
subdevices will be removed by calling their unbind() method.
However, the DRM device itself will only be freed when the last user will
have closed it.
It means that there is a time window where the device and its resources
aren't there anymore, but the userspace can still call into our driver.
Fortunately, the DRM framework provides the drm_dev_enter() and
drm_dev_exit() functions to make sure our underlying device is still there
for the section protected by those calls. Let's add them to the HVS driver.
Reviewed-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hvs.c | 99 ---
1 file changed, 93 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index fbaa741dda5f..f2d6e62e7585 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -25,6 +25,7 @@
#include
#include
+#include
#include
#include "vc4_drv.h"
@@ -66,11 +67,15 @@ static const struct debugfs_reg32 hvs_regs[] = {
void vc4_hvs_dump_state(struct vc4_hvs *hvs)
{
+ struct drm_device *drm = &hvs->vc4->base;
struct drm_printer p = drm_info_printer(&hvs->pdev->dev);
- int i;
+ int idx, i;
drm_print_regset32(&p, &hvs->regset);
+ if (!drm_dev_enter(drm, &idx))
+ return;
+
DRM_INFO("HVS ctx:\n");
for (i = 0; i < 64; i += 4) {
DRM_INFO("0x%08x (%s): 0x%08x 0x%08x 0x%08x 0x%08x\n",
@@ -80,6 +85,8 @@ void vc4_hvs_dump_state(struct vc4_hvs *hvs)
readl((u32 __iomem *)hvs->dlist + i + 2),
readl((u32 __iomem *)hvs->dlist + i + 3));
}
+
+ drm_dev_exit(idx);
}
static int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data)
@@ -175,6 +182,11 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs
*hvs,
int ret, i;
u32 __iomem *dst_kernel;
+ /*
+* NOTE: We don't need a call to drm_dev_enter()/drm_dev_exit()
+* here since that function is only called from vc4_hvs_bind().
+*/
+
ret = drm_mm_insert_node(&hvs->dlist_mm, space, VC4_KERNEL_DWORDS);
if (ret) {
DRM_ERROR("Failed to allocate space for filter kernel: %d\n",
@@ -199,10 +211,15 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs
*hvs,
static void vc4_hvs_lut_load(struct vc4_hvs *hvs,
struct vc4_crtc *vc4_crtc)
{
+ struct drm_device *drm = &hvs->vc4->base;
struct drm_crtc *crtc = &vc4_crtc->base;
struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
+ int idx;
u32 i;
+ if (!drm_dev_enter(drm, &idx))
+ return;
+
/* The LUT memory is laid out with each HVS channel in order,
* each of which takes 256 writes for R, 256 for G, then 256
* for B.
@@ -217,6 +234,8 @@ static void vc4_hvs_lut_load(struct vc4_hvs *hvs,
HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_g[i]);
for (i = 0; i < crtc->gamma_size; i++)
HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_b[i]);
+
+ drm_dev_exit(idx);
}
static void vc4_hvs_update_gamma_lut(struct vc4_hvs *hvs,
@@ -238,7 +257,12 @@ static void vc4_hvs_update_gamma_lut(struct vc4_hvs *hvs,
u8 vc4_hvs_get_fifo_frame_count(struct vc4_hvs *hvs, unsigned int fifo)
{
+ struct drm_device *drm = &hvs->vc4->base;
u8 field = 0;
+ int idx;
+
+ if (!drm_dev_enter(drm, &idx))
+ return 0;
switch (fifo) {
case 0:
@@ -255,6 +279,7 @@ u8 vc4_hvs_get_fifo_frame_count(struct vc4_hvs *hvs,
unsigned int fifo)
break;
}
+ drm_dev_exit(idx);
return field;
}
@@ -267,6 +292,12 @@ int vc4_hvs_get_fifo_from_output(struct vc4_hvs *hvs,
unsigned int output)
if (!vc4->is_vc5)
return output;
+ /*
+* NOTE: We should probably use drm_dev_enter()/drm_dev_exit()
+* here, but this function is only used during the DRM device
+* initialization, so we should be fine.
+*/
+
switch (output) {
case 0:
return 0;
@@ -315,12 +346,17 @@ static int vc4_hvs_init_channel(struct vc4_hvs *hvs,
struct drm_crtc *crtc,
struct drm_display_mode *mode, bool oneshot)
{
struct vc4_dev *vc4 = hvs->vc4;
+ struct drm_device *drm = &vc4->base;
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
struct vc4_crtc_state *vc4_crtc_state = to_vc4_crtc_state(crtc->state);
unsigned int chan = vc4_crtc_state->assigned_channel;
bool interlace = mode->flags & DRM_MODE_FLAG_INTERLACE;
u32 dispbkgndx;
u32 dispctrl;
+ int idx;
+
+ if (!drm_dev_enter(drm, &idx))
+ return -ENODEV;
HVS_WRITE(SCALER_DISPCTR
[PATCH v3 16/71] drm/vc4: hvs: Remove planes currently allocated before taking down
When the HVS driver is unbound, a lot of memory allocations in the LBM and DLIST RAM are still assigned to planes that are still allocated. Thus, we hit a warning when calling drm_mm_takedown() since the memory pool is not completely free of allocations. Let's free all the currently live entries before calling drm_mm_takedown(). Reviewed-by: Dave Stevenson Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hvs.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index f2d6e62e7585..a62f55ce 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -873,11 +873,18 @@ static void vc4_hvs_unbind(struct device *dev, struct device *master, struct drm_device *drm = dev_get_drvdata(master); struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_hvs *hvs = vc4->hvs; + struct drm_mm_node *node, *next; if (drm_mm_node_allocated(&vc4->hvs->mitchell_netravali_filter)) drm_mm_remove_node(&vc4->hvs->mitchell_netravali_filter); + drm_mm_for_each_node_safe(node, next, &vc4->hvs->dlist_mm) + drm_mm_remove_node(node); + drm_mm_takedown(&vc4->hvs->dlist_mm); + + drm_mm_for_each_node_safe(node, next, &vc4->hvs->lbm_mm) + drm_mm_remove_node(node); drm_mm_takedown(&vc4->hvs->lbm_mm); clk_disable_unprepare(hvs->core_clk); -- 2.36.1
[PATCH v3 14/71] drm/vc4: crtc: Create vblank reporting function
We'll need that code in the HVS driver, so let's create a shared function
to reuse it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 23 +++
drivers/gpu/drm/vc4/vc4_drv.h | 1 +
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 029be98660b3..faad9e564772 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -544,6 +544,20 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
return 0;
}
+void vc4_crtc_send_vblank(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ unsigned long flags;
+
+ if (!crtc->state || !crtc->state->event)
+ return;
+
+ spin_lock_irqsave(&dev->event_lock, flags);
+ drm_crtc_send_vblank_event(crtc, crtc->state->event);
+ crtc->state->event = NULL;
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+}
+
static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
@@ -567,14 +581,7 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
* Make sure we issue a vblank event after disabling the CRTC if
* someone was waiting it.
*/
- if (crtc->state->event) {
- unsigned long flags;
-
- spin_lock_irqsave(&dev->event_lock, flags);
- drm_crtc_send_vblank_event(crtc, crtc->state->event);
- crtc->state->event = NULL;
- spin_unlock_irqrestore(&dev->event_lock, flags);
- }
+ vc4_crtc_send_vblank(crtc);
}
static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index c48a73914200..da52d1e0c7b5 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -861,6 +861,7 @@ void vc4_crtc_destroy_state(struct drm_crtc *crtc,
struct drm_crtc_state *state);
void vc4_crtc_reset(struct drm_crtc *crtc);
void vc4_crtc_handle_vblank(struct vc4_crtc *crtc);
+void vc4_crtc_send_vblank(struct drm_crtc *crtc);
void vc4_crtc_get_margins(struct drm_crtc_state *state,
unsigned int *left, unsigned int *right,
unsigned int *top, unsigned int *bottom);
--
2.36.1
[PATCH v3 08/71] drm/connector: Check for destroy implementation
Connectors need to be cleaned up with a call to drm_connector_cleanup()
in their drm_connector_funcs.destroy implementation.
Let's check for this and complain if there's no such function.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_connector.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index d7dab4242290..855267116e68 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -347,6 +347,9 @@ int drm_connector_init(struct drm_device *dev,
const struct drm_connector_funcs *funcs,
int connector_type)
{
+ if (WARN_ON(!(funcs && funcs->destroy)))
+ return -EINVAL;
+
return __drm_connector_init(dev, connector, funcs, connector_type,
NULL);
}
EXPORT_SYMBOL(drm_connector_init);
@@ -377,6 +380,9 @@ int drm_connector_init_with_ddc(struct drm_device *dev,
int connector_type,
struct i2c_adapter *ddc)
{
+ if (WARN_ON(!(funcs && funcs->destroy)))
+ return -EINVAL;
+
return __drm_connector_init(dev, connector, funcs, connector_type, ddc);
}
EXPORT_SYMBOL(drm_connector_init_with_ddc);
--
2.36.1
[PATCH v3 17/71] drm/vc4: plane: Take possible_crtcs as an argument
vc4_plane_init() currently initialises the plane with no possible CRTCs,
and will expect the caller to set it up by itself.
Let's change that logic a bit to follow the syntax of
drm_universal_plane_init() and pass the possible CRTCs bitmask as an
argument to the function instead.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 2 +-
drivers/gpu/drm/vc4/vc4_drv.h | 3 ++-
drivers/gpu/drm/vc4/vc4_plane.c | 15 +++
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index faad9e564772..d391e894ee6c 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1244,7 +1244,7 @@ int vc4_crtc_init(struct drm_device *drm, struct vc4_crtc
*vc4_crtc,
* requirement of the plane configuration, and reject ones
* that will take too much.
*/
- primary_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_PRIMARY);
+ primary_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_PRIMARY, 0);
if (IS_ERR(primary_plane)) {
dev_err(drm->dev, "failed to construct primary plane\n");
return PTR_ERR(primary_plane);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index da52d1e0c7b5..91983fbd4d83 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -959,7 +959,8 @@ int vc4_kms_load(struct drm_device *dev);
/* vc4_plane.c */
struct drm_plane *vc4_plane_init(struct drm_device *dev,
-enum drm_plane_type type);
+enum drm_plane_type type,
+uint32_t possible_crtcs);
int vc4_plane_create_additional_planes(struct drm_device *dev);
u32 vc4_plane_write_dlist(struct drm_plane *plane, u32 __iomem *dlist);
u32 vc4_plane_dlist_size(const struct drm_plane_state *state);
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index f27e87a23df7..a344762d86eb 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -1492,7 +1492,8 @@ static const struct drm_plane_funcs vc4_plane_funcs = {
};
struct drm_plane *vc4_plane_init(struct drm_device *dev,
-enum drm_plane_type type)
+enum drm_plane_type type,
+uint32_t possible_crtcs)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);
struct drm_plane *plane = NULL;
@@ -1523,7 +1524,7 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
}
plane = &vc4_plane->base;
- ret = drm_universal_plane_init(dev, plane, 0,
+ ret = drm_universal_plane_init(dev, plane, possible_crtcs,
&vc4_plane_funcs,
formats, num_formats,
modifiers, type, NULL);
@@ -1575,13 +1576,11 @@ int vc4_plane_create_additional_planes(struct
drm_device *drm)
*/
for (i = 0; i < 16; i++) {
struct drm_plane *plane =
- vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY);
+ vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY,
+ GENMASK(drm->mode_config.num_crtc - 1,
0));
if (IS_ERR(plane))
continue;
-
- plane->possible_crtcs =
- GENMASK(drm->mode_config.num_crtc - 1, 0);
}
drm_for_each_crtc(crtc, drm) {
@@ -1589,9 +1588,9 @@ int vc4_plane_create_additional_planes(struct drm_device
*drm)
* since we overlay planes on the CRTC in the order they were
* initialized.
*/
- cursor_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_CURSOR);
+ cursor_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_CURSOR,
+ drm_crtc_mask(crtc));
if (!IS_ERR(cursor_plane)) {
- cursor_plane->possible_crtcs = drm_crtc_mask(crtc);
crtc->cursor = cursor_plane;
}
}
--
2.36.1
[PATCH v3 20/71] drm/vc4: crtc: Move debugfs_name to crtc_data
All the CRTCs, including the TXP, have a debugfs file and name so we can
consolidate it into vc4_crtc_data.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 18 +-
drivers/gpu/drm/vc4/vc4_drv.h | 4 ++--
drivers/gpu/drm/vc4/vc4_txp.c | 1 +
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index d7cc006b22c9..bcce61879d53 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1084,10 +1084,10 @@ static const struct drm_crtc_helper_funcs
vc4_crtc_helper_funcs = {
static const struct vc4_pv_data bcm2835_pv0_data = {
.base = {
+ .debugfs_name = "crtc0_regs",
.hvs_available_channels = BIT(0),
.hvs_output = 0,
},
- .debugfs_name = "crtc0_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1098,10 +1098,10 @@ static const struct vc4_pv_data bcm2835_pv0_data = {
static const struct vc4_pv_data bcm2835_pv1_data = {
.base = {
+ .debugfs_name = "crtc1_regs",
.hvs_available_channels = BIT(2),
.hvs_output = 2,
},
- .debugfs_name = "crtc1_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1112,10 +1112,10 @@ static const struct vc4_pv_data bcm2835_pv1_data = {
static const struct vc4_pv_data bcm2835_pv2_data = {
.base = {
+ .debugfs_name = "crtc2_regs",
.hvs_available_channels = BIT(1),
.hvs_output = 1,
},
- .debugfs_name = "crtc2_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1126,10 +1126,10 @@ static const struct vc4_pv_data bcm2835_pv2_data = {
static const struct vc4_pv_data bcm2711_pv0_data = {
.base = {
+ .debugfs_name = "crtc0_regs",
.hvs_available_channels = BIT(0),
.hvs_output = 0,
},
- .debugfs_name = "crtc0_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1140,10 +1140,10 @@ static const struct vc4_pv_data bcm2711_pv0_data = {
static const struct vc4_pv_data bcm2711_pv1_data = {
.base = {
+ .debugfs_name = "crtc1_regs",
.hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
.hvs_output = 3,
},
- .debugfs_name = "crtc1_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1154,10 +1154,10 @@ static const struct vc4_pv_data bcm2711_pv1_data = {
static const struct vc4_pv_data bcm2711_pv2_data = {
.base = {
+ .debugfs_name = "crtc2_regs",
.hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
.hvs_output = 4,
},
- .debugfs_name = "crtc2_regs",
.fifo_depth = 256,
.pixels_per_clock = 2,
.encoder_types = {
@@ -1167,10 +1167,10 @@ static const struct vc4_pv_data bcm2711_pv2_data = {
static const struct vc4_pv_data bcm2711_pv3_data = {
.base = {
+ .debugfs_name = "crtc3_regs",
.hvs_available_channels = BIT(1),
.hvs_output = 1,
},
- .debugfs_name = "crtc3_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1180,10 +1180,10 @@ static const struct vc4_pv_data bcm2711_pv3_data = {
static const struct vc4_pv_data bcm2711_pv4_data = {
.base = {
+ .debugfs_name = "crtc4_regs",
.hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
.hvs_output = 5,
},
- .debugfs_name = "crtc4_regs",
.fifo_depth = 64,
.pixels_per_clock = 2,
.encoder_types = {
@@ -1320,7 +1320,7 @@ static int vc4_crtc_bind(struct device *dev, struct
device *master, void *data)
platform_set_drvdata(pdev, vc4_crtc);
- vc4_debugfs_add_regset32(drm, pv_data->debugfs_name,
+ vc4_debugfs_add_regset32(drm, pv_data->base.debugfs_name,
&vc4_crtc->regset);
return 0;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 91983fbd4d83..df4ff6747eeb 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -458,6 +458,8 @@ to_vc4_encoder(struct drm_encoder *encoder)
}
struct vc4_crtc_data {
+ const char *debugfs_name;
+
/* Bitmask of channels (FIFOs) of the HVS that the output can source
from */
unsigned int hvs_available_channels;
@@ -475,8 +477,6 @@ struct vc4_pv_data {
u8 pixels_per_clock;
enum vc4_encoder_type encoder_types[4];
- const char *debugfs_name;
-
};
struct vc4_crtc {
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index d20b0bc
[PATCH v3 21/71] drm/vc4: crtc: Switch to drmm_kzalloc
Our internal structure that stores the DRM entities structure is allocated through a device-managed kzalloc. This means that this will eventually be freed whenever the device is removed. In our case, the most likely source of removal is that the main device is going to be unbound, and component_unbind_all() is being run. However, it occurs while the DRM device is still registered, which will create dangling pointers, eventually resulting in use-after-free. Switch to a DRM-managed allocation to keep our structure until the DRM driver doesn't need it anymore. Reviewed-by: Dave Stevenson Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index bcce61879d53..d5158a6c0f21 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -1284,7 +1284,7 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) struct drm_crtc *crtc; int ret; - vc4_crtc = devm_kzalloc(dev, sizeof(*vc4_crtc), GFP_KERNEL); + vc4_crtc = drmm_kzalloc(drm, sizeof(*vc4_crtc), GFP_KERNEL); if (!vc4_crtc) return -ENOMEM; crtc = &vc4_crtc->base; -- 2.36.1
[PATCH v3 19/71] drm/vc4: plane: Switch to drmm_universal_plane_alloc()
Let's switch to drmm_universal_plane_alloc() for our plane allocation and
initialisation to make the driver a bit simpler.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 1 -
drivers/gpu/drm/vc4/vc4_plane.c | 23 ---
2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 03a799ba9ee8..d7cc006b22c9 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1282,7 +1282,6 @@ static int vc4_crtc_bind(struct device *dev, struct
device *master, void *data)
const struct vc4_pv_data *pv_data;
struct vc4_crtc *vc4_crtc;
struct drm_crtc *crtc;
- struct drm_plane *destroy_plane, *temp;
int ret;
vc4_crtc = devm_kzalloc(dev, sizeof(*vc4_crtc), GFP_KERNEL);
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index a344762d86eb..82a650268f19 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -1483,8 +1483,6 @@ static bool vc4_format_mod_supported(struct drm_plane
*plane,
static const struct drm_plane_funcs vc4_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
- .destroy = drm_plane_cleanup,
- .set_property = NULL,
.reset = vc4_plane_reset,
.atomic_duplicate_state = vc4_plane_duplicate_state,
.atomic_destroy_state = vc4_plane_destroy_state,
@@ -1496,11 +1494,10 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
uint32_t possible_crtcs)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);
- struct drm_plane *plane = NULL;
+ struct drm_plane *plane;
struct vc4_plane *vc4_plane;
u32 formats[ARRAY_SIZE(hvs_formats)];
int num_formats = 0;
- int ret = 0;
unsigned i;
static const uint64_t modifiers[] = {
DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED,
@@ -1511,11 +1508,6 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
DRM_FORMAT_MOD_INVALID
};
- vc4_plane = devm_kzalloc(dev->dev, sizeof(*vc4_plane),
-GFP_KERNEL);
- if (!vc4_plane)
- return ERR_PTR(-ENOMEM);
-
for (i = 0; i < ARRAY_SIZE(hvs_formats); i++) {
if (!hvs_formats[i].hvs5_only || vc4->is_vc5) {
formats[num_formats] = hvs_formats[i].drm;
@@ -1523,13 +1515,14 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
}
}
+ vc4_plane = drmm_universal_plane_alloc(dev, struct vc4_plane, base,
+ possible_crtcs,
+ &vc4_plane_funcs,
+ formats, num_formats,
+ modifiers, type, NULL);
+ if (IS_ERR(vc4_plane))
+ return ERR_CAST(vc4_plane);
plane = &vc4_plane->base;
- ret = drm_universal_plane_init(dev, plane, possible_crtcs,
- &vc4_plane_funcs,
- formats, num_formats,
- modifiers, type, NULL);
- if (ret)
- return ERR_PTR(ret);
if (vc4->is_vc5)
drm_plane_helper_add(plane, &vc5_plane_helper_funcs);
--
2.36.1
[PATCH v3 22/71] drm/vc4: crtc: Switch to DRM-managed CRTC initialization
The current code will call drm_crtc_cleanup() when the device is
unbound. However, by then, there might still be some references held to
that CRTC, including by the userspace that might still have the DRM
device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 16 ++--
drivers/gpu/drm/vc4/vc4_drv.h | 1 -
drivers/gpu/drm/vc4/vc4_txp.c | 1 -
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index d5158a6c0f21..3768a2a57ca9 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -206,11 +206,6 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
*crtc,
return ret;
}
-void vc4_crtc_destroy(struct drm_crtc *crtc)
-{
- drm_crtc_cleanup(crtc);
-}
-
static u32 vc4_get_fifo_full_level(struct vc4_crtc *vc4_crtc, u32 format)
{
const struct vc4_crtc_data *crtc_data =
vc4_crtc_to_vc4_crtc_data(vc4_crtc);
@@ -1059,7 +1054,6 @@ void vc4_crtc_reset(struct drm_crtc *crtc)
static const struct drm_crtc_funcs vc4_crtc_funcs = {
.set_config = drm_atomic_helper_set_config,
- .destroy = vc4_crtc_destroy,
.page_flip = vc4_page_flip,
.set_property = NULL,
.cursor_set = NULL, /* handled by drm_mode_cursor_universal */
@@ -1237,6 +1231,7 @@ int vc4_crtc_init(struct drm_device *drm, struct vc4_crtc
*vc4_crtc,
struct drm_crtc *crtc = &vc4_crtc->base;
struct drm_plane *primary_plane;
unsigned int i;
+ int ret;
/* For now, we create just the primary and the legacy cursor
* planes. We should be able to stack more planes on easily,
@@ -1251,8 +1246,11 @@ int vc4_crtc_init(struct drm_device *drm, struct
vc4_crtc *vc4_crtc,
}
spin_lock_init(&vc4_crtc->irq_lock);
- drm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
- crtc_funcs, NULL);
+ ret = drmm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
+crtc_funcs, NULL);
+ if (ret)
+ return ret;
+
drm_crtc_helper_add(crtc, crtc_helper_funcs);
if (!vc4->is_vc5) {
@@ -1332,8 +1330,6 @@ static void vc4_crtc_unbind(struct device *dev, struct
device *master,
struct platform_device *pdev = to_platform_device(dev);
struct vc4_crtc *vc4_crtc = dev_get_drvdata(dev);
- vc4_crtc_destroy(&vc4_crtc->base);
-
CRTC_WRITE(PV_INTEN, 0);
platform_set_drvdata(pdev, NULL);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index df4ff6747eeb..6afa873f0def 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -850,7 +850,6 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc);
int vc4_crtc_init(struct drm_device *drm, struct vc4_crtc *vc4_crtc,
const struct drm_crtc_funcs *crtc_funcs,
const struct drm_crtc_helper_funcs *crtc_helper_funcs);
-void vc4_crtc_destroy(struct drm_crtc *crtc);
int vc4_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_pending_vblank_event *event,
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index 20e08e31aa1b..448d48e7e99f 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -384,7 +384,6 @@ static void vc4_txp_disable_vblank(struct drm_crtc *crtc) {}
static const struct drm_crtc_funcs vc4_txp_crtc_funcs = {
.set_config = drm_atomic_helper_set_config,
- .destroy= vc4_crtc_destroy,
.page_flip = vc4_page_flip,
.reset = vc4_crtc_reset,
.atomic_duplicate_state = vc4_crtc_duplicate_state,
--
2.36.1
[PATCH v3 18/71] drm/vc4: crtc: Remove manual plane removal on error
When vc4_crtc_bind() fails after vc4_crtc_init() has been called, we have
a loop undoing the plane creation and calling destroy on each plane
registered and matching the possible_crtcs mask.
However, this is redundant with what drm_mode_config_cleanup() is doing, so
let's remove it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index d391e894ee6c..03a799ba9ee8 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1317,7 +1317,7 @@ static int vc4_crtc_bind(struct device *dev, struct
device *master, void *data)
IRQF_SHARED,
"vc4 crtc", vc4_crtc);
if (ret)
- goto err_destroy_planes;
+ return ret;
platform_set_drvdata(pdev, vc4_crtc);
@@ -1325,15 +1325,6 @@ static int vc4_crtc_bind(struct device *dev, struct
device *master, void *data)
&vc4_crtc->regset);
return 0;
-
-err_destroy_planes:
- list_for_each_entry_safe(destroy_plane, temp,
-&drm->mode_config.plane_list, head) {
- if (destroy_plane->possible_crtcs == drm_crtc_mask(crtc))
- destroy_plane->funcs->destroy(destroy_plane);
- }
-
- return ret;
}
static void vc4_crtc_unbind(struct device *dev, struct device *master,
--
2.36.1
[PATCH v3 24/71] drm/vc4: dpi: Embed DRM structures into the private structure
The VC4 DPI driver private structure contains only a pointer to the
encoder it implements. This makes the overall structure somewhat
inconsistent with the rest of the driver, and complicates its
initialisation without any apparent gain.
Let's embed the drm_encoder structure (through the vc4_encoder one) into
struct vc4_dpi to fix both issues.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dpi.c | 49 ---
1 file changed, 16 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 52348e1e1ee7..7d4bb74cd500 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -83,10 +83,10 @@
/* General DPI hardware state. */
struct vc4_dpi {
+ struct vc4_encoder encoder;
+
struct platform_device *pdev;
- struct drm_encoder *encoder;
-
void __iomem *regs;
struct clk *pixel_clock;
@@ -95,21 +95,15 @@ struct vc4_dpi {
struct debugfs_regset32 regset;
};
+static inline struct vc4_dpi *
+to_vc4_dpi(struct drm_encoder *encoder)
+{
+ return container_of(encoder, struct vc4_dpi, encoder.base);
+}
+
#define DPI_READ(offset) readl(dpi->regs + (offset))
#define DPI_WRITE(offset, val) writel(val, dpi->regs + (offset))
-/* VC4 DPI encoder KMS struct */
-struct vc4_dpi_encoder {
- struct vc4_encoder base;
- struct vc4_dpi *dpi;
-};
-
-static inline struct vc4_dpi_encoder *
-to_vc4_dpi_encoder(struct drm_encoder *encoder)
-{
- return container_of(encoder, struct vc4_dpi_encoder, base.base);
-}
-
static const struct debugfs_reg32 dpi_regs[] = {
VC4_REG32(DPI_C),
VC4_REG32(DPI_ID),
@@ -117,8 +111,7 @@ static const struct debugfs_reg32 dpi_regs[] = {
static void vc4_dpi_encoder_disable(struct drm_encoder *encoder)
{
- struct vc4_dpi_encoder *vc4_encoder = to_vc4_dpi_encoder(encoder);
- struct vc4_dpi *dpi = vc4_encoder->dpi;
+ struct vc4_dpi *dpi = to_vc4_dpi(encoder);
clk_disable_unprepare(dpi->pixel_clock);
}
@@ -127,8 +120,7 @@ static void vc4_dpi_encoder_enable(struct drm_encoder
*encoder)
{
struct drm_device *dev = encoder->dev;
struct drm_display_mode *mode = &encoder->crtc->mode;
- struct vc4_dpi_encoder *vc4_encoder = to_vc4_dpi_encoder(encoder);
- struct vc4_dpi *dpi = vc4_encoder->dpi;
+ struct vc4_dpi *dpi = to_vc4_dpi(encoder);
struct drm_connector_list_iter conn_iter;
struct drm_connector *connector = NULL, *connector_scan;
u32 dpi_c = DPI_ENABLE;
@@ -261,7 +253,7 @@ static int vc4_dpi_init_bridge(struct vc4_dpi *dpi)
return PTR_ERR(bridge);
}
- return drm_bridge_attach(dpi->encoder, bridge, NULL, 0);
+ return drm_bridge_attach(&dpi->encoder.base, bridge, NULL, 0);
}
static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
@@ -269,21 +261,12 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
struct platform_device *pdev = to_platform_device(dev);
struct drm_device *drm = dev_get_drvdata(master);
struct vc4_dpi *dpi;
- struct vc4_dpi_encoder *vc4_dpi_encoder;
int ret;
dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL);
if (!dpi)
return -ENOMEM;
-
- vc4_dpi_encoder = devm_kzalloc(dev, sizeof(*vc4_dpi_encoder),
- GFP_KERNEL);
- if (!vc4_dpi_encoder)
- return -ENOMEM;
- vc4_dpi_encoder->base.type = VC4_ENCODER_TYPE_DPI;
- vc4_dpi_encoder->dpi = dpi;
- dpi->encoder = &vc4_dpi_encoder->base.base;
-
+ dpi->encoder.type = VC4_ENCODER_TYPE_DPI;
dpi->pdev = pdev;
dpi->regs = vc4_ioremap_regs(pdev, 0);
if (IS_ERR(dpi->regs))
@@ -317,8 +300,8 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
if (ret)
DRM_ERROR("Failed to turn on core clock: %d\n", ret);
- drm_simple_encoder_init(drm, dpi->encoder, DRM_MODE_ENCODER_DPI);
- drm_encoder_helper_add(dpi->encoder, &vc4_dpi_encoder_helper_funcs);
+ drm_simple_encoder_init(drm, &dpi->encoder.base, DRM_MODE_ENCODER_DPI);
+ drm_encoder_helper_add(&dpi->encoder.base,
&vc4_dpi_encoder_helper_funcs);
ret = vc4_dpi_init_bridge(dpi);
if (ret)
@@ -331,7 +314,7 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
return 0;
err_destroy_encoder:
- drm_encoder_cleanup(dpi->encoder);
+ drm_encoder_cleanup(&dpi->encoder.base);
clk_disable_unprepare(dpi->core_clock);
return ret;
}
@@ -343,7 +326,7 @@ static void vc4_dpi_unbind(struct device *dev, struct
device *master,
drm_of_panel_bridge_remove(dev->of_node, 0, 0);
- drm_encoder_cleanup(dpi->encoder);
+ drm_encoder_cleanup(&dpi->encoder.base);
[PATCH v3 27/71] drm/vc4: dpi: Remove unnecessary drm_of_panel_bridge_remove call
Since we have a managed call to create our panel_bridge instance, the call
to drm_of_panel_bridge_remove() at unbind is both redundant and dangerous
since it might lead to a use-after-free.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dpi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 2febfe3d854d..22e388d17508 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -328,8 +328,6 @@ static void vc4_dpi_unbind(struct device *dev, struct
device *master,
{
struct vc4_dpi *dpi = dev_get_drvdata(dev);
- drm_of_panel_bridge_remove(dev->of_node, 0, 0);
-
drm_encoder_cleanup(&dpi->encoder.base);
clk_disable_unprepare(dpi->core_clock);
--
2.36.1
[PATCH v3 23/71] drm/vc4: dpi: Remove vc4_dev dpi pointer
There's no user for that pointer so let's just get rid of it.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dpi.c | 7 ---
drivers/gpu/drm/vc4/vc4_drv.h | 1 -
2 files changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 44355b347ff2..52348e1e1ee7 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -268,7 +268,6 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
{
struct platform_device *pdev = to_platform_device(dev);
struct drm_device *drm = dev_get_drvdata(master);
- struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_dpi *dpi;
struct vc4_dpi_encoder *vc4_dpi_encoder;
int ret;
@@ -327,8 +326,6 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
dev_set_drvdata(dev, dpi);
- vc4->dpi = dpi;
-
vc4_debugfs_add_regset32(drm, "dpi_regs", &dpi->regset);
return 0;
@@ -342,8 +339,6 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
static void vc4_dpi_unbind(struct device *dev, struct device *master,
void *data)
{
- struct drm_device *drm = dev_get_drvdata(master);
- struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_dpi *dpi = dev_get_drvdata(dev);
drm_of_panel_bridge_remove(dev->of_node, 0, 0);
@@ -351,8 +346,6 @@ static void vc4_dpi_unbind(struct device *dev, struct
device *master,
drm_encoder_cleanup(dpi->encoder);
clk_disable_unprepare(dpi->core_clock);
-
- vc4->dpi = NULL;
}
static const struct component_ops vc4_dpi_ops = {
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 6afa873f0def..db51dd3e20b8 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -83,7 +83,6 @@ struct vc4_dev {
struct vc4_hvs *hvs;
struct vc4_v3d *v3d;
- struct vc4_dpi *dpi;
struct vc4_vec *vec;
struct vc4_txp *txp;
--
2.36.1
[PATCH v3 30/71] drm/vc4: dpi: Switch to drmm_of_get_bridge
The current code uses a device-managed function to retrieve the next bridge
downstream.
However, that means that it will be removed at unbind time, where the DRM
device is still very much live and might still have some applications that
still have it open.
Switch to a DRM-managed variant to clean everything up once the DRM device
has been last closed.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dpi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 6e0d8da4ae1e..b4059820c9f5 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -239,10 +239,11 @@ static const struct of_device_id vc4_dpi_dt_match[] = {
*/
static int vc4_dpi_init_bridge(struct vc4_dpi *dpi)
{
+ struct drm_device *drm = dpi->encoder.base.dev;
struct device *dev = &dpi->pdev->dev;
struct drm_bridge *bridge;
- bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
+ bridge = drmm_of_get_bridge(drm, dev->of_node, 0, 0);
if (IS_ERR(bridge)) {
/* If nothing was connected in the DT, that's not an
* error.
--
2.36.1
[PATCH v3 28/71] drm/vc4: dpi: Add action to disable the clock
The DPI controller has two clocks called core and pixel, the core clock
being enabled at bind time.
Adding a device-managed action will make the error path easier, so let's
create one to disable it.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dpi.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 22e388d17508..9c5629e9e446 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -256,6 +256,13 @@ static int vc4_dpi_init_bridge(struct vc4_dpi *dpi)
return drm_bridge_attach(&dpi->encoder.base, bridge, NULL, 0);
}
+static void vc4_dpi_disable_clock(void *ptr)
+{
+ struct vc4_dpi *dpi = ptr;
+
+ clk_disable_unprepare(dpi->core_clock);
+}
+
static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
{
struct platform_device *pdev = to_platform_device(dev);
@@ -304,6 +311,10 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
return ret;
}
+ ret = devm_add_action_or_reset(dev, vc4_dpi_disable_clock, dpi);
+ if (ret)
+ return ret;
+
drm_simple_encoder_init(drm, &dpi->encoder.base, DRM_MODE_ENCODER_DPI);
drm_encoder_helper_add(&dpi->encoder.base,
&vc4_dpi_encoder_helper_funcs);
@@ -319,7 +330,6 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
err_destroy_encoder:
drm_encoder_cleanup(&dpi->encoder.base);
- clk_disable_unprepare(dpi->core_clock);
return ret;
}
@@ -329,8 +339,6 @@ static void vc4_dpi_unbind(struct device *dev, struct
device *master,
struct vc4_dpi *dpi = dev_get_drvdata(dev);
drm_encoder_cleanup(&dpi->encoder.base);
-
- clk_disable_unprepare(dpi->core_clock);
}
static const struct component_ops vc4_dpi_ops = {
--
2.36.1
[PATCH v3 29/71] drm/vc4: dpi: Switch to DRM-managed encoder initialization
The current code will call drm_encoder_cleanup() when the device is
unbound. However, by then, there might still be some references held to
that encoder, including by the userspace that might still have the DRM
device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dpi.c | 23 ---
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 9c5629e9e446..6e0d8da4ae1e 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -315,35 +315,28 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
if (ret)
return ret;
- drm_simple_encoder_init(drm, &dpi->encoder.base, DRM_MODE_ENCODER_DPI);
+ ret = drmm_encoder_init(drm, &dpi->encoder.base,
+ NULL,
+ DRM_MODE_ENCODER_DPI,
+ NULL);
+ if (ret)
+ return ret;
+
drm_encoder_helper_add(&dpi->encoder.base,
&vc4_dpi_encoder_helper_funcs);
ret = vc4_dpi_init_bridge(dpi);
if (ret)
- goto err_destroy_encoder;
+ return ret;
dev_set_drvdata(dev, dpi);
vc4_debugfs_add_regset32(drm, "dpi_regs", &dpi->regset);
return 0;
-
-err_destroy_encoder:
- drm_encoder_cleanup(&dpi->encoder.base);
- return ret;
-}
-
-static void vc4_dpi_unbind(struct device *dev, struct device *master,
- void *data)
-{
- struct vc4_dpi *dpi = dev_get_drvdata(dev);
-
- drm_encoder_cleanup(&dpi->encoder.base);
}
static const struct component_ops vc4_dpi_ops = {
.bind = vc4_dpi_bind,
- .unbind = vc4_dpi_unbind,
};
static int vc4_dpi_dev_probe(struct platform_device *pdev)
--
2.36.1
[PATCH v3 32/71] drm/vc4: dsi: Embed DRM structures into the private structure
The VC4 DSI driver private structure contains only a pointer to the
encoder it implements. This makes the overall structure somewhat
inconsistent with the rest of the driver, and complicates its
initialisation without any apparent gain.
Let's embed the drm_encoder structure (through the vc4_encoder one) into
struct vc4_dsi to fix both issues.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dsi.c | 60 ++-
1 file changed, 23 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index b7b2c76770dc..72889524540e 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -549,10 +549,11 @@ struct vc4_dsi_variant {
/* General DSI hardware state. */
struct vc4_dsi {
- struct platform_device *pdev;
-
+ struct vc4_encoder encoder;
struct mipi_dsi_host dsi_host;
- struct drm_encoder *encoder;
+
+ struct platform_device *pdev;
+
struct drm_bridge *bridge;
struct list_head bridge_chain;
@@ -600,6 +601,12 @@ struct vc4_dsi {
#define host_to_dsi(host) container_of(host, struct vc4_dsi, dsi_host)
+static inline struct vc4_dsi *
+to_vc4_dsi(struct drm_encoder *encoder)
+{
+ return container_of(encoder, struct vc4_dsi, encoder.base);
+}
+
static inline void
dsi_dma_workaround_write(struct vc4_dsi *dsi, u32 offset, u32 val)
{
@@ -644,18 +651,6 @@ dsi_dma_workaround_write(struct vc4_dsi *dsi, u32 offset,
u32 val)
DSI_WRITE(dsi->variant->port ? DSI1_##offset : DSI0_##offset, val)
#define DSI_PORT_BIT(bit) (dsi->variant->port ? DSI1_##bit : DSI0_##bit)
-/* VC4 DSI encoder KMS struct */
-struct vc4_dsi_encoder {
- struct vc4_encoder base;
- struct vc4_dsi *dsi;
-};
-
-static inline struct vc4_dsi_encoder *
-to_vc4_dsi_encoder(struct drm_encoder *encoder)
-{
- return container_of(encoder, struct vc4_dsi_encoder, base.base);
-}
-
static const struct debugfs_reg32 dsi0_regs[] = {
VC4_REG32(DSI0_CTRL),
VC4_REG32(DSI0_STAT),
@@ -795,8 +790,7 @@ dsi_esc_timing(u32 ns)
static void vc4_dsi_encoder_disable(struct drm_encoder *encoder)
{
- struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
- struct vc4_dsi *dsi = vc4_encoder->dsi;
+ struct vc4_dsi *dsi = to_vc4_dsi(encoder);
struct device *dev = &dsi->pdev->dev;
struct drm_bridge *iter;
@@ -839,8 +833,7 @@ static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder
*encoder,
const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
{
- struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
- struct vc4_dsi *dsi = vc4_encoder->dsi;
+ struct vc4_dsi *dsi = to_vc4_dsi(encoder);
struct clk *phy_parent = clk_get_parent(dsi->pll_phy_clock);
unsigned long parent_rate = clk_get_rate(phy_parent);
unsigned long pixel_clock_hz = mode->clock * 1000;
@@ -875,8 +868,7 @@ static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder
*encoder,
static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
{
struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
- struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
- struct vc4_dsi *dsi = vc4_encoder->dsi;
+ struct vc4_dsi *dsi = to_vc4_dsi(encoder);
struct device *dev = &dsi->pdev->dev;
bool debug_dump_regs = false;
struct drm_bridge *iter;
@@ -1569,21 +1561,14 @@ static int vc4_dsi_bind(struct device *dev, struct
device *master, void *data)
struct platform_device *pdev = to_platform_device(dev);
struct drm_device *drm = dev_get_drvdata(master);
struct vc4_dsi *dsi = dev_get_drvdata(dev);
- struct vc4_dsi_encoder *vc4_dsi_encoder;
+ struct drm_encoder *encoder = &dsi->encoder.base;
int ret;
dsi->variant = of_device_get_match_data(dev);
- vc4_dsi_encoder = devm_kzalloc(dev, sizeof(*vc4_dsi_encoder),
- GFP_KERNEL);
- if (!vc4_dsi_encoder)
- return -ENOMEM;
-
INIT_LIST_HEAD(&dsi->bridge_chain);
- vc4_dsi_encoder->base.type = dsi->variant->port ?
- VC4_ENCODER_TYPE_DSI1 : VC4_ENCODER_TYPE_DSI0;
- vc4_dsi_encoder->dsi = dsi;
- dsi->encoder = &vc4_dsi_encoder->base.base;
+ dsi->encoder.type = dsi->variant->port ?
+ VC4_ENCODER_TYPE_DSI1 : VC4_ENCODER_TYPE_DSI0;
dsi->regs = vc4_ioremap_regs(pdev, 0);
if (IS_ERR(dsi->regs))
@@ -1702,10 +1687,10 @@ static int vc4_dsi_bind(struct device *dev, struct
device *master, void *data)
if (ret)
return ret;
- drm_simple_encoder_init(drm, dsi->encoder, DRM_MODE_ENCODER_DSI);
- drm_encoder_helper_add(dsi->encoder, &vc4_dsi_encoder_helper_funcs);
+ drm_simple_encode
[PATCH v3 26/71] drm/vc4: dpi: Return an error if we can't enable our clock
If we fail to enable the DPI clock, we just ignore the error and moves
forward. Let's return an error instead.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dpi.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index cfd89fd0695c..2febfe3d854d 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -289,6 +289,7 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
DRM_ERROR("Failed to get core clock: %d\n", ret);
return ret;
}
+
dpi->pixel_clock = devm_clk_get(dev, "pixel");
if (IS_ERR(dpi->pixel_clock)) {
ret = PTR_ERR(dpi->pixel_clock);
@@ -298,8 +299,10 @@ static int vc4_dpi_bind(struct device *dev, struct device
*master, void *data)
}
ret = clk_prepare_enable(dpi->core_clock);
- if (ret)
+ if (ret) {
DRM_ERROR("Failed to turn on core clock: %d\n", ret);
+ return ret;
+ }
drm_simple_encoder_init(drm, &dpi->encoder.base, DRM_MODE_ENCODER_DPI);
drm_encoder_helper_add(&dpi->encoder.base,
&vc4_dpi_encoder_helper_funcs);
--
2.36.1
[PATCH v3 36/71] drm/vc4: dsi: Switch to devm_pm_runtime_enable
devm_pm_runtime_enable() simplifies the driver a bit since it will call pm_runtime_disable() automatically through a device-managed action. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_dsi.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 4f5bdb9a328b..52c3215fef49 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1729,6 +1729,10 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) drm_encoder_helper_add(encoder, &vc4_dsi_encoder_helper_funcs); + ret = devm_pm_runtime_enable(dev); + if (ret) + return ret; + ret = drm_bridge_attach(encoder, dsi->bridge, NULL, 0); if (ret) return ret; @@ -1741,8 +1745,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) vc4_debugfs_add_regset32(drm, dsi->variant->debugfs_name, &dsi->regset); - pm_runtime_enable(dev); - return 0; } @@ -1752,8 +1754,6 @@ static void vc4_dsi_unbind(struct device *dev, struct device *master, struct vc4_dsi *dsi = dev_get_drvdata(dev); struct drm_encoder *encoder = &dsi->encoder.base; - pm_runtime_disable(dev); - /* * Restore the bridge_chain so the bridge detach procedure can happen * normally. -- 2.36.1
[PATCH v3 31/71] drm/vc4: dpi: Protect device resources
Our current code now mixes some resources whose lifetime are tied to the
device (clocks, IO mappings, etc.) and some that are tied to the DRM device
(encoder, bridge).
The device one will be freed at unbind time, but the DRM one will only be
freed when the last user of the DRM device closes its file handle.
So we end up with a time window during which we can call the encoder hooks,
but we don't have access to the underlying resources and device.
Let's protect all those sections with drm_dev_enter() and drm_dev_exit() so
that we bail out if we are during that window.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dpi.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index b4059820c9f5..e892ac853f64 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -13,6 +13,7 @@
#include
#include
+#include
#include
#include
#include
@@ -111,9 +112,16 @@ static const struct debugfs_reg32 dpi_regs[] = {
static void vc4_dpi_encoder_disable(struct drm_encoder *encoder)
{
+ struct drm_device *dev = encoder->dev;
struct vc4_dpi *dpi = to_vc4_dpi(encoder);
+ int idx;
+
+ if (!drm_dev_enter(dev, &idx))
+ return;
clk_disable_unprepare(dpi->pixel_clock);
+
+ drm_dev_exit(idx);
}
static void vc4_dpi_encoder_enable(struct drm_encoder *encoder)
@@ -124,6 +132,7 @@ static void vc4_dpi_encoder_enable(struct drm_encoder
*encoder)
struct drm_connector_list_iter conn_iter;
struct drm_connector *connector = NULL, *connector_scan;
u32 dpi_c = DPI_ENABLE;
+ int idx;
int ret;
/* Look up the connector attached to DPI so we can get the
@@ -203,6 +212,9 @@ static void vc4_dpi_encoder_enable(struct drm_encoder
*encoder)
dpi_c |= DPI_VSYNC_DISABLE;
}
+ if (!drm_dev_enter(dev, &idx))
+ return;
+
DPI_WRITE(DPI_C, dpi_c);
ret = clk_set_rate(dpi->pixel_clock, mode->clock * 1000);
@@ -212,6 +224,8 @@ static void vc4_dpi_encoder_enable(struct drm_encoder
*encoder)
ret = clk_prepare_enable(dpi->pixel_clock);
if (ret)
DRM_ERROR("Failed to set clock rate: %d\n", ret);
+
+ drm_dev_exit(idx);
}
static enum drm_mode_status vc4_dpi_encoder_mode_valid(struct drm_encoder
*encoder,
--
2.36.1
[PATCH v3 33/71] drm/vc4: dsi: Switch to DRM-managed encoder initialization
The current code will call drm_encoder_cleanup() when the device is
unbound. However, by then, there might still be some references held to
that encoder, including by the userspace that might still have the DRM
device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dsi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 72889524540e..1a55b7ea52a8 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -1687,7 +1687,13 @@ static int vc4_dsi_bind(struct device *dev, struct
device *master, void *data)
if (ret)
return ret;
- drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_DSI);
+ ret = drmm_encoder_init(drm, encoder,
+ NULL,
+ DRM_MODE_ENCODER_DSI,
+ NULL);
+ if (ret)
+ return ret;
+
drm_encoder_helper_add(encoder, &vc4_dsi_encoder_helper_funcs);
ret = drm_bridge_attach(encoder, dsi->bridge, NULL, 0);
@@ -1720,7 +1726,6 @@ static void vc4_dsi_unbind(struct device *dev, struct
device *master,
* normally.
*/
list_splice_init(&dsi->bridge_chain, &encoder->bridge_chain);
- drm_encoder_cleanup(encoder);
}
static const struct component_ops vc4_dsi_ops = {
--
2.36.1
[PATCH v3 25/71] drm/vc4: dpi: Switch to drmm_kzalloc
Our internal structure that stores the DRM entities structure is allocated through a device-managed kzalloc. This means that this will eventually be freed whenever the device is removed. In our case, the most likely source of removal is that the main device is going to be unbound, and component_unbind_all() is being run. However, it occurs while the DRM device is still registered, which will create dangling pointers, eventually resulting in use-after-free. Switch to a DRM-managed allocation to keep our structure until the DRM driver doesn't need it anymore. Reviewed-by: Dave Stevenson Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_dpi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index 7d4bb74cd500..cfd89fd0695c 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -263,9 +263,10 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) struct vc4_dpi *dpi; int ret; - dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL); + dpi = drmm_kzalloc(drm, sizeof(*dpi), GFP_KERNEL); if (!dpi) return -ENOMEM; + dpi->encoder.type = VC4_ENCODER_TYPE_DPI; dpi->pdev = pdev; dpi->regs = vc4_ioremap_regs(pdev, 0); -- 2.36.1
[PATCH v3 40/71] drm/vc4: hdmi: Remove call to drm_connector_unregister()
drm_connector_unregister() is only to be used for connectors that have been
registered through drm_connector_register() after drm_dev_register() has
been called. This is our case here so let's remove the call.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index fba549edcfc5..05f769474903 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -267,12 +267,6 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
return connector_status_disconnected;
}
-static void vc4_hdmi_connector_destroy(struct drm_connector *connector)
-{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-
static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
@@ -380,7 +374,7 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector
*connector)
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
.detect = vc4_hdmi_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = vc4_hdmi_connector_destroy,
+ .destroy = drm_connector_cleanup,
.reset = vc4_hdmi_connector_reset,
.atomic_duplicate_state = vc4_hdmi_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -3022,7 +3016,7 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
err_free_hotplug:
vc4_hdmi_hotplug_exit(vc4_hdmi);
err_destroy_conn:
- vc4_hdmi_connector_destroy(&vc4_hdmi->connector);
+ drm_connector_cleanup(&vc4_hdmi->connector);
err_destroy_encoder:
drm_encoder_cleanup(encoder);
pm_runtime_put_sync(dev);
@@ -3066,7 +3060,7 @@ static void vc4_hdmi_unbind(struct device *dev, struct
device *master,
vc4_hdmi_audio_exit(vc4_hdmi);
vc4_hdmi_cec_exit(vc4_hdmi);
vc4_hdmi_hotplug_exit(vc4_hdmi);
- vc4_hdmi_connector_destroy(&vc4_hdmi->connector);
+ drm_connector_cleanup(&vc4_hdmi->connector);
drm_encoder_cleanup(&vc4_hdmi->encoder.base);
pm_runtime_disable(dev);
--
2.36.1
[PATCH v3 34/71] drm/vc4: dsi: Switch to drmm_of_get_bridge
The current code uses a device-managed function to retrieve the next bridge downstream. However, that means that it will be removed at unbind time, where the DRM device is still very much live and might still have some applications that still have it open. Switch to a DRM-managed variant to clean everything up once the DRM device has been last closed. Reviewed-by: Dave Stevenson Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 1a55b7ea52a8..13266ff334d0 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1672,7 +1672,7 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) return ret; } - dsi->bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0); + dsi->bridge = drmm_of_get_bridge(drm, dev->of_node, 0, 0); if (IS_ERR(dsi->bridge)) return PTR_ERR(dsi->bridge); -- 2.36.1
[PATCH v3 37/71] drm/vc4: hdmi: Depends on CONFIG_PM
We already depend on runtime PM to get the power domains and clocks for
most of the devices supported by the vc4 driver, so let's just select it
to make sure it's there.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/Kconfig| 1 +
drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 061be9a6619d..b0f3117102ca 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -8,6 +8,7 @@ config DRM_VC4
depends on DRM
depends on SND && SND_SOC
depends on COMMON_CLK
+ depends on PM
select DRM_DISPLAY_HDMI_HELPER
select DRM_DISPLAY_HELPER
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 6ab83296b0e4..77e3ec52b175 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2854,7 +2854,7 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi
*vc4_hdmi)
return 0;
}
-static int __maybe_unused vc4_hdmi_runtime_suspend(struct device *dev)
+static int vc4_hdmi_runtime_suspend(struct device *dev)
{
struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
--
2.36.1
[PATCH v3 38/71] drm/vc4: hdmi: Rework power up
The current code tries to handle the case where CONFIG_PM isn't selected
by first calling our runtime_resume implementation and then properly
report the power state to the runtime_pm core.
This allows to have a functionning device even if pm_runtime_get_*
functions are nops.
However, the device power state if CONFIG_PM is enabled is
RPM_SUSPENDED, and thus our vc4_hdmi_write() and vc4_hdmi_read() calls
in the runtime_pm hooks will now report a warning since the device might
not be properly powered.
Even more so, we need CONFIG_PM enabled since the previous RaspberryPi
have a power domain that needs to be powered up for the HDMI controller
to be usable.
The previous patch has created a dependency on CONFIG_PM, now we can
just assume it's there and only call pm_runtime_resume_and_get() to make
sure our device is powered in bind.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 77e3ec52b175..73fb2f91c3e4 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2971,18 +2971,16 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
vc4_hdmi->disable_4kp60 = true;
}
- /*
-* We need to have the device powered up at this point to call
-* our reset hook and for the CEC init.
-*/
- ret = vc4_hdmi_runtime_resume(dev);
- if (ret)
- goto err_put_ddc;
-
- pm_runtime_get_noresume(dev);
- pm_runtime_set_active(dev);
pm_runtime_enable(dev);
+ /*
+* We need to have the device powered up at this point to call
+* our reset hook and for the CEC init.
+*/
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret)
+ goto err_disable_runtime_pm;
+
if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") ||
of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) &&
HDMI_READ(HDMI_VID_CTL) & VC4_HD_VID_CTL_ENABLE) {
@@ -3027,6 +3025,7 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
err_destroy_encoder:
drm_encoder_cleanup(encoder);
pm_runtime_put_sync(dev);
+err_disable_runtime_pm:
pm_runtime_disable(dev);
err_put_ddc:
put_device(&vc4_hdmi->ddc->dev);
--
2.36.1
[PATCH v3 39/71] drm/vc4: hdmi: Switch to drmm_kzalloc
Our internal structure that stores the DRM entities structure is allocated through a device-managed kzalloc. This means that this will eventually be freed whenever the device is removed. In our case, the most likely source of removal is that the main device is going to be unbound, and component_unbind_all() is being run. However, it occurs while the DRM device is still registered, which will create dangling pointers, eventually resulting in use-after-free. Switch to a DRM-managed allocation to keep our structure until the DRM driver doesn't need it anymore. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 73fb2f91c3e4..fba549edcfc5 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2907,9 +2907,10 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) struct device_node *ddc_node; int ret; - vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL); + vc4_hdmi = drmm_kzalloc(drm, sizeof(*vc4_hdmi), GFP_KERNEL); if (!vc4_hdmi) return -ENOMEM; + mutex_init(&vc4_hdmi->mutex); spin_lock_init(&vc4_hdmi->hw_lock); INIT_DELAYED_WORK(&vc4_hdmi->scrambling_work, vc4_hdmi_scrambling_wq); -- 2.36.1
[PATCH v3 35/71] drm/vc4: dsi: Fix the driver structure lifetime
The vc4_dsi structure is currently allocated through a device-managed
allocation. This can lead to use-after-free issues however in the unbinding
path since the DRM entities will stick around, but the underlying structure
has been freed.
However, we can't just fix it by using a DRM-managed allocation like we did
for the other drivers since the DSI case is a bit more intricate.
Indeed, the structure will be allocated at probe time, when we don't have a
DRM device yet, to be able to register the DSI bus driver. We will then
reuse it at bind time to register our KMS entities in the framework.
In order to work around both constraints, we can use a kref to track the
users of the structure (DSI host, and KMS), and then put our structure when
the DSI host will have been unregistered, and through a DRM-managed action
that will execute once we won't need the KMS entities anymore.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dsi.c | 38 ++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 13266ff334d0..4f5bdb9a328b 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -552,6 +552,8 @@ struct vc4_dsi {
struct vc4_encoder encoder;
struct mipi_dsi_host dsi_host;
+ struct kref kref;
+
struct platform_device *pdev;
struct drm_bridge *bridge;
@@ -1556,6 +1558,31 @@ static void vc4_dsi_dma_chan_release(void *ptr)
dsi->reg_dma_chan = NULL;
}
+static void vc4_dsi_release(struct kref *kref)
+{
+ struct vc4_dsi *dsi =
+ container_of(kref, struct vc4_dsi, kref);
+
+ kfree(dsi);
+}
+
+static void vc4_dsi_get(struct vc4_dsi *dsi)
+{
+ kref_get(&dsi->kref);
+}
+
+static void vc4_dsi_put(struct vc4_dsi *dsi)
+{
+ kref_put(&dsi->kref, &vc4_dsi_release);
+}
+
+static void vc4_dsi_release_action(struct drm_device *drm, void *ptr)
+{
+ struct vc4_dsi *dsi = ptr;
+
+ vc4_dsi_put(dsi);
+}
+
static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
{
struct platform_device *pdev = to_platform_device(dev);
@@ -1564,6 +1591,12 @@ static int vc4_dsi_bind(struct device *dev, struct
device *master, void *data)
struct drm_encoder *encoder = &dsi->encoder.base;
int ret;
+ vc4_dsi_get(dsi);
+
+ ret = drmm_add_action_or_reset(drm, vc4_dsi_release_action, dsi);
+ if (ret)
+ return ret;
+
dsi->variant = of_device_get_match_data(dev);
INIT_LIST_HEAD(&dsi->bridge_chain);
@@ -1738,11 +1771,12 @@ static int vc4_dsi_dev_probe(struct platform_device
*pdev)
struct device *dev = &pdev->dev;
struct vc4_dsi *dsi;
- dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
+ dsi = kzalloc(sizeof(*dsi), GFP_KERNEL);
if (!dsi)
return -ENOMEM;
dev_set_drvdata(dev, dsi);
+ kref_init(&dsi->kref);
dsi->pdev = pdev;
dsi->dsi_host.ops = &vc4_dsi_host_ops;
dsi->dsi_host.dev = dev;
@@ -1757,6 +1791,8 @@ static int vc4_dsi_dev_remove(struct platform_device
*pdev)
struct vc4_dsi *dsi = dev_get_drvdata(dev);
mipi_dsi_host_unregister(&dsi->dsi_host);
+ vc4_dsi_put(dsi);
+
return 0;
}
--
2.36.1
[PATCH v3 41/71] drm/vc4: hdmi: Switch to DRM-managed encoder initialization
The current code will call drm_encoder_cleanup() when the device is unbound. However, by then, there might still be some references held to that encoder, including by the userspace that might still have the DRM device open. Let's switch to a DRM-managed initialization to clean up after ourselves only once the DRM device has been last closed. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hdmi.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 05f769474903..6c44faea4af1 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2984,12 +2984,18 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) clk_prepare_enable(vc4_hdmi->pixel_bvb_clock); } - drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS); + ret = drmm_encoder_init(drm, encoder, + NULL, + DRM_MODE_ENCODER_TMDS, + NULL); + if (ret) + goto err_put_runtime_pm; + drm_encoder_helper_add(encoder, &vc4_hdmi_encoder_helper_funcs); ret = vc4_hdmi_connector_init(drm, vc4_hdmi); if (ret) - goto err_destroy_encoder; + goto err_put_runtime_pm; ret = vc4_hdmi_hotplug_init(vc4_hdmi); if (ret) @@ -3017,8 +3023,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) vc4_hdmi_hotplug_exit(vc4_hdmi); err_destroy_conn: drm_connector_cleanup(&vc4_hdmi->connector); -err_destroy_encoder: - drm_encoder_cleanup(encoder); +err_put_runtime_pm: pm_runtime_put_sync(dev); err_disable_runtime_pm: pm_runtime_disable(dev); @@ -3061,7 +3066,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, vc4_hdmi_cec_exit(vc4_hdmi); vc4_hdmi_hotplug_exit(vc4_hdmi); drm_connector_cleanup(&vc4_hdmi->connector); - drm_encoder_cleanup(&vc4_hdmi->encoder.base); pm_runtime_disable(dev); -- 2.36.1
[PATCH v3 43/71] drm/vc4: hdmi: Switch to device-managed ALSA initialization
The current code to unregister our ALSA device needs to be undone manually
when we remove the HDMI driver.
Since ALSA doesn't seem to support any mechanism to defer freeing something
until the last user of the ALSA device is gone, we can either use a
device-managed or a DRM-managed action.
The consistent way would be to use a DRM-managed one, just like pretty much
any framework-facing structure should be doing. However, ALSA does a lot of
allocation and registration using device-managed calls. Thus, if we're
going that way, by the time the DRM-managed action would run all of those
allocation would have been freed and we would end up with a use-after-free.
Thus, let's do a device-managed action. It's been tested with KASAN enabled
and doesn't seem to trigger any issue, so it's as good as anything.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 43 --
1 file changed, 36 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 283fea78f3cf..bf7d56bbcba0 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2056,6 +2056,14 @@ static struct hdmi_codec_pdata vc4_hdmi_codec_pdata = {
.i2s = 1,
};
+static void vc4_hdmi_audio_codec_release(void *ptr)
+{
+ struct vc4_hdmi *vc4_hdmi = ptr;
+
+ platform_device_unregister(vc4_hdmi->audio.codec_pdev);
+ vc4_hdmi->audio.codec_pdev = NULL;
+}
+
static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
{
const struct vc4_hdmi_register *mai_data =
@@ -2097,6 +2105,30 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
vc4_hdmi->audio.dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
vc4_hdmi->audio.dma_data.maxburst = 2;
+ /*
+* NOTE: Strictly speaking, we should probably use a DRM-managed
+* registration there to avoid removing all the audio components
+* by the time the driver doesn't have any user anymore.
+*
+* However, the ASoC core uses a number of devm_kzalloc calls
+* when registering, even when using non-device-managed
+* functions (such as in snd_soc_register_component()).
+*
+* If we call snd_soc_unregister_component() in a DRM-managed
+* action, the device-managed actions have already been executed
+* and thus we would access memory that has been freed.
+*
+* Using device-managed hooks here probably leaves us open to a
+* bunch of issues if userspace still has a handle on the ALSA
+* device when the device is removed. However, this is mitigated
+* by the use of drm_dev_enter()/drm_dev_exit() in the audio
+* path to prevent the access to the device resources if it
+* isn't there anymore.
+*
+* Then, the vc4_hdmi structure is DRM-managed and thus only
+* freed whenever the last user has closed the DRM device file.
+* It should thus outlive ALSA in most situations.
+*/
ret = devm_snd_dmaengine_pcm_register(dev, &pcm_conf, 0);
if (ret) {
dev_err(dev, "Could not register PCM component: %d\n", ret);
@@ -2120,6 +2152,10 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
}
vc4_hdmi->audio.codec_pdev = codec_pdev;
+ ret = devm_add_action_or_reset(dev, vc4_hdmi_audio_codec_release,
vc4_hdmi);
+ if (ret)
+ return ret;
+
dai_link->cpus = &vc4_hdmi->audio.cpu;
dai_link->codecs= &vc4_hdmi->audio.codec;
dai_link->platforms = &vc4_hdmi->audio.platform;
@@ -2158,12 +2194,6 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
}
-static void vc4_hdmi_audio_exit(struct vc4_hdmi *vc4_hdmi)
-{
- platform_device_unregister(vc4_hdmi->audio.codec_pdev);
- vc4_hdmi->audio.codec_pdev = NULL;
-}
-
static irqreturn_t vc4_hdmi_hpd_irq_thread(int irq, void *priv)
{
struct vc4_hdmi *vc4_hdmi = priv;
@@ -3062,7 +3092,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct
device *master,
kfree(vc4_hdmi->hdmi_regset.regs);
kfree(vc4_hdmi->hd_regset.regs);
- vc4_hdmi_audio_exit(vc4_hdmi);
vc4_hdmi_cec_exit(vc4_hdmi);
vc4_hdmi_hotplug_exit(vc4_hdmi);
--
2.36.1
[PATCH v3 42/71] drm/vc4: hdmi: Switch to DRM-managed connector initialization
The current code will call drm_connector_unregister() and
drm_connector_cleanup() when the device is unbound. However, by then, there
might still be some references held to that connector, including by the
userspace that might still have the DRM device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 6c44faea4af1..283fea78f3cf 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -374,7 +374,6 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector
*connector)
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
.detect = vc4_hdmi_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
.reset = vc4_hdmi_connector_reset,
.atomic_duplicate_state = vc4_hdmi_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -392,10 +391,13 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
struct drm_encoder *encoder = &vc4_hdmi->encoder.base;
int ret;
- drm_connector_init_with_ddc(dev, connector,
- &vc4_hdmi_connector_funcs,
- DRM_MODE_CONNECTOR_HDMIA,
- vc4_hdmi->ddc);
+ ret = drmm_connector_init(dev, connector,
+ &vc4_hdmi_connector_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ vc4_hdmi->ddc);
+ if (ret)
+ return ret;
+
drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
/*
@@ -2999,7 +3001,7 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
ret = vc4_hdmi_hotplug_init(vc4_hdmi);
if (ret)
- goto err_destroy_conn;
+ goto err_put_runtime_pm;
ret = vc4_hdmi_cec_init(vc4_hdmi);
if (ret)
@@ -3021,8 +3023,6 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
vc4_hdmi_cec_exit(vc4_hdmi);
err_free_hotplug:
vc4_hdmi_hotplug_exit(vc4_hdmi);
-err_destroy_conn:
- drm_connector_cleanup(&vc4_hdmi->connector);
err_put_runtime_pm:
pm_runtime_put_sync(dev);
err_disable_runtime_pm:
@@ -3065,7 +3065,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct
device *master,
vc4_hdmi_audio_exit(vc4_hdmi);
vc4_hdmi_cec_exit(vc4_hdmi);
vc4_hdmi_hotplug_exit(vc4_hdmi);
- drm_connector_cleanup(&vc4_hdmi->connector);
pm_runtime_disable(dev);
--
2.36.1
[PATCH v3 45/71] drm/vc4: hdmi: Use a device-managed action for DDC
The reference to the DDC controller device needs to be put back when we're
done with it. Let's use a device-managed action to simplify the driver.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 83ca1c02dde2..274f17880455 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2932,6 +2932,13 @@ static int vc4_hdmi_runtime_resume(struct device *dev)
return 0;
}
+static void vc4_hdmi_put_ddc_device(void *ptr)
+{
+ struct vc4_hdmi *vc4_hdmi = ptr;
+
+ put_device(&vc4_hdmi->ddc->dev);
+}
+
static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
{
const struct vc4_hdmi_variant *variant = of_device_get_match_data(dev);
@@ -2987,13 +2994,16 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
return -EPROBE_DEFER;
}
+ ret = devm_add_action_or_reset(dev, vc4_hdmi_put_ddc_device, vc4_hdmi);
+ if (ret)
+ return ret;
+
/* Only use the GPIO HPD pin if present in the DT, otherwise
* we'll use the HDMI core's register.
*/
vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
if (IS_ERR(vc4_hdmi->hpd_gpio)) {
- ret = PTR_ERR(vc4_hdmi->hpd_gpio);
- goto err_put_ddc;
+ return PTR_ERR(vc4_hdmi->hpd_gpio);
}
vc4_hdmi->disable_wifi_frequencies =
@@ -3064,8 +3074,6 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
pm_runtime_put_sync(dev);
err_disable_runtime_pm:
pm_runtime_disable(dev);
-err_put_ddc:
- put_device(&vc4_hdmi->ddc->dev);
return ret;
}
@@ -3102,8 +3110,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct
device *master,
vc4_hdmi_hotplug_exit(vc4_hdmi);
pm_runtime_disable(dev);
-
- put_device(&vc4_hdmi->ddc->dev);
}
static const struct component_ops vc4_hdmi_ops = {
--
2.36.1
[PATCH v3 44/71] drm/vc4: hdmi: Switch to device-managed CEC initialization
The current code to unregister our CEC device needs to be undone manually
when we remove the HDMI driver.
Since the CEC framework will allocate its main structure, and will defer
its deallocation to when the last user will have closed it, we don't really
need to take any particular measure to prevent any use-after-free and can
thus use any managed action.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 94 ++
1 file changed, 50 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index bf7d56bbcba0..83ca1c02dde2 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2576,6 +2576,14 @@ static const struct cec_adap_ops vc4_hdmi_cec_adap_ops =
{
.adap_transmit = vc4_hdmi_cec_adap_transmit,
};
+static void vc4_hdmi_cec_release(void *ptr)
+{
+ struct vc4_hdmi *vc4_hdmi = ptr;
+
+ cec_unregister_adapter(vc4_hdmi->cec_adap);
+ vc4_hdmi->cec_adap = NULL;
+}
+
static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
{
struct cec_connector_info conn_info;
@@ -2600,70 +2608,71 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
cec_s_conn_info(vc4_hdmi->cec_adap, &conn_info);
if (vc4_hdmi->variant->external_irq_controller) {
- ret = request_threaded_irq(platform_get_irq_byname(pdev,
"cec-rx"),
- vc4_cec_irq_handler_rx_bare,
- vc4_cec_irq_handler_rx_thread, 0,
- "vc4 hdmi cec rx", vc4_hdmi);
+ ret = devm_request_threaded_irq(dev,
platform_get_irq_byname(pdev, "cec-rx"),
+ vc4_cec_irq_handler_rx_bare,
+ vc4_cec_irq_handler_rx_thread,
0,
+ "vc4 hdmi cec rx", vc4_hdmi);
if (ret)
goto err_delete_cec_adap;
- ret = request_threaded_irq(platform_get_irq_byname(pdev,
"cec-tx"),
- vc4_cec_irq_handler_tx_bare,
- vc4_cec_irq_handler_tx_thread, 0,
- "vc4 hdmi cec tx", vc4_hdmi);
+ ret = devm_request_threaded_irq(dev,
platform_get_irq_byname(pdev, "cec-tx"),
+ vc4_cec_irq_handler_tx_bare,
+ vc4_cec_irq_handler_tx_thread,
0,
+ "vc4 hdmi cec tx", vc4_hdmi);
if (ret)
- goto err_remove_cec_rx_handler;
+ goto err_delete_cec_adap;
} else {
- ret = request_threaded_irq(platform_get_irq(pdev, 0),
- vc4_cec_irq_handler,
- vc4_cec_irq_handler_thread, 0,
- "vc4 hdmi cec", vc4_hdmi);
+ ret = devm_request_threaded_irq(dev, platform_get_irq(pdev, 0),
+ vc4_cec_irq_handler,
+ vc4_cec_irq_handler_thread, 0,
+ "vc4 hdmi cec", vc4_hdmi);
if (ret)
goto err_delete_cec_adap;
}
ret = cec_register_adapter(vc4_hdmi->cec_adap, &pdev->dev);
if (ret < 0)
- goto err_remove_handlers;
+ goto err_delete_cec_adap;
+
+ /*
+* NOTE: Strictly speaking, we should probably use a DRM-managed
+* registration there to avoid removing the CEC adapter by the
+* time the DRM driver doesn't have any user anymore.
+*
+* However, the CEC framework already cleans up the CEC adapter
+* only when the last user has closed its file descriptor, so we
+* don't need to handle it in DRM.
+*
+* By the time the device-managed hook is executed, we will give
+* up our reference to the CEC adapter and therefore don't
+* really care when it's actually freed.
+*
+* There's still a problematic sequence: if we unregister our
+* CEC adapter, but the userspace keeps a handle on the CEC
+* adapter but not the DRM device for some reason. In such a
+* case, our vc4_hdmi structure will be freed, but the
+* cec_adapter structure will have a dangling pointer to what
+* used to be our HDMI controller. If we get a CEC call at that
+* moment, we could end up with a use-after-free. Fortunately,
+* the CEC framework already handles this too, by calling
+* cec_is_registered() in cec_ioctl() and cec_poll().
+*/
+ ret = devm_add_action_or_reset(dev, vc4_hdmi_cec_release,
[PATCH v3 49/71] drm/vc4: hdmi: Protect device resources after removal
Whenever the device and driver are unbound, the main device and all the
subdevices will be removed by calling their unbind() method.
However, the DRM device itself will only be freed when the last user will
have closed it.
It means that there is a time window where the device and its resources
aren't there anymore, but the userspace can still call into our driver.
Fortunately, the DRM framework provides the drm_dev_enter() and
drm_dev_exit() functions to make sure our underlying device is still there
for the section protected by those calls. Let's add them to the HDMI driver.
Acked-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 308 +++--
1 file changed, 291 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a130b7d48e46..a826faf8b02d 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -146,7 +147,12 @@ static int vc4_hdmi_debugfs_regs(struct seq_file *m, void
*unused)
{
struct drm_info_node *node = (struct drm_info_node *)m->private;
struct vc4_hdmi *vc4_hdmi = node->info_ent->data;
+ struct drm_device *drm = vc4_hdmi->connector.dev;
struct drm_printer p = drm_seq_file_printer(m);
+ int idx;
+
+ if (!drm_dev_enter(drm, &idx))
+ return -ENODEV;
drm_print_regset32(&p, &vc4_hdmi->hdmi_regset);
drm_print_regset32(&p, &vc4_hdmi->hd_regset);
@@ -157,12 +163,23 @@ static int vc4_hdmi_debugfs_regs(struct seq_file *m, void
*unused)
drm_print_regset32(&p, &vc4_hdmi->ram_regset);
drm_print_regset32(&p, &vc4_hdmi->rm_regset);
+ drm_dev_exit(idx);
+
return 0;
}
static void vc4_hdmi_reset(struct vc4_hdmi *vc4_hdmi)
{
+ struct drm_device *drm = vc4_hdmi->connector.dev;
unsigned long flags;
+ int idx;
+
+ /*
+* We can be called by our bind callback, when the
+* connector->dev pointer might not be initialised yet.
+*/
+ if (drm && !drm_dev_enter(drm, &idx))
+ return;
spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
@@ -179,11 +196,23 @@ static void vc4_hdmi_reset(struct vc4_hdmi *vc4_hdmi)
HDMI_WRITE(HDMI_SW_RESET_CONTROL, 0);
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
+
+ if (drm)
+ drm_dev_exit(idx);
}
static void vc5_hdmi_reset(struct vc4_hdmi *vc4_hdmi)
{
+ struct drm_device *drm = vc4_hdmi->connector.dev;
unsigned long flags;
+ int idx;
+
+ /*
+* We can be called by our bind callback, when the
+* connector->dev pointer might not be initialised yet.
+*/
+ if (drm && !drm_dev_enter(drm, &idx))
+ return;
reset_control_reset(vc4_hdmi->reset);
@@ -195,15 +224,31 @@ static void vc5_hdmi_reset(struct vc4_hdmi *vc4_hdmi)
HDMI_READ(HDMI_CLOCK_STOP) | VC4_DVP_HT_CLOCK_STOP_PIXEL);
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
+
+ if (drm)
+ drm_dev_exit(idx);
}
#ifdef CONFIG_DRM_VC4_HDMI_CEC
static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi)
{
- unsigned long cec_rate = clk_get_rate(vc4_hdmi->cec_clock);
+ struct drm_device *drm = vc4_hdmi->connector.dev;
+ unsigned long cec_rate;
unsigned long flags;
u16 clk_cnt;
u32 value;
+ int idx;
+
+ /*
+* This function is called by our runtime_resume implementation
+* and thus at bind time, when we haven't registered our
+* connector yet and thus don't have a pointer to the DRM
+* device.
+*/
+ if (drm && !drm_dev_enter(drm, &idx))
+ return;
+
+ cec_rate = clk_get_rate(vc4_hdmi->cec_clock);
spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
@@ -219,6 +264,9 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi
*vc4_hdmi)
HDMI_WRITE(HDMI_CEC_CNTRL_1, value);
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
+
+ if (drm)
+ drm_dev_exit(idx);
}
#else
static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
@@ -440,25 +488,34 @@ static int vc4_hdmi_stop_packet(struct drm_encoder
*encoder,
bool poll)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_device *drm = vc4_hdmi->connector.dev;
u32 packet_id = type - 0x80;
unsigned long flags;
+ int ret = 0;
+ int idx;
+
+ if (!drm_dev_enter(drm, &idx))
+ return -ENODEV;
spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
HDMI_WRITE(HDMI_RAM_PACKET_CONFIG,
HDMI_READ(HDMI_RAM_PACKET_CONFIG) & ~BIT(packet_id));
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, f
[PATCH v3 47/71] drm/vc4: hdmi: Use devm to register hotplug interrupts
Commit 776efe800fed ("drm/vc4: hdmi: Drop devm interrupt handler for
hotplug interrupts") dropped the device-managed interrupt registration
because it was creating bugs and races whenever an interrupt was coming in
while the device was removed.
However, our latest patches to the HDMI controller driver fix this as well,
so we can use device-managed interrupt handlers again.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 41 +-
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index c93100fe4b42..6275db463afd 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2216,21 +2216,19 @@ static int vc4_hdmi_hotplug_init(struct vc4_hdmi
*vc4_hdmi)
unsigned int hpd_con = platform_get_irq_byname(pdev,
"hpd-connected");
unsigned int hpd_rm = platform_get_irq_byname(pdev,
"hpd-removed");
- ret = request_threaded_irq(hpd_con,
- NULL,
- vc4_hdmi_hpd_irq_thread,
IRQF_ONESHOT,
- "vc4 hdmi hpd connected", vc4_hdmi);
+ ret = devm_request_threaded_irq(&pdev->dev, hpd_con,
+ NULL,
+ vc4_hdmi_hpd_irq_thread,
IRQF_ONESHOT,
+ "vc4 hdmi hpd connected",
vc4_hdmi);
if (ret)
return ret;
- ret = request_threaded_irq(hpd_rm,
- NULL,
- vc4_hdmi_hpd_irq_thread,
IRQF_ONESHOT,
- "vc4 hdmi hpd disconnected",
vc4_hdmi);
- if (ret) {
- free_irq(hpd_con, vc4_hdmi);
+ ret = devm_request_threaded_irq(&pdev->dev, hpd_rm,
+ NULL,
+ vc4_hdmi_hpd_irq_thread,
IRQF_ONESHOT,
+ "vc4 hdmi hpd disconnected",
vc4_hdmi);
+ if (ret)
return ret;
- }
connector->polled = DRM_CONNECTOR_POLL_HPD;
}
@@ -2238,16 +2236,6 @@ static int vc4_hdmi_hotplug_init(struct vc4_hdmi
*vc4_hdmi)
return 0;
}
-static void vc4_hdmi_hotplug_exit(struct vc4_hdmi *vc4_hdmi)
-{
- struct platform_device *pdev = vc4_hdmi->pdev;
-
- if (vc4_hdmi->variant->external_irq_controller) {
- free_irq(platform_get_irq_byname(pdev, "hpd-connected"),
vc4_hdmi);
- free_irq(platform_get_irq_byname(pdev, "hpd-removed"),
vc4_hdmi);
- }
-}
-
#ifdef CONFIG_DRM_VC4_HDMI_CEC
static irqreturn_t vc4_cec_irq_handler_rx_thread(int irq, void *priv)
{
@@ -3069,11 +3057,11 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
ret = vc4_hdmi_cec_init(vc4_hdmi);
if (ret)
- goto err_free_hotplug;
+ goto err_put_runtime_pm;
ret = vc4_hdmi_audio_init(vc4_hdmi);
if (ret)
- goto err_free_hotplug;
+ goto err_put_runtime_pm;
vc4_debugfs_add_file(drm, variant->debugfs_name,
vc4_hdmi_debugfs_regs,
@@ -3083,8 +3071,6 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
return 0;
-err_free_hotplug:
- vc4_hdmi_hotplug_exit(vc4_hdmi);
err_put_runtime_pm:
pm_runtime_put_sync(dev);
err_disable_runtime_pm:
@@ -3096,8 +3082,6 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
static void vc4_hdmi_unbind(struct device *dev, struct device *master,
void *data)
{
- struct vc4_hdmi *vc4_hdmi;
-
/*
* ASoC makes it a bit hard to retrieve a pointer to the
* vc4_hdmi structure. Registering the card will overwrite our
@@ -3117,9 +3101,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct
device *master,
*/
BUILD_BUG_ON(offsetof(struct vc4_hdmi_audio, card) != 0);
BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0);
- vc4_hdmi = dev_get_drvdata(dev);
-
- vc4_hdmi_hotplug_exit(vc4_hdmi);
pm_runtime_disable(dev);
}
--
2.36.1
[PATCH v3 46/71] drm/vc4: hdmi: Switch to DRM-managed kfree to build regsets
The current code to build the registers set later exposed in debugfs for
the HDMI controller relies on traditional allocations, that are later
free'd as part of the driver unbind hook.
Since krealloc doesn't have a DRM-managed equivalent, let's add an action
to free the buffer later on.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 46 +-
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ++-
2 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 274f17880455..c93100fe4b42 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2675,7 +2675,15 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
}
#endif
-static int vc4_hdmi_build_regset(struct vc4_hdmi *vc4_hdmi,
+static void vc4_hdmi_free_regset(struct drm_device *drm, void *ptr)
+{
+ struct debugfs_reg32 *regs = ptr;
+
+ kfree(regs);
+}
+
+static int vc4_hdmi_build_regset(struct drm_device *drm,
+struct vc4_hdmi *vc4_hdmi,
struct debugfs_regset32 *regset,
enum vc4_hdmi_regs reg)
{
@@ -2683,6 +2691,7 @@ static int vc4_hdmi_build_regset(struct vc4_hdmi
*vc4_hdmi,
struct debugfs_reg32 *regs, *new_regs;
unsigned int count = 0;
unsigned int i;
+ int ret;
regs = kcalloc(variant->num_registers, sizeof(*regs),
GFP_KERNEL);
@@ -2708,10 +2717,15 @@ static int vc4_hdmi_build_regset(struct vc4_hdmi
*vc4_hdmi,
regset->regs = new_regs;
regset->nregs = count;
+ ret = drmm_add_action_or_reset(drm, vc4_hdmi_free_regset, new_regs);
+ if (ret)
+ return ret;
+
return 0;
}
-static int vc4_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi)
+static int vc4_hdmi_init_resources(struct drm_device *drm,
+ struct vc4_hdmi *vc4_hdmi)
{
struct platform_device *pdev = vc4_hdmi->pdev;
struct device *dev = &pdev->dev;
@@ -2725,11 +2739,11 @@ static int vc4_hdmi_init_resources(struct vc4_hdmi
*vc4_hdmi)
if (IS_ERR(vc4_hdmi->hd_regs))
return PTR_ERR(vc4_hdmi->hd_regs);
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hd_regset, VC4_HD);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->hd_regset,
VC4_HD);
if (ret)
return ret;
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hdmi_regset, VC4_HDMI);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->hdmi_regset,
VC4_HDMI);
if (ret)
return ret;
@@ -2752,7 +2766,8 @@ static int vc4_hdmi_init_resources(struct vc4_hdmi
*vc4_hdmi)
return 0;
}
-static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi)
+static int vc5_hdmi_init_resources(struct drm_device *drm,
+ struct vc4_hdmi *vc4_hdmi)
{
struct platform_device *pdev = vc4_hdmi->pdev;
struct device *dev = &pdev->dev;
@@ -2854,35 +2869,35 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi
*vc4_hdmi)
return PTR_ERR(vc4_hdmi->reset);
}
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hdmi_regset, VC4_HDMI);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->hdmi_regset,
VC4_HDMI);
if (ret)
return ret;
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hd_regset, VC4_HD);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->hd_regset,
VC4_HD);
if (ret)
return ret;
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->cec_regset, VC5_CEC);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->cec_regset,
VC5_CEC);
if (ret)
return ret;
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->csc_regset, VC5_CSC);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->csc_regset,
VC5_CSC);
if (ret)
return ret;
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->dvp_regset, VC5_DVP);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->dvp_regset,
VC5_DVP);
if (ret)
return ret;
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->phy_regset, VC5_PHY);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->phy_regset,
VC5_PHY);
if (ret)
return ret;
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->ram_regset, VC5_RAM);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->ram_regset,
VC5_RAM);
if (ret)
return ret;
- ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->rm_regset, VC5_RM);
+ ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->rm_regset,
VC5_RM);
if (ret)
return ret;
@@ -2977,7 +2992,7 @@ static int vc4_hdmi_bind(struct device *dev, st
[PATCH v3 48/71] drm/vc4: hdmi: Move audio structure offset checks
The HDMI driver unbind hook doesn't have any ALSA-related code anymore, so
let's move the ALSA sanity checks and comments we have to some other part
of the driver dedicated to ALSA.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 40 +-
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 6275db463afd..a130b7d48e46 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2076,6 +2076,26 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
int index, len;
int ret;
+ /*
+* ASoC makes it a bit hard to retrieve a pointer to the
+* vc4_hdmi structure. Registering the card will overwrite our
+* device drvdata with a pointer to the snd_soc_card structure,
+* which can then be used to retrieve whatever drvdata we want
+* to associate.
+*
+* However, that doesn't fly in the case where we wouldn't
+* register an ASoC card (because of an old DT that is missing
+* the dmas properties for example), then the card isn't
+* registered and the device drvdata wouldn't be set.
+*
+* We can deal with both cases by making sure a snd_soc_card
+* pointer and a vc4_hdmi structure are pointing to the same
+* memory address, so we can treat them indistinctly without any
+* issue.
+*/
+ BUILD_BUG_ON(offsetof(struct vc4_hdmi_audio, card) != 0);
+ BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0);
+
if (!of_find_property(dev->of_node, "dmas", &len) || !len) {
dev_warn(dev,
"'dmas' DT property is missing or empty, no HDMI
audio\n");
@@ -3082,26 +3102,6 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
static void vc4_hdmi_unbind(struct device *dev, struct device *master,
void *data)
{
- /*
-* ASoC makes it a bit hard to retrieve a pointer to the
-* vc4_hdmi structure. Registering the card will overwrite our
-* device drvdata with a pointer to the snd_soc_card structure,
-* which can then be used to retrieve whatever drvdata we want
-* to associate.
-*
-* However, that doesn't fly in the case where we wouldn't
-* register an ASoC card (because of an old DT that is missing
-* the dmas properties for example), then the card isn't
-* registered and the device drvdata wouldn't be set.
-*
-* We can deal with both cases by making sure a snd_soc_card
-* pointer and a vc4_hdmi structure are pointing to the same
-* memory address, so we can treat them indistinctly without any
-* issue.
-*/
- BUILD_BUG_ON(offsetof(struct vc4_hdmi_audio, card) != 0);
- BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0);
-
pm_runtime_disable(dev);
}
--
2.36.1
