[Bug 110886] After S3 resume, kernel: [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:57:crtc-0] flip_done timed out
https://bugs.freedesktop.org/show_bug.cgi?id=110886 --- Comment #5 from Alex Deucher --- Does disabling the IOMMU help? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110865] Rx480 consumes 20w more power in idle than under Windows
https://bugs.freedesktop.org/show_bug.cgi?id=110865 --- Comment #5 from Alex Deucher --- Created attachment 144978 --> https://bugs.freedesktop.org/attachment.cgi?id=144978=edit possible fix Does this patch fix the issue? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110865] Rx480 consumes 20w more power in idle than under Windows
https://bugs.freedesktop.org/show_bug.cgi?id=110865 --- Comment #4 from Alex Deucher --- (In reply to Martin from comment #3) > Thank you for your explanation. > How do I find out the blanking periods? They are based on the timing for the mode on the display. As for the relevant driver code, take a look at smu7_apply_state_adjust_rules(). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: The issue with page allocation 5.3 rc1-rc2 (seems drm culprit here)
On Wed, Aug 7, 2019 at 11:49 PM Mikhail Gavrilov
wrote:
>
> On Tue, 6 Aug 2019 at 06:48, Hillf Danton wrote:
> >
> > My bad, respin with one header file added.
> >
> > Hillf
> > -8<---
> >
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> > @@ -23,6 +23,7 @@
> > */
> >
> > #include
> > +#include
> >
> > #include "dm_services.h"
> >
> > @@ -1174,8 +1175,12 @@ struct dc_state *dc_create_state(struct
> > struct dc_state *context = kzalloc(sizeof(struct dc_state),
> >GFP_KERNEL);
> >
> > - if (!context)
> > - return NULL;
> > + if (!context) {
> > + context = kvzalloc(sizeof(struct dc_state),
> > + GFP_KERNEL);
> > + if (!context)
> > + return NULL;
> > + }
> > /* Each context must have their own instance of VBA and in order to
> > * initialize and obtain IP and SOC the base DML instance from DC is
> > * initially copied into every context
> > @@ -1195,8 +1200,13 @@ struct dc_state *dc_copy_state(struct dc
> > struct dc_state *new_ctx = kmemdup(src_ctx,
> > sizeof(struct dc_state), GFP_KERNEL);
> >
> > - if (!new_ctx)
> > - return NULL;
> > + if (!new_ctx) {
> > + new_ctx = kvmalloc(sizeof(*new_ctx), GFP_KERNEL);
> > + if (new_ctx)
> > + *new_ctx = *src_ctx;
> > + else
> > + return NULL;
> > + }
> >
> > for (i = 0; i < MAX_PIPES; i++) {
> > struct pipe_ctx *cur_pipe =
> > _ctx->res_ctx.pipe_ctx[i];
> > @@ -1230,7 +1240,7 @@ static void dc_state_free(struct kref *k
> > {
> > struct dc_state *context = container_of(kref, struct dc_state,
> > refcount);
> > dc_resource_state_destruct(context);
> > - kfree(context);
> > + kvfree(context);
> > }
> >
> > void dc_release_state(struct dc_state *context)
> > --
> >
>
> Unfortunately error "gnome-shell: page allocation failure: order:4,
> mode:0x40cc0(GFP_KERNEL|__GFP_COMP),
> nodemask=(null),cpuset=/,mems_allowed=0" still happens even with
> applying this patch.
I think we can just drop the kmalloc altogether. How about this patch?
Alex
>
> Thanks.
>
>
> --
> Best Regards,
> Mike Gavrilov.
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
From c3ba6f05ca3e0371254fbfb1a8c06274e3cdb96e Mon Sep 17 00:00:00 2001
From: Alex Deucher
Date: Thu, 8 Aug 2019 00:29:23 -0500
Subject: [PATCH] drm/amd/display: use kvmalloc for dc_state
It's large and doesn't need contiguous memory.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 252b621d93a9..ef780a4e484a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -23,6 +23,7 @@
*/
#include
+#include
#include "dm_services.h"
@@ -1183,8 +1184,8 @@ bool dc_post_update_surfaces_to_stream(struct dc *dc)
struct dc_state *dc_create_state(struct dc *dc)
{
- struct dc_state *context = kzalloc(sizeof(struct dc_state),
- GFP_KERNEL);
+ struct dc_state *context = kvzalloc(sizeof(struct dc_state),
+ GFP_KERNEL);
if (!context)
return NULL;
@@ -1204,11 +1205,11 @@ struct dc_state *dc_create_state(struct dc *dc)
struct dc_state *dc_copy_state(struct dc_state *src_ctx)
{
int i, j;
- struct dc_state *new_ctx = kmemdup(src_ctx,
- sizeof(struct dc_state), GFP_KERNEL);
+ struct dc_state *new_ctx = kvmalloc(sizeof(struct dc_state), GFP_KERNEL);
if (!new_ctx)
return NULL;
+ memcpy(new_ctx, src_ctx, sizeof(struct dc_state));
for (i = 0; i < MAX_PIPES; i++) {
struct pipe_ctx *cur_pipe = _ctx->res_ctx.pipe_ctx[i];
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: Add better overview documentation for syncobj (v2)
Christan, any thoughts on v2? --Jason On August 7, 2019 09:06:47 Lionel Landwerlin wrote: On 06/08/2019 19:19, Jason Ekstrand wrote: This patch only brings the syncobj documentation up-to-date for the original form of syncobj. It does not contain any information about the design of timeline syncobjs. v2: Incorporate feedback from Lionel and Christian: - Mention actual ioctl and flag names - Better language around reference counting - Misc. language cleanups Signed-off-by: Jason Ekstrand Reviewed-by: Lionel Landwerlin --- drivers/gpu/drm/drm_syncobj.c | 98 +++ 1 file changed, 87 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 1438dcb3ebb1..4b5c7b0ed714 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -29,21 +29,97 @@ /** * DOC: Overview * - * DRM synchronisation objects (syncobj, see struct _syncobj) are - * persistent objects that contain an optional fence. The fence can be updated - * with a new fence, or be NULL. + * DRM synchronisation objects (syncobj, see struct _syncobj) provide a + * container for a synchronization primitive which can be used by userspace + * to explicitly synchronize GPU commands, can be shared between userspace + * processes, and can be shared between different DRM drivers. + * Their primary use-case is to implement Vulkan fences and semaphores. + * The syncobj userspace API provides ioctls for several operations: * - * syncobj's can be waited upon, where it will wait for the underlying - * fence. + * - Creation and destruction of syncobjs + * - Import and export of syncobjs to/from a syncobj file descriptor + * - Import and export a syncobj's underlying fence to/from a sync file + * - Reset a syncobj (set its fence to NULL) + * - Signal a syncobj (set a trivially signaled fence) + * - Wait for a syncobj's fence to appear and be signaled * - * syncobj's can be export to fd's and back, these fd's are opaque and - * have no other use case, except passing the syncobj between processes. + * At it's core, a syncobj is simply a wrapper around a pointer to a struct + * _fence which may be NULL. + * When a syncobj is first created, its pointer is either NULL or a pointer + * to an already signaled fence depending on whether the + * _SYNCOBJ_CREATE_SIGNALED flag is passed to + * _IOCTL_SYNCOBJ_CREATE. + * When GPU work which signals a syncobj is enqueued in a DRM driver, + * the syncobj fence is replaced with a fence which will be signaled by the + * completion of that work. + * When GPU work which waits on a syncobj is enqueued in a DRM driver, the + * driver retrieves syncobj's current fence at the time the work is enqueued + * waits on that fence before submitting the work to hardware. + * If the syncobj's fence is NULL, the enqueue operation is expected to fail. + * All manipulation of the syncobjs's fence happens in terms of the current + * fence at the time the ioctl is called by userspace regardless of whether + * that operation is an immediate host-side operation (signal or reset) or + * or an operation which is enqueued in some driver queue. + * _IOCTL_SYNCOBJ_RESET and _IOCTL_SYNCOBJ_SIGNAL can be used to + * manipulate a syncobj from the host by resetting its pointer to NULL or + * setting its pointer to a fence which is already signaled. * - * Their primary use-case is to implement Vulkan fences and semaphores. * - * syncobj have a kref reference count, but also have an optional file. - * The file is only created once the syncobj is exported. - * The file takes a reference on the kref. + * Host-side wait on syncobjs + * -- + * + * _IOCTL_SYNCOBJ_WAIT takes an array of syncobj handles and does a + * host-side wait on all of the syncobj fences simultaneously. + * If _SYNCOBJ_WAIT_FLAGS_WAIT_ALL is set, the wait ioctl will wait on + * all of the syncobj fences to be signaled before it returns. + * Otherwise, it returns once at least one syncobj fence has been signaled + * and the index of a signaled fence is written back to the client. + * + * Unlike the enqueued GPU work dependencies which fail if they see a NULL + * fence in a syncobj, if _SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT is set, + * the host-side wait will first wait for the syncobj to receive a non-NULL + * fence and then wait on that fence. + * If _SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT is not set and any one of the + * syncobjs in the array has a NULL fence, -EINVAL will be returned. + * Assuming the syncobj starts off with a NULL fence, this allows a client + * to do a host wait in one thread (or process) which waits on GPU work + * submitted in another thread (or process) without having to manually + * synchronize between the two. + * This requirement is inherited from the Vulkan fence API. + * + * + * Import/export of syncobjs + * - + * + * _IOCTL_SYNCOBJ_FD_TO_HANDLE and
Re: [PATCH 00/34] put_user_pages(): miscellaneous call sites
On Wed, Aug 07, 2019 at 10:46:49AM +0200, Michal Hocko wrote:
> On Wed 07-08-19 10:37:26, Jan Kara wrote:
> > On Fri 02-08-19 12:14:09, John Hubbard wrote:
> > > On 8/2/19 7:52 AM, Jan Kara wrote:
> > > > On Fri 02-08-19 07:24:43, Matthew Wilcox wrote:
> > > > > On Fri, Aug 02, 2019 at 02:41:46PM +0200, Jan Kara wrote:
> > > > > > On Fri 02-08-19 11:12:44, Michal Hocko wrote:
> > > > > > > On Thu 01-08-19 19:19:31, john.hubb...@gmail.com wrote:
> > > > > > > [...]
> > > > > > > > 2) Convert all of the call sites for get_user_pages*(), to
> > > > > > > > invoke put_user_page*(), instead of put_page(). This involves
> > > > > > > > dozens of
> > > > > > > > call sites, and will take some time.
> > > > > > >
> > > > > > > How do we make sure this is the case and it will remain the case
> > > > > > > in the
> > > > > > > future? There must be some automagic to enforce/check that. It is
> > > > > > > simply
> > > > > > > not manageable to do it every now and then because then 3) will
> > > > > > > simply
> > > > > > > be never safe.
> > > > > > >
> > > > > > > Have you considered coccinele or some other scripted way to do the
> > > > > > > transition? I have no idea how to deal with future changes that
> > > > > > > would
> > > > > > > break the balance though.
> > >
> > > Hi Michal,
> > >
> > > Yes, I've thought about it, and coccinelle falls a bit short (it's not
> > > smart
> > > enough to know which put_page()'s to convert). However, there is a debug
> > > option planned: a yet-to-be-posted commit [1] uses struct page extensions
> > > (obviously protected by CONFIG_DEBUG_GET_USER_PAGES_REFERENCES) to add
> > > a redundant counter. That allows:
> > >
> > > void __put_page(struct page *page)
> > > {
> > > ...
> > > /* Someone called put_page() instead of put_user_page() */
> > > WARN_ON_ONCE(atomic_read(_ext->pin_count) > 0);
> > >
> > > > > >
> > > > > > Yeah, that's why I've been suggesting at LSF/MM that we may need to
> > > > > > create
> > > > > > a gup wrapper - say vaddr_pin_pages() - and track which sites
> > > > > > dropping
> > > > > > references got converted by using this wrapper instead of gup. The
> > > > > > counterpart would then be more logically named as unpin_page() or
> > > > > > whatever
> > > > > > instead of put_user_page(). Sure this is not completely foolproof
> > > > > > (you can
> > > > > > create new callsite using vaddr_pin_pages() and then just drop refs
> > > > > > using
> > > > > > put_page()) but I suppose it would be a high enough barrier for
> > > > > > missed
> > > > > > conversions... Thoughts?
> > >
> > > The debug option above is still a bit simplistic in its implementation
> > > (and maybe not taking full advantage of the data it has), but I think
> > > it's preferable, because it monitors the "core" and WARNs.
> > >
> > > Instead of the wrapper, I'm thinking: documentation and the passage of
> > > time, plus the debug option (perhaps enhanced--probably once I post it
> > > someone will notice opportunities), yes?
> >
> > So I think your debug option and my suggested renaming serve a bit
> > different purposes (and thus both make sense). If you do the renaming, you
> > can just grep to see unconverted sites. Also when someone merges new GUP
> > user (unaware of the new rules) while you switch GUP to use pins instead of
> > ordinary references, you'll get compilation error in case of renaming
> > instead of hard to debug refcount leak without the renaming. And such
> > conflict is almost bound to happen given the size of GUP patch set... Also
> > the renaming serves against the "coding inertia" - i.e., GUP is around for
> > ages so people just use it without checking any documentation or comments.
> > After switching how GUP works, what used to be correct isn't anymore so
> > renaming the function serves as a warning that something has really
> > changed.
>
> Fully agreed!
Ok Prior to this I've been basing all my work for the RDMA/FS DAX stuff in
Johns put_user_pages()... (Including when I proposed failing truncate with a
lease in June [1])
However, based on the suggestions in that thread it became clear that a new
interface was going to need to be added to pass in the "RDMA file" information
to GUP to associate file pins with the correct processes...
I have many drawings on my white board with "a whole lot of lines" on them to
make sure that if a process opens a file, mmaps it, pins it with RDMA, _closes_
it, and ummaps it; that the resulting file pin can still be traced back to the
RDMA context and all the processes which may have access to it No matter
where the original context may have come from. I believe I have accomplished
that.
Before I go on, I would like to say that the "imbalance" of get_user_pages()
and put_page() bothers me from a purist standpoint... However, since this
discussion cropped up I went ahead and ported my work to Linus' current master
(5.3-rc3+) and in doing so I only had to steal a bit of Johns
Re: [PATCHv2 2/3] i915: convert to new mount API
On Wed, Aug 07, 2019 at 08:30:02AM +0200, Christoph Hellwig wrote: > On Tue, Aug 06, 2019 at 12:50:10AM -0700, Hugh Dickins wrote: > > Though personally I'm averse to managing "f"objects through > > "m"interfaces, which can get ridiculous (notably, MADV_HUGEPAGE works > > on the virtual address of a mapping, but the huge-or-not alignment of > > that mapping must have been decided previously). In Google we do use > > fcntls F_HUGEPAGE and F_NOHUGEPAGE to override on a per-file basis - > > one day I'll get to upstreaming those. > > Such an interface seems very useful, although the two fcntls seem a bit > odd. > > But I think the point here is that the i915 has its own somewhat odd > instance of tmpfs. If we could pass the equivalent of the huge=* > options to shmem_file_setup all that garbage (including the > shmem_file_setup_with_mnt function) could go away. ... or follow shmem_file_super() with whatever that fcntl maps to internally. I would really love to get rid of that i915 kludge.
Re: [PATCHv2 2/3] i915: convert to new mount API
On Tue, Aug 06, 2019 at 12:50:10AM -0700, Hugh Dickins wrote: > that mapping must have been decided previously). In Google we do use > fcntls F_HUGEPAGE and F_NOHUGEPAGE to override on a per-file basis - > one day I'll get to upstreaming those. That'd be nice - we could kill the i915 wierd private shmem instance, along with some kludges in mm/shmem.c.
[PATCH v2 2/2] drm/nouveau/dispnv50: Fix runtime PM ref tracking for non-blocking modesets
This is something that got noticed a while ago back when I was fixing a
large number of runtime PM related issues in nouveau, but never got
fixed:
https://patchwork.freedesktop.org/series/46815/#rev7
It's not safe to iterate the entire list of CRTCs in
nv50_disp_atomic_commit(), as we could be doing a non-blocking modeset
on one CRTC in parallel with one or more other CRTCs. Likewise, this
means it's also not safe to do so in order to track runtime PM state.
While this code is certainly wrong, so far the only issues I've seen
this cause in the wild is the occasional PM ref unbalance after an
atomic check failure + module reloading (since the PCI device will
outlive nouveau in such scenarios).
So, do this far more elegantly: grab a runtime PM ref across the modeset
and commit tail, then grab/put references for each CRTC enable/disable.
This also ends up being much simpler then the previous broken solution
we had.
Finally, since we've removed all it's users: get rid of
nouveau_drm->have_disp_power_ref.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 38 +++--
drivers/gpu/drm/nouveau/nouveau_drv.h | 3 --
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 126703816794..659e6fa645cb 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1826,8 +1826,11 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
NV_ATOMIC(drm, "%s: clr %04x (set %04x)\n", crtc->name,
asyh->clr.mask, asyh->set.mask);
- if (old_crtc_state->active && !new_crtc_state->active)
+
+ if (old_crtc_state->active && !new_crtc_state->active) {
+ pm_runtime_put_noidle(dev->dev);
drm_crtc_vblank_off(crtc);
+ }
if (asyh->clr.mask) {
nv50_head_flush_clr(head, asyh, atom->flush_disable);
@@ -1913,8 +1916,10 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
}
if (new_crtc_state->active) {
- if (!old_crtc_state->active)
+ if (!old_crtc_state->active) {
drm_crtc_vblank_on(crtc);
+ pm_runtime_get_noresume(dev->dev);
+ }
if (new_crtc_state->event)
drm_crtc_vblank_get(crtc);
}
@@ -1979,6 +1984,10 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
drm_atomic_helper_cleanup_planes(dev, state);
drm_atomic_helper_commit_cleanup_done(state);
drm_atomic_state_put(state);
+
+ /* Drop the RPM ref we got from nv50_disp_atomic_commit() */
+ pm_runtime_mark_last_busy(dev->dev);
+ pm_runtime_put_autosuspend(dev->dev);
}
static void
@@ -1993,11 +2002,8 @@ static int
nv50_disp_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state, bool nonblock)
{
- struct nouveau_drm *drm = nouveau_drm(dev);
struct drm_plane_state *new_plane_state;
struct drm_plane *plane;
- struct drm_crtc *crtc;
- bool active = false;
int ret, i;
ret = pm_runtime_get_sync(dev->dev);
@@ -2034,27 +2040,17 @@ nv50_disp_atomic_commit(struct drm_device *dev,
drm_atomic_state_get(state);
+ /*
+* Grab another RPM ref for the commit tail, which will release the
+* ref when it's finished
+*/
+ pm_runtime_get_noresume(dev->dev);
+
if (nonblock)
queue_work(system_unbound_wq, >commit_work);
else
nv50_disp_atomic_commit_tail(state);
- drm_for_each_crtc(crtc, dev) {
- if (crtc->state->active) {
- if (!drm->have_disp_power_ref) {
- drm->have_disp_power_ref = true;
- return 0;
- }
- active = true;
- break;
- }
- }
-
- if (!active && drm->have_disp_power_ref) {
- pm_runtime_put_autosuspend(dev->dev);
- drm->have_disp_power_ref = false;
- }
-
err_cleanup:
if (ret)
drm_atomic_helper_cleanup_planes(dev, state);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index aae035816383..411352dd5390 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -204,9 +204,6 @@ struct nouveau_drm {
/* led management */
struct nouveau_led *led;
- /* display power reference */
- bool have_disp_power_ref;
-
struct dev_pm_domain vga_pm_domain;
struct nouveau_svm *svm;
--
2.21.0
[PATCH v2 1/2] drm/nouveau/dispnv04: Remove runtime PM
Originally when trying to fix the issue of runtime PM references with
non-blocking CRTCs on nv50, I ended up stumbling on this code when
trying to remove nouveau_drm->have_disp_power_ref, and attempted to fix
it to remove the dependency on have_disp_power_ref. However, Ilia Mirkin
pointed out that this code is actually completely useless, as pre-nv50
never had runtime PM support in the first place! Go figure.
So, since it's useless just get rid of it. Note that since the only
thing nouveau_crtc_set_config() was doing was grabbing a runtime PM ref,
calling drm_crtc_helper_set_config() then dropping the ref; we can just
remove the function entirely and just call drm_crtc_helper_set_config()
directly.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 51 +
1 file changed, 1 insertion(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index f22f01020625..050eb5b7dd13 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -22,8 +22,6 @@
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*/
-#include
-
#include
#include
#include
@@ -1031,53 +1029,6 @@ nv04_crtc_cursor_move(struct drm_crtc *crtc, int x, int
y)
return 0;
}
-static int
-nouveau_crtc_set_config(struct drm_mode_set *set,
- struct drm_modeset_acquire_ctx *ctx)
-{
- struct drm_device *dev;
- struct nouveau_drm *drm;
- int ret;
- struct drm_crtc *crtc;
- bool active = false;
- if (!set || !set->crtc)
- return -EINVAL;
-
- dev = set->crtc->dev;
-
- /* get a pm reference here */
- ret = pm_runtime_get_sync(dev->dev);
- if (ret < 0 && ret != -EACCES)
- return ret;
-
- ret = drm_crtc_helper_set_config(set, ctx);
-
- drm = nouveau_drm(dev);
-
- /* if we get here with no crtcs active then we can drop a reference */
- list_for_each_entry(crtc, >mode_config.crtc_list, head) {
- if (crtc->enabled)
- active = true;
- }
-
- pm_runtime_mark_last_busy(dev->dev);
- /* if we have active crtcs and we don't have a power ref,
- take the current one */
- if (active && !drm->have_disp_power_ref) {
- drm->have_disp_power_ref = true;
- return ret;
- }
- /* if we have no active crtcs, then drop the power ref
- we got before */
- if (!active && drm->have_disp_power_ref) {
- pm_runtime_put_autosuspend(dev->dev);
- drm->have_disp_power_ref = false;
- }
- /* drop the power reference we got coming in here */
- pm_runtime_put_autosuspend(dev->dev);
- return ret;
-}
-
struct nv04_page_flip_state {
struct list_head head;
struct drm_pending_vblank_event *event;
@@ -1293,7 +1244,7 @@ static const struct drm_crtc_funcs nv04_crtc_funcs = {
.cursor_set = nv04_crtc_cursor_set,
.cursor_move = nv04_crtc_cursor_move,
.gamma_set = nv_crtc_gamma_set,
- .set_config = nouveau_crtc_set_config,
+ .set_config = drm_crtc_helper_set_config,
.page_flip = nv04_crtc_page_flip,
.destroy = nv_crtc_destroy,
};
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/2] drm/nouveau: CRTC Runtime PM ref tracking fixes
Just some runtime PM fixes for some much less noticeable runtime PM ref tracking issues that I got reminded of when fixing some unrelated issues with nouveau. Changes since v1: * Don't fix CRTC RPM code in dispnv04, because it's not actually doing anything in the first place. Just get rid of it. - imirkin Lyude Paul (2): drm/nouveau/dispnv04: Remove runtime PM drm/nouveau/dispnv50: Fix runtime PM ref tracking for non-blocking modesets drivers/gpu/drm/nouveau/dispnv04/crtc.c | 51 + drivers/gpu/drm/nouveau/dispnv50/disp.c | 38 +- drivers/gpu/drm/nouveau/nouveau_drv.h | 3 -- 3 files changed, 18 insertions(+), 74 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/nouveau/dispnv04: Grab/put runtime PM refs on DPMS on/off
On Wed, 2019-08-07 at 19:06 -0400, Ilia Mirkin wrote:
> On Wed, Aug 7, 2019 at 5:55 PM Daniel Vetter wrote:
> > On Wed, Aug 07, 2019 at 05:33:00PM -0400, Lyude Paul wrote:
> > > The code claims to grab a runtime PM ref when at least one CRTC is
> > > active, but that's not actually the case as we grab a runtime PM ref
> > > whenever a CRTC is enabled regardless of it's DPMS state. Meaning that
> > > we can end up keeping the GPU awake when there are no screens enabled,
> > > something we don't really want to do.
> > >
> > > Note that we fixed this same issue for nv50 a while ago in:
> > >
> > > commit e5d54f193572 ("drm/nouveau/drm/nouveau: Fix runtime PM leak in
> > > nv50_disp_atomic_commit()")
> > >
> > > Since we're about to remove nouveau_drm->have_disp_power_ref in the next
> > > commit, let's also simplify the RPM code here while we're at it: grab a
> > > ref during a modeset, grab additional RPM refs for each CRTC enabled by
> > > said modeset, and drop an RPM ref for each CRTC disabled by said
> > > modeset. This allows us to keep the GPU awake whenever screens are
> > > turned on, without needing to use nouveau_drm->have_disp_power_ref.
> > >
> > > Signed-off-by: Lyude Paul
> > > ---
> > > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 18 --
> > > 1 file changed, 4 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > > b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > > index f22f01020625..08ad8e3b9cd2 100644
> > > --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > > +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > > @@ -183,6 +183,10 @@ nv_crtc_dpms(struct drm_crtc *crtc, int mode)
> > > return;
> > >
> > > nv_crtc->last_dpms = mode;
> > > + if (mode == DRM_MODE_DPMS_ON)
> > > + pm_runtime_get_noresume(dev->dev);
> > > + else
> > > + pm_runtime_put_noidle(dev->dev);
> >
> > it's after we filter out duplicate operations, so that part looks good.
> > But not all of nouveau's legacy helper crtc callbacks go throuh ->dpms I
> > think: nv_crtc_disable doesn't, and crtc helpers use that in preference
> > over ->dpms in some cases.
> >
> > I think the only way to actually hit that path is if you switch an active
> > connector from an active CRTC to an inactive one. This implicitly disables
> > the crtc (well, should, nv_crtc_disable doesn't seem to shut down hw), and
> > I think would leak your runtime PM reference here. At least temporarily.
> >
> > No idea how to best fix that. Aside from "use atomic" :-)
>
> Not sure if this is relevant to the discussion at hand, but I'd like
> to point out that dispnv04 is for pre-nv50 things, which definitely
> didn't support any kind of ACPI-based runtime suspend.
I thought it was really suspicious that such an old chipset had any kind of
runtime PM, but didn't question the code lol. I guess a more appropriate patch
would be to just remove runtime PM support entirely for pre-nv50. Will respin
soon and do this.
>
> -ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/nouveau/dispnv04: Grab/put runtime PM refs on DPMS on/off
On Wed, Aug 7, 2019 at 5:55 PM Daniel Vetter wrote:
>
> On Wed, Aug 07, 2019 at 05:33:00PM -0400, Lyude Paul wrote:
> > The code claims to grab a runtime PM ref when at least one CRTC is
> > active, but that's not actually the case as we grab a runtime PM ref
> > whenever a CRTC is enabled regardless of it's DPMS state. Meaning that
> > we can end up keeping the GPU awake when there are no screens enabled,
> > something we don't really want to do.
> >
> > Note that we fixed this same issue for nv50 a while ago in:
> >
> > commit e5d54f193572 ("drm/nouveau/drm/nouveau: Fix runtime PM leak in
> > nv50_disp_atomic_commit()")
> >
> > Since we're about to remove nouveau_drm->have_disp_power_ref in the next
> > commit, let's also simplify the RPM code here while we're at it: grab a
> > ref during a modeset, grab additional RPM refs for each CRTC enabled by
> > said modeset, and drop an RPM ref for each CRTC disabled by said
> > modeset. This allows us to keep the GPU awake whenever screens are
> > turned on, without needing to use nouveau_drm->have_disp_power_ref.
> >
> > Signed-off-by: Lyude Paul
> > ---
> > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 18 --
> > 1 file changed, 4 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > index f22f01020625..08ad8e3b9cd2 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > @@ -183,6 +183,10 @@ nv_crtc_dpms(struct drm_crtc *crtc, int mode)
> > return;
> >
> > nv_crtc->last_dpms = mode;
> > + if (mode == DRM_MODE_DPMS_ON)
> > + pm_runtime_get_noresume(dev->dev);
> > + else
> > + pm_runtime_put_noidle(dev->dev);
>
> it's after we filter out duplicate operations, so that part looks good.
> But not all of nouveau's legacy helper crtc callbacks go throuh ->dpms I
> think: nv_crtc_disable doesn't, and crtc helpers use that in preference
> over ->dpms in some cases.
>
> I think the only way to actually hit that path is if you switch an active
> connector from an active CRTC to an inactive one. This implicitly disables
> the crtc (well, should, nv_crtc_disable doesn't seem to shut down hw), and
> I think would leak your runtime PM reference here. At least temporarily.
>
> No idea how to best fix that. Aside from "use atomic" :-)
Not sure if this is relevant to the discussion at hand, but I'd like
to point out that dispnv04 is for pre-nv50 things, which definitely
didn't support any kind of ACPI-based runtime suspend.
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110214] Raven Ridge (2400G): xterm scrollback buffer disappears while Shift+PgUp and Shift+PgDn
https://bugs.freedesktop.org/show_bug.cgi?id=110214 --- Comment #106 from Diego Viola --- (In reply to Pierre-Eric Pelloux-Prayer from comment #105) > The commit fixing this issue has changed a bit, it would be great if you > could confirm the latest version of the MR works well for you. > > Thanks! Yes, I can confirm it's working great with the latest version. Thank you! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: fix VOP_WIN_GET macro
Am Mittwoch, 3. Juli 2019, 11:51:11 CEST schrieb John Keeping:
> Commit 9a61c54b9bff ("drm/rockchip: vop: group vop registers") seems to
> have unintentionally changed the defintion of this macro. Since it is
> unused, this was not spotted but any attempt to use it results in
> compilation errors.
>
> Revert to the previous definition.
>
> Fixes: 9a61c54b9bff ("drm/rockchip: vop: group vop registers")
> Signed-off-by: John Keeping
applied to drm-misc-next
Thanks
Heiko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()"
On Wed, Aug 7, 2019 at 3:01 PM Daniel Vetter wrote: > > On Wed, Aug 07, 2019 at 10:52:47AM -0400, Sean Paul wrote: > > From: Rob Herring > > > > This reverts commit 220df83a5394fbf7c1486ba7848794b7b351d598. > > > > Turns out drm_gem_dumb_map_offset really only worked for the dumb buffer > > case, so revert the name change. > > > > Signed-off-by: Rob Herring > > Signed-off-by: Sean Paul > > This part of the series seems unecessary to revert? Already committed, so let me go revert that right now for you. I can only hope that's enough to get my commit rights revoked too. :) > iow I still like this, and Im trying to sell Gerd Hoffmann on it for some > of his ttm refactoring ... revert of the revert of the revert of the > revert? Probably better if Gerd cherry-picks into his series (if my > suggestion works out) and maybe references this entire chain for > entertainment purposes :-) > -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/nouveau/dispnv04: Grab/put runtime PM refs on DPMS on/off
On Wed, Aug 07, 2019 at 05:33:00PM -0400, Lyude Paul wrote:
> The code claims to grab a runtime PM ref when at least one CRTC is
> active, but that's not actually the case as we grab a runtime PM ref
> whenever a CRTC is enabled regardless of it's DPMS state. Meaning that
> we can end up keeping the GPU awake when there are no screens enabled,
> something we don't really want to do.
>
> Note that we fixed this same issue for nv50 a while ago in:
>
> commit e5d54f193572 ("drm/nouveau/drm/nouveau: Fix runtime PM leak in
> nv50_disp_atomic_commit()")
>
> Since we're about to remove nouveau_drm->have_disp_power_ref in the next
> commit, let's also simplify the RPM code here while we're at it: grab a
> ref during a modeset, grab additional RPM refs for each CRTC enabled by
> said modeset, and drop an RPM ref for each CRTC disabled by said
> modeset. This allows us to keep the GPU awake whenever screens are
> turned on, without needing to use nouveau_drm->have_disp_power_ref.
>
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 18 --
> 1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index f22f01020625..08ad8e3b9cd2 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -183,6 +183,10 @@ nv_crtc_dpms(struct drm_crtc *crtc, int mode)
> return;
>
> nv_crtc->last_dpms = mode;
> + if (mode == DRM_MODE_DPMS_ON)
> + pm_runtime_get_noresume(dev->dev);
> + else
> + pm_runtime_put_noidle(dev->dev);
it's after we filter out duplicate operations, so that part looks good.
But not all of nouveau's legacy helper crtc callbacks go throuh ->dpms I
think: nv_crtc_disable doesn't, and crtc helpers use that in preference
over ->dpms in some cases.
I think the only way to actually hit that path is if you switch an active
connector from an active CRTC to an inactive one. This implicitly disables
the crtc (well, should, nv_crtc_disable doesn't seem to shut down hw), and
I think would leak your runtime PM reference here. At least temporarily.
No idea how to best fix that. Aside from "use atomic" :-)
Cheers, Daniel
>
> if (nv_two_heads(dev))
> NVSetOwner(dev, nv_crtc->index);
> @@ -1045,7 +1049,6 @@ nouveau_crtc_set_config(struct drm_mode_set *set,
>
> dev = set->crtc->dev;
>
> - /* get a pm reference here */
> ret = pm_runtime_get_sync(dev->dev);
> if (ret < 0 && ret != -EACCES)
> return ret;
> @@ -1061,19 +1064,6 @@ nouveau_crtc_set_config(struct drm_mode_set *set,
> }
>
> pm_runtime_mark_last_busy(dev->dev);
> - /* if we have active crtcs and we don't have a power ref,
> -take the current one */
> - if (active && !drm->have_disp_power_ref) {
> - drm->have_disp_power_ref = true;
> - return ret;
> - }
> - /* if we have no active crtcs, then drop the power ref
> -we got before */
> - if (!active && drm->have_disp_power_ref) {
> - pm_runtime_put_autosuspend(dev->dev);
> - drm->have_disp_power_ref = false;
> - }
> - /* drop the power reference we got coming in here */
> pm_runtime_put_autosuspend(dev->dev);
> return ret;
> }
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110214] Raven Ridge (2400G): xterm scrollback buffer disappears while Shift+PgUp and Shift+PgDn
https://bugs.freedesktop.org/show_bug.cgi?id=110214 --- Comment #105 from Pierre-Eric Pelloux-Prayer --- The commit fixing this issue has changed a bit, it would be great if you could confirm the latest version of the MR works well for you. Thanks! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111244] amdgpu kernel 5.2 blank display after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=111244 --- Comment #23 from Samuele Decarli --- amdgpu.dc=1 had no effect on my machine. On my computer resume fails quite consistently Any idea on what should be done to fix this, or even what is the cause? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/nouveau/dispnv50: Fix runtime PM ref tracking for non-blocking modesets
This is something that got noticed a while ago back when I was fixing a
large number of runtime PM related issues in nouveau, but never got
fixed:
https://patchwork.freedesktop.org/series/46815/#rev7
It's not safe to iterate the entire list of CRTCs in
nv50_disp_atomic_commit(), as we could be doing a non-blocking modeset
on one CRTC in parallel with one or more other CRTCs. Likewise, this
means it's also not safe to do so in order to track runtime PM state.
While this code is certainly wrong, so far the only issues I've seen
this cause in the wild is the occasional PM ref unbalance after an
atomic check failure + module reloading (since the PCI device will
outlive nouveau in such scenarios).
So, do this far more elegantly: grab a runtime PM ref across the modeset
and commit tail, then grab/put references for each CRTC enable/disable.
This also ends up being much simpler then the previous broken solution
we had.
Finally, since we've removed all it's users: get rid of
nouveau_drm->have_disp_power_ref.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 38 +++--
drivers/gpu/drm/nouveau/nouveau_drv.h | 3 --
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 126703816794..659e6fa645cb 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1826,8 +1826,11 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
NV_ATOMIC(drm, "%s: clr %04x (set %04x)\n", crtc->name,
asyh->clr.mask, asyh->set.mask);
- if (old_crtc_state->active && !new_crtc_state->active)
+
+ if (old_crtc_state->active && !new_crtc_state->active) {
+ pm_runtime_put_noidle(dev->dev);
drm_crtc_vblank_off(crtc);
+ }
if (asyh->clr.mask) {
nv50_head_flush_clr(head, asyh, atom->flush_disable);
@@ -1913,8 +1916,10 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
}
if (new_crtc_state->active) {
- if (!old_crtc_state->active)
+ if (!old_crtc_state->active) {
drm_crtc_vblank_on(crtc);
+ pm_runtime_get_noresume(dev->dev);
+ }
if (new_crtc_state->event)
drm_crtc_vblank_get(crtc);
}
@@ -1979,6 +1984,10 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
drm_atomic_helper_cleanup_planes(dev, state);
drm_atomic_helper_commit_cleanup_done(state);
drm_atomic_state_put(state);
+
+ /* Drop the RPM ref we got from nv50_disp_atomic_commit() */
+ pm_runtime_mark_last_busy(dev->dev);
+ pm_runtime_put_autosuspend(dev->dev);
}
static void
@@ -1993,11 +2002,8 @@ static int
nv50_disp_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state, bool nonblock)
{
- struct nouveau_drm *drm = nouveau_drm(dev);
struct drm_plane_state *new_plane_state;
struct drm_plane *plane;
- struct drm_crtc *crtc;
- bool active = false;
int ret, i;
ret = pm_runtime_get_sync(dev->dev);
@@ -2034,27 +2040,17 @@ nv50_disp_atomic_commit(struct drm_device *dev,
drm_atomic_state_get(state);
+ /*
+* Grab another RPM ref for the commit tail, which will release the
+* ref when it's finished
+*/
+ pm_runtime_get_noresume(dev->dev);
+
if (nonblock)
queue_work(system_unbound_wq, >commit_work);
else
nv50_disp_atomic_commit_tail(state);
- drm_for_each_crtc(crtc, dev) {
- if (crtc->state->active) {
- if (!drm->have_disp_power_ref) {
- drm->have_disp_power_ref = true;
- return 0;
- }
- active = true;
- break;
- }
- }
-
- if (!active && drm->have_disp_power_ref) {
- pm_runtime_put_autosuspend(dev->dev);
- drm->have_disp_power_ref = false;
- }
-
err_cleanup:
if (ret)
drm_atomic_helper_cleanup_planes(dev, state);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index aae035816383..411352dd5390 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -204,9 +204,6 @@ struct nouveau_drm {
/* led management */
struct nouveau_led *led;
- /* display power reference */
- bool have_disp_power_ref;
-
struct dev_pm_domain vga_pm_domain;
struct nouveau_svm *svm;
--
2.21.0
[PATCH 0/2] drm/nouveau: CRTC Runtime PM ref tracking fixes
Just some runtime PM fixes for some much less noticeable runtime PM ref tracking issues that I got reminded of when fixing some unrelated issues with nouveau. Lyude Paul (2): drm/nouveau/dispnv04: Grab/put runtime PM refs on DPMS on/off drm/nouveau/dispnv50: Fix runtime PM ref tracking for non-blocking modesets drivers/gpu/drm/nouveau/dispnv04/crtc.c | 18 +++- drivers/gpu/drm/nouveau/dispnv50/disp.c | 38 +++-- drivers/gpu/drm/nouveau/nouveau_drv.h | 3 -- 3 files changed, 21 insertions(+), 38 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/nouveau/dispnv04: Grab/put runtime PM refs on DPMS on/off
The code claims to grab a runtime PM ref when at least one CRTC is
active, but that's not actually the case as we grab a runtime PM ref
whenever a CRTC is enabled regardless of it's DPMS state. Meaning that
we can end up keeping the GPU awake when there are no screens enabled,
something we don't really want to do.
Note that we fixed this same issue for nv50 a while ago in:
commit e5d54f193572 ("drm/nouveau/drm/nouveau: Fix runtime PM leak in
nv50_disp_atomic_commit()")
Since we're about to remove nouveau_drm->have_disp_power_ref in the next
commit, let's also simplify the RPM code here while we're at it: grab a
ref during a modeset, grab additional RPM refs for each CRTC enabled by
said modeset, and drop an RPM ref for each CRTC disabled by said
modeset. This allows us to keep the GPU awake whenever screens are
turned on, without needing to use nouveau_drm->have_disp_power_ref.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 18 --
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index f22f01020625..08ad8e3b9cd2 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -183,6 +183,10 @@ nv_crtc_dpms(struct drm_crtc *crtc, int mode)
return;
nv_crtc->last_dpms = mode;
+ if (mode == DRM_MODE_DPMS_ON)
+ pm_runtime_get_noresume(dev->dev);
+ else
+ pm_runtime_put_noidle(dev->dev);
if (nv_two_heads(dev))
NVSetOwner(dev, nv_crtc->index);
@@ -1045,7 +1049,6 @@ nouveau_crtc_set_config(struct drm_mode_set *set,
dev = set->crtc->dev;
- /* get a pm reference here */
ret = pm_runtime_get_sync(dev->dev);
if (ret < 0 && ret != -EACCES)
return ret;
@@ -1061,19 +1064,6 @@ nouveau_crtc_set_config(struct drm_mode_set *set,
}
pm_runtime_mark_last_busy(dev->dev);
- /* if we have active crtcs and we don't have a power ref,
- take the current one */
- if (active && !drm->have_disp_power_ref) {
- drm->have_disp_power_ref = true;
- return ret;
- }
- /* if we have no active crtcs, then drop the power ref
- we got before */
- if (!active && drm->have_disp_power_ref) {
- pm_runtime_put_autosuspend(dev->dev);
- drm->have_disp_power_ref = false;
- }
- /* drop the power reference we got coming in here */
pm_runtime_put_autosuspend(dev->dev);
return ret;
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: ✗ Fi.CI.BAT: failure for series starting with [1/6] dma-buf: add dynamic DMA-buf handling v13
On Wed, Jul 31, 2019 at 10:55:02AM +0200, Daniel Vetter wrote: > On Thu, Jun 27, 2019 at 09:28:11AM +0200, Christian König wrote: > > Hi Daniel, > > > > those fails look like something random to me and not related to my patch > > set. Correct? > > First one I looked at has the reservation_obj all over: > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13438/fi-cml-u/igt@gem_exec_fe...@basic-busy-default.html > > So 5 second guees is ... probably real? > > Note that with the entire lmem stuff going on right now there's massive > discussions about how we're doing resv_obj vs obj->mm.lock the wrong way > round in i915, so I'm not surprised at all that you managed to trip this. > > The way I see it right now is that obj->mm.lock needs to be limited to > dealing with the i915 shrinker interactions only, and only for i915 native > objects. And for dma-bufs we need to make sure it's not anywhere in the > callchain. Unfortunately that's a massive refactor I guess ... Thought about this some more, aside from just breaking i915 or waiting until it's refactored (Both not awesome) I think the only option is get back to the original caching. And figure out whether we really need to take the direction into account for that, or whether upgrading to bidirectional unconditionally won't be ok. I think there's only really two cases where this matters: - display drivers using the cma/dma_alloc helpers. Everything is allocated fully coherent, cpu side wc, no flushing. - Everyone else (on platforms where there's actually some flushing going on) is for rendering gpus, and those always map bidirectional and want the mapping cached for as long as possible. With that we could go back to creating the cached mapping at attach time and avoid inflicting the reservation object lock to places that would keel over. Thoughts? -Daniel > -Daniel > > > > > Christian. > > > > Am 26.06.19 um 15:32 schrieb Patchwork: > > > == Series Details == > > > > > > Series: series starting with [1/6] dma-buf: add dynamic DMA-buf handling > > > v13 > > > URL : https://patchwork.freedesktop.org/series/62777/ > > > State : failure > > > > > > == Summary == > > > > > > CI Bug Log - changes from CI_DRM_6358 -> Patchwork_13438 > > > > > > > > > Summary > > > --- > > > > > >**FAILURE** > > > > > >Serious unknown changes coming with Patchwork_13438 absolutely need to > > > be > > >verified manually. > > >If you think the reported changes have nothing to do with the changes > > >introduced in Patchwork_13438, please notify your bug team to allow > > > them > > >to document this new failure mode, which will reduce false positives > > > in CI. > > > > > >External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13438/ > > > > > > Possible new issues > > > --- > > > > > >Here are the unknown changes that may have been introduced in > > > Patchwork_13438: > > > > > > ### IGT changes ### > > > > > > Possible regressions > > > > > >* igt@i915_selftest@live_contexts: > > > - fi-kbl-7567u: [PASS][1] -> [INCOMPLETE][2] > > > [1]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6358/fi-kbl-7567u/igt@i915_selftest@live_contexts.html > > > [2]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13438/fi-kbl-7567u/igt@i915_selftest@live_contexts.html > > > > > >* igt@i915_selftest@live_hugepages: > > > - fi-skl-gvtdvm: [PASS][3] -> [INCOMPLETE][4] > > > [3]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6358/fi-skl-gvtdvm/igt@i915_selftest@live_hugepages.html > > > [4]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13438/fi-skl-gvtdvm/igt@i915_selftest@live_hugepages.html > > > > > > Known issues > > > > > > > > >Here are the changes found in Patchwork_13438 that come from known > > > issues: > > > > > > ### IGT changes ### > > > > > > Issues hit > > > > > >* igt@gem_basic@create-close: > > > - fi-icl-u3: [PASS][5] -> [DMESG-WARN][6] ([fdo#107724]) > > > [5]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6358/fi-icl-u3/igt@gem_ba...@create-close.html > > > [6]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13438/fi-icl-u3/igt@gem_ba...@create-close.html > > > > > >* igt@gem_ctx_switch@basic-default: > > > - fi-icl-guc: [PASS][7] -> [INCOMPLETE][8] ([fdo#107713] / > > > [fdo#108569]) > > > [7]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6358/fi-icl-guc/igt@gem_ctx_swi...@basic-default.html > > > [8]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13438/fi-icl-guc/igt@gem_ctx_swi...@basic-default.html > > > > > >* igt@gem_exec_create@basic: > > > - fi-icl-u2: [PASS][9] -> [INCOMPLETE][10] ([fdo#107713]) > > > [9]: > > >
Re: [Intel-gfx] [PATCH] dma-buf: make dma_fence structure a bit smaller
On Wed, Aug 07, 2019 at 03:01:59PM +0100, Chris Wilson wrote: > Quoting Christian König (2019-08-07 14:54:05) > > The ruc and cb_list are never used at the same time. > > This smal change is actually making the structure 16% smaller. > (Trivial pair of typos) > > Yes. We clear the callback list on kref_put so that by the time we > release the fence it is unused. No one should be adding to the cb_list > that they don't themselves hold a reference for, this only now makes for > a much more spectacular use-after-free. :) ^^ stuff the above as an inline kerneldoc comment into the patch? And into the commit message too pls. We need better docs for all this scorcery :-) Thanks, Daniel > > > Signed-off-by: Christian König > Reviewed-by: Chris Wilson > -Chris > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: add gem ttm helpers
On Wed, Aug 07, 2019 at 01:51:33PM +0200, Gerd Hoffmann wrote: > Hi, > > > > > > I don't think so. drm_gem_dumb_map_offset() calls > > > > > drm_gem_create_mmap_offset(), which I think is not correct for ttm > > > > > objects because ttm_bo_init() handles vma_node initialization. > > > Ok I looked again, and your ttm version seems to exactly match > > drm_gem_dumb_map_offset(), > > No. The difference outlined above is still there. See also v2 which > adds an comment saying so. Creating an mmap offset is idempotent. Otherwise the gem version would already blow up real bad, since it's getting called multiple times by userspace already. So I still think ttm isn't special here, how did this blow up when you tried? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/3] Send a hotplug when edid changes
On Wed, Aug 07, 2019 at 07:43:18AM +, Lisovskiy, Stanislav wrote: > On Tue, 2019-08-06 at 19:43 +0200, Daniel Vetter wrote: > > On Tue, Aug 6, 2019 at 4:06 PM Lisovskiy, Stanislav > > wrote: > > > > > > On Tue, 2019-08-06 at 15:51 +0200, Daniel Vetter wrote: > > > > On Tue, Aug 06, 2019 at 03:55:48PM +0300, Stanislav Lisovskiy > > > > wrote: > > > > > This series introduce to drm a way to determine if something > > > > > else > > > > > except connection_status had changed during probing, which > > > > > can be used by other drivers as well. Another i915 specific > > > > > part > > > > > uses this approach to determine if edid had changed without > > > > > changing the connection status and send a hotplug event. > > > > > > > > Did you read through the entire huge thread on all this (with I > > > > think > > > > Paul, Pekka, Ram and some others)? I feel like this is mostly > > > > taking > > > > that > > > > idea, but not taking a lot of the details we've discussed there. > > > > Specifically I'm not sure how userspace should handle this > > > > without > > > > also > > > > exposing the epoch counter, or at least generating a hotplug > > > > event > > > > for the > > > > specific connector. All that and more we discussed there. > > > > > > > > And then there's the follow-up question: What's the userspace for > > > > this? > > > > Existing expectations are a minefield here, so even if you don't > > > > plan > > > > to > > > > change userspace we need to know what this is aimed for, and see > > > > above I > > > > don't think this is possible to use without userspace changes ... > > > > > > Yes, sure I agree about userspace. However I guess we must start > > > from > > > something at least. > > > I think I have seen some discussion regarding exposing this epoch > > > counter as a property. Was even implementing this at some point but > > > didn't dare to send to mailing list :) > > > > > > My idea was just to expose this epoch counter as a drm property, to > > > let > > > userspace then to figure out, what has happened, as imho adding > > > different events for this and that is a bit of an overkill and > > > brings > > > additional complexity.. > > > > Adding Ram and link to the (warning, huge!) thread: > > > > https://patchwork.freedesktop.org/patch/303905/?series=57232=9 > > > > > However, please let me know, what do you think we should do for > > > userspace. > > > > That seems backwards, since this is an uapi change I'd expect you're > > solving some specific issue in some specific userspace? If we're > > doing > > this just because I'm not really following. > > Specifically, I'm solving an issue of changed edid during suspend, like > we for example have in kms_chamelium hotplug tests(some of which now > fail because of that). > So even if connection status hasn't changed, we still need to send > hotplug event and userspace needs to be able to understand that > something had changed and whether we need to do a full reprobe or not. > Epoch counter property would be responsible for this. > As I understand in general there might be other connector updates, > except edid which we need propagate in a similar way. So igt isn't valid userspace (it's just good testcases). Can we repro the same on real userspace? Does this work with real userspace? We've had userspace which tries to be clever and filters out what looks like redundant hotplug events. And then gets it wrong in cases like this. Also, we've had forever an unconditional uevent on resume, exactly because anything could have changed. Did we loose this one on the way somewhere? Or maybe I misremember ... If all we care about is resume re-adding that uncondtional uevent on resume is going to be a lot easier than this here. -Daniel > > -Stanislav > > > > > Cheers, Daniel > > > > > > > > > > > -Stanislav > > > > > > > > > > -Daniel > > > > > > > > > > > > > > Stanislav Lisovskiy (3): > > > > > drm: Add helper to compare edids. > > > > > drm: Introduce change counter to drm_connector > > > > > drm/i915: Send hotplug event if edid had changed. > > > > > > > > > > drivers/gpu/drm/drm_connector.c | 1 + > > > > > drivers/gpu/drm/drm_edid.c | 33 > > > > > > > > > > drivers/gpu/drm/drm_probe_helper.c | 29 > > > > > +++- > > > > > - > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 16 +- > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c| 16 -- > > > > > drivers/gpu/drm/i915/display/intel_hotplug.c | 21 ++ > > > > > --- > > > > > include/drm/drm_connector.h | 3 ++ > > > > > include/drm/drm_edid.h | 9 ++ > > > > > 8 files changed, 117 insertions(+), 11 deletions(-) > > > > > > > > > > -- > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 1/2] Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()"
On Wed, Aug 07, 2019 at 10:52:47AM -0400, Sean Paul wrote:
> From: Rob Herring
>
> This reverts commit 220df83a5394fbf7c1486ba7848794b7b351d598.
>
> Turns out drm_gem_dumb_map_offset really only worked for the dumb buffer
> case, so revert the name change.
>
> Signed-off-by: Rob Herring
> Signed-off-by: Sean Paul
This part of the series seems unecessary to revert?
iow I still like this, and Im trying to sell Gerd Hoffmann on it for some
of his ttm refactoring ... revert of the revert of the revert of the
revert? Probably better if Gerd cherry-picks into his series (if my
suggestion works out) and maybe references this entire chain for
entertainment purposes :-)
-Daniel
> ---
> drivers/gpu/drm/drm_dumb_buffers.c | 4 ++--
> drivers/gpu/drm/drm_gem.c | 10 +++---
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++-
> include/drm/drm_gem.h | 4 ++--
> 4 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
> b/drivers/gpu/drm/drm_dumb_buffers.c
> index b55cfc9e8772..d18a740fe0f1 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -48,7 +48,7 @@
> * To support dumb objects drivers must implement the _driver.dumb_create
> * operation. _driver.dumb_destroy defaults to drm_gem_dumb_destroy() if
> * not set and _driver.dumb_map_offset defaults to
> - * drm_gem_map_offset(). See the callbacks for further details.
> + * drm_gem_dumb_map_offset(). See the callbacks for further details.
> *
> * Note that dumb objects may not be used for gpu acceleration, as has been
> * attempted on some ARM embedded platforms. Such drivers really must have
> @@ -127,7 +127,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
> args->handle,
> >offset);
> else
> - return drm_gem_map_offset(file_priv, dev, args->handle,
> + return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
> >offset);
> }
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 8cbfd60e09c0..afc38cece3f5 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -298,7 +298,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
> EXPORT_SYMBOL(drm_gem_handle_delete);
>
> /**
> - * drm_gem_map_offset - return the fake mmap offset for a gem object
> + * drm_gem_dumb_map_offset - return the fake mmap offset for a gem object
> * @file: drm file-private structure containing the gem object
> * @dev: corresponding drm_device
> * @handle: gem object handle
> @@ -307,14 +307,10 @@ EXPORT_SYMBOL(drm_gem_handle_delete);
> * This implements the _driver.dumb_map_offset kms driver callback for
> * drivers which use gem to manage their backing storage.
> *
> - * It can also be used by drivers using GEM BO implementations which
> - * have same restriction that imported objects cannot be mapped. The
> - * shmem backend is one example.
> - *
> * Returns:
> * 0 on success or a negative error code on failure.
> */
> -int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
> +int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
> u32 handle, u64 *offset)
> {
> struct drm_gem_object *obj;
> @@ -340,7 +336,7 @@ int drm_gem_map_offset(struct drm_file *file, struct
> drm_device *dev,
>
> return ret;
> }
> -EXPORT_SYMBOL_GPL(drm_gem_map_offset);
> +EXPORT_SYMBOL_GPL(drm_gem_dumb_map_offset);
>
> /**
> * drm_gem_dumb_destroy - dumb fb callback helper for gem based drivers
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index bf0ad8e5a02b..d734d9d51762 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -273,7 +273,8 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void
> *data,
> {
> struct drm_exynos_gem_map *args = data;
>
> - return drm_gem_map_offset(file_priv, dev, args->handle, >offset);
> + return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
> +>offset);
> }
>
> struct exynos_drm_gem *exynos_drm_gem_get(struct drm_file *filp,
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 0d6445fa9541..ae693c0666cd 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -401,8 +401,8 @@ int drm_gem_fence_array_add(struct xarray *fence_array,
> int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
>struct drm_gem_object *obj,
>bool write);
> -int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
> -u32 handle, u64 *offset);
> +int drm_gem_dumb_map_offset(struct drm_file
Re: [PATCH 2/2] Revert "drm/panfrost: Use drm_gem_map_offset()"
On Wed, Aug 07, 2019 at 04:59:51PM +0100, Emil Velikov wrote: > On Wed, 7 Aug 2019 at 15:53, Sean Paul wrote: > > > > From: Rob Herring > > > > This reverts commit 583bbf46133c726bae277e8f4e32bfba2a528c7f. > > > > Turns out we need mmap to work on imported BOs even if the current code > > is buggy. > > > Personally I would have mentioned a use case where imported BOs are used. > > > Signed-off-by: Rob Herring > > Signed-off-by: Sean Paul > > Regardless of the above nitpick, with the patch order fixed the series is: > Reviewed-by: Emil Velikov > > ... in case you haven't picked it already. Yeah a follow-up patch to add a comment here about why exactly this went all kaboom, plus which userspace (since panfrost is moving fast) would be real nice here. Atm we need to hope someone does a git blame on this before the break this again, which seems a bit hopeful ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/4] backlight: Expose brightness curve type through sysfs
On Tue, Jul 09, 2019 at 12:00:05PM -0700, Matthias Kaehlcke wrote: > Backlight brightness curves can have different shapes. The two main > types are linear and non-linear curves. The human eye doesn't > perceive linearly increasing/decreasing brightness as linear (see > also 88ba95bedb79 "backlight: pwm_bl: Compute brightness of LED > linearly to human eye"), hence many backlights use non-linear (often > logarithmic) brightness curves. The type of curve currently is opaque > to userspace, so userspace often uses more or less reliable heuristics > (like the number of brightness levels) to decide whether to treat a > backlight device as linear or non-linear. > > Export the type of the brightness curve via the new sysfs attribute > 'scale'. The value of the attribute can be 'linear', 'non-linear' or > 'unknown'. For devices that don't provide information about the scale > of their brightness curve the value of the 'scale' attribute is 'unknown'. > > Signed-off-by: Matthias Kaehlcke Daniel (et al): do you have any more comments on this patch/series or is it ready to land? Thanks Matthias
Re: drm pull for v5.3-rc1
On Tue, Aug 6, 2019 at 11:40 PM Christoph Hellwig wrote: > > I'm not an all that huge fan of super magic macro loops. But in this > case I don't see how it could even work, as we get special callbacks > for huge pages and holes, and people are trying to add a few more ops > as well. Yeah, in this case we definitely don't want to make some magic loop walker. Loops are certainly simpler than callbacks for most cases (and often faster because you don't have indirect calls which now are getting quite expensive), but the walker code really does end up having tons of different cases that you'd have to handle with magic complex conditionals or switch statements instead. So the "walk over range using this set of callbacks" is generally the right interface. If there is some particular case that might be very simple and the callback model is expensive due to indirect calls for each page, then such a case should probably use the normal page walking loops (that we *used* to have everywhere - the "walk_range()" interface is the "new" model for all the random odd special cases). Linus
Re: [Freedreno] [PATCH] drm/msm: Make DRM_MSM default to 'm'
Hi Jordan/Rob. On Wed, Aug 07, 2019 at 12:46:49PM -0600, Jordan Crouse wrote: > On Wed, Aug 07, 2019 at 11:08:53AM -0700, Rob Clark wrote: > > On Wed, Aug 7, 2019 at 10:38 AM Sam Ravnborg wrote: > > > > > > Hi Jordan. > > > On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote: > > > > Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as > > > > modules but there are some cases where DRM might be built in for > > > > whatever > > > > reason and in those situations it is preferable to still keep MSM as a > > > > module by default and let the user decide if they _really_ want to build > > > > it in. > > > > > > > > Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure > > > > it doesn't get missed when we need it for a6xx tarets. > > > > > > > > Signed-off-by: Jordan Crouse > > > > --- > > > > > > > > drivers/gpu/drm/msm/Kconfig | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > > > > index 9c37e4d..3b2334b 100644 > > > > --- a/drivers/gpu/drm/msm/Kconfig > > > > +++ b/drivers/gpu/drm/msm/Kconfig > > > > @@ -14,11 +14,12 @@ config DRM_MSM > > > > select SHMEM > > > > select TMPFS > > > > select QCOM_SCM if ARCH_QCOM > > > > + select QCOM_COMMAND_DB if ARCH_QCOM > > > > select WANT_DEV_COREDUMP > > > > select SND_SOC_HDMI_CODEC if SND_SOC > > > > select SYNC_FILE > > > > select PM_OPP > > > > - default y > > > > + default m > > > > > > As a general comment the right thing would be to drop this default. > > > As it is now the Kconfig says that when DRM is selected then all of the > > > world would then also get DRM_MSM, which only a small part of this world > > > you see any benefit in. > > > So they now have to de-select MSM. > > > > If the default is dropped, it should probably be accompanied by adding > > CONFIG_DRM_MSM=m to defconfig's, I think That would be best. So the defconfigs end up with the same config as before. > > In general I prefer to not use a default but this is the only GPU driver for > ARCH_QCOM and I think its safe to stay that 99% of ARCH_QCOM users would > select > this module and those that wouldn't will omit DRM entirely. > > I feel it is net negative if we dropped the default but then had to turn > around > and enable it in every defconfig. "in every" equals three defconfigs: $ git grep ARCH_QCOM | grep defconfig arch/arm/configs/multi_v7_defconfig:CONFIG_ARCH_QCOM=y arch/arm/configs/qcom_defconfig:CONFIG_ARCH_QCOM=y arch/arm64/configs/defconfig:CONFIG_ARCH_QCOM=y Sam
[Bug 110413] GPU crash and failed reset leading to deadlock on Polaris 22 XL [Radeon RX Vega M GL]
https://bugs.freedesktop.org/show_bug.cgi?id=110413 --- Comment #12 from Utku Helvacı (tuxutku) --- as it turns out this is not a bug in kernel but amd's aco compiler so its irrelevant -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dsi: Fix return value check for clk_get_parent
From: Sean Paul
clk_get_parent returns an error pointer upon failure, not NULL. So the
checks as they exist won't catch a failure. This patch changes the
checks and the return values to properly handle an error pointer.
Fixes: c4d8cfe516dc ("drm/msm/dsi: add implementation for helper functions")
Cc: Sibi Sankar
Cc: Sean Paul
Cc: Rob Clark
Cc: # v4.19+
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index aa35d18ab43c9..02acb4338721a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -421,15 +421,15 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
}
msm_host->byte_clk_src = clk_get_parent(msm_host->byte_clk);
- if (!msm_host->byte_clk_src) {
- ret = -ENODEV;
+ if (IS_ERR(msm_host->byte_clk_src)) {
+ ret = PTR_ERR(msm_host->byte_clk_src);
pr_err("%s: can't find byte_clk clock. ret=%d\n", __func__,
ret);
goto exit;
}
msm_host->pixel_clk_src = clk_get_parent(msm_host->pixel_clk);
- if (!msm_host->pixel_clk_src) {
- ret = -ENODEV;
+ if (IS_ERR(msm_host->pixel_clk_src)) {
+ ret = PTR_ERR(msm_host->pixel_clk_src);
pr_err("%s: can't find pixel_clk clock. ret=%d\n", __func__,
ret);
goto exit;
}
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Wed, Aug 7, 2019 at 10:45 AM Jason Gunthorpe wrote:
>
> On Tue, Aug 06, 2019 at 07:05:42PM +0300, Christoph Hellwig wrote:
> > There is only a single place where the pgmap is passed over a function
> > call, so replace it with local variables in the places where we deal
> > with the pgmap.
> >
> > Signed-off-by: Christoph Hellwig
> > mm/hmm.c | 62
> > 1 file changed, 27 insertions(+), 35 deletions(-)
> >
> > diff --git a/mm/hmm.c b/mm/hmm.c
> > index 9a908902e4cc..d66fa29b42e0 100644
> > +++ b/mm/hmm.c
> > @@ -278,7 +278,6 @@ EXPORT_SYMBOL(hmm_mirror_unregister);
> >
> > struct hmm_vma_walk {
> > struct hmm_range*range;
> > - struct dev_pagemap *pgmap;
> > unsigned long last;
> > unsigned intflags;
> > };
> > @@ -475,6 +474,7 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk,
> > #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > struct hmm_vma_walk *hmm_vma_walk = walk->private;
> > struct hmm_range *range = hmm_vma_walk->range;
> > + struct dev_pagemap *pgmap = NULL;
> > unsigned long pfn, npages, i;
> > bool fault, write_fault;
> > uint64_t cpu_flags;
> > @@ -490,17 +490,14 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk,
> > pfn = pmd_pfn(pmd) + pte_index(addr);
> > for (i = 0; addr < end; addr += PAGE_SIZE, i++, pfn++) {
> > if (pmd_devmap(pmd)) {
> > - hmm_vma_walk->pgmap = get_dev_pagemap(pfn,
> > - hmm_vma_walk->pgmap);
> > - if (unlikely(!hmm_vma_walk->pgmap))
> > + pgmap = get_dev_pagemap(pfn, pgmap);
> > + if (unlikely(!pgmap))
> > return -EBUSY;
>
> Unrelated to this patch, but what is the point of getting checking
> that the pgmap exists for the page and then immediately releasing it?
> This code has this pattern in several places.
>
> It feels racy
Agree, not sure what the intent is here. The only other reason call
get_dev_pagemap() is to just check in general if the pfn is indeed
owned by some ZONE_DEVICE instance, but if the intent is to make sure
the device is still attached/enabled that check is invalidated at
put_dev_pagemap().
If it's the former case, validating ZONE_DEVICE pfns, I imagine we can
do something cheaper with a helper that is on the order of the same
cost as pfn_valid(). I.e. replace PTE_DEVMAP with a mem_section flag
or something similar.
>
> > }
> > pfns[i] = hmm_device_entry_from_pfn(range, pfn) | cpu_flags;
> > }
> > - if (hmm_vma_walk->pgmap) {
> > - put_dev_pagemap(hmm_vma_walk->pgmap);
> > - hmm_vma_walk->pgmap = NULL;
>
> Putting the value in the hmm_vma_walk would have made some sense to me
> if the pgmap was not set to NULL all over the place. Then the most
> xa_loads would be eliminated, as I would expect the pgmap tends to be
> mostly uniform for these use cases.
>
> Is there some reason the pgmap ref can't be held across
> faulting/sleeping? ie like below.
No restriction on holding refs over faulting / sleeping.
>
> Anyhow, I looked over this pretty carefully and the change looks
> functionally OK, I just don't know why the code is like this in the
> first place.
>
> Reviewed-by: Jason Gunthorpe
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 9a908902e4cc38..4e30128c23a505 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -497,10 +497,6 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk,
> }
> pfns[i] = hmm_device_entry_from_pfn(range, pfn) | cpu_flags;
> }
> - if (hmm_vma_walk->pgmap) {
> - put_dev_pagemap(hmm_vma_walk->pgmap);
> - hmm_vma_walk->pgmap = NULL;
> - }
> hmm_vma_walk->last = end;
> return 0;
> #else
> @@ -604,10 +600,6 @@ static int hmm_vma_handle_pte(struct mm_walk *walk,
> unsigned long addr,
> return 0;
>
> fault:
> - if (hmm_vma_walk->pgmap) {
> - put_dev_pagemap(hmm_vma_walk->pgmap);
> - hmm_vma_walk->pgmap = NULL;
> - }
> pte_unmap(ptep);
> /* Fault any virtual address we were asked to fault */
> return hmm_vma_walk_hole_(addr, end, fault, write_fault, walk);
> @@ -690,16 +682,6 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
> return r;
> }
> }
> - if (hmm_vma_walk->pgmap) {
> - /*
> -* We do put_dev_pagemap() here and not in
> hmm_vma_handle_pte()
> -* so that we can leverage get_dev_pagemap() optimization
> which
> -* will not re-take a reference on a pgmap if we already have
> -* one.
> -*/
> - put_dev_pagemap(hmm_vma_walk->pgmap);
> - hmm_vma_walk->pgmap = NULL;
> - }
> pte_unmap(ptep - 1);
>
Re: [Freedreno] [PATCH] drm/msm: Make DRM_MSM default to 'm'
On Wed, Aug 07, 2019 at 11:08:53AM -0700, Rob Clark wrote: > On Wed, Aug 7, 2019 at 10:38 AM Sam Ravnborg wrote: > > > > Hi Jordan. > > On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote: > > > Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as > > > modules but there are some cases where DRM might be built in for whatever > > > reason and in those situations it is preferable to still keep MSM as a > > > module by default and let the user decide if they _really_ want to build > > > it in. > > > > > > Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure > > > it doesn't get missed when we need it for a6xx tarets. > > > > > > Signed-off-by: Jordan Crouse > > > --- > > > > > > drivers/gpu/drm/msm/Kconfig | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > > > index 9c37e4d..3b2334b 100644 > > > --- a/drivers/gpu/drm/msm/Kconfig > > > +++ b/drivers/gpu/drm/msm/Kconfig > > > @@ -14,11 +14,12 @@ config DRM_MSM > > > select SHMEM > > > select TMPFS > > > select QCOM_SCM if ARCH_QCOM > > > + select QCOM_COMMAND_DB if ARCH_QCOM > > > select WANT_DEV_COREDUMP > > > select SND_SOC_HDMI_CODEC if SND_SOC > > > select SYNC_FILE > > > select PM_OPP > > > - default y > > > + default m > > > > As a general comment the right thing would be to drop this default. > > As it is now the Kconfig says that when DRM is selected then all of the > > world would then also get DRM_MSM, which only a small part of this world > > you see any benefit in. > > So they now have to de-select MSM. > > If the default is dropped, it should probably be accompanied by adding > CONFIG_DRM_MSM=m to defconfig's, I think In general I prefer to not use a default but this is the only GPU driver for ARCH_QCOM and I think its safe to stay that 99% of ARCH_QCOM users would select this module and those that wouldn't will omit DRM entirely. I feel it is net negative if we dropped the default but then had to turn around and enable it in every defconfig. Jordan > BR, > -R > > > Kconfig has: > > depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST) > > > > So maybe not all of the world but all QCOM or IMX5 users. Maybe they are all > > interested in MSM. Otherwise the default should rather be dropped. > > If there is any good hints then the help text could anyway use some > > love, and then add the info there. > > > > The other change with QCOM_COMMAND_DB seems on the other hand to make > > sense but then this is another patch. > > > > Sam > > ___ > > Freedreno mailing list > > freedr...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] amdgpu, amdkfd drm-fixes-5.3
Hi Dave, Daniel, Fixes for 5.3. Nothing too major bug-wise. I'm reverting the kfd GWS ioctl that was added this cycle. After working with it for a while the kfd team decided it wasn't quite right. I should have been stricter with it in the beginning. Revert it. The following changes since commit 9c8c9c7cdb4c8fb48a2bc70f41a07920f761d2cd: Merge tag 'exynos-drm-fixes-for-v5.3-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-fixes (2019-08-02 17:10:17 +0200) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/drm-fixes-5.3-2019-08-07 for you to fetch changes up to 4b3e30ed3ec7864e798403a63ff2e96bd0c19ab0: Revert "drm/amdkfd: New IOCTL to allocate queue GWS" (2019-08-07 10:21:38 -0500) drm-fixes-5.3-2019-08-07: amdgpu: - Fixes VCN to handle the latest navi10 firmware - Fixes for fan control on navi10 - Properly handle SMU metrics table on navi10 - Fix a resume regression on Stoney amdkfd: - Revert new GWS ioctl. It's not ready. Alex Deucher (1): Revert "drm/amdkfd: New IOCTL to allocate queue GWS" Evan Quan (1): drm/amd/powerplay: correct navi10 vcn powergate Kevin Wang (1): drm/amd/powerplay: honor hw limit on fetching metrics data for navi10 Likun Gao (1): drm/amdgpu: pin the csb buffer on hw init for gfx v8 Marek Olšák (1): Revert "drm/amdgpu: fix transform feedback GDS hang on gfx10 (v2)" Matt Coffin (1): drm/amd/powerplay: Allow changing of fan_control in smu_v11_0 Thong Thai (2): drm/amd/amdgpu/vcn_v2_0: Mark RB commands as KMD commands drm/amd/amdgpu/vcn_v2_0: Move VCN 2.0 specific dec ring test to vcn_v2_0 drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h| 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h| 1 + drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 12 +--- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 40 + drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 44 +++--- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 28 - drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 4 +- drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 + drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 79 +- drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 2 +- include/uapi/linux/kfd_ioctl.h | 20 +-- 11 files changed, 138 insertions(+), 94 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH] drm/msm: Make DRM_MSM default to 'm'
On Wed, Aug 7, 2019 at 10:38 AM Sam Ravnborg wrote: > > Hi Jordan. > On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote: > > Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as > > modules but there are some cases where DRM might be built in for whatever > > reason and in those situations it is preferable to still keep MSM as a > > module by default and let the user decide if they _really_ want to build > > it in. > > > > Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure > > it doesn't get missed when we need it for a6xx tarets. > > > > Signed-off-by: Jordan Crouse > > --- > > > > drivers/gpu/drm/msm/Kconfig | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > > index 9c37e4d..3b2334b 100644 > > --- a/drivers/gpu/drm/msm/Kconfig > > +++ b/drivers/gpu/drm/msm/Kconfig > > @@ -14,11 +14,12 @@ config DRM_MSM > > select SHMEM > > select TMPFS > > select QCOM_SCM if ARCH_QCOM > > + select QCOM_COMMAND_DB if ARCH_QCOM > > select WANT_DEV_COREDUMP > > select SND_SOC_HDMI_CODEC if SND_SOC > > select SYNC_FILE > > select PM_OPP > > - default y > > + default m > > As a general comment the right thing would be to drop this default. > As it is now the Kconfig says that when DRM is selected then all of the > world would then also get DRM_MSM, which only a small part of this world > you see any benefit in. > So they now have to de-select MSM. If the default is dropped, it should probably be accompanied by adding CONFIG_DRM_MSM=m to defconfig's, I think BR, -R > Kconfig has: > depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST) > > So maybe not all of the world but all QCOM or IMX5 users. Maybe they are all > interested in MSM. Otherwise the default should rather be dropped. > If there is any good hints then the help text could anyway use some > love, and then add the info there. > > The other change with QCOM_COMMAND_DB seems on the other hand to make > sense but then this is another patch. > > Sam > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203471] Tearing on Raven Ridge and RX560X PRIME setup even with Vsync enabled
https://bugzilla.kernel.org/show_bug.cgi?id=203471 vr00m (vmuppa...@outlook.com) changed: What|Removed |Added CC||vmuppa...@outlook.com --- Comment #11 from vr00m (vmuppa...@outlook.com) --- ICYMI, I was able to solve the tearing problem with raven ridge using iommu=soft boot param. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204181] NULL pointer dereference regression in amdgpu
https://bugzilla.kernel.org/show_bug.cgi?id=204181 vr00m (vmuppa...@outlook.com) changed: What|Removed |Added CC||vmuppa...@outlook.com --- Comment #28 from vr00m (vmuppa...@outlook.com) --- I experienced issues after upgrading kernel from 5.1 to 5.2 on my notebook with 2500 U. I tried kernel boot param iommu=soft and that fixed it. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm: Make DRM_MSM default to 'm'
Hi Jordan. On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote: > Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as > modules but there are some cases where DRM might be built in for whatever > reason and in those situations it is preferable to still keep MSM as a > module by default and let the user decide if they _really_ want to build > it in. > > Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure > it doesn't get missed when we need it for a6xx tarets. > > Signed-off-by: Jordan Crouse > --- > > drivers/gpu/drm/msm/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > index 9c37e4d..3b2334b 100644 > --- a/drivers/gpu/drm/msm/Kconfig > +++ b/drivers/gpu/drm/msm/Kconfig > @@ -14,11 +14,12 @@ config DRM_MSM > select SHMEM > select TMPFS > select QCOM_SCM if ARCH_QCOM > + select QCOM_COMMAND_DB if ARCH_QCOM > select WANT_DEV_COREDUMP > select SND_SOC_HDMI_CODEC if SND_SOC > select SYNC_FILE > select PM_OPP > - default y > + default m As a general comment the right thing would be to drop this default. As it is now the Kconfig says that when DRM is selected then all of the world would then also get DRM_MSM, which only a small part of this world you see any benefit in. So they now have to de-select MSM. Kconfig has: depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST) So maybe not all of the world but all QCOM or IMX5 users. Maybe they are all interested in MSM. Otherwise the default should rather be dropped. If there is any good hints then the help text could anyway use some love, and then add the info there. The other change with QCOM_COMMAND_DB seems on the other hand to make sense but then this is another patch. Sam
Re: next/master boot: 263 boots: 11 failed, 186 passed with 64 offline, 1 untried/unknown, 1 conflict (next-20190802)
On Fri, Aug 02, 2019 at 05:13:30AM -0700, kernelci.org bot wrote: Today's -next still fails to boot on CM-QS600 with qcom_defconfig: > qcom_defconfig: > gcc-8: > qcom-apq8064-cm-qs600: 1 failed lab This has been going on since June. It crashes initializing the GPU: [4.261135] adreno 430.adreno-3xx: 430.adreno-3xx supply vddcx not found, using dummy regulator [4.270254] msm 510.mdp: [drm:msm_gpu_init] A320: using IOMMU [4.280025] 8<--- cut here --- [4.285557] Unable to handle kernel paging request at virtual address 4000 [4.288430] pgd = (ptrval) [4.295714] [4000] *pgd= [4.298329] Internal error: Oops: 805 [#1] PREEMPT SMP ARM [4.302054] Modules linked in: [4.307352] CPU: 2 PID: 88 Comm: kworker/2:1 Tainted: GW 5.3.0-rc3-next-20190807 #1 [4.310391] Hardware name: Generic DT based system [4.319353] Workqueue: events deferred_probe_work_func [4.319930] usb 1-1: New USB device found, idVendor=04b4, idProduct=6570, bcdDevice=32.99 [4.324201] PC is at v7_dma_clean_range+0x1c/0x34 [4.324214] LR is at __dma_page_cpu_to_dev+0x28/0x8c ... [4.753642] [] (v7_dma_clean_range) from [] (__dma_page_cpu_to_dev+0x28/0x8c) [4.761795] [] (__dma_page_cpu_to_dev) from [] (arm_dma_sync_sg_for_device+0x4c/0x64) [4.770654] [] (arm_dma_sync_sg_for_device) from [] (get_pages+0x1bc/0x218) [4.780199] [] (get_pages) from [] (msm_gem_get_and_pin_iova+0xb4/0x13c) [4.788704] [] (msm_gem_get_and_pin_iova) from [] (_msm_gem_kernel_new+0x38/0xa8) [4.797386] [] (_msm_gem_kernel_new) from [] (msm_gem_kernel_new+0x24/0x2c) [4.806501] [] (msm_gem_kernel_new) from [] (msm_gpu_init+0x4a4/0x614) [4.815021] [] (msm_gpu_init) from [] (adreno_gpu_init+0x17c/0x288) [4.823342] [] (adreno_gpu_init) from [] (a3xx_gpu_init+0x84/0x108) [4.831239] [] (a3xx_gpu_init) from [] (adreno_bind+0x1c4/0x268) [4.839224] [] (adreno_bind) from [] (component_bind_all+0x11c/0x258) [4.847213] [] (component_bind_all) from [] (msm_drm_bind+0xf8/0x638) [4.855282] [] (msm_drm_bind) from [] (try_to_bring_up_master+0x1fc/0x2b8) More details including full logs and the image file at: https://kernelci.org/boot/id/5d4ac1e659b514754b31b293/ signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Fix kerneldoc warns in connector-related docs
Hi Sean.
On Wed, Aug 07, 2019 at 12:28:04PM -0400, Sean Paul wrote:
> From: Sean Paul
>
> Fixes the following warnings:
> ../drivers/gpu/drm/drm_connector.c:989: WARNING: Unexpected indentation.
> ../drivers/gpu/drm/drm_connector.c:993: WARNING: Unexpected indentation.
> ../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or
> phrase reference start-string without end-string.
> ../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or
> phrase reference start-string without end-string.
Thanks, 4 less warnings..
>
> Fixes: 1b27fbdde1df ("drm: Add
> drm_atomic_get_(old|new)_connector_for_encoder() helpers")
> Fixes: bb5a45d40d50 ("drm/hdcp: update content protection property with
> uevent")
> Cc: Ramalingam C
> Cc: Daniel Vetter
> Cc: Pekka Paalanen
> Cc: Sam Ravnborg
> Cc: Laurent Pinchart
> Cc: Jani Nikula
> Cc: Sean Paul
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Sean Paul
> Cc: David Airlie
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Sean Paul
> ---
> drivers/gpu/drm/drm_connector.c | 10 ++
> include/drm/drm_connector.h | 4 ++--
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 354798bad576..4c766624b20d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -986,12 +986,14 @@ static const struct drm_prop_enum_list
> hdmi_colorspaces[] = {
> * - Kernel sends uevent with the connector id and property id through
> * @drm_hdcp_update_content_protection, upon below kernel triggered
> * scenarios:
> - * DESIRED -> ENABLED (authentication success)
> - * ENABLED -> DESIRED (termination of authentication)
> + *
> + * - DESIRED -> ENABLED (authentication success)
> + * - ENABLED -> DESIRED (termination of authentication)
> * - Please note no uevents for userspace triggered property state changes,
> * which can't fail such as
> - * DESIRED/ENABLED -> UNDESIRED
> - * UNDESIRED -> DESIRED
> + *
> + * - DESIRED/ENABLED -> UNDESIRED
> + * - UNDESIRED -> DESIRED
> * - Userspace is responsible for polling the property or listen to uevents
> * to determine when the value transitions from ENABLED to DESIRED.
> * This signifies the link is no longer protected and userspace should
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 0b9997e27689..e391f9c05f98 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -543,8 +543,8 @@ struct drm_connector_state {
>*
>* This is also used in the atomic helpers to map encoders to their
>* current and previous connectors, see
> - * _atomic_get_old_connector_for_encoder() and
> - * _atomic_get_new_connector_for_encoder().
> + * _atomic_get_old_connector_for_encoder and
> + * _atomic_get_new_connector_for_encoder.
Please fix this to use () for the functions and drop the "&".
This is what we use in drm kernel-doc for functions.
See for example function references in doc of writeback_job in the same file.
With this fixed:
Reviewed-by: Sam Ravnborg
>*
>* NOTE: Atomic drivers must fill this out (either themselves or through
>* helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will
> --
> Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: add cache support for arm64
On Wed, Aug 7, 2019 at 9:50 AM Mark Rutland wrote: > > On Wed, Aug 07, 2019 at 09:15:54AM -0700, Rob Clark wrote: > > On Wed, Aug 7, 2019 at 5:38 AM Mark Rutland wrote: > > > > > > On Tue, Aug 06, 2019 at 09:31:55AM -0700, Rob Clark wrote: > > > > On Tue, Aug 6, 2019 at 7:35 AM Mark Rutland > > > > wrote: > > > > > > > > > > On Tue, Aug 06, 2019 at 07:11:41AM -0700, Rob Clark wrote: > > > > > > On Tue, Aug 6, 2019 at 1:48 AM Christoph Hellwig > > > > > > wrote: > > > > > > > > > > > > > > This goes in the wrong direction. drm_cflush_* are a bad API we > > > > > > > need to > > > > > > > get rid of, not add use of it. The reason for that is two-fold: > > > > > > > > > > > > > > a) it doesn't address how cache maintaince actually works in most > > > > > > > platforms. When talking about a cache we three fundamental > > > > > > > operations: > > > > > > > > > > > > > > 1) write back - this writes the content of the cache back > > > > > > > to the > > > > > > >backing memory > > > > > > > 2) invalidate - this remove the content of the cache > > > > > > > 3) write back + invalidate - do both of the above > > > > > > > > > > > > Agreed that drm_cflush_* isn't a great API. In this particular case > > > > > > (IIUC), I need wb+inv so that there aren't dirty cache lines that > > > > > > drop > > > > > > out to memory later, and so that I don't get a cache hit on > > > > > > uncached/wc mmap'ing. > > > > > > > > > > Is there a cacheable alias lying around (e.g. the linear map), or are > > > > > these addresses only mapped uncached/wc? > > > > > > > > > > If there's a cacheable alias, performing an invalidate isn't > > > > > sufficient, > > > > > since a CPU can allocate a new (clean) entry at any point in time > > > > > (e.g. > > > > > as a result of prefetching or arbitrary speculation). > > > > > > > > I *believe* that there are not alias mappings (that I don't control > > > > myself) for pages coming from > > > > shmem_file_setup()/shmem_read_mapping_page().. > > > > > > AFAICT, that's regular anonymous memory, so there will be a cacheable > > > alias in the linear/direct map. > > > > tbh, I'm not 100% sure whether there is a cacheable alias, or whether > > any potential linear map is torn down. > > I'm fairly confident that the linear/direct map cacheable alias is not > torn down when pages are allocated. The gneeric page allocation code > doesn't do so, and I see nothing the shmem code to do so. > > For arm64, we can tear down portions of the linear map, but that has to > be done explicitly, and this is only possible when using rodata_full. If > not using rodata_full, it is not possible to dynamically tear down the > cacheable alias. So, we do end up using GFP_HIGHUSER, which appears to get passed thru when shmem gets to the point of actually allocating pages.. not sure if that just ends up being a hint, or if it guarantees that we don't get something in the linear map. (Bear with me while I "page" this all back in.. last time I dug thru the shmem code was probably pre-armv8, or at least before I had any armv8 hw) BR, -R
[PATCH] drm/msm: Make DRM_MSM default to 'm'
Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as modules but there are some cases where DRM might be built in for whatever reason and in those situations it is preferable to still keep MSM as a module by default and let the user decide if they _really_ want to build it in. Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure it doesn't get missed when we need it for a6xx tarets. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 9c37e4d..3b2334b 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -14,11 +14,12 @@ config DRM_MSM select SHMEM select TMPFS select QCOM_SCM if ARCH_QCOM + select QCOM_COMMAND_DB if ARCH_QCOM select WANT_DEV_COREDUMP select SND_SOC_HDMI_CODEC if SND_SOC select SYNC_FILE select PM_OPP - default y + default m help DRM/KMS driver for MSM/snapdragon. -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109524] "Invalid glsl version in shading_language_version()" when trying to run directX games using wine
https://bugs.freedesktop.org/show_bug.cgi?id=109524 --- Comment #14 from Juan A. Suarez --- (In reply to Juan A. Suarez from comment #13) > The patch landed in Mesa 19.1.5. I meant 19.1.4. Sorry. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109524] "Invalid glsl version in shading_language_version()" when trying to run directX games using wine
https://bugs.freedesktop.org/show_bug.cgi?id=109524 Juan A. Suarez changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #13 from Juan A. Suarez --- The patch landed in Mesa 19.1.5. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: add cache support for arm64
On Wed, Aug 07, 2019 at 09:15:54AM -0700, Rob Clark wrote:
> On Wed, Aug 7, 2019 at 5:38 AM Mark Rutland wrote:
> >
> > On Tue, Aug 06, 2019 at 09:31:55AM -0700, Rob Clark wrote:
> > > On Tue, Aug 6, 2019 at 7:35 AM Mark Rutland wrote:
> > > >
> > > > On Tue, Aug 06, 2019 at 07:11:41AM -0700, Rob Clark wrote:
> > > > > On Tue, Aug 6, 2019 at 1:48 AM Christoph Hellwig wrote:
> > > > > >
> > > > > > This goes in the wrong direction. drm_cflush_* are a bad API we
> > > > > > need to
> > > > > > get rid of, not add use of it. The reason for that is two-fold:
> > > > > >
> > > > > > a) it doesn't address how cache maintaince actually works in most
> > > > > > platforms. When talking about a cache we three fundamental
> > > > > > operations:
> > > > > >
> > > > > > 1) write back - this writes the content of the cache back
> > > > > > to the
> > > > > >backing memory
> > > > > > 2) invalidate - this remove the content of the cache
> > > > > > 3) write back + invalidate - do both of the above
> > > > >
> > > > > Agreed that drm_cflush_* isn't a great API. In this particular case
> > > > > (IIUC), I need wb+inv so that there aren't dirty cache lines that drop
> > > > > out to memory later, and so that I don't get a cache hit on
> > > > > uncached/wc mmap'ing.
> > > >
> > > > Is there a cacheable alias lying around (e.g. the linear map), or are
> > > > these addresses only mapped uncached/wc?
> > > >
> > > > If there's a cacheable alias, performing an invalidate isn't sufficient,
> > > > since a CPU can allocate a new (clean) entry at any point in time (e.g.
> > > > as a result of prefetching or arbitrary speculation).
> > >
> > > I *believe* that there are not alias mappings (that I don't control
> > > myself) for pages coming from
> > > shmem_file_setup()/shmem_read_mapping_page()..
> >
> > AFAICT, that's regular anonymous memory, so there will be a cacheable
> > alias in the linear/direct map.
>
> tbh, I'm not 100% sure whether there is a cacheable alias, or whether
> any potential linear map is torn down.
I'm fairly confident that the linear/direct map cacheable alias is not
torn down when pages are allocated. The gneeric page allocation code
doesn't do so, and I see nothing the shmem code to do so.
For arm64, we can tear down portions of the linear map, but that has to
be done explicitly, and this is only possible when using rodata_full. If
not using rodata_full, it is not possible to dynamically tear down the
cacheable alias.
> My understanding is that a cacheable alias is "ok", with some
> caveats.. ie. that the cacheable alias is not accessed.
Unfortunately, that is not true. You'll often get away with it in
practice, but that's a matter of probability rather than a guarantee.
You cannot prevent a CPU from accessing a VA arbitrarily (e.g. as the
result of wild speculation). The ARM ARM (ARM DDI 0487E.a) points this
out explicitly:
| Entries for addresses that are Normal Cacheable can be allocated to
| the cache at any time, and so the cache invalidate instruction cannot
| ensure that the address is not present in a cache.
... along with:
| Caches introduce a number of potential problems, mainly because:
|
| * Memory accesses can occur at times other than when the programmer
| would expect them.
;)
> I'm not entirely sure about pre-fetch from access to adjacent pages.
> We have been using shmem as a source for pages since the beginning,
> and I haven't seen it cause any problems in the last 6 years. (This
> is limited to armv7 and armv8, I'm not really sure what would happen
> on armv6, but that is a combo I don't have to care about.)
Over time, CPUs get more aggressive w.r.t. prefetching and speculation,
so having not seen such issues in the past does not imply we won't in
future.
Anecdotally, we had an issue a few years ago that we were only able to
reproduce under heavy stress testing. A CPU would make speculative
instruction fetches from a read-destructive MMIO register, despite the
PC never going near the corresponding VA, and despite that code having
(apparently) caused no issue for years before that.
See commit:
b6ccb9803e90c16b ("ARM: 7954/1: mm: remove remaining domain support from
ARMv6")
... which unfortunately lacks the full war story.
Thanks,
Mark.
[PATCH] drm/nouveau/nvif/mmu: Use struct_size() helper
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct nvif_mmu_kind_v0 {
...
__u8 data[];
};
Make use of the struct_size() helper instead of an open-coded version
in order to avoid any potential type mistakes.
So, replace the following form:
sizeof(*kind) + sizeof(*kind->data) * mmu->kind_nr
with:
struct_size(kind, data, mmu->kind_nr)
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/nouveau/nvif/mmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/mmu.c
b/drivers/gpu/drm/nouveau/nvif/mmu.c
index ae08a1ca8044..5641bda2046d 100644
--- a/drivers/gpu/drm/nouveau/nvif/mmu.c
+++ b/drivers/gpu/drm/nouveau/nvif/mmu.c
@@ -110,7 +110,7 @@ nvif_mmu_init(struct nvif_object *parent, s32 oclass,
struct nvif_mmu *mmu)
if (mmu->kind_nr) {
struct nvif_mmu_kind_v0 *kind;
- u32 argc = sizeof(*kind) + sizeof(*kind->data) * mmu->kind_nr;
+ size_t argc = struct_size(kind, data, mmu->kind_nr);
if (ret = -ENOMEM, !(kind = kmalloc(argc, GFP_KERNEL)))
goto done;
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] video: fbdev/mmp/core: Use struct_size() in kzalloc()
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct mmp_path {
...
struct mmp_overlay overlays[0];
};
size = sizeof(struct mmp_path) + count * sizeof(struct mmp_overlay);
instance = kzalloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
instance = kzalloc(struct_size(instance, overlays, count), GFP_KERNEL)
Notice that, in this case, variable size is not necessary, hence it
is removed.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva
---
drivers/video/fbdev/mmp/core.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/mmp/core.c b/drivers/video/fbdev/mmp/core.c
index 0ffc1b7b7052..154127256a2c 100644
--- a/drivers/video/fbdev/mmp/core.c
+++ b/drivers/video/fbdev/mmp/core.c
@@ -153,13 +153,11 @@ EXPORT_SYMBOL_GPL(mmp_get_path);
struct mmp_path *mmp_register_path(struct mmp_path_info *info)
{
int i;
- size_t size;
struct mmp_path *path = NULL;
struct mmp_panel *panel;
- size = sizeof(struct mmp_path)
- + sizeof(struct mmp_overlay) * info->overlay_num;
- path = kzalloc(size, GFP_KERNEL);
+ path = kzalloc(struct_size(path, overlays, info->overlay_num),
+ GFP_KERNEL);
if (!path)
return NULL;
--
2.22.0
[PATCH] drm: Fix kerneldoc warns in connector-related docs
From: Sean Paul
Fixes the following warnings:
../drivers/gpu/drm/drm_connector.c:989: WARNING: Unexpected indentation.
../drivers/gpu/drm/drm_connector.c:993: WARNING: Unexpected indentation.
../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or phrase
reference start-string without end-string.
../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or phrase
reference start-string without end-string.
Fixes: 1b27fbdde1df ("drm: Add drm_atomic_get_(old|new)_connector_for_encoder()
helpers")
Fixes: bb5a45d40d50 ("drm/hdcp: update content protection property with uevent")
Cc: Ramalingam C
Cc: Daniel Vetter
Cc: Pekka Paalanen
Cc: Sam Ravnborg
Cc: Laurent Pinchart
Cc: Jani Nikula
Cc: Sean Paul
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Sean Paul
Cc: David Airlie
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Sean Paul
---
drivers/gpu/drm/drm_connector.c | 10 ++
include/drm/drm_connector.h | 4 ++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 354798bad576..4c766624b20d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -986,12 +986,14 @@ static const struct drm_prop_enum_list hdmi_colorspaces[]
= {
* - Kernel sends uevent with the connector id and property id through
* @drm_hdcp_update_content_protection, upon below kernel triggered
* scenarios:
- * DESIRED -> ENABLED (authentication success)
- * ENABLED -> DESIRED (termination of authentication)
+ *
+ * - DESIRED -> ENABLED (authentication success)
+ * - ENABLED -> DESIRED (termination of authentication)
* - Please note no uevents for userspace triggered property state changes,
* which can't fail such as
- * DESIRED/ENABLED -> UNDESIRED
- * UNDESIRED -> DESIRED
+ *
+ * - DESIRED/ENABLED -> UNDESIRED
+ * - UNDESIRED -> DESIRED
* - Userspace is responsible for polling the property or listen to uevents
* to determine when the value transitions from ENABLED to DESIRED.
* This signifies the link is no longer protected and userspace should
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 0b9997e27689..e391f9c05f98 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -543,8 +543,8 @@ struct drm_connector_state {
*
* This is also used in the atomic helpers to map encoders to their
* current and previous connectors, see
-* _atomic_get_old_connector_for_encoder() and
-* _atomic_get_new_connector_for_encoder().
+* _atomic_get_old_connector_for_encoder and
+* _atomic_get_new_connector_for_encoder.
*
* NOTE: Atomic drivers must fill this out (either themselves or through
* helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: add cache support for arm64
On Wed, Aug 7, 2019 at 5:38 AM Mark Rutland wrote: > > On Tue, Aug 06, 2019 at 09:31:55AM -0700, Rob Clark wrote: > > On Tue, Aug 6, 2019 at 7:35 AM Mark Rutland wrote: > > > > > > On Tue, Aug 06, 2019 at 07:11:41AM -0700, Rob Clark wrote: > > > > On Tue, Aug 6, 2019 at 1:48 AM Christoph Hellwig wrote: > > > > > > > > > > This goes in the wrong direction. drm_cflush_* are a bad API we need > > > > > to > > > > > get rid of, not add use of it. The reason for that is two-fold: > > > > > > > > > > a) it doesn't address how cache maintaince actually works in most > > > > > platforms. When talking about a cache we three fundamental > > > > > operations: > > > > > > > > > > 1) write back - this writes the content of the cache back to > > > > > the > > > > >backing memory > > > > > 2) invalidate - this remove the content of the cache > > > > > 3) write back + invalidate - do both of the above > > > > > > > > Agreed that drm_cflush_* isn't a great API. In this particular case > > > > (IIUC), I need wb+inv so that there aren't dirty cache lines that drop > > > > out to memory later, and so that I don't get a cache hit on > > > > uncached/wc mmap'ing. > > > > > > Is there a cacheable alias lying around (e.g. the linear map), or are > > > these addresses only mapped uncached/wc? > > > > > > If there's a cacheable alias, performing an invalidate isn't sufficient, > > > since a CPU can allocate a new (clean) entry at any point in time (e.g. > > > as a result of prefetching or arbitrary speculation). > > > > I *believe* that there are not alias mappings (that I don't control > > myself) for pages coming from > > shmem_file_setup()/shmem_read_mapping_page().. > > AFAICT, that's regular anonymous memory, so there will be a cacheable > alias in the linear/direct map. tbh, I'm not 100% sure whether there is a cacheable alias, or whether any potential linear map is torn down. My understanding is that a cacheable alias is "ok", with some caveats.. ie. that the cacheable alias is not accessed. I'm not entirely sure about pre-fetch from access to adjacent pages. We have been using shmem as a source for pages since the beginning, and I haven't seen it cause any problems in the last 6 years. (This is limited to armv7 and armv8, I'm not really sure what would happen on armv6, but that is a combo I don't have to care about.) BR, -R > > digging around at what dma_sync_sg_* does under the hood, it looks > > like it is just arch_sync_dma_for_cpu/device(), so I guess that should > > be sufficient for what I need. > > I don't think that's the case, per the example I gave above. > > Thanks, > Mark.
Re: [PATCH 8/8] drm/bridge: dw-hdmi-i2s: add .get_eld support
On Wed 07 Aug 2019 at 14:57, Jonas Karlman wrote:
> On 2019-08-05 15:41, Jerome Brunet wrote:
>> Provide the eld to the generic hdmi-codec driver.
>> This will let the driver enforce the maximum channel number and set the
>> channel allocation depending on the hdmi sink.
>>
>> Cc: Jonas Karlman
>> Signed-off-by: Jerome Brunet
>> ---
>> drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h | 1 +
>> drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 10 ++
>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 +
>> 3 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
>> index 63b5756f463b..cb07dc0da5a7 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
>> @@ -14,6 +14,7 @@ struct dw_hdmi_audio_data {
>>
>> struct dw_hdmi_i2s_audio_data {
>> struct dw_hdmi *hdmi;
>> +u8 *eld;
>>
>> void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
>> u8 (*read)(struct dw_hdmi *hdmi, int offset);
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>> index b8ece9c1ba2c..14d499b344c0 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>> @@ -121,6 +121,15 @@ static void dw_hdmi_i2s_audio_shutdown(struct device
>> *dev, void *data)
>> dw_hdmi_audio_disable(hdmi);
>> }
>>
>> +static int dw_hdmi_i2s_get_eld(struct device *dev, void *data, uint8_t *buf,
>> + size_t len)
>> +{
>> +struct dw_hdmi_i2s_audio_data *audio = data;
>> +
>> +memcpy(buf, audio->eld, min(sizeof(audio->eld), len));
>
> Above sizeof does not work as intended, sizeof(audio->eld) is probably the
> size of a pointer and not MAX_ELD_BYTES (128),
> resulting in only part of the ELD gets copied to buf.
Silly ... thanks for pointing this out. I'll respin
>
>
> Thanks for reworking dw-hdmi multi channel lpcm support!, this works on
> Rockchip for most parts.
> Without the [1] patch (reset cts+n after clock is enabled) audio sometime
> stay silent when switching between e.g. 2ch 44.1khz and 6ch 48khz tracks.
> It is not fully clear to me why reset cts+n helps, if that have
> negative affects on other platforms or if there is another way to
> solve loosing audio.
I did not see that issue my self but I guess could propose this change ?
>
> I also have a small issue with the channel allocation being selected by
> hdmi-codec, my soundbar reports LPCM 6.1ch instead of LPCM 7.1ch when I play
> a 7.1ch speaker test clip.
> I have a hdmi-codec patch to fix selection of channel allocation in
> queue.
Yes, I know there is still a few problems around that and stale eld.
But those problem are not really coming from the i2s interface of the
dw-hdmi itself and should be dealt with separatly.
This is just the beginning ;)
>
>
> For patch 1-7:
>
> Reviewed-by: Jonas Karlman
>
>
> [1]
> https://github.com/Kwiboo/linux-rockchip/commit/c0839e874f843ad173ddde958303d6878394ef92
>
> Regards,
> Jonas
>
>> +return 0;
>> +}
>> +
>> static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
>>struct device_node *endpoint)
>> {
>> @@ -144,6 +153,7 @@ static int dw_hdmi_i2s_get_dai_id(struct
>> snd_soc_component *component,
>> static struct hdmi_codec_ops dw_hdmi_i2s_ops = {
>> .hw_params = dw_hdmi_i2s_hw_params,
>> .audio_shutdown = dw_hdmi_i2s_audio_shutdown,
>> +.get_eld= dw_hdmi_i2s_get_eld,
>> .get_dai_id = dw_hdmi_i2s_get_dai_id,
>> };
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index bed4bb017afd..8df69c9dbfad 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2797,6 +2797,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>> struct dw_hdmi_i2s_audio_data audio;
>>
>> audio.hdmi = hdmi;
>> +audio.eld = hdmi->connector.eld;
>> audio.write = hdmi_writeb;
>> audio.read = hdmi_readb;
>> hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
Re: [PATCH 3/4] dma-buf: further relax reservation_object_add_shared_fence
Quoting Christian König (2019-08-07 14:53:11)
> Other cores don't busy wait any more and we removed the last user of checking
> the seqno for changes. Drop updating the number for shared fences altogether.
>
> Signed-off-by: Christian König
Reviewed-by: Chris Wilson
> ---
> drivers/dma-buf/reservation.c| 6 --
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 7 +--
> 2 files changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 8fcaddffd5d4..90bc6ef03598 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -237,9 +237,6 @@ void reservation_object_add_shared_fence(struct
> reservation_object *obj,
> fobj = reservation_object_get_list(obj);
> count = fobj->shared_count;
>
> - preempt_disable();
> - write_seqcount_begin(>seq);
> -
> for (i = 0; i < count; ++i) {
>
> old = rcu_dereference_protected(fobj->shared[i],
> @@ -257,9 +254,6 @@ void reservation_object_add_shared_fence(struct
> reservation_object *obj,
> RCU_INIT_POINTER(fobj->shared[i], fence);
> /* pointer update must be visible before we extend the shared_count */
> smp_store_mb(fobj->shared_count, count);
> -
> - write_seqcount_end(>seq);
> - preempt_enable();
> dma_fence_put(old);
> }
> EXPORT_SYMBOL(reservation_object_add_shared_fence);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index fe062b76ec91..a4640ddc24d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -251,12 +251,7 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct
> amdgpu_bo *bo,
> new->shared_max = old->shared_max;
> new->shared_count = k;
>
> - /* Install the new fence list, seqcount provides the barriers */
> - preempt_disable();
> - write_seqcount_begin(>seq);
> - RCU_INIT_POINTER(resv->fence, new);
> - write_seqcount_end(>seq);
> - preempt_enable();
> + rcu_assign_pointer(resv->fence, new);
but you'll probably want a local ack for amdgpu/
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: add cache support for arm64
On Tue, Aug 6, 2019 at 11:25 PM Christoph Hellwig wrote: > > On Tue, Aug 06, 2019 at 09:23:51AM -0700, Rob Clark wrote: > > On Tue, Aug 6, 2019 at 8:50 AM Christoph Hellwig wrote: > > > > > > On Tue, Aug 06, 2019 at 07:11:41AM -0700, Rob Clark wrote: > > > > Agreed that drm_cflush_* isn't a great API. In this particular case > > > > (IIUC), I need wb+inv so that there aren't dirty cache lines that drop > > > > out to memory later, and so that I don't get a cache hit on > > > > uncached/wc mmap'ing. > > > > > > So what is the use case here? Allocate pages using the page allocator > > > (or CMA for that matter), and then mmaping them to userspace and never > > > touching them again from the kernel? > > > > Currently, it is pages coming from tmpfs. Ideally we want pages that > > are swappable when unpinned. > > tmpfs is basically a (complicated) frontend for alloc pages as far > as page allocation is concerned. > > > CPU mappings are *mostly* just mapping to userspace. There are a few > > exceptions that are vmap'd (fbcon, and ringbuffer). > > And those use the same backend? yes > > (Eventually I'd like to support pages passed in from userspace.. but > > that is down the road.) > > Eww. Please talk to the iommu list before starting on that. This is more of a long term goal, we can't do it until we have per-context/process pagetables, ofc. Getting a bit off topic, but I'm curious about what problems you are concerned about. Userspace can shoot it's own foot, but if it is not sharing GPU pagetables with other processes, it can't shoot other's feet. (I'm guessing you are concerned about non-page-aligned mappings?) > > > > Tying it in w/ iommu seems a bit weird to me.. but maybe that is just > > > > me, I'm certainly willing to consider proposals or to try things and > > > > see how they work out. > > > > > > This was just my through as the fit seems easy. But maybe you'll > > > need to explain your use case(s) a bit more so that we can figure out > > > what a good high level API is. > > > > Tying it to iommu_map/unmap would be awkward, as we could need to > > setup cpu mmap before it ends up mapped to iommu. And the plan to > > support per-process pagetables involved creating an iommu_domain per > > userspace gl context.. some buffers would end up mapped into multiple > > contexts/iommu_domains. > > > > If the cache operation was detached from iommu_map/unmap, then it > > would seem weird to be part of the iommu API. > > > > I guess I'm not entirely sure what you had in mind, but this is why > > iommu seemed to me like a bad fit. > > So back to the question, I'd like to understand your use case (and > maybe hear from the other drm folks if that is common): > > - you allocate pages from shmem (why shmem, btw? if this is done by >other drm drivers how do they guarantee addressability without an >iommu?) shmem for swappable pages. I don't unpin and let things get swapped out yet, but I'm told it starts to become important when you have 50 browser tabs open ;-) There are some display-only drm drivers with no IOMMU, which use CMA rather than shmem. Basically every real GPU has some form of MMU or IOMMU for memory protection. (The couple exceptions do expensive kernel side cmdstream validation.) > - then the memory is either mapped to userspace or vmapped (or even >both, althrough the lack of aliasing you mentioned would speak >against it) as writecombine (aka arm v6+ normal uncached). Does >the mapping live on until the memory is freed? (side note, *most* of the drm/msm supported devices are armv8, the exceptions are 8060 and 8064 which are armv7.. I don't think drm/msm will ever have to deal w/ armv6) Userspace keeps the userspace mmap around opportunistically (once it is mmap'd, not all buffers will be accessed from CPU). In fact there is a userspace buffer cache, where we try to re-use buffers that are already allocated and possibly mmap'd, since allocation and setting up mmap is expensive. (There is an MADVISE ioctl so userspace can tell kernel about buffers in the cache, which are available to be purged by shrinker.. if a buffer is purged, it's userspace mmap is torn down... along with it's IOMMU map, ofc) > - as you mention swapping - how do you guarantee there are no >aliases in the kernel direct mapping after the page has been swapped >in? Before unpinning and allowing pages to be swapped out, CPU and IOMMU maps would be torn down. (I don't think we could unpin buffers w/ a vmap, but those are just a drop in the bucket.) Currently, the kernel always knows buffers associated w/ a submit (job) queued to the GPU, so it could bring pages back in and re-store iommu map.. the fault handler can be used to bring things back in for CPU access. (For more free-form HMM style access to userspace memory, we'd need to be able to sleep in IOMMU fault handler before the IOMMU resumes translation.) As far as cache is concerned, it would be basically the same
Re: [PATCH 2/4] drm/i915: use new reservation_object_fences helper
Quoting Christian König (2019-08-07 14:53:10) > Instead of open coding the sequence loop use the new helper. > > Signed-off-by: Christian König Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] dma-buf: add reservation_object_fences helper
Quoting Christian König (2019-08-07 14:53:09) > Add a new helper to get a consistent set of pointers from the reservation > object. While at it group all access helpers together in the header file. > > v2: correctly return shared_count as well > > Signed-off-by: Christian König Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Regression] "drm/amdgpu: enable gfxoff again on raven series (v2)"
May I know the all firmware version in your system?
Thanks,
Ray
From: Kai-Heng Feng
Sent: Wednesday, August 7, 2019 8:50 PM
To: Huang, Ray
Cc: Deucher, Alexander; Koenig, Christian; Zhou, David(ChunMing); amd-gfx list;
dri-devel@lists.freedesktop.org; LKML; Anthony Wong
Subject: [Regression] "drm/amdgpu: enable gfxoff again on raven series (v2)"
Hi,
After commit 005440066f92 ("drm/amdgpu: enable gfxoff again on raven series
(v2)”), browsers on Raven Ridge systems cause serious corruption like this:
https://launchpadlibrarian.net/436319772/Screenshot%20from%202019-08-07%2004-20-34.png
Firmwares for Raven Ridge is up-to-date.
Kai-Heng
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] drm-misc-next: Revert patches missing reviews
On Wed, Aug 07, 2019 at 10:20:53AM -0400, Sean Paul wrote: > From: Sean Paul > > Hellooo, > This has been covered ad nauseam on the m-l and irc, but for the record: > > Reviews are a mandatory requirement for patches in drm-misc-next, it's > what keeps us all honest in the committer model. The most recent > drm-misc-next pull included a handful of patches that were missing > reviews. There was absolutely zero nefarious intent, but rules are > rules, so we're reverting them. > > I feel confident re-applying all of these with my SoB, so I plan on > doing that in the same push. Applied to drm-misc-next with all patches re-applied. Sean > > Thanks to everyone for handling this so well, seriously awesome that > we can stay constructive as a community \o/. > > Lastly, this will be caught by our tooling in the future so this should > be the last time this happens (on accident). > > Sean > > Sean Paul (5): > Revert "Revert "drm/gem: Rename drm_gem_dumb_map_offset() to > drm_gem_map_offset()"" > Revert "Revert "drm/panfrost: Use drm_gem_map_offset()"" > Revert "drm/vgem: drop DRM_AUTH usage from the driver" > Revert "drm/msm: drop DRM_AUTH usage from the driver" > Revert "drm/nouveau: remove open-coded drm_invalid_op()" > > drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- > drivers/gpu/drm/drm_gem.c | 10 +++--- > drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 +-- > drivers/gpu/drm/msm/msm_drv.c | 22 +++--- > drivers/gpu/drm/nouveau/nouveau_abi16.c | 6 ++ > drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 + > drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-- > drivers/gpu/drm/vgem/vgem_drv.c | 4 ++-- > include/drm/drm_gem.h | 4 ++-- > 10 files changed, 35 insertions(+), 37 deletions(-) > > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm/panfrost: Revert drm_gem_map_offset changes
On Wed, Aug 07, 2019 at 10:52:46AM -0400, Sean Paul wrote: > From: Sean Paul > > When I started re-applying these I realized they hadn't hit the list. > Applied to drm-misc-next Sean > Sean > > Rob Herring (2): > Revert "drm/gem: Rename drm_gem_dumb_map_offset() to > drm_gem_map_offset()" > Revert "drm/panfrost: Use drm_gem_map_offset()" > > drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- > drivers/gpu/drm/drm_gem.c | 10 +++--- > drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++- > drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-- > include/drm/drm_gem.h | 4 ++-- > 5 files changed, 23 insertions(+), 14 deletions(-) > > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] Revert "drm/panfrost: Use drm_gem_map_offset()"
On Wed, 7 Aug 2019 at 15:53, Sean Paul wrote: > > From: Rob Herring > > This reverts commit 583bbf46133c726bae277e8f4e32bfba2a528c7f. > > Turns out we need mmap to work on imported BOs even if the current code > is buggy. > Personally I would have mentioned a use case where imported BOs are used. > Signed-off-by: Rob Herring > Signed-off-by: Sean Paul Regardless of the above nitpick, with the patch order fixed the series is: Reviewed-by: Emil Velikov ... in case you haven't picked it already. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm pull for v5.3-rc1
On Wed, Aug 07, 2019 at 04:32:51PM +0100, Steven Price wrote:
> On 07/08/2019 15:56, Matthew Wilcox wrote:
> > On Wed, Aug 07, 2019 at 03:30:38PM +0100, Steven Price wrote:
> >> On 07/08/2019 15:15, Matthew Wilcox wrote:
> >>> On Tue, Aug 06, 2019 at 11:40:00PM -0700, Christoph Hellwig wrote:
> On Tue, Aug 06, 2019 at 12:09:38PM -0700, Matthew Wilcox wrote:
> > Has anyone looked at turning the interface inside-out? ie something
> > like:
> >
> > struct mm_walk_state state = { .mm = mm, .start = start, .end =
> > end, };
> >
> > for_each_page_range(, page) {
> > ... do something with page ...
> > }
> >
> > with appropriate macrology along the lines of:
> >
> > #define for_each_page_range(state, page)
> > \
> > while ((page = page_range_walk_next(state)))
> >
> > Then you don't need to package anything up into structs that are shared
> > between the caller and the iterated function.
>
> I'm not an all that huge fan of super magic macro loops. But in this
> case I don't see how it could even work, as we get special callbacks
> for huge pages and holes, and people are trying to add a few more ops
> as well.
> >>>
> >>> We could have bits in the mm_walk_state which indicate what things to
> >>> return
> >>> and what things to skip. We could (and probably should) also use
> >>> different
> >>> iterator names if people actually want to iterate different things. eg
> >>> for_each_pte_range(, pte) as well as for_each_page_range().
> >>>
> >>
> >> The iterator approach could be awkward for the likes of my generic
> >> ptdump implementation[1]. It would require an iterator which returns all
> >> levels and allows skipping levels when required (to prevent KASAN
> >> slowing things down too much). So something like:
> >>
> >> start_walk_range();
> >> for_each_page_range(, page) {
> >>switch(page->level) {
> >>case PTE:
> >>...
> >>case PMD:
> >>if (...)
> >>skip_pmd();
> >>...
> >>case HOLE:
> >>
> >>...
> >>}
> >> }
> >> end_walk_range();
> >>
> >> It seems a little fragile - e.g. we wouldn't (easily) get type checking
> >> that you are actually treating a PTE as a pte_t. The state mutators like
> >> skip_pmd() also seem a bit clumsy.
> >
> > Once you're on-board with using a state structure, you can use it in all
> > kinds of fun ways. For example:
> >
> > struct mm_walk_state {
> > struct mm_struct *mm;
> > unsigned long start;
> > unsigned long end;
> > unsigned long curr;
> > p4d_t p4d;
> > pud_t pud;
> > pmd_t pmd;
> > pte_t pte;
> > enum page_entry_size size;
> > int flags;
> > };
> >
> > For this user, I'd expect something like ...
> >
> > DECLARE_MM_WALK_FLAGS(state, mm, start, end,
> > MM_WALK_HOLES | MM_WALK_ALL_SIZES);
> >
> > walk_each_pte(state) {
> > switch (state->size) {
> > case PE_SIZE_PTE:
> > ...
> > case PE_SIZE_PMD:
> > if (...(state->pmd))
> > continue;
>
> You need to be able to signal whether you want to descend into the PMD
> or skip the entire part of the tree. This was my skip_pmd() function above.
Do you? My assumption was that if there's a PMD entry, you either want
to be called once for the entire PMD entry, or 512 times for each PTE
entry that would be in the PMD if it hadn't been collapsed, and you
could indicate this through a flag in the state. Is it more dynamic
than that for some users?
In any case, we could have a skip_pmd() function; I'm just not
sure we need it.
> > ...
> > }
> > }
> >
> > There's no need to have start / end walk function calls.
>
> You've got a start walk function (it's your DECLARE_MM_WALK_FLAGS
> above). The end walk I agree I think you don't actually need it since
> struct mm_walk_state contains all the state.
Ah, I misunderstood what you meant.
Re: drm pull for v5.3-rc1
On 07/08/2019 15:56, Matthew Wilcox wrote:
> On Wed, Aug 07, 2019 at 03:30:38PM +0100, Steven Price wrote:
>> On 07/08/2019 15:15, Matthew Wilcox wrote:
>>> On Tue, Aug 06, 2019 at 11:40:00PM -0700, Christoph Hellwig wrote:
On Tue, Aug 06, 2019 at 12:09:38PM -0700, Matthew Wilcox wrote:
> Has anyone looked at turning the interface inside-out? ie something like:
>
> struct mm_walk_state state = { .mm = mm, .start = start, .end = end, };
>
> for_each_page_range(, page) {
> ... do something with page ...
> }
>
> with appropriate macrology along the lines of:
>
> #define for_each_page_range(state, page) \
> while ((page = page_range_walk_next(state)))
>
> Then you don't need to package anything up into structs that are shared
> between the caller and the iterated function.
I'm not an all that huge fan of super magic macro loops. But in this
case I don't see how it could even work, as we get special callbacks
for huge pages and holes, and people are trying to add a few more ops
as well.
>>>
>>> We could have bits in the mm_walk_state which indicate what things to return
>>> and what things to skip. We could (and probably should) also use different
>>> iterator names if people actually want to iterate different things. eg
>>> for_each_pte_range(, pte) as well as for_each_page_range().
>>>
>>
>> The iterator approach could be awkward for the likes of my generic
>> ptdump implementation[1]. It would require an iterator which returns all
>> levels and allows skipping levels when required (to prevent KASAN
>> slowing things down too much). So something like:
>>
>> start_walk_range();
>> for_each_page_range(, page) {
>> switch(page->level) {
>> case PTE:
>> ...
>> case PMD:
>> if (...)
>> skip_pmd();
>> ...
>> case HOLE:
>>
>> ...
>> }
>> }
>> end_walk_range();
>>
>> It seems a little fragile - e.g. we wouldn't (easily) get type checking
>> that you are actually treating a PTE as a pte_t. The state mutators like
>> skip_pmd() also seem a bit clumsy.
>
> Once you're on-board with using a state structure, you can use it in all
> kinds of fun ways. For example:
>
> struct mm_walk_state {
> struct mm_struct *mm;
> unsigned long start;
> unsigned long end;
> unsigned long curr;
> p4d_t p4d;
> pud_t pud;
> pmd_t pmd;
> pte_t pte;
> enum page_entry_size size;
> int flags;
> };
>
> For this user, I'd expect something like ...
>
> DECLARE_MM_WALK_FLAGS(state, mm, start, end,
> MM_WALK_HOLES | MM_WALK_ALL_SIZES);
>
> walk_each_pte(state) {
> switch (state->size) {
> case PE_SIZE_PTE:
> ...
> case PE_SIZE_PMD:
> if (...(state->pmd))
> continue;
You need to be able to signal whether you want to descend into the PMD
or skip the entire part of the tree. This was my skip_pmd() function above.
> ...
> }
> }
>
> There's no need to have start / end walk function calls.
>
You've got a start walk function (it's your DECLARE_MM_WALK_FLAGS
above). The end walk I agree I think you don't actually need it since
struct mm_walk_state contains all the state.
Steve
[Bug 110865] Rx480 consumes 20w more power in idle than under Windows
https://bugs.freedesktop.org/show_bug.cgi?id=110865 --- Comment #3 from Martin --- Thank you for your explanation. How do I find out the blanking periods? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] nouveau/hmm: map pages after migration
When memory is migrated to the GPU it is likely to be accessed by GPU
code soon afterwards. Instead of waiting for a GPU fault, map the
migrated memory into the GPU page tables with the same access permissions
as the source CPU page table entries. This preserves copy on write
semantics.
Signed-off-by: Ralph Campbell
Cc: Christoph Hellwig
Cc: Jason Gunthorpe
Cc: "Jérôme Glisse"
Cc: Ben Skeggs
---
This patch is based on top of Christoph Hellwig's 9 patch series
https://lore.kernel.org/linux-mm/20190729234611.gc7...@redhat.com/T/#u
"turn the hmm migrate_vma upside down" but without patch 9
"mm: remove the unused MIGRATE_PFN_WRITE" and adds a use for the flag.
drivers/gpu/drm/nouveau/nouveau_dmem.c | 45 +-
drivers/gpu/drm/nouveau/nouveau_svm.c | 86 ++
drivers/gpu/drm/nouveau/nouveau_svm.h | 19 ++
3 files changed, 133 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c
b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index ef9de82b0744..c83e6f118817 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -25,11 +25,13 @@
#include "nouveau_dma.h"
#include "nouveau_mem.h"
#include "nouveau_bo.h"
+#include "nouveau_svm.h"
#include
#include
#include
#include
+#include
#include
#include
@@ -560,11 +562,12 @@ nouveau_dmem_init(struct nouveau_drm *drm)
}
static unsigned long nouveau_dmem_migrate_copy_one(struct nouveau_drm *drm,
- struct vm_area_struct *vma, unsigned long addr,
- unsigned long src, dma_addr_t *dma_addr)
+ struct vm_area_struct *vma, unsigned long src,
+ dma_addr_t *dma_addr, u64 *pfn)
{
struct device *dev = drm->dev->dev;
struct page *dpage, *spage;
+ unsigned long paddr;
spage = migrate_pfn_to_page(src);
if (!spage || !(src & MIGRATE_PFN_MIGRATE))
@@ -572,17 +575,21 @@ static unsigned long nouveau_dmem_migrate_copy_one(struct
nouveau_drm *drm,
dpage = nouveau_dmem_page_alloc_locked(drm);
if (!dpage)
- return 0;
+ goto out;
*dma_addr = dma_map_page(dev, spage, 0, PAGE_SIZE, DMA_BIDIRECTIONAL);
if (dma_mapping_error(dev, *dma_addr))
goto out_free_page;
+ paddr = nouveau_dmem_page_addr(dpage);
if (drm->dmem->migrate.copy_func(drm, 1, NOUVEAU_APER_VRAM,
- nouveau_dmem_page_addr(dpage), NOUVEAU_APER_HOST,
- *dma_addr))
+ paddr, NOUVEAU_APER_HOST, *dma_addr))
goto out_dma_unmap;
+ *pfn = NVIF_VMM_PFNMAP_V0_V | NVIF_VMM_PFNMAP_V0_VRAM |
+ ((paddr >> PAGE_SHIFT) << NVIF_VMM_PFNMAP_V0_ADDR_SHIFT);
+ if (src & MIGRATE_PFN_WRITE)
+ *pfn |= NVIF_VMM_PFNMAP_V0_W;
return migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
out_dma_unmap:
@@ -590,18 +597,19 @@ static unsigned long nouveau_dmem_migrate_copy_one(struct
nouveau_drm *drm,
out_free_page:
nouveau_dmem_page_free_locked(drm, dpage);
out:
+ *pfn = NVIF_VMM_PFNMAP_V0_NONE;
return 0;
}
static void nouveau_dmem_migrate_chunk(struct migrate_vma *args,
- struct nouveau_drm *drm, dma_addr_t *dma_addrs)
+ struct nouveau_drm *drm, dma_addr_t *dma_addrs, u64 *pfns)
{
struct nouveau_fence *fence;
unsigned long addr = args->start, nr_dma = 0, i;
for (i = 0; addr < args->end; i++) {
args->dst[i] = nouveau_dmem_migrate_copy_one(drm, args->vma,
- addr, args->src[i], _addrs[nr_dma]);
+ args->src[i], _addrs[nr_dma], [i]);
if (args->dst[i])
nr_dma++;
addr += PAGE_SIZE;
@@ -615,10 +623,6 @@ static void nouveau_dmem_migrate_chunk(struct migrate_vma
*args,
dma_unmap_page(drm->dev->dev, dma_addrs[nr_dma], PAGE_SIZE,
DMA_BIDIRECTIONAL);
}
- /*
-* FIXME optimization: update GPU page table to point to newly migrated
-* memory.
-*/
migrate_vma_finalize(args);
}
@@ -631,11 +635,12 @@ nouveau_dmem_migrate_vma(struct nouveau_drm *drm,
unsigned long npages = (end - start) >> PAGE_SHIFT;
unsigned long max = min(SG_MAX_SINGLE_ALLOC, npages);
dma_addr_t *dma_addrs;
+ u64 *pfns;
struct migrate_vma args = {
.vma= vma,
.start = start,
};
- unsigned long c, i;
+ unsigned long i;
int ret = -ENOMEM;
args.src = kcalloc(max, sizeof(args.src), GFP_KERNEL);
@@ -649,19 +654,25 @@ nouveau_dmem_migrate_vma(struct nouveau_drm *drm,
if (!dma_addrs)
goto out_free_dst;
- for (i = 0; i < npages; i += c) {
- c = min(SG_MAX_SINGLE_ALLOC, npages);
-
Re: [PATCH 8/8] drm/bridge: dw-hdmi-i2s: add .get_eld support
On 2019-08-05 15:41, Jerome Brunet wrote:
> Provide the eld to the generic hdmi-codec driver.
> This will let the driver enforce the maximum channel number and set the
> channel allocation depending on the hdmi sink.
>
> Cc: Jonas Karlman
> Signed-off-by: Jerome Brunet
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h | 1 +
> drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 10 ++
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 +
> 3 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> index 63b5756f463b..cb07dc0da5a7 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> @@ -14,6 +14,7 @@ struct dw_hdmi_audio_data {
>
> struct dw_hdmi_i2s_audio_data {
> struct dw_hdmi *hdmi;
> + u8 *eld;
>
> void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
> u8 (*read)(struct dw_hdmi *hdmi, int offset);
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> index b8ece9c1ba2c..14d499b344c0 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> @@ -121,6 +121,15 @@ static void dw_hdmi_i2s_audio_shutdown(struct device
> *dev, void *data)
> dw_hdmi_audio_disable(hdmi);
> }
>
> +static int dw_hdmi_i2s_get_eld(struct device *dev, void *data, uint8_t *buf,
> +size_t len)
> +{
> + struct dw_hdmi_i2s_audio_data *audio = data;
> +
> + memcpy(buf, audio->eld, min(sizeof(audio->eld), len));
Above sizeof does not work as intended, sizeof(audio->eld) is probably the size
of a pointer and not MAX_ELD_BYTES (128),
resulting in only part of the ELD gets copied to buf.
Thanks for reworking dw-hdmi multi channel lpcm support!, this works on
Rockchip for most parts.
Without the [1] patch (reset cts+n after clock is enabled) audio sometime stay
silent when switching between e.g. 2ch 44.1khz and 6ch 48khz tracks.
It is not fully clear to me why reset cts+n helps, if that have negative
affects on other platforms or if there is another way to solve loosing audio.
I also have a small issue with the channel allocation being selected by
hdmi-codec, my soundbar reports LPCM 6.1ch instead of LPCM 7.1ch when I play a
7.1ch speaker test clip.
I have a hdmi-codec patch to fix selection of channel allocation in queue.
For patch 1-7:
Reviewed-by: Jonas Karlman
[1]
https://github.com/Kwiboo/linux-rockchip/commit/c0839e874f843ad173ddde958303d6878394ef92
Regards,
Jonas
> + return 0;
> +}
> +
> static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
> struct device_node *endpoint)
> {
> @@ -144,6 +153,7 @@ static int dw_hdmi_i2s_get_dai_id(struct
> snd_soc_component *component,
> static struct hdmi_codec_ops dw_hdmi_i2s_ops = {
> .hw_params = dw_hdmi_i2s_hw_params,
> .audio_shutdown = dw_hdmi_i2s_audio_shutdown,
> + .get_eld= dw_hdmi_i2s_get_eld,
> .get_dai_id = dw_hdmi_i2s_get_dai_id,
> };
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index bed4bb017afd..8df69c9dbfad 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2797,6 +2797,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
> struct dw_hdmi_i2s_audio_data audio;
>
> audio.hdmi = hdmi;
> + audio.eld = hdmi->connector.eld;
> audio.write = hdmi_writeb;
> audio.read = hdmi_readb;
> hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
Re: [PATCH 0/2] drm/panfrost: Revert drm_gem_map_offset changes
On Wed, Aug 07, 2019 at 10:52:46AM -0400, Sean Paul wrote: > From: Sean Paul > > When I started re-applying these I realized they hadn't hit the list. > > Sean > > Rob Herring (2): > Revert "drm/gem: Rename drm_gem_dumb_map_offset() to > drm_gem_map_offset()" > Revert "drm/panfrost: Use drm_gem_map_offset()" Note I have the order messed up, so will swap it when I apply (and properly compile test in between :/) Sean > > drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- > drivers/gpu/drm/drm_gem.c | 10 +++--- > drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++- > drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-- > include/drm/drm_gem.h | 4 ++-- > 5 files changed, 23 insertions(+), 14 deletions(-) > > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()"
From: Rob Herring
This reverts commit 220df83a5394fbf7c1486ba7848794b7b351d598.
Turns out drm_gem_dumb_map_offset really only worked for the dumb buffer
case, so revert the name change.
Signed-off-by: Rob Herring
Signed-off-by: Sean Paul
---
drivers/gpu/drm/drm_dumb_buffers.c | 4 ++--
drivers/gpu/drm/drm_gem.c | 10 +++---
drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++-
include/drm/drm_gem.h | 4 ++--
4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
b/drivers/gpu/drm/drm_dumb_buffers.c
index b55cfc9e8772..d18a740fe0f1 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -48,7 +48,7 @@
* To support dumb objects drivers must implement the _driver.dumb_create
* operation. _driver.dumb_destroy defaults to drm_gem_dumb_destroy() if
* not set and _driver.dumb_map_offset defaults to
- * drm_gem_map_offset(). See the callbacks for further details.
+ * drm_gem_dumb_map_offset(). See the callbacks for further details.
*
* Note that dumb objects may not be used for gpu acceleration, as has been
* attempted on some ARM embedded platforms. Such drivers really must have
@@ -127,7 +127,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
args->handle,
>offset);
else
- return drm_gem_map_offset(file_priv, dev, args->handle,
+ return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
>offset);
}
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 8cbfd60e09c0..afc38cece3f5 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -298,7 +298,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
EXPORT_SYMBOL(drm_gem_handle_delete);
/**
- * drm_gem_map_offset - return the fake mmap offset for a gem object
+ * drm_gem_dumb_map_offset - return the fake mmap offset for a gem object
* @file: drm file-private structure containing the gem object
* @dev: corresponding drm_device
* @handle: gem object handle
@@ -307,14 +307,10 @@ EXPORT_SYMBOL(drm_gem_handle_delete);
* This implements the _driver.dumb_map_offset kms driver callback for
* drivers which use gem to manage their backing storage.
*
- * It can also be used by drivers using GEM BO implementations which
- * have same restriction that imported objects cannot be mapped. The
- * shmem backend is one example.
- *
* Returns:
* 0 on success or a negative error code on failure.
*/
-int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
+int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
u32 handle, u64 *offset)
{
struct drm_gem_object *obj;
@@ -340,7 +336,7 @@ int drm_gem_map_offset(struct drm_file *file, struct
drm_device *dev,
return ret;
}
-EXPORT_SYMBOL_GPL(drm_gem_map_offset);
+EXPORT_SYMBOL_GPL(drm_gem_dumb_map_offset);
/**
* drm_gem_dumb_destroy - dumb fb callback helper for gem based drivers
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index bf0ad8e5a02b..d734d9d51762 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -273,7 +273,8 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void
*data,
{
struct drm_exynos_gem_map *args = data;
- return drm_gem_map_offset(file_priv, dev, args->handle, >offset);
+ return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
+ >offset);
}
struct exynos_drm_gem *exynos_drm_gem_get(struct drm_file *filp,
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 0d6445fa9541..ae693c0666cd 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -401,8 +401,8 @@ int drm_gem_fence_array_add(struct xarray *fence_array,
int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
struct drm_gem_object *obj,
bool write);
-int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
- u32 handle, u64 *offset);
+int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
+ u32 handle, u64 *offset);
int drm_gem_dumb_destroy(struct drm_file *file,
struct drm_device *dev,
uint32_t handle);
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] Revert "drm/panfrost: Use drm_gem_map_offset()"
From: Rob Herring
This reverts commit 583bbf46133c726bae277e8f4e32bfba2a528c7f.
Turns out we need mmap to work on imported BOs even if the current code
is buggy.
Signed-off-by: Rob Herring
Signed-off-by: Sean Paul
---
drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index b2e325e270b7..b187daa4da85 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -291,14 +291,26 @@ static int panfrost_ioctl_mmap_bo(struct drm_device *dev,
void *data,
struct drm_file *file_priv)
{
struct drm_panfrost_mmap_bo *args = data;
+ struct drm_gem_object *gem_obj;
+ int ret;
if (args->flags != 0) {
DRM_INFO("unknown mmap_bo flags: %d\n", args->flags);
return -EINVAL;
}
- return drm_gem_map_offset(file_priv, dev, args->handle,
- >offset);
+ gem_obj = drm_gem_object_lookup(file_priv, args->handle);
+ if (!gem_obj) {
+ DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle);
+ return -ENOENT;
+ }
+
+ ret = drm_gem_create_mmap_offset(gem_obj);
+ if (ret == 0)
+ args->offset = drm_vma_node_offset_addr(_obj->vma_node);
+ drm_gem_object_put_unlocked(gem_obj);
+
+ return ret;
}
static int panfrost_ioctl_get_bo_offset(struct drm_device *dev, void *data,
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm/panfrost: Revert drm_gem_map_offset changes
From: Sean Paul When I started re-applying these I realized they hadn't hit the list. Sean Rob Herring (2): Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()" Revert "drm/panfrost: Use drm_gem_map_offset()" drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 10 +++--- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-- include/drm/drm_gem.h | 4 ++-- 5 files changed, 23 insertions(+), 14 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] drm-misc-next: Revert patches missing reviews
On Wed, 7 Aug 2019 at 15:21, Sean Paul wrote: > > From: Sean Paul > > Hellooo, > This has been covered ad nauseam on the m-l and irc, but for the record: > > Reviews are a mandatory requirement for patches in drm-misc-next, it's > what keeps us all honest in the committer model. The most recent > drm-misc-next pull included a handful of patches that were missing > reviews. There was absolutely zero nefarious intent, but rules are > rules, so we're reverting them. > > I feel confident re-applying all of these with my SoB, so I plan on > doing that in the same push. > Thanks. > Thanks to everyone for handling this so well, seriously awesome that > we can stay constructive as a community \o/. > > Lastly, this will be caught by our tooling in the future so this should > be the last time this happens (on accident). > Hear, hear. > Sean > > Sean Paul (5): > Revert "drm/vgem: drop DRM_AUTH usage from the driver" > Revert "drm/msm: drop DRM_AUTH usage from the driver" > Revert "drm/nouveau: remove open-coded drm_invalid_op()" > For these three: Acked-by: Emil Velikov -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] drm-misc-next: Revert patches missing reviews
Hi, On Wed, Aug 07, 2019 at 10:20:53AM -0400, Sean Paul wrote: > From: Sean Paul > > Hellooo, > This has been covered ad nauseam on the m-l and irc, but for the record: > > Reviews are a mandatory requirement for patches in drm-misc-next, it's > what keeps us all honest in the committer model. The most recent > drm-misc-next pull included a handful of patches that were missing > reviews. There was absolutely zero nefarious intent, but rules are > rules, so we're reverting them. > > I feel confident re-applying all of these with my SoB, so I plan on > doing that in the same push. > > Thanks to everyone for handling this so well, seriously awesome that > we can stay constructive as a community \o/. > > Lastly, this will be caught by our tooling in the future so this should > be the last time this happens (on accident). Thanks for taking care of this: Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110865] Rx480 consumes 20w more power in idle than under Windows
https://bugs.freedesktop.org/show_bug.cgi?id=110865 Alex Deucher changed: What|Removed |Added Severity|normal |enhancement --- Comment #2 from Alex Deucher --- This is the expected behavior for multiple monitors on Linux. mclk switching must happen in the monitors' blanking period. Since they likely don't align, especially if the monitors have different timing, we have to use a fixed mclk. The DC modesetting code can lock the timing of multiple monitors if they are using the exact same timing so that the blanking periods align, but I don't think the Linux power management code takes this into account at the moment. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm pull for v5.3-rc1
On 07/08/2019 15:15, Matthew Wilcox wrote:
> On Tue, Aug 06, 2019 at 11:40:00PM -0700, Christoph Hellwig wrote:
>> On Tue, Aug 06, 2019 at 12:09:38PM -0700, Matthew Wilcox wrote:
>>> Has anyone looked at turning the interface inside-out? ie something like:
>>>
>>> struct mm_walk_state state = { .mm = mm, .start = start, .end = end, };
>>>
>>> for_each_page_range(, page) {
>>> ... do something with page ...
>>> }
>>>
>>> with appropriate macrology along the lines of:
>>>
>>> #define for_each_page_range(state, page)\
>>> while ((page = page_range_walk_next(state)))
>>>
>>> Then you don't need to package anything up into structs that are shared
>>> between the caller and the iterated function.
>>
>> I'm not an all that huge fan of super magic macro loops. But in this
>> case I don't see how it could even work, as we get special callbacks
>> for huge pages and holes, and people are trying to add a few more ops
>> as well.
>
> We could have bits in the mm_walk_state which indicate what things to return
> and what things to skip. We could (and probably should) also use different
> iterator names if people actually want to iterate different things. eg
> for_each_pte_range(, pte) as well as for_each_page_range().
>
The iterator approach could be awkward for the likes of my generic
ptdump implementation[1]. It would require an iterator which returns all
levels and allows skipping levels when required (to prevent KASAN
slowing things down too much). So something like:
start_walk_range();
for_each_page_range(, page) {
switch(page->level) {
case PTE:
...
case PMD:
if (...)
skip_pmd();
...
case HOLE:
...
}
}
end_walk_range();
It seems a little fragile - e.g. we wouldn't (easily) get type checking
that you are actually treating a PTE as a pte_t. The state mutators like
skip_pmd() also seem a bit clumsy.
Steve
[1]
https://lore.kernel.org/lkml/20190731154603.41797-20-steven.pr...@arm.com/
Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi Artur, On 8/1/19 10:59, Artur Świgoń wrote: > Hi Georgi, > > On Fri, 2019-07-26 at 11:05 +0300, Georgi Djakov wrote: >> Hi Artur, >> >> On 7/23/19 15:20, Artur Świgoń wrote: >>> This patch adds interconnect functionality to the exynos-bus devfreq >>> driver. >>> >>> The SoC topology is a graph (or, more specifically, a tree) and most of its >>> edges are taken from the devfreq parent-child hierarchy (cf. >>> Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous >>> patch adds missing edges to the DT (under the name 'parent'). Due to >>> unspecified relative probing order, -EPROBE_DEFER may be propagated to >>> guarantee that a child is probed before its parent. >>> >>> Each bus is now an interconnect provider and an interconnect node as well >>> (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers >>> itself as a node. Node IDs are not hardcoded but rather assigned at >>> runtime, in probing order (subject to the above-mentioned exception >>> regarding relative order). This approach allows for using this driver with >>> various Exynos SoCs. >> >> I am not familiar with the Exynos bus topology, but it seems to me that it's >> not >> represented correctly. An interconnect provider with just a single node >> (port) >> is odd. I would expect that each provider consists of multiple master and >> slave >> nodes. This data would be used by a framework to understand what are the >> links >> and how the traffic flows between the IP blocks and through which buses. > > To summarize the exynos-bus topology[1] used by the devfreq driver: There are > many data buses for data transfer in Samsung Exynos SoC. Every bus has its own > clock. Buses often share power lines, in which case one of the buses on the > power line is referred to as 'parent' (or as 'devfreq' in the DT). In the > particular case of Exynos4412[1][2], the topology can be expressed as follows: > > bus_dmc > -- bus_acp > -- bus_c2c > > bus_leftbus > -- bus_rightbus > -- bus_display > -- bus_fsys > -- bus_peri > -- bus_mfc > > Where bus_dmc and bus_leftbus probably could be referred to as masters, and > the > following indented nodes as slaves. Patch 08/11 of this RFC additionally adds > the following to the DT: > > bus_dmc > -- bus_leftbus > > Which makes the topology a valid tree. > > The exynos-bus concept in devfreq[3] is designed in such a way that every bus > is > probed separately as a platform device, and is a largely independent entity. > > This RFC proposes an extension to the existing devfreq driver that basically > provides a simple QoS to ensure minimum clock frequency for selected buses > (possibly overriding devfreq governor calculations) using the interconnect > framework. > > The hierarchy is modelled in such a way that every bus is an interconnect > node. > On the other hand, what is considered an interconnect provider here is quite > arbitrary, but for the reasons mentioned in the above paragraph, this RFC > assumes that every bus is a provider of itself as a node. Using an alternative IIUC, in case we want to transfer data between the display and the memory controller, the path would look like this: display --> bus_display --> bus_leftbus --> bus_dmc --> memory But the bus_display for example would have not one, but two nodes (ports), right? One representing the link to the display controller and another one representing the link to bus_leftbus? So i think that all the buses should have at least two nodes, to represent each end of the wire. > singleton provider approach was deemed more complicated since the 'dev' field > in > 'struct icc_provider' has to be set to something meaningful and we are tied to > the 'samsung,exynos-bus' compatible string in the driver (and multiple > instances > of exynos-bus probed in indeterminate relative order). > Sure, the rest makes sense to me. Thanks, Georgi
[PATCH 2/5] Revert "Revert "drm/panfrost: Use drm_gem_map_offset()""
From: Sean Paul
This reverts commit be855382bacb5ccfd24f9be6098d87acf4cfbb15.
Mandatory review was missing from this patch.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++--
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index b187daa4da85..b2e325e270b7 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -291,26 +291,14 @@ static int panfrost_ioctl_mmap_bo(struct drm_device *dev,
void *data,
struct drm_file *file_priv)
{
struct drm_panfrost_mmap_bo *args = data;
- struct drm_gem_object *gem_obj;
- int ret;
if (args->flags != 0) {
DRM_INFO("unknown mmap_bo flags: %d\n", args->flags);
return -EINVAL;
}
- gem_obj = drm_gem_object_lookup(file_priv, args->handle);
- if (!gem_obj) {
- DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle);
- return -ENOENT;
- }
-
- ret = drm_gem_create_mmap_offset(gem_obj);
- if (ret == 0)
- args->offset = drm_vma_node_offset_addr(_obj->vma_node);
- drm_gem_object_put_unlocked(gem_obj);
-
- return ret;
+ return drm_gem_map_offset(file_priv, dev, args->handle,
+ >offset);
}
static int panfrost_ioctl_get_bo_offset(struct drm_device *dev, void *data,
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] Revert "drm/nouveau: remove open-coded drm_invalid_op()"
From: Sean Paul
This reverts commit ccdae42575695ab442941310bd67c7ed1714e273.
Mandatory review was missing from this patch.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 6 ++
drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index e2bae1424502..94387e62b338 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -244,6 +244,12 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
return 0;
}
+int
+nouveau_abi16_ioctl_setparam(ABI16_IOCTL_ARGS)
+{
+ return -EINVAL;
+}
+
int
nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS)
{
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.h
b/drivers/gpu/drm/nouveau/nouveau_abi16.h
index 70f6aa5c9dd1..195546719bfe 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.h
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.h
@@ -6,6 +6,7 @@
struct drm_device *dev, void *data, struct drm_file *file_priv
int nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS);
+int nouveau_abi16_ioctl_setparam(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_channel_free(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_grobj_alloc(ABI16_IOCTL_ARGS);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 7e045580a3a4..551c4ee2ceed 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1047,7 +1047,7 @@ nouveau_drm_postclose(struct drm_device *dev, struct
drm_file *fpriv)
static const struct drm_ioctl_desc
nouveau_ioctls[] = {
DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(NOUVEAU_SETPARAM, drm_invalid_op,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+ DRM_IOCTL_DEF_DRV(NOUVEAU_SETPARAM, nouveau_abi16_ioctl_setparam,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC,
nouveau_abi16_ioctl_channel_alloc, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE,
nouveau_abi16_ioctl_channel_free, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc,
DRM_RENDER_ALLOW),
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] Revert "Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()""
From: Sean Paul
This reverts commit 415d2e9e07574d3de63b8df77dc686e0ebf64865.
Mandatory review was missing from this patch.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/drm_dumb_buffers.c | 4 ++--
drivers/gpu/drm/drm_gem.c | 10 +++---
drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 +--
include/drm/drm_gem.h | 4 ++--
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_dumb_buffers.c
b/drivers/gpu/drm/drm_dumb_buffers.c
index d18a740fe0f1..b55cfc9e8772 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -48,7 +48,7 @@
* To support dumb objects drivers must implement the _driver.dumb_create
* operation. _driver.dumb_destroy defaults to drm_gem_dumb_destroy() if
* not set and _driver.dumb_map_offset defaults to
- * drm_gem_dumb_map_offset(). See the callbacks for further details.
+ * drm_gem_map_offset(). See the callbacks for further details.
*
* Note that dumb objects may not be used for gpu acceleration, as has been
* attempted on some ARM embedded platforms. Such drivers really must have
@@ -127,7 +127,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
args->handle,
>offset);
else
- return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
+ return drm_gem_map_offset(file_priv, dev, args->handle,
>offset);
}
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index afc38cece3f5..8cbfd60e09c0 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -298,7 +298,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
EXPORT_SYMBOL(drm_gem_handle_delete);
/**
- * drm_gem_dumb_map_offset - return the fake mmap offset for a gem object
+ * drm_gem_map_offset - return the fake mmap offset for a gem object
* @file: drm file-private structure containing the gem object
* @dev: corresponding drm_device
* @handle: gem object handle
@@ -307,10 +307,14 @@ EXPORT_SYMBOL(drm_gem_handle_delete);
* This implements the _driver.dumb_map_offset kms driver callback for
* drivers which use gem to manage their backing storage.
*
+ * It can also be used by drivers using GEM BO implementations which
+ * have same restriction that imported objects cannot be mapped. The
+ * shmem backend is one example.
+ *
* Returns:
* 0 on success or a negative error code on failure.
*/
-int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
+int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
u32 handle, u64 *offset)
{
struct drm_gem_object *obj;
@@ -336,7 +340,7 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct
drm_device *dev,
return ret;
}
-EXPORT_SYMBOL_GPL(drm_gem_dumb_map_offset);
+EXPORT_SYMBOL_GPL(drm_gem_map_offset);
/**
* drm_gem_dumb_destroy - dumb fb callback helper for gem based drivers
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index d734d9d51762..bf0ad8e5a02b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -273,8 +273,7 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void
*data,
{
struct drm_exynos_gem_map *args = data;
- return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
- >offset);
+ return drm_gem_map_offset(file_priv, dev, args->handle, >offset);
}
struct exynos_drm_gem *exynos_drm_gem_get(struct drm_file *filp,
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index ae693c0666cd..0d6445fa9541 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -401,8 +401,8 @@ int drm_gem_fence_array_add(struct xarray *fence_array,
int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
struct drm_gem_object *obj,
bool write);
-int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
- u32 handle, u64 *offset);
+int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
+ u32 handle, u64 *offset);
int drm_gem_dumb_destroy(struct drm_file *file,
struct drm_device *dev,
uint32_t handle);
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] Revert "drm/msm: drop DRM_AUTH usage from the driver"
From: Sean Paul
This reverts commit 88209d2c5035737f96bcfc2fd73c0fd8d80e9bf1.
Mandatory review was missing from this patch.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/msm_drv.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index ea335ca25eca..abf8f4e4e543 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -984,17 +984,17 @@ static int msm_ioctl_submitqueue_close(struct drm_device
*dev, void *data,
}
static const struct drm_ioctl_desc msm_ioctls[] = {
- DRM_IOCTL_DEF_DRV(MSM_GET_PARAM,msm_ioctl_get_param,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_GEM_NEW, msm_ioctl_gem_new,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_GEM_INFO, msm_ioctl_gem_info,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_PREP, msm_ioctl_gem_cpu_prep,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_FINI, msm_ioctl_gem_cpu_fini,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_GEM_SUBMIT, msm_ioctl_gem_submit,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_WAIT_FENCE, msm_ioctl_wait_fence,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_GEM_MADVISE, msm_ioctl_gem_madvise,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_NEW, msm_ioctl_submitqueue_new,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_CLOSE, msm_ioctl_submitqueue_close,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query,
DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_GET_PARAM,msm_ioctl_get_param,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_GEM_NEW, msm_ioctl_gem_new,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_GEM_INFO, msm_ioctl_gem_info,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_PREP, msm_ioctl_gem_cpu_prep,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_FINI, msm_ioctl_gem_cpu_fini,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_GEM_SUBMIT, msm_ioctl_gem_submit,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_WAIT_FENCE, msm_ioctl_wait_fence,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_GEM_MADVISE, msm_ioctl_gem_madvise,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_NEW, msm_ioctl_submitqueue_new,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_CLOSE, msm_ioctl_submitqueue_close,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query,
DRM_AUTH|DRM_RENDER_ALLOW),
};
static const struct vm_operations_struct vm_ops = {
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] Revert "drm/vgem: drop DRM_AUTH usage from the driver"
From: Sean Paul
This reverts commit e4eee93d25776da998ec2dfaabe7d2206598d26d.
Mandatory review was missing from this patch.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/vgem/vgem_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 5bd60ded3d81..1d0ccfcbc472 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -253,8 +253,8 @@ static int vgem_gem_dumb_map(struct drm_file *file, struct
drm_device *dev,
}
static struct drm_ioctl_desc vgem_ioctls[] = {
- DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl,
DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl,
DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl,
DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl,
DRM_AUTH|DRM_RENDER_ALLOW),
};
static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/5] drm-misc-next: Revert patches missing reviews
From: Sean Paul Hellooo, This has been covered ad nauseam on the m-l and irc, but for the record: Reviews are a mandatory requirement for patches in drm-misc-next, it's what keeps us all honest in the committer model. The most recent drm-misc-next pull included a handful of patches that were missing reviews. There was absolutely zero nefarious intent, but rules are rules, so we're reverting them. I feel confident re-applying all of these with my SoB, so I plan on doing that in the same push. Thanks to everyone for handling this so well, seriously awesome that we can stay constructive as a community \o/. Lastly, this will be caught by our tooling in the future so this should be the last time this happens (on accident). Sean Sean Paul (5): Revert "Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()"" Revert "Revert "drm/panfrost: Use drm_gem_map_offset()"" Revert "drm/vgem: drop DRM_AUTH usage from the driver" Revert "drm/msm: drop DRM_AUTH usage from the driver" Revert "drm/nouveau: remove open-coded drm_invalid_op()" drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 10 +++--- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 +-- drivers/gpu/drm/msm/msm_drv.c | 22 +++--- drivers/gpu/drm/nouveau/nouveau_abi16.c | 6 ++ drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 + drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-- drivers/gpu/drm/vgem/vgem_drv.c | 4 ++-- include/drm/drm_gem.h | 4 ++-- 10 files changed, 35 insertions(+), 37 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] dma-buf: nuke reservation_object seq number
Quoting Christian König (2019-08-07 14:53:12)
> The only remaining use for this is to protect against setting a new exclusive
> fence while we grab both exclusive and shared. That can also be archived by
> looking if the exclusive fence has changed or not after completing the
> operation.
>
> v2: switch setting excl fence to rcu_assign_pointer
>
> Signed-off-by: Christian König
> ---
> drivers/dma-buf/reservation.c | 24 +---
> include/linux/reservation.h | 9 ++---
> 2 files changed, 7 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 90bc6ef03598..f7f4a0858c2a 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -49,12 +49,6 @@
> DEFINE_WD_CLASS(reservation_ww_class);
> EXPORT_SYMBOL(reservation_ww_class);
>
> -struct lock_class_key reservation_seqcount_class;
> -EXPORT_SYMBOL(reservation_seqcount_class);
> -
> -const char reservation_seqcount_string[] = "reservation_seqcount";
> -EXPORT_SYMBOL(reservation_seqcount_string);
> -
> /**
> * reservation_object_list_alloc - allocate fence list
> * @shared_max: number of fences we need space for
> @@ -103,9 +97,6 @@ static void reservation_object_list_free(struct
> reservation_object_list *list)
> void reservation_object_init(struct reservation_object *obj)
> {
> ww_mutex_init(>lock, _ww_class);
> -
> - __seqcount_init(>seq, reservation_seqcount_string,
> - _seqcount_class);
> RCU_INIT_POINTER(obj->fence, NULL);
> RCU_INIT_POINTER(obj->fence_excl, NULL);
> }
> @@ -282,12 +273,10 @@ void reservation_object_add_excl_fence(struct
> reservation_object *obj,
> dma_fence_get(fence);
>
> preempt_disable();
> - write_seqcount_begin(>seq);
> - /* write_seqcount_begin provides the necessary memory barrier */
> - RCU_INIT_POINTER(obj->fence_excl, fence);
> + rcu_assign_pointer(obj->fence_excl, fence);
> + /* pointer update must be visible before we modify the shared_count */
> if (old)
> - old->shared_count = 0;
> - write_seqcount_end(>seq);
> + smp_store_mb(old->shared_count, 0);
> preempt_enable();
>
> /* inplace update, no shared fences */
> @@ -368,11 +357,8 @@ int reservation_object_copy_fences(struct
> reservation_object *dst,
> old = reservation_object_get_excl(dst);
>
> preempt_disable();
> - write_seqcount_begin(>seq);
> - /* write_seqcount_begin provides the necessary memory barrier */
> - RCU_INIT_POINTER(dst->fence_excl, new);
> - RCU_INIT_POINTER(dst->fence, dst_list);
> - write_seqcount_end(>seq);
> + rcu_assign_pointer(dst->fence_excl, new);
> + rcu_assign_pointer(dst->fence, dst_list);
> preempt_enable();
>
> reservation_object_list_free(src_list);
> diff --git a/include/linux/reservation.h b/include/linux/reservation.h
> index 044a5cd4af50..fd29baad0be3 100644
> --- a/include/linux/reservation.h
> +++ b/include/linux/reservation.h
> @@ -46,8 +46,6 @@
> #include
>
> extern struct ww_class reservation_ww_class;
> -extern struct lock_class_key reservation_seqcount_class;
> -extern const char reservation_seqcount_string[];
>
> /**
> * struct reservation_object_list - a list of shared fences
> @@ -71,7 +69,6 @@ struct reservation_object_list {
> */
> struct reservation_object {
> struct ww_mutex lock;
> - seqcount_t seq;
>
> struct dma_fence __rcu *fence_excl;
> struct reservation_object_list __rcu *fence;
> @@ -156,14 +153,12 @@ reservation_object_fences(struct reservation_object
> *obj,
> struct reservation_object_list **list,
> u32 *shared_count)
> {
> - unsigned int seq;
> -
> do {
> - seq = read_seqcount_begin(>seq);
> *excl = rcu_dereference(obj->fence_excl);
> *list = rcu_dereference(obj->fence);
> *shared_count = *list ? (*list)->shared_count : 0;
> - } while (read_seqcount_retry(>seq, seq));
> + smp_rmb(); /* See reservation_object_add_excl_fence */
> + } while (rcu_access_pointer(obj->fence_excl) != *excl);
> }
Reviewed-by: Chris Wilson
I think this is correct. Now see if we can convince Daniel!
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm pull for v5.3-rc1
On Tue, Aug 06, 2019 at 11:40:00PM -0700, Christoph Hellwig wrote:
> On Tue, Aug 06, 2019 at 12:09:38PM -0700, Matthew Wilcox wrote:
> > Has anyone looked at turning the interface inside-out? ie something like:
> >
> > struct mm_walk_state state = { .mm = mm, .start = start, .end = end, };
> >
> > for_each_page_range(, page) {
> > ... do something with page ...
> > }
> >
> > with appropriate macrology along the lines of:
> >
> > #define for_each_page_range(state, page)\
> > while ((page = page_range_walk_next(state)))
> >
> > Then you don't need to package anything up into structs that are shared
> > between the caller and the iterated function.
>
> I'm not an all that huge fan of super magic macro loops. But in this
> case I don't see how it could even work, as we get special callbacks
> for huge pages and holes, and people are trying to add a few more ops
> as well.
We could have bits in the mm_walk_state which indicate what things to return
and what things to skip. We could (and probably should) also use different
iterator names if people actually want to iterate different things. eg
for_each_pte_range(, pte) as well as for_each_page_range().
Re: [PATCH] drm/syncobj: Add better overview documentation for syncobj (v2)
On 06/08/2019 19:19, Jason Ekstrand wrote: This patch only brings the syncobj documentation up-to-date for the original form of syncobj. It does not contain any information about the design of timeline syncobjs. v2: Incorporate feedback from Lionel and Christian: - Mention actual ioctl and flag names - Better language around reference counting - Misc. language cleanups Signed-off-by: Jason Ekstrand Reviewed-by: Lionel Landwerlin --- drivers/gpu/drm/drm_syncobj.c | 98 +++ 1 file changed, 87 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 1438dcb3ebb1..4b5c7b0ed714 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -29,21 +29,97 @@ /** * DOC: Overview * - * DRM synchronisation objects (syncobj, see struct _syncobj) are - * persistent objects that contain an optional fence. The fence can be updated - * with a new fence, or be NULL. + * DRM synchronisation objects (syncobj, see struct _syncobj) provide a + * container for a synchronization primitive which can be used by userspace + * to explicitly synchronize GPU commands, can be shared between userspace + * processes, and can be shared between different DRM drivers. + * Their primary use-case is to implement Vulkan fences and semaphores. + * The syncobj userspace API provides ioctls for several operations: * - * syncobj's can be waited upon, where it will wait for the underlying - * fence. + * - Creation and destruction of syncobjs + * - Import and export of syncobjs to/from a syncobj file descriptor + * - Import and export a syncobj's underlying fence to/from a sync file + * - Reset a syncobj (set its fence to NULL) + * - Signal a syncobj (set a trivially signaled fence) + * - Wait for a syncobj's fence to appear and be signaled * - * syncobj's can be export to fd's and back, these fd's are opaque and - * have no other use case, except passing the syncobj between processes. + * At it's core, a syncobj is simply a wrapper around a pointer to a struct + * _fence which may be NULL. + * When a syncobj is first created, its pointer is either NULL or a pointer + * to an already signaled fence depending on whether the + * _SYNCOBJ_CREATE_SIGNALED flag is passed to + * _IOCTL_SYNCOBJ_CREATE. + * When GPU work which signals a syncobj is enqueued in a DRM driver, + * the syncobj fence is replaced with a fence which will be signaled by the + * completion of that work. + * When GPU work which waits on a syncobj is enqueued in a DRM driver, the + * driver retrieves syncobj's current fence at the time the work is enqueued + * waits on that fence before submitting the work to hardware. + * If the syncobj's fence is NULL, the enqueue operation is expected to fail. + * All manipulation of the syncobjs's fence happens in terms of the current + * fence at the time the ioctl is called by userspace regardless of whether + * that operation is an immediate host-side operation (signal or reset) or + * or an operation which is enqueued in some driver queue. + * _IOCTL_SYNCOBJ_RESET and _IOCTL_SYNCOBJ_SIGNAL can be used to + * manipulate a syncobj from the host by resetting its pointer to NULL or + * setting its pointer to a fence which is already signaled. * - * Their primary use-case is to implement Vulkan fences and semaphores. * - * syncobj have a kref reference count, but also have an optional file. - * The file is only created once the syncobj is exported. - * The file takes a reference on the kref. + * Host-side wait on syncobjs + * -- + * + * _IOCTL_SYNCOBJ_WAIT takes an array of syncobj handles and does a + * host-side wait on all of the syncobj fences simultaneously. + * If _SYNCOBJ_WAIT_FLAGS_WAIT_ALL is set, the wait ioctl will wait on + * all of the syncobj fences to be signaled before it returns. + * Otherwise, it returns once at least one syncobj fence has been signaled + * and the index of a signaled fence is written back to the client. + * + * Unlike the enqueued GPU work dependencies which fail if they see a NULL + * fence in a syncobj, if _SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT is set, + * the host-side wait will first wait for the syncobj to receive a non-NULL + * fence and then wait on that fence. + * If _SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT is not set and any one of the + * syncobjs in the array has a NULL fence, -EINVAL will be returned. + * Assuming the syncobj starts off with a NULL fence, this allows a client + * to do a host wait in one thread (or process) which waits on GPU work + * submitted in another thread (or process) without having to manually + * synchronize between the two. + * This requirement is inherited from the Vulkan fence API. + * + * + * Import/export of syncobjs + * - + * + * _IOCTL_SYNCOBJ_FD_TO_HANDLE and _IOCTL_SYNCOBJ_HANDLE_TO_FD + * provide two mechanisms for import/export of syncobjs. + * + * The first lets the client
[GIT PULL] drm/tegra: Fixes for v5.3-rc4
Hi Dave, The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b: Linus 5.3-rc1 (2019-07-21 14:05:38 -0700) are available in the Git repository at: git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-5.3-rc4 for you to fetch changes up to 2a6fc3cb5cb68597f1072bfeef28d2ca02310220: drm/tegra: Fix gpiod_get_from_of_node() regression (2019-07-25 15:23:26 +0200) Thanks, Thierry drm/tegra: Fixes for v5.3-rc4 This contains a single fix for a regression introduced by a combination of a GPIO and a drm/tegra patch merged in v5.3-rc1. Dmitry Osipenko (1): drm/tegra: Fix gpiod_get_from_of_node() regression drivers/gpu/drm/tegra/output.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: make dma_fence structure a bit smaller
Quoting Christian König (2019-08-07 14:54:05) > The ruc and cb_list are never used at the same time. > This smal change is actually making the structure 16% smaller. (Trivial pair of typos) Yes. We clear the callback list on kref_put so that by the time we release the fence it is unused. No one should be adding to the cb_list that they don't themselves hold a reference for, this only now makes for a much more spectacular use-after-free. :) > Signed-off-by: Christian König Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111244] amdgpu kernel 5.2 blank display after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=111244 --- Comment #22 from Michel Dänzer --- (In reply to cspack from comment #21) > The default is -1 according to the docs and > /sys/module/amdgpu/parameters/dc. What I meant is it's enabled by default for you, so amdgpu.dc=1 has no effect. > I assume it should effectively be the same but it seems to result in different > behavior vs. setting it to 1. The different behaviour is just luck, which is why you had trouble bisecting initially, not related to amdgpu.dc=1. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm/syncobj: add syncobj sideband payload for threaded submission
On 07/08/2019 16:49, Koenig, Christian wrote: Well first of all I strongly suggest to make the sideband information a separate IOCTL and not use the existing signal/query IOCTLs for it. It felt like at least the query ioctl was the right place to get the sideband value. I can change. Then as far as I see this basically sets a sequence number to use for binary semaphores, is that correct? If yes then that would be a rather elegant idea. Yeah that's pretty much it. From the vulkan API point view, this doesn't even need to be atomic, it just needs to happen within vkQueueSubmit(). Again to explain the issue, it's that syncobj is container and we might race picking up the fence from one thread while the submission thread updates the fence. Essentially reusing the fence chain mechanism solve the issue because we can wait on a particular replacement sequence number (matching a vkQueueSubmit() call). -Lionel Christian. Am 07.08.19 um 15:37 schrieb Lionel Landwerlin: Hi, The goal of this feature is to solve an issue that was seen in our testing. After discussing on [1] I thought we could solve this problem with stalling the application thread after each vkQueueSubmit() call where a binary semaphore is signaled but I don't think it's a good option because of performance impacts on the application. Unfortunately there isn't a good way to reproduce this problem 100% because it essentially exposes a race between the application thread and the submission thread. I've uploaded tests in the Khronos repository to try to expose the issue. Thanks, [1] : https://lists.freedesktop.org/archives/dri-devel/2019-August/229188.html Lionel Landwerlin (2): drm/syncobj: add sideband payload drm/syncobj: add submit point query capability drivers/gpu/drm/drm_syncobj.c | 132 ++ include/drm/drm_syncobj.h | 9 +++ include/uapi/drm/drm.h| 5 +- 3 files changed, 100 insertions(+), 46 deletions(-) -- 2.23.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dma-buf: make dma_fence structure a bit smaller
The ruc and cb_list are never used at the same time.
This smal change is actually making the structure 16% smaller.
Signed-off-by: Christian König
---
include/linux/dma-fence.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 05d29dbc7e62..3985c72cd0c2 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -65,8 +65,10 @@ struct dma_fence_cb;
struct dma_fence {
struct kref refcount;
const struct dma_fence_ops *ops;
- struct rcu_head rcu;
- struct list_head cb_list;
+ union {
+ struct rcu_head rcu;
+ struct list_head cb_list;
+ };
spinlock_t *lock;
u64 context;
u64 seqno;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] dma-buf: further relax reservation_object_add_shared_fence
Other cores don't busy wait any more and we removed the last user of checking
the seqno for changes. Drop updating the number for shared fences altogether.
Signed-off-by: Christian König
---
drivers/dma-buf/reservation.c| 6 --
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 7 +--
2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 8fcaddffd5d4..90bc6ef03598 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -237,9 +237,6 @@ void reservation_object_add_shared_fence(struct
reservation_object *obj,
fobj = reservation_object_get_list(obj);
count = fobj->shared_count;
- preempt_disable();
- write_seqcount_begin(>seq);
-
for (i = 0; i < count; ++i) {
old = rcu_dereference_protected(fobj->shared[i],
@@ -257,9 +254,6 @@ void reservation_object_add_shared_fence(struct
reservation_object *obj,
RCU_INIT_POINTER(fobj->shared[i], fence);
/* pointer update must be visible before we extend the shared_count */
smp_store_mb(fobj->shared_count, count);
-
- write_seqcount_end(>seq);
- preempt_enable();
dma_fence_put(old);
}
EXPORT_SYMBOL(reservation_object_add_shared_fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index fe062b76ec91..a4640ddc24d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -251,12 +251,7 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct
amdgpu_bo *bo,
new->shared_max = old->shared_max;
new->shared_count = k;
- /* Install the new fence list, seqcount provides the barriers */
- preempt_disable();
- write_seqcount_begin(>seq);
- RCU_INIT_POINTER(resv->fence, new);
- write_seqcount_end(>seq);
- preempt_enable();
+ rcu_assign_pointer(resv->fence, new);
/* Drop the references to the removed fences or move them to ef_list */
for (i = j, k = 0; i < old->shared_count; ++i) {
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] dma-buf: nuke reservation_object seq number
The only remaining use for this is to protect against setting a new exclusive
fence while we grab both exclusive and shared. That can also be archived by
looking if the exclusive fence has changed or not after completing the
operation.
v2: switch setting excl fence to rcu_assign_pointer
Signed-off-by: Christian König
---
drivers/dma-buf/reservation.c | 24 +---
include/linux/reservation.h | 9 ++---
2 files changed, 7 insertions(+), 26 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 90bc6ef03598..f7f4a0858c2a 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -49,12 +49,6 @@
DEFINE_WD_CLASS(reservation_ww_class);
EXPORT_SYMBOL(reservation_ww_class);
-struct lock_class_key reservation_seqcount_class;
-EXPORT_SYMBOL(reservation_seqcount_class);
-
-const char reservation_seqcount_string[] = "reservation_seqcount";
-EXPORT_SYMBOL(reservation_seqcount_string);
-
/**
* reservation_object_list_alloc - allocate fence list
* @shared_max: number of fences we need space for
@@ -103,9 +97,6 @@ static void reservation_object_list_free(struct
reservation_object_list *list)
void reservation_object_init(struct reservation_object *obj)
{
ww_mutex_init(>lock, _ww_class);
-
- __seqcount_init(>seq, reservation_seqcount_string,
- _seqcount_class);
RCU_INIT_POINTER(obj->fence, NULL);
RCU_INIT_POINTER(obj->fence_excl, NULL);
}
@@ -282,12 +273,10 @@ void reservation_object_add_excl_fence(struct
reservation_object *obj,
dma_fence_get(fence);
preempt_disable();
- write_seqcount_begin(>seq);
- /* write_seqcount_begin provides the necessary memory barrier */
- RCU_INIT_POINTER(obj->fence_excl, fence);
+ rcu_assign_pointer(obj->fence_excl, fence);
+ /* pointer update must be visible before we modify the shared_count */
if (old)
- old->shared_count = 0;
- write_seqcount_end(>seq);
+ smp_store_mb(old->shared_count, 0);
preempt_enable();
/* inplace update, no shared fences */
@@ -368,11 +357,8 @@ int reservation_object_copy_fences(struct
reservation_object *dst,
old = reservation_object_get_excl(dst);
preempt_disable();
- write_seqcount_begin(>seq);
- /* write_seqcount_begin provides the necessary memory barrier */
- RCU_INIT_POINTER(dst->fence_excl, new);
- RCU_INIT_POINTER(dst->fence, dst_list);
- write_seqcount_end(>seq);
+ rcu_assign_pointer(dst->fence_excl, new);
+ rcu_assign_pointer(dst->fence, dst_list);
preempt_enable();
reservation_object_list_free(src_list);
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index 044a5cd4af50..fd29baad0be3 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -46,8 +46,6 @@
#include
extern struct ww_class reservation_ww_class;
-extern struct lock_class_key reservation_seqcount_class;
-extern const char reservation_seqcount_string[];
/**
* struct reservation_object_list - a list of shared fences
@@ -71,7 +69,6 @@ struct reservation_object_list {
*/
struct reservation_object {
struct ww_mutex lock;
- seqcount_t seq;
struct dma_fence __rcu *fence_excl;
struct reservation_object_list __rcu *fence;
@@ -156,14 +153,12 @@ reservation_object_fences(struct reservation_object *obj,
struct reservation_object_list **list,
u32 *shared_count)
{
- unsigned int seq;
-
do {
- seq = read_seqcount_begin(>seq);
*excl = rcu_dereference(obj->fence_excl);
*list = rcu_dereference(obj->fence);
*shared_count = *list ? (*list)->shared_count : 0;
- } while (read_seqcount_retry(>seq, seq));
+ smp_rmb(); /* See reservation_object_add_excl_fence */
+ } while (rcu_access_pointer(obj->fence_excl) != *excl);
}
/**
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/i915: use new reservation_object_fences helper
Instead of open coding the sequence loop use the new helper.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/gem/i915_gem_busy.c | 24 +++-
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
index 6ad93a09968c..8473292096cb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
@@ -83,7 +83,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_busy *args = data;
struct drm_i915_gem_object *obj;
struct reservation_object_list *list;
- unsigned int seq;
+ unsigned int i, shared_count;
+ struct dma_fence *excl;
int err;
err = -ENOENT;
@@ -109,29 +110,18 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
* to report the overall busyness. This is what the wait-ioctl does.
*
*/
-retry:
- seq = raw_read_seqcount(>base.resv->seq);
+ reservation_object_fences(obj->base.resv, , , _count);
/* Translate the exclusive fence to the READ *and* WRITE engine */
- args->busy =
- busy_check_writer(rcu_dereference(obj->base.resv->fence_excl));
+ args->busy = busy_check_writer(excl);
/* Translate shared fences to READ set of engines */
- list = rcu_dereference(obj->base.resv->fence);
- if (list) {
- unsigned int shared_count = list->shared_count, i;
+ for (i = 0; i < shared_count; ++i) {
+ struct dma_fence *fence = rcu_dereference(list->shared[i]);
- for (i = 0; i < shared_count; ++i) {
- struct dma_fence *fence =
- rcu_dereference(list->shared[i]);
-
- args->busy |= busy_check_reader(fence);
- }
+ args->busy |= busy_check_reader(fence);
}
- if (args->busy && read_seqcount_retry(>base.resv->seq, seq))
- goto retry;
-
err = 0;
out:
rcu_read_unlock();
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] dma-buf: add reservation_object_fences helper
Add a new helper to get a consistent set of pointers from the reservation
object. While at it group all access helpers together in the header file.
v2: correctly return shared_count as well
Signed-off-by: Christian König
---
drivers/dma-buf/dma-buf.c | 31 ++---
drivers/dma-buf/reservation.c | 82
include/linux/reservation.h | 115 +-
3 files changed, 101 insertions(+), 127 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f45bfb29ef96..67510f2be8bc 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -199,7 +199,7 @@ static __poll_t dma_buf_poll(struct file *file, poll_table
*poll)
struct reservation_object_list *fobj;
struct dma_fence *fence_excl;
__poll_t events;
- unsigned shared_count, seq;
+ unsigned shared_count;
dmabuf = file->private_data;
if (!dmabuf || !dmabuf->resv)
@@ -213,21 +213,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table
*poll)
if (!events)
return 0;
-retry:
- seq = read_seqcount_begin(>seq);
rcu_read_lock();
-
- fobj = rcu_dereference(resv->fence);
- if (fobj)
- shared_count = fobj->shared_count;
- else
- shared_count = 0;
- fence_excl = rcu_dereference(resv->fence_excl);
- if (read_seqcount_retry(>seq, seq)) {
- rcu_read_unlock();
- goto retry;
- }
-
+ reservation_object_fences(resv, _excl, , _count);
if (fence_excl && (!(events & EPOLLOUT) || shared_count == 0)) {
struct dma_buf_poll_cb_t *dcb = >cb_excl;
__poll_t pevents = EPOLLIN;
@@ -1157,7 +1144,6 @@ static int dma_buf_debug_show(struct seq_file *s, void
*unused)
struct reservation_object *robj;
struct reservation_object_list *fobj;
struct dma_fence *fence;
- unsigned seq;
int count = 0, attach_count, shared_count, i;
size_t size = 0;
@@ -1188,16 +1174,9 @@ static int dma_buf_debug_show(struct seq_file *s, void
*unused)
buf_obj->name ?: "");
robj = buf_obj->resv;
- while (true) {
- seq = read_seqcount_begin(>seq);
- rcu_read_lock();
- fobj = rcu_dereference(robj->fence);
- shared_count = fobj ? fobj->shared_count : 0;
- fence = rcu_dereference(robj->fence_excl);
- if (!read_seqcount_retry(>seq, seq))
- break;
- rcu_read_unlock();
- }
+ rcu_read_lock();
+ reservation_object_fences(robj, , , _count);
+ rcu_read_unlock();
if (fence)
seq_printf(s, "\tExclusive fence: %s %s %ssignalled\n",
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index ad6775b32a73..8fcaddffd5d4 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -317,17 +317,15 @@ int reservation_object_copy_fences(struct
reservation_object *dst,
{
struct reservation_object_list *src_list, *dst_list;
struct dma_fence *old, *new;
- unsigned i;
+ unsigned int i, shared_count;
reservation_object_assert_held(dst);
rcu_read_lock();
- src_list = rcu_dereference(src->fence);
retry:
- if (src_list) {
- unsigned shared_count = src_list->shared_count;
-
+ reservation_object_fences(src, , _list, _count);
+ if (shared_count) {
rcu_read_unlock();
dst_list = reservation_object_list_alloc(shared_count);
@@ -335,14 +333,14 @@ int reservation_object_copy_fences(struct
reservation_object *dst,
return -ENOMEM;
rcu_read_lock();
- src_list = rcu_dereference(src->fence);
- if (!src_list || src_list->shared_count > shared_count) {
+ reservation_object_fences(src, , _list, _count);
+ if (!src_list || shared_count > dst_list->shared_max) {
kfree(dst_list);
goto retry;
}
dst_list->shared_count = 0;
- for (i = 0; i < src_list->shared_count; ++i) {
+ for (i = 0; i < shared_count; ++i) {
struct dma_fence *fence;
fence = rcu_dereference(src_list->shared[i]);
@@ -352,7 +350,6 @@ int reservation_object_copy_fences(struct
reservation_object *dst,
if (!dma_fence_get_rcu(fence)) {
reservation_object_list_free(dst_list);
- src_list = rcu_dereference(src->fence);
goto retry;
Re: [PATCH 0/2] drm/syncobj: add syncobj sideband payload for threaded submission
Well first of all I strongly suggest to make the sideband information a separate IOCTL and not use the existing signal/query IOCTLs for it. Then as far as I see this basically sets a sequence number to use for binary semaphores, is that correct? If yes then that would be a rather elegant idea. Christian. Am 07.08.19 um 15:37 schrieb Lionel Landwerlin: > Hi, > > The goal of this feature is to solve an issue that was seen in our > testing. After discussing on [1] I thought we could solve this problem > with stalling the application thread after each vkQueueSubmit() call > where a binary semaphore is signaled but I don't think it's a good > option because of performance impacts on the application. > > Unfortunately there isn't a good way to reproduce this problem 100% > because it essentially exposes a race between the application thread > and the submission thread. > > I've uploaded tests in the Khronos repository to try to expose the > issue. > > Thanks, > > [1] : https://lists.freedesktop.org/archives/dri-devel/2019-August/229188.html > > Lionel Landwerlin (2): >drm/syncobj: add sideband payload >drm/syncobj: add submit point query capability > > drivers/gpu/drm/drm_syncobj.c | 132 ++ > include/drm/drm_syncobj.h | 9 +++ > include/uapi/drm/drm.h| 5 +- > 3 files changed, 100 insertions(+), 46 deletions(-) > > -- > 2.23.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111244] amdgpu kernel 5.2 blank display after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=111244 --- Comment #21 from csp...@verizon.net --- The default is -1 according to the docs and /sys/module/amdgpu/parameters/dc. I assume it should effectively be the same but it seems to result in different behavior vs. setting it to 1. DC is enabled in both cases (the log shows "Display Core initialized"), but setting it to default results in a suspend/resume failure 100% of the time. Whereas setting it to 1 results in success most of time, although it did fail eventually after several reboots. Very strange. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/syncobj: add sideband payload
The Vulkan timeline semaphores allow signaling to happen on the point
of the timeline without all of the its dependencies to be created.
The current 2 implementations (AMD/Intel) of the Vulkan spec on top of
the Linux kernel are using a thread to wait on the dependencies of a
given point to materialize and delay actual submission to the kernel
driver until the wait completes.
If a binary semaphore is submitted for signaling along the side of a
timeline semaphore waiting for completion that means that the drm
syncobj associated with that binary semaphore will not have a DMA
fence associated with it by the time vkQueueSubmit() returns. This and
the fact that a binary semaphore can be signaled and unsignaled as
before its DMA fences materialize mean that we cannot just rely on the
fence within the syncobj but we also need a sideband payload verifying
that the fence in the syncobj matches the last submission from the
Vulkan API point of view.
This change adds a sideband payload that is incremented with signaled
syncobj when vkQueueSubmit() is called. The next vkQueueSubmit()
waiting on a the syncobj will read the sideband payload and wait for a
fence chain element with a seqno superior or equal to the sideband
payload value to be added into the fence chain and use that fence to
trigger the submission on the kernel driver.
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/drm_syncobj.c | 119 +-
include/drm/drm_syncobj.h | 9 +++
include/uapi/drm/drm.h| 4 +-
3 files changed, 86 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index b927e482e554..c437fb6aaf7c 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1150,8 +1150,10 @@ drm_syncobj_reset_ioctl(struct drm_device *dev, void
*data,
if (ret < 0)
return ret;
- for (i = 0; i < args->count_handles; i++)
+ for (i = 0; i < args->count_handles; i++) {
drm_syncobj_replace_fence(syncobjs[i], NULL);
+ syncobjs[i]->sideband_payload = 0;
+ }
drm_syncobj_array_free(syncobjs, args->count_handles);
@@ -1197,7 +1199,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev,
void *data,
{
struct drm_syncobj_timeline_array *args = data;
struct drm_syncobj **syncobjs;
- struct dma_fence_chain **chains;
+ struct dma_fence_chain **chains = NULL;
uint64_t *points;
uint32_t i, j;
int ret;
@@ -1205,7 +1207,8 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev,
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
- if (args->pad != 0)
+ if (args->selector !=
DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SIGNALED_POINT &&
+ args->selector !=
DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SIDEBAND_PAYLOAD)
return -EINVAL;
if (args->count_handles == 0)
@@ -1232,30 +1235,41 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device
*dev, void *data,
goto err_points;
}
- chains = kmalloc_array(args->count_handles, sizeof(void *), GFP_KERNEL);
- if (!chains) {
- ret = -ENOMEM;
- goto err_points;
- }
- for (i = 0; i < args->count_handles; i++) {
- chains[i] = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL);
- if (!chains[i]) {
- for (j = 0; j < i; j++)
- kfree(chains[j]);
+ switch (args->selector) {
+ case DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SIGNALED_POINT:
+ chains = kmalloc_array(args->count_handles, sizeof(void *),
GFP_KERNEL);
+ if (!chains) {
ret = -ENOMEM;
- goto err_chains;
+ goto err_points;
+ }
+ for (i = 0; i < args->count_handles; i++) {
+ chains[i] = kzalloc(sizeof(struct dma_fence_chain),
GFP_KERNEL);
+ if (!chains[i]) {
+ for (j = 0; j < i; j++)
+ kfree(chains[j]);
+ ret = -ENOMEM;
+ goto err_chains;
+ }
}
- }
- for (i = 0; i < args->count_handles; i++) {
- struct dma_fence *fence = dma_fence_get_stub();
+ for (i = 0; i < args->count_handles; i++) {
+ struct dma_fence *fence = dma_fence_get_stub();
- drm_syncobj_add_point(syncobjs[i], chains[i],
- fence, points[i]);
- dma_fence_put(fence);
+ drm_syncobj_add_point(syncobjs[i], chains[i],
+ fence, points[i]);
+
[PATCH 0/2] drm/syncobj: add syncobj sideband payload for threaded submission
Hi, The goal of this feature is to solve an issue that was seen in our testing. After discussing on [1] I thought we could solve this problem with stalling the application thread after each vkQueueSubmit() call where a binary semaphore is signaled but I don't think it's a good option because of performance impacts on the application. Unfortunately there isn't a good way to reproduce this problem 100% because it essentially exposes a race between the application thread and the submission thread. I've uploaded tests in the Khronos repository to try to expose the issue. Thanks, [1] : https://lists.freedesktop.org/archives/dri-devel/2019-August/229188.html Lionel Landwerlin (2): drm/syncobj: add sideband payload drm/syncobj: add submit point query capability drivers/gpu/drm/drm_syncobj.c | 132 ++ include/drm/drm_syncobj.h | 9 +++ include/uapi/drm/drm.h| 5 +- 3 files changed, 100 insertions(+), 46 deletions(-) -- 2.23.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/syncobj: add submit point query capability
This feature was talked about by David. It allows userspace to query
the last submitted point on a timeline.
Following the previous commit it made sense to add it.
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/drm_syncobj.c | 15 ++-
include/uapi/drm/drm.h| 1 +
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index c437fb6aaf7c..ad2f5672d707 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1291,7 +1291,8 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void
*data,
return -EOPNOTSUPP;
if (args->selector !=
DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SIGNALED_POINT &&
- args->selector !=
DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SIDEBAND_PAYLOAD)
+ args->selector !=
DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SIDEBAND_PAYLOAD &&
+ args->selector != DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SUBMIT_POINT)
return -EINVAL;
if (args->count_handles == 0)
@@ -1346,6 +1347,18 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void
*data,
if (ret)
goto error;
break;
+
+ case DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SUBMIT_POINT:
+ fence = drm_syncobj_fence_get(syncobjs[i]);
+ chain = to_dma_fence_chain(fence);
+ point = chain ? chain->base.seqno : 0;
+ dma_fence_put(fence);
+
+ ret = copy_to_user([i], ,
sizeof(uint64_t));
+ ret = ret ? -EFAULT : 0;
+ if (ret)
+ goto error;
+ break;
}
}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index dea759a36d37..3b8cdb3ffa94 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -785,6 +785,7 @@ struct drm_syncobj_timeline_array {
__u32 selector;
#define DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SIGNALED_POINT (0)
#define DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SIDEBAND_PAYLOAD (1)
+#define DRM_SYNCOBJ_TIMELINE_ARRAY_SELECTOR_SUBMIT_POINT (2)
};
--
2.23.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Fix some NULL vs IS_ERR() conditions
Quoting Chris Wilson (2019-08-07 13:32:15)
> Quoting Dan Carpenter (2019-08-07 13:28:32)
> > There were several places which check for NULL when they should have
> > been checking for IS_ERR().
> >
> > Fixes: d8af05ff38ae ("drm/i915: Allow sharing the idle-barrier from other
> > kernel requests")
> > Signed-off-by: Dan Carpenter
>
> Oops,
> Reviewed-by: Chris Wilson
And pushed, ta.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 8/8] dma-buf: nuke reservation_object seq number
Am 07.08.19 um 14:19 schrieb Chris Wilson:
> Quoting Christian König (2019-08-07 13:08:38)
>> Am 06.08.19 um 21:57 schrieb Chris Wilson:
>>> If we add to shared-list during the read, ... Hmm, actually we should
>>> return num_list, i.e.
>>>
>>> do {
>>>*list = rcu_dereference(obj->fence);
>>>num_list = *list ? (*list)->count : 0;
>>>smp_rmb();
>>> } while (...)
>>>
>>> return num_list.
>>>
>>> as the relationship between the count and the fence entries is also
>>> determined by the mb in add_shared_fence.
>> I've read that multiple times now, but can't follow. Why should we do this?
>>
>> The only important thing is that the readers see the new fence before
>> the increment of the number of fences.
> Exactly. We order the store so that the fence is in the list before we
> update the count (so that we don't read garbage because the fence isn't
> there yet).
>
> But we don't have the equivalent here for the read once the rmb is
> removed from the seqcount_read_begin/end looping. We need to see the
> update in the same order as was stored, and only use the coherent
> portion of the list.
Ok that makes sense. Going to fix up the code regarding to that.
Christian.
> -Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/crc-debugfs: Add notes about CRC<->commit interactions
On Tue, Aug 06, 2019 at 01:46:22PM +0100, Brian Starkey wrote:
> CRC generation can be impacted by commits coming from userspace, and
> enabling CRC generation may itself trigger a commit. Add notes about
> this to the kerneldoc.
>
> Changes since v1:
> - Clarified that anything that would disable CRCs counts as a full
>modeset, and so userspace needs to reconfigure after full modesets
>
> Changes since v2:
> - Add these notes
> - Rebase onto drm-misc-next (trivial conflict in comment)
>
> Signed-off-by: Brian Starkey
> Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_debugfs_crc.c | 9 +
> include/drm/drm_crtc.h| 4
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c
> index 6604ed223160..be1b7ba92ffe 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -69,6 +69,15 @@
> * implement _crtc_funcs.set_crc_source and
> _crtc_funcs.verify_crc_source.
> * The debugfs files are automatically set up if those vfuncs are set. CRC
> samples
> * need to be captured in the driver by calling drm_crtc_add_crc_entry().
> + * Depending on the driver and HW requirements,
> _crtc_funcs.set_crc_source
> + * may result in a commit (even a full modeset).
> + *
> + * CRC results must be reliable across non-full-modeset atomic commits, so
> if a
> + * commit via DRM_IOCTL_MODE_ATOMIC would disable or otherwise interfere with
> + * CRC generation, then the driver must mark that commit as a full modeset
> + * (drm_atomic_crtc_needs_modeset() should return true). As a result, to
> ensure
> + * consistent results, generic userspace must re-setup CRC generation after a
> + * legacy SETCRTC or an atomic commit with DRM_MODE_ATOMIC_ALLOW_MODESET.
> */
>
> static int crc_control_show(struct seq_file *m, void *data)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 128d8b210621..7d14c11bdc0a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -756,6 +756,9 @@ struct drm_crtc_funcs {
>* provided from the configured source. Drivers must accept an "auto"
>* source name that will select a default source for this CRTC.
>*
> + * This may trigger an atomic modeset commit if necessary, to enable CRC
> + * generation.
> + *
>* Note that "auto" can depend upon the current modeset configuration,
>* e.g. it could pick an encoder or output specific CRC sampling point.
>*
> @@ -767,6 +770,7 @@ struct drm_crtc_funcs {
>* 0 on success or a negative error code on failure.
>*/
> int (*set_crc_source)(struct drm_crtc *crtc, const char *source);
> +
> /**
>* @verify_crc_source:
>*
> --
> 2.17.1
>
Pushed to drm-misc-next.
Commit id :- 178e5f3a5bc1d67d1248a74c0abab41040abe7c4
-Ayan
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
