- 26 Mar, 2020 37 commits
-
-
Daniel Vetter authored
It's right above the drm_dev_put(). This is made possible by a preceeding patch which added a drmm_ cleanup action to drm_mode_config_init(), hence all we need to do to ensure that drm_mode_config_cleanup() is run on final drm_device cleanup is check the new error code for _init(). Aside: Another driver with a bit much devm_kzalloc, which should probably use drmm_kzalloc instead ... v2: Explain why this cleanup is possible (Laurent). v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas) Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> (v2) Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by:
Daniel Vetter <daniel.vetter@intel.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Cc: linux-renesas-soc@vger.kernel.org Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-37-daniel.vetter@ffwll.ch
-
Daniel Vetter authored
It's right above the drm_dev_put(). This is made possible by a preceeding patch which added a drmm_ cleanup action to drm_mode_config_init(), hence all we need to do to ensure that drm_mode_config_cleanup() is run on final drm_device cleanup is check the new error code for _init(). Aside: This driver gets its devm_ stuff all wrong wrt drm_device and anything hanging off that. Not the only one unfortunately. v2: Explain why this cleanup is possible (Laurent). v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas) Acked-by:
Sam Ravnborg <sam@ravnborg.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by:
-
Daniel Vetter authored
It's right above the drm_dev_put(). This is made possible by a preceeding patch which added a drmm_ cleanup action to drm_mode_config_init(), hence all we need to do to ensure that drm_mode_config_cleanup() is run on final drm_device cleanup is check the new error code for _init(). Aside: This driver gets its devm_ stuff all wrong wrt drm_device and anything hanging off that. Not the only one unfortunately. v2: Explain why this cleanup is possible (Laurent). v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas) Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> (v2) Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by:
-
Daniel Vetter authored
Auto-unwind ftw, now possible with the fixed drm_device related management. Aside, clk/regulator seem to be missing devm versions for a bunch of functions, preventing a pile of these simpler drivers from outright losing their ->remove hook. Reviewed-by:
Linus Walleij <linus.walleij@linaro.org> Signed-off-by:
-
Daniel Vetter authored
Allows us to drop the drm_driver.release callback. This is made possible by a preceeding patch which added a drmm_ cleanup action to drm_mode_config_init(), hence all we need to do to ensure that drm_mode_config_cleanup() is run on final drm_device cleanup is check the new error code for _init(). v2: Explain why this cleanup is possible (Laurent). v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas) Acked-by:
-
Daniel Vetter authored
Allows us to drop the drm_driver.release callback. This is made possible by a preceeding patch which added a drmm_ cleanup action to drm_mode_config_init(), hence all we need to do to ensure that drm_mode_config_cleanup() is run on final drm_device cleanup is check the new error code for _init(). v2: Explain why this cleanup is possible (Laurent). v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas) Acked-by:
-
Daniel Vetter authored
With the drm_device lifetime fun cleaned up there's nothing in the way anymore to use devm_ for everything hw releated. Do it, and in the process, throw out the entire onion unwinding. Acked-by:
Gerd Hoffmann <kraxel@redhat.com> Signed-off-by:
-
Daniel Vetter authored
We can even delete the drm_driver.release hook now! This is made possible by a preceeding patch which added a drmm_ cleanup action to drm_mode_config_init(), hence all we need to do to ensure that drm_mode_config_cleanup() is run on final drm_device cleanup is check the new error code for _init(). v2: Explain why this cleanup is possible (Laurent). v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas) Acked-by:
-
Daniel Vetter authored
Instead rely on the automatic clean, for which we just need to check that drm_mode_config_init succeeded. To avoid an inversion in the cleanup we also have to move the dev_private allocation over to drmm_kzalloc. This is made possible by a preceeding patch which added a drmm_ cleanup action to drm_mode_config_init(), hence all we need to do to ensure that drm_mode_config_cleanup() is run on final drm_device cleanup is check the new error code for _init(). v2: Explain why this cleanup is possible (Laurent). v3: Use drmm_mode_config_init() for more clarity (Sam, Thomas) Acked-by:
-
Daniel Vetter authored
Small mistake that crept into commit 81da8c3b Author: Gerd Hoffmann <kraxel@redhat.com> Date: Tue Feb 11 14:52:18 2020 +0100 drm/bochs: add drm_driver.release callback where drm_atomic_helper_shutdown was left in both places. The ->release callback really shouldn't touch hardware. Acked-by:
-
Daniel Vetter authored
drm_mode_config_cleanup is idempotent, so no harm in calling this twice. This allows us to gradually switch drivers over by removing explicit drm_mode_config_cleanup calls. With this step it's now also possible that (at least for simple drivers) automatic resource cleanup can be done correctly without a drm_driver->release hook. Therefore allow this now in devm_drm_dev_init(). Also with drmm_ explicit drm_driver->release hooks are kinda not the best option: Drivers can always just register their current release hook with drmm_add_action, but even better they could split them up to simplify the unwinding for the driver load failure case. So deprecate that hook to discourage future users. v2: Fixup the example in the kerneldoc too. v3: - For paranoia, double check that minor->dev == dev in the release hook, because I botched the pointer math in the drmm library. - Call drm_mode_config_cleanup when drmm_add_action fails, we'd be missing some mutex_destroy and ida_cleanup otherwise (Laurent) v4: Add a drmm_add_action_or_reset (like devm_ has) to encapsulate this pattern (Noralf). v5: Fix oversight in the new drmm_add_action_or_reset macro (Noralf) v4: Review from Sam: - drmm_mode_config_init wrapper (also suggested by Thomas) - improve commit message, explain better why ->relase is deprecated v5: - Make drmm_ the main function, with the old one as compat wrapper (Sam) - Add FIXME comments to drm_mode_config_cleanup/init() that drivers shouldn't use these anymore. - Move drmm_add_action_or_reset helper to an earlier patch. Reviewed-by:
Noralf Trønnes <noralf@tronnes.org> Signed-off-by:
-
Daniel Vetter authored
It has become empty. Given the few users I figured not much point splitting this up. v2: Rebase over i915 changes. v3: Rebase over patch split fix. Acked-by:
-
Daniel Vetter authored
Nothing special here, except that this is the first time that we automatically clean up something that's initialized with an explicit driver call. But the cleanup was done at the very end of the release sequence for all drivers, and that's still the case. At least without more uses of drmm_ through explicit driver calls. Also for this one we need drmm_kcalloc, so lets add those. The motivation here is to allow us to remove the explicit calls to drm_dev_fini() from all drivers. v2: Sort includes (Laurent) v3: Motivate the change in the commit message better (Sam) Acked-by:
-
Daniel Vetter authored
We might want to look into pushing this down into drm_mm_init, but that would mean rolling out return codes to a pile of functions unfortunately. So let's leave that for now. Acked-by:
-
Daniel Vetter authored
The cleanup here is somewhat tricky, since we can't tell apart the allocated minor index from 0. So register a cleanup action first, and if the index allocation fails, unregister that cleanup action again to avoid bad mistakes. The kdev for the minor already handles NULL, so no problem there. Hence add drmm_remove_action() to the drm_managed library. v2: Make pointer math around void ** consistent with what Laurent suggested. v3: Use drmm_add_action_or_reset and remove drmm_remove_action. Noticed because of some questions from Thomas. This also means we need to move the drmm_add_action_or_reset helper earlier in the series. v4: Uh ... fix slightly embarrassing bug CI spotted. Acked-by:
Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by:
-
Daniel Vetter authored
Well for the simple stuff at least, vblank, gem and minor cleanup I want to further split up as a demonstration. v2: We need to clear drm_device->dev otherwise the debug drm printing after our cleanup hook (e.g. in drm_manged_release) will chase released memory and result in a use-after-free. Not really pretty, but oh well. Reviewed-by:
-
Daniel Vetter authored
We need to add a drmm_kstrdup for this, but let's start somewhere. This is not exactly perfect onion unwinding, but it's jsut a kfree so doesn't really matter at all. Reviewed-by:
-
Daniel Vetter authored
A few things: - Update the example driver in the documentation. - We can drop the old kfree in drm_dev_release. - Add a WARN_ON check in drm_dev_register to make sure everyone calls drmm_add_final_kfree and there's no leaks. v2: Restore the full cleanup, I accidentally left some moved code behind when fixing the bisectability of the series. Acked-by:
-
Daniel Vetter authored
These are the leftover drivers that didn't have a ->release hook that needed to be updated. Acked-by:
Liviu Dudau <liviu.dudau@arm.com> Signed-off-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. Acked-by:
Hans de Goede <hdegoede@redhat.com> Signed-off-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. Acked-by:
Paul Cercueil <paul@crapouillou.net> Signed-off-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. Acked-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. v2: After drm_dev_init/drmm_add_final_kfree we need to clean up everything through a drm_dev_put. Rework the unwind code to match that. Reviewed-by:
Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. v2: After drm_dev_init/drmm_add_final_kfree we need to clean up everything through a drm_dev_put. Rework the unwind code to match that. Acked-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. v2: Fix unused variable warning, spotted while applying. Reviewed-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. Acked-by:
Jyri Sarha <jsarha@ti.com> Signed-off-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. I also noticed that the unwind code is wrong, after drm_dev_init the drm_device owns the v3d allocation, so the kfree(v3d) is a double-free. Reorder the setup to fix this issue. After a bit more prep in drivers and drm core v3d should be able to switch over to devm_drm_dev_init, which should clean this up further. Acked-by:
Eric Anholt <eric@anholt.net> Signed-off-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. I also noticed that cirrus forgot to call drm_dev_fini(). v2: Don't call kfree(cirrus) after we've handed overship of that to drm_device and the drmm_ stuff. Acked-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. The mock device in the selftests needed it's pci_device split up from the drm_device. In the future we could simplify this again by allocating the pci_device as a managed allocation too. v2: I overlooked that i915_driver_destroy is also called in the unwind code of the error path. There we need a drm_dev_put. Similar for the mock object. Now the problem with that is that the drm_driver->release callbacks for both the real driver and the mock one assume everything has been set up. Hence going through that path for a partially set up driver will result in issues. Quickest fix is to disable the ->release() hook until the driver is fully initialized, and keep the onion unwinding. Long term would be cleanest to move everything over to drmm_ release actions, but that's a lot of work for a big driver like i915. Plus more core work needed first anyway. v3: Fix i915_drm pointer wrangling in mock_gem_device. Also switch over to start using drm_dev_put() to clean up even on the error path. Aside I think the current error path is leaking the allocation. v4: more fixes for intel-gfx-ci, some if it damage from v3 :-/ Reviewed-by:
Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Maarten Lankhorst authored
Requested for getting some i915 fixes back into drm-misc-next by danvet. Signed-off-by:Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. Acked-by:
-
Daniel Vetter authored
With this we can drop the final kfree from the release function. v2: We need drm_dev_put to unroll the driver creation (once drm_dev_init and drmm_add_final_kfree suceeded), otherwise the drmm_ magic doesn't happen. v3: Actually squash in the fixup (Laurent). Acked-by:
-
Daniel Vetter authored
They all share mipi_dbi_release so we need to switch them all together. With this we can drop the final kfree from the release function. Aside, I think we could perhaps have a tiny additional helper for these mipi_dbi drivers, the first few lines around devm_drm_dev_init are all the same (except for the drm_driver pointer). Acked-by:
-
Daniel Vetter authored
I also did a full review of all callers, and only the xen driver forgot to call drm_dev_put in the failure path. Fix that up too. v2: I noticed that xen has a drm_driver.release hook, and uses drm_dev_alloc(). We need to remove the kfree from xen_drm_drv_release(). bochs also has a release hook, but leaked the drm_device ever since commit 0a6659bd Author: Gerd Hoffmann <kraxel@redhat.com> Date: Tue Dec 17 18:04:46 2013 +0100 drm/bochs: new driver This patch here fixes that leak. Same for virtio, started leaking with commit b1df3a2b Author: Gerd Hoffmann <kraxel@redhat.com> Date: Tue Feb 11 14:58:04 2020 +0100 drm/virtio: add drm_driver.release callback. Acked-by:
Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: xen-devel@lists.xenproject.org Signed-off-by:
-
Daniel Vetter authored
We have lots of these. And the cleanup code tends to be of dubious quality. The biggest wrong pattern is that developers use devm_, which ties the release action to the underlying struct device, whereas all the userspace visible stuff attached to a drm_device can long outlive that one (e.g. after a hotunplug while userspace has open files and mmap'ed buffers). Give people what they want, but with more correctness. Mostly copied from devres.c, with types adjusted to fit drm_device and a few simplifications - I didn't (yet) copy over everything. Since the types don't match code sharing looked like a hopeless endeavour. For now it's only super simplified, no groups, you can't remove actions (but kfree exists, we'll need that soon). Plus all specific to drm_device ofc, including the logging. Which I didn't bother to make compile-time optional, since none of the other drm logging is compile time optional either. One tricky bit here is the chicken&egg between allocating your drm_device structure and initiliazing it with drm_dev_init. For perfect onion unwinding we'd need to have the action to kfree the allocation registered before drm_dev_init registers any of its own release handlers. But drm_dev_init doesn't know where exactly the drm_device is emebedded into the overall structure, and by the time it returns it'll all be too late. And forcing drivers to be able clean up everything except the one kzalloc is silly. Work around this by having a very special final_kfree pointer. This also avoids troubles with the list head possibly disappearing from underneath us when we release all resources attached to the drm_device. v2: Do all the kerneldoc at the end, to avoid lots of fairly pointless shuffling while getting everything into shape. v3: Add static to add/del_dr (Neil) Move typo fix to the right patch (Neil) v4: Enforce contract for drmm_add_final_kfree: Use ksize() to check that the drm_device is indeed contained somewhere in the final kfree(). Because we need that or the entire managed release logic blows up in a pile of use-after-frees. Motivated by a discussion with Laurent. v5: Review from Laurent: - %zu instead of casting size_t - header guards - sorting of includes - guarding of data assignment if we didn't allocate it for a NULL pointer - delete spurious newline - cast void* data parameter correctly in ->release call, no idea how this even worked before v6: Review from Sam - Add the kerneldoc for the managed sub-struct back in, even if it doesn't show up in the generated html somehow. - Explain why __always_inline. - Fix bisectability around the final kfree() in drm_dev_relase(). This is just interim code which will disappear again. - Some whitespace polish. - Add debug output when drmm_add_action or drmm_kmalloc fail. v7: My bisectability fix wasn't up to par as noticed by smatch. v8: Remove unecessary {} around if else v9: Use kstrdup_const, which requires kfree_const and introducing a free_dr() helper (Thomas). v10: kfree_const goes boom on the plain "kmalloc" assignment, somehow we need to wrap that in kstrdup_const() too!! Also renumber revision log, I somehow reset it midway thruh. Reviewed-by: -
Daniel Vetter authored
For two reasons: - The driver core clears this already for us after we're unloaded in __device_release_driver(). - It's way too late, the drm_device ->release callback might massively outlive the underlying physical device, since a drm_device can be kept alive by open drm_file or well really anything else userspace is still hanging onto. So if we clear this ourselves, we should clear it in the pci ->remove callback, not in the drm_device ->release callback. Looking at git history this was fixed in the driver core with commit 0998d063 Author: Hans de Goede <hdegoede@redhat.com> Date: Wed May 23 00:09:34 2012 +0200 device-core: Ensure drvdata = NULL when no driver is bound v2: Cite the core fix in the commit message (Chris). v3: Fix commit message and unused variable warning (Jani). Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Daniel Vetter authored
slab does this already, and I want to use this in a memory allocation tracker in drm for stuff that's tied to the lifetime of a drm_device, not the underlying struct device. Kinda like devres, but for drm. Acked-by:
Andrew Morton <akpm@linux-foundation.org> Signed-off-by:
-
- 25 Mar, 2020 2 commits
-
-
git://people.freedesktop.org/~sroland/linuxDave Airlie authored
vmwgfx pull for for 5.7. Needed for GL4 functionality. Sync up device headers, add support for new commands, code refactoring around surface definition. Signed-off-by:
Dave Airlie <airlied@redhat.com> From: "Roland Scheidegger (VMware)" <rscheidegger.oss@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200323235434.11780-1-rscheidegger.oss@gmail.com
-
https://git.pengutronix.de/git/lst/linuxDave Airlie authored
- fix for potential out-of-bounds reads in the perfmon ioctl implementation from Christian - override to expose proper feature flags for the GC400 found on the STM32MP1 SoC, also from Christian - Guido fixed an issue where we would spuriously fail to enter runtime suspend due to a new GPU engine status bit on GC7000 - tree-wide change from Gustavo to get rid of zero-length arrays - fix for missed TS cache flush on GC7000, leading to spurious MMU faults from me - request pages from DMA32 zone on systems where we can't address all present memory from me Signed-off-by:
-
- 24 Mar, 2020 1 commit
-
-
Jernej Skrabec authored
is_color_space_conversion() is a misnomer. It checks not only if color space conversion is needed, but also if format conversion is needed. This is actually desired behaviour because result of this function determines if CSC block should be enabled or not (CSC block can also do format conversion). In order to clear misunderstandings, let's rework is_color_space_conversion() to do exactly what is supposed to do and add another function which will determine if CSC block must be enabled or not. Reviewed-by:
Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by:
Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200304232512.51616-5-jernej.skrabec@siol.net
-
