Most platforms with a Mali-400 or Mali-450 GPU also have support for
changing the GPU clock frequency. Add devfreq support so the GPU clock
rate is updated based on the actual GPU usage when the
"operating-points-v2" property is present in the board.dts.
The actual devfreq code is taken from panfrost_devfreq.c and modified so
it matches what the lima hardware needs:
- a call to dev_pm_opp_set_clkname() during initialization because there
are two clocks on Mali-4x0 IPs. "core" is the one that actually clocks
the GPU so we need to control it using devfreq.
- locking when reading or writing the devfreq statistics because (unlike
than panfrost) we have multiple PP and GP IRQs which may finish jobs
concurrently.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Qiang Yu <yuq825@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200319203427.2259891-3-martin.blumenstingl@googlemail.com
Currently we have one down reply message servicing the mst manager, so
we need to serialize all tx msgs to ensure we only have one message in
flight at a time. For obvious reasons this is suboptimal (but less
suboptimal than the free-for-all we had before serialization).
This patch removes the single down_rep_recv message from manager and
adds 2 replies in the branch structure. The 2 replies mirrors the tx_slots
which we use to rate-limit outgoing messages and correspond to seqno in
the packet headers.
Cc: Wayne Lin <Wayne.Lin@amd.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Wayne Lin <waynelin@amd.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200213211523.156998-3-sean@poorly.run
Currently drivers using drm_fbdev_generic_setup() end up with a single
empty aperture in their fb_info struct.
Not having the proper info in the apertures list causes
register_framebuffer to not remove conflicting framebuffers,
which some drivers currently workaround by manually calling
drm_fb_helper_remove_conflicting_pci_framebuffers().
Add a TODO as a reminder that we need to fix this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20200326151009.102377-1-hdegoede@redhat.com
All collected together to provide a consistent story in one patch,
instead of the somewhat bumpy refactor-evolution leading to this.
Also some thoughts on what the next steps could be:
- Create a macro called devm_drm_dev_alloc() which essentially wraps
the kzalloc(); devm_drm_dev_init(); drmm_add_final_kfree() combo.
Needs to be a macro since we'll have to do some typeof trickery and
casting to make this fully generic for all drivers that embed struct
drm_device into their own thing.
- A lot of the simple drivers now have essentially just
drm_dev_unplug(); drm_atomic_helper_shutdown(); as their
$bus_driver->remove hook. We could create a devm_mode_config_reset
which sets drm_atomic_helper_shutdown as it's cleanup action, and a
devm_drm_dev_register with drm_dev_unplug as it's cleanup action,
and simple drivers wouldn't have a need for a ->remove function at
all, and we could delete them.
- For more complicated drivers we need drmm_ versions of a _lot_ more
things. All the userspace visible objects (crtc, plane, encoder,
crtc), anything else hanging of those (maybe a drmm_get_edid, at
least for panels and other built-in stuff).
Also some more thoughts on why we're not reusing devm_ with maybe a
fake struct device embedded into the drm_device (we can't use the
kdev, since that's in each drm_minor).
- Code review gets extremely tricky, since every time you see a devm_
you need to carefully check whether the fake device (with the
drm_device lifetim) or the real device (with the lifetim of the
underlying physical device and driver binding) are used. That's not
going to help at all, and we have enormous amounts of drivers who
use devm_ where they really shouldn't. Having different types makes
sure the compiler type checks this for us and ensures correctness.
- The set of functions are very much non-overlapping. E.g.
devm_ioremap makes total sense, drmm_ioremap has the wrong lifetime,
since hw resources need to be cleaned out at driver unbind and wont
outlive that like a drm_device. Similar, but other way round for
drmm_connector_init (which is the only correct version, devm_ for
drm_connector is just buggy). Simply not having the wrong version
again prevents bugs.
Finally I guess this opens a huge todo for all the drivers. I'm
semi-tempted to do a tree-wide s/devm_kzalloc/drmm_kzalloc/ since most
likely that'll fix an enormous amount of bugs and most likely not
cause any issues at all (aside from maybe holding onto memory slightly
too long).
v2:
- Doc improvements from Laurent.
- Also add kerneldoc for the new drmm_add_action_or_reset.
v3:
- Remove kerneldoc for drmm_remove_action.
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
fixup docs
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-52-daniel.vetter@ffwll.ch
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)
I also noticed that I've failed to add the error checking,
__must_check caught that.
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org> (v2)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-47-daniel.vetter@ffwll.ch
Instead of having a work item that never stops (which really should be
a kthread), with a dedicated workqueue to not upset anyone else, use a
delayed work. A bunch of changes:
- We can throw out all the custom wakeup and requeue logic and state
tracking. If we schedule the work with a 0 delay it'll get
scheduled immediately.
- Persistent state (frame & draw_status_timeout) need to be moved out
of the work.
- diff is bigger than the changes, biggest chunk is reindenting the
work fn because it lost its while loop.
Lots of code deleting as consequence all over. Specifically we can
delete the drm_driver.release code now!
v2: Review from Hans:
- Use mod_delayed_work in the plane update path to make sure we do
actually schedule immediately). In the worker we still want
queue_delayed_work, which won't modify the timeout when the work is
already scheduled. Which is exactly what we want if the work races
with a plane update.
- Switch to system_long_wq, Hans says on usb2 a plane upload can take
80 ms.
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-46-daniel.vetter@ffwll.ch
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 ...
I'm pretty sure this one blows up already under KASAN because it's
using devm_drm_dev_init, and later on devm_kzalloc. Hence the memory
will get freed before the final drm_dev_put (all from the devres
code), but the cleanup in that final drm_dev_put will access the just
freed memory.
Unfortunately fixing this properly needs slightly more work, namely
drmm_ versions for all the drm objects (planes, crtc, ...), so that
the cleanup actually happens before even drmm_kzalloc would release
the underlying memory. Not quite there yet.
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>
Acked-by: Jyri Sarha <jsarha@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-42-daniel.vetter@ffwll.ch
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 <daniel.vetter@intel.com>
Cc: Eric Anholt <eric@anholt.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-36-daniel.vetter@ffwll.ch
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: Sam Ravnborg <sam@ravnborg.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-27-daniel.vetter@ffwll.ch
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: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-25-daniel.vetter@ffwll.ch
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: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200324203936.3330994-1-daniel.vetter@ffwll.ch
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 <daniel.vetter@intel.com>
Cc: Eric Anholt <eric@anholt.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-11-daniel.vetter@ffwll.ch
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: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-9-daniel.vetter@ffwll.ch
