Re: [PATCH 0/2] drm/fourcc.h: Add libcamera to Open Source Waiver
On Thu, 14 Mar 2024 at 10:12, Jacopo Mondi wrote: > gentle nudge for > > *) libcamera: are we ok being listed here ? > *) DRM/KMS: is it ok splitting the list of projects in the way I've >done ? My bikeshed would be to change the list to 1) OpenGL / OpenGL ES / EGL and its extensions, 2) Vulkan and its extensions, 3) libcamera. But it doesn't really make much difference; people are going to get the point. With whatever reasonable wording, series is: Reviewed-by: Daniel Stone Thanks Jacopo! -d
Re: Safety of opening up /dev/dma_heap/* to physically present users (udev uaccess tag) ?
Hi, On Wed, 8 May 2024 at 16:49, Daniel Vetter wrote: > On Wed, May 08, 2024 at 09:38:33AM +0100, Daniel Stone wrote: > > Right now, if your platform requires CMA for display, then the app > > needs access to the GPU render node and the display node too, in order > > to allocate buffers which the compositor can scan out directly. If it > > only has access to the render nodes and not the display node, it won't > > be able to allocate correctly, so its content will need a composition > > pass, i.e. performance penalty for sandboxing. But if it can allocate > > correctly, then hey, it can exhaust CMA just like heaps can. > > > > Personally I think we'd be better off just allowing access and > > figuring out cgroups later. It's not like the OOM story is great > > generally, and hey, you can get there with just render nodes ... > > Imo the right fix is to ask the compositor to allocate the buffers in this > case, and then maybe have some kind of revoke/purge behaviour on these > buffers. Compositor has an actual idea of who's a candidate for direct > scanout after all, not the app. Or well at least force migrate the memory > from cma to shmem. > > If you only whack cgroups on this issue you're still stuck in the world > where either all apps together can ddos the display or no one can > realistically direct scanout. Mmm, back to DRI2. I can't say I'm wildly enthused about that, not least because a client using GPU/codec/etc for those buffers would have to communicate its requirements (alignment etc) forward to the compositor in order for the compositor to allocate for it. Obviously passing the constraints etc around isn't a solved problem yet, but it is at least contained down in clients rather than making it back and forth between client and compositor. I'm extremely not-wild about the compositor migrating memory from CMA to shmem behind the client's back, and tbh I'm not sure how that would even work if the client has it pinned through whatever API it's imported into. Anyway, like Laurent says, if we're deciding that heaps can't be used by generic apps (unlike DRM/V4L2/etc), then we need gralloc. Cheers, Daniel
Re: Safety of opening up /dev/dma_heap/* to physically present users (udev uaccess tag) ?
On Wed, 8 May 2024 at 09:33, Daniel Vetter wrote: > On Wed, May 08, 2024 at 06:46:53AM +0100, Daniel Stone wrote: > > That would have the unfortunate side effect of making sandboxed apps > > less efficient on some platforms, since they wouldn't be able to do > > direct scanout anymore ... > > I was assuming that everyone goes through pipewire, and ideally that is > the only one that can even get at these special chardev. > > If pipewire is only for sandboxed apps then yeah this aint great :-/ No, PipeWire is fine, I mean graphical apps. Right now, if your platform requires CMA for display, then the app needs access to the GPU render node and the display node too, in order to allocate buffers which the compositor can scan out directly. If it only has access to the render nodes and not the display node, it won't be able to allocate correctly, so its content will need a composition pass, i.e. performance penalty for sandboxing. But if it can allocate correctly, then hey, it can exhaust CMA just like heaps can. Personally I think we'd be better off just allowing access and figuring out cgroups later. It's not like the OOM story is great generally, and hey, you can get there with just render nodes ... Cheers, Daniel
Re: Safety of opening up /dev/dma_heap/* to physically present users (udev uaccess tag) ?
Hi, On Tue, 7 May 2024 at 12:15, Daniel Vetter wrote: > On Mon, May 06, 2024 at 04:01:42PM +0200, Hans de Goede wrote: > > On 5/6/24 3:38 PM, Daniel Vetter wrote: > > I agree that bad applications are an issue, but not for the flathub / snaps > > case. Flatpacks / snaps run sandboxed and don't have access to a full /dev > > so those should not be able to open /dev/dma_heap/* independent of > > the ACLs on /dev/dma_heap/*. The plan is for cameras using the > > libcamera software ISP to always be accessed through pipewire and > > the camera portal, so in this case pipewere is taking the place of > > the compositor in your kms vs render node example. > > Yeah essentially if you clarify to "set the permissions such that pipewire > can do allocations", then I think that makes sense. And is at the same > level as e.g. drm kms giving compsitors (but _only_ compositors) special > access rights. That would have the unfortunate side effect of making sandboxed apps less efficient on some platforms, since they wouldn't be able to do direct scanout anymore ... Cheers, Daniel
Re: [petsc-users] Spack build and ptscotch
Hi Satish, Many thanks for the interesting response. I'll see if I can figure out something useful to do for our goals (a reliable spack package for our simulator, which is dependent on bringing in the exact right petsc build as dependency), but as you say, maintaining these things and accounting for everything is a lot harder than it looks. Time constraints from other projects might limit how much more tinkering I can do, but we have a better idea of the issues we face now. Daniel On Wed, Apr 24, 2024 at 4:25 PM Satish Balay wrote: > This is the complexity with maintaining dependencies (and dependencies > of dependencies), and different build systems > > - Its not easy to keep the "defaults" in both builds exactly the same. > - And its not easy to expose all "variants" or keep the same variants in > both builds. > - And each pkg has its own issues that prevents some combinations to > work or not [or tested combinations vs untested]. > > This e-mail query has multiple things: > > - understand "why" the current impl of [spack, petsc] build tools are the > way they are. > - if they can be improved > - and build use cases that you need working > - [and subsequently your code working] > > Addressing them all is not easy - so lets stick with what you need to make > progress. > > For one - we recommend using latest petsc version [i.e 3.21 - not 3.19] - > any fixes we have will address the current release. > > > - spack: ptscotch will always be built without parmetis wrappers, can't > turn on > > diff --git a/var/spack/repos/builtin/packages/petsc/package.py > b/var/spack/repos/builtin/packages/petsc/package.py > index b7b1d86b15..ae27ba4c4e 100644 > --- a/var/spack/repos/builtin/packages/petsc/package.py > +++ b/var/spack/repos/builtin/packages/petsc/package.py > @@ -268,9 +268,7 @@ def check_fortran_compiler(self): > depends_on("metis@5:~int64", when="@3.8:+metis~int64") > depends_on("metis@5:+int64", when="@3.8:+metis+int64") > > -# PTScotch: Currently disable Parmetis wrapper, this means > -# nested disection won't be available thought PTScotch > -depends_on("scotch+esmumps~metis+mpi", when="+ptscotch") > +depends_on("scotch+esmumps+mpi", when="+ptscotch") > depends_on("scotch+int64", when="+ptscotch+int64") > > depends_on("hdf5@:1.10+mpi", when="@:3.12+hdf5+mpi") > > Now you can try: > > spack install petsc~metis+ptscotch ^scotch+metis > vs > spack install petsc~metis+ptscotch ^scotch~metis [~metis is the default > for scotch] > > Note the following comment in > spack/var/spack/repos/builtin/packages/scotch/package.py > > >>>> > # Vendored dependency of METIS/ParMETIS conflicts with standard > # installations > conflicts("metis", when="+metis") > conflicts("parmetis", when="+metis") > <<<<< > > > - classical: ptscotch will always be built with parmetis wrappers, can't > seem to turn off > > Looks like spack uses cmake build of ptscotch. PETSc uses Makefile > interface. It likely doesn't support turning off metis wrappers [without > hacks]. > > So you might either need to hack scotch build via petsc - or just install > it separately - and use it with petsc. > > I see an effort to migrate scotch build in petsc to cmake > > https://urldefense.us/v3/__https://gitlab.com/petsc/petsc/-/merge_requests/7242/__;!!G_uCfscf7eWS!axLWsOWdCnVQSjIurDkWvFmG4riOizNNPbVM78TQoVScHx7ERMUENiQ-VW2Lh5e83QHhKcA7-HO0nDJ_hTez8JffqQz8h1I$ > > https://urldefense.us/v3/__https://gitlab.com/petsc/petsc/-/merge_requests/7495/__;!!G_uCfscf7eWS!axLWsOWdCnVQSjIurDkWvFmG4riOizNNPbVM78TQoVScHx7ERMUENiQ-VW2Lh5e83QHhKcA7-HO0nDJ_hTez8JffgEb5IwI$ > > > Satish > > On Wed, 24 Apr 2024, Daniel Stone wrote: > > > Hi PETSc community, > > > > I've been looking at using Spack to build PETSc, in particular I need to > > disable the default metis/parmetis dependencies and use PTScotch instead, > > for our software. > > I've had quite a bit of trouble with this - it seems like something in > the > > resulting build of our simulator ends up badly optimised and an mpi > > bottleneck, when I build against > > PETSc built with Spack. > > > > I've been trying to track this down, and noticed this in the PETSc Spack > > build recipe: > > > > # PTScotch: Currently disable Parmetis wrapper, this means > > # nested disection won't be available thought PTScotch > > depends_on("scotch+esmumps~metis+mpi", when="+ptscotch&q
Re: Possible Performance Regression with Mesa
Hi Joao, On Fri, 26 Apr 2024 at 08:42, Joao Paulo Silva Goncalves wrote: > On Thu, Apr 25, 2024 at 9:08 AM Lucas Stach wrote: > > I can reproduce the issue, but sadly there is no simple fix for this, > > as it's a bad interaction between some of the new features. > > At the core of the issue is the dmabuf-feedback support with the chain > > of events being as follows: > > > 1. weston switches to the scanout tranche, as it would like to put the > > surface on a plane > > 2. the client reallocates as linear but does so on the render node > > 3. weston still isn't able to put the buffer on the plane, as it's > > still scanout incompatible due to being non-contig, so needs to fall > > back to rendering > > 4. now we are stuck at a linear buffer being used for rendering, which > > is very non-optimal > > > I'll look into improving this, but can make no commitments as to when > > I'll be able to get around to this. > > Seem to be tricky. > If you want, we at least can help you test it. Just reach out. > We also saw similar behaviour on more modern hardware, like the iMX8MM. > I will do a bit more testing on the iMX8MM and also some on the iMX8MP to > geather more data and > I am thinking in also opening an issue on the gitlab of Mesa, for better > tracking. What do you think? One thing you can try is to edit weston/libweston/backend-drm/state-propose.c and, inside dmabuf_feedback_maybe_update(), prevent action_needed from ever being set to ACTION_NEEDED_ADD_SCANOUT_TRANCHE. It would be interesting to know if this restores full performance. Cheers, Daniel
Re: Possible Performance Regression with Mesa
On Thu, 25 Apr 2024 at 13:08, Lucas Stach wrote: > I can reproduce the issue, but sadly there is no simple fix for this, > as it's a bad interaction between some of the new features. > At the core of the issue is the dmabuf-feedback support with the chain > of events being as follows: > > 1. weston switches to the scanout tranche, as it would like to put the > surface on a plane > 2. the client reallocates as linear but does so on the render node > 3. weston still isn't able to put the buffer on the plane, as it's > still scanout incompatible due to being non-contig, so needs to fall > back to rendering > 4. now we are stuck at a linear buffer being used for rendering, which > is very non-optimal Oh man, sorry about that, that shouldn't happen. As long as drmModeAddFB2 is failing, we should be marking the buffer as non-importable, and then hinting the client back towards tiled. That being said, yeah, having the client render to linear and skip composition is definitely going to be better! Cheers, Daniel
[petsc-users] Spack build and ptscotch
Hi PETSc community, I've been looking at using Spack to build PETSc, in particular I need to disable the default metis/parmetis dependencies and use PTScotch instead, for our software. I've had quite a bit of trouble with this - it seems like something in the resulting build of our simulator ends up badly optimised and an mpi bottleneck, when I build against PETSc built with Spack. I've been trying to track this down, and noticed this in the PETSc Spack build recipe: # PTScotch: Currently disable Parmetis wrapper, this means # nested disection won't be available thought PTScotch depends_on("scotch+esmumps~metis+mpi", when="+ptscotch") depends_on("scotch+int64", when="+ptscotch+int64") Sure enough - when I compare the build with Spack and a traditional build with ./configure etc, I see that, in the traditional build, Scotch is always built with the parmetis wrapper, but not in the Scotch build. In fact, I'm not sure how to turn off the parmetis wrapper option for scotch, in the case of a traditional build (i.e. there doesn't seem to be a flag in the configure script for it) - which would be a very useful test for me (I can of course do similar experiments by doing a classical build of petsc against ptscotch built separately without the wrappers, etc - will try that). Does anyone know why the parmetis wrapper is always disabled in the spack build options? Is there something about Spack that would prevent it from working? It's notable - but I might be missing it - that there's no warning that there's a difference in the way ptscotch is built between the spack and classical builds: - classical: ptscotch will always be built with parmetis wrappers, can't seem to turn off - spack: ptscotch will always be built without parmetis wrappers, can't turn on Any insight at all would be great, I'm new to Spack and am not super familiar with the logic that goes into setting up builds for the system. Here is the kind of command I give to Spack for PETSc builds, which may well be less than ideal: spack install petsc@3.19.1 ~metis +ptscotch ^hdf5 +fortran +hl Separate tiny note: when building with hdf5, I have to ensure that the fortran flag is set for it, as above. There's a fortran flag for the petsc module, default true, and a fortran flag for the hdf5 module, default false. A naive user (i.e. me), will see the fortran flag for the petsc module, and assume that all dependencies will correspondingly be built with fortran capability - then see that hdf5.mod is missing when trying to build their software against petsc. It's the old "did you forget --with-hdf5-fortran-bindings?" issue, resurrected for a new build system. Thanks, Daniel
New maintainer for XDG specs
Hi all, After a bit of a period of inactivity, Matthias Klumpp has agreed to help update and maintain the XDG specs, including the menu and icon-theme specs in particular. If you have any suggestions or anything you want addressed, please discuss it on the list, or file an issue or merge request. Thanks to Matthias for stepping up, as well as to Bastien Nocera and David Faure in particular for all their work over the years to get us to this point. Cheers, Daniel
Re: [PATCH v2 1/2] drm: Introduce plane SIZE_HINTS property
Hi, On Tue, 27 Feb 2024 at 19:35, Ville Syrjala wrote: > Add a new immutable plane property by which a plane can advertise > a handful of recommended plane sizes. This would be mostly exposed > by cursor planes as a slightly more capable replacement for > the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare > a one size fits all limit for the whole device. Acked-by: Daniel Stone Cheers, Daniel
Re: [PATCH v2 1/2] drm: Introduce plane SIZE_HINTS property
Hi, On Tue, 27 Feb 2024 at 19:35, Ville Syrjala wrote: > Add a new immutable plane property by which a plane can advertise > a handful of recommended plane sizes. This would be mostly exposed > by cursor planes as a slightly more capable replacement for > the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare > a one size fits all limit for the whole device. Acked-by: Daniel Stone Cheers, Daniel
Re: drm-misc migration to Gitlab server
On Mon, 26 Feb 2024 at 12:03, Jani Nikula wrote: > On Mon, 26 Feb 2024, Daniel Stone wrote: > > It's a fair question. If you want to verify that someone is > > @intel.com, maybe get them to email you out-of-band to check it. If > > you want to check something else, just ask an admin I suppose. > > I thought you wanted not to be bothered with access requests! ;D Hey, we have like four other people with GitLab admin privileges ... I just want to not have to deal with LDAP mainly.
Re: drm-misc migration to Gitlab server
On Mon, 26 Feb 2024 at 11:57, Jani Nikula wrote: > On Mon, 26 Feb 2024, Maxime Ripard wrote: > > For the recent-ish subscriptions, it's possible since we've required to > > open a Gitlab issue for a while, so we have the association between the > > Gitlab account and the SSH account already. > > > > During the Gitlab setup, the groups were also created already with the > > people that had an SSH account at the time, and Gitlab account. > > > > But for the rest, yeah, I had to ping Daniel S. about it. He could find > > a few matches, but there's some where we just don't know if or what the > > Gitlab account is. > > > > Generally speaking, we've been conservative about it, and only added > > accounts we were sure of. > > Ah, I didn't make myself clear. I'm more interested in the process going > forward, for new access requests. Anyone can create an account and > request access; how does a maintainer verify the request? For our > purposes it's basically just matching againt the email addresses in > existing commits in the repo. It's a fair question. If you want to verify that someone is @intel.com, maybe get them to email you out-of-band to check it. If you want to check something else, just ask an admin I suppose. Cheers, Daniel
Re: [PULL] drm-xe-next
Hi, On Mon, 26 Feb 2024 at 03:21, Lucas De Marchi wrote: > All of this should be fixed by now: dim is used for applying and pushing > patches, which has additional checks so that doesn't happen again. Still > pending confirmation from Daniel Stone if the git server hooks are ready > in gitlab so we properly forbid pushes without dim, like we do with the > git.fd.o infra. Yeah, I did that last week. Cheers, Daniel
Re: [PULL] drm-xe-next
Hi, On Mon, 26 Feb 2024 at 03:21, Lucas De Marchi wrote: > All of this should be fixed by now: dim is used for applying and pushing > patches, which has additional checks so that doesn't happen again. Still > pending confirmation from Daniel Stone if the git server hooks are ready > in gitlab so we properly forbid pushes without dim, like we do with the > git.fd.o infra. Yeah, I did that last week. Cheers, Daniel
Re: [PULL] drm-xe-next
Hi, On Mon, 26 Feb 2024 at 03:21, Lucas De Marchi wrote: > All of this should be fixed by now: dim is used for applying and pushing > patches, which has additional checks so that doesn't happen again. Still > pending confirmation from Daniel Stone if the git server hooks are ready > in gitlab so we properly forbid pushes without dim, like we do with the > git.fd.o infra. Yeah, I did that last week. Cheers, Daniel
Re: drm-misc migration to Gitlab server
Hi Stephen, On Tue, 20 Feb 2024 at 22:46, Stephen Rothwell wrote: > On Tue, 20 Feb 2024 11:25:05 +0000 Daniel Stone wrote: > > cc sfr - once we move the DRM repos to a different location, what's > > the best way to update linux-next? > > These are (I think) all the drm trees/branches that I fetch every day: > > [...] > > If someone could just send me all the new equivalent URLs when the > change happens, I will fix them up in my config. Thanks a lot! We'll give you a shout with a couple of days' heads-up before we move then. Cheers, Daniel
Re: drm-misc migration to Gitlab server
On Tue, 20 Feb 2024 at 09:05, Maxime Ripard wrote: > On Tue, Feb 20, 2024 at 09:49:25AM +0100, Maxime Ripard wrote: > > This will be mostly transparent to current committers and users: we'll > > still use dim, in the exact same way, the only change will be the URL of > > the repo. This will also be transparent to linux-next, since the > > linux-next branch lives in its own repo and is pushed by dim when > > pushing a branch. > > Actually, I double-checked and linux-next pulls our branches directly, > so once the transition is over we'll have to notify them too. cc sfr - once we move the DRM repos to a different location, what's the best way to update linux-next? That being said, we could set up read-only pull mirrors in the old location ... something I want to do in March (because what else are you going to do on holiday?) is to kill the write repos on kemper (git.fd.o), move them to being on molly (cgit/anongit.fd.o) only, and just have a cronjob that regularly pulls from all the gl.fd.o repos, rather than pushing from GitLab. Cheers, Daniel
Re: [PATCH 1/2] drm/tidss: Fix initial plane zpos values
Hi, On Fri, 16 Feb 2024 at 09:00, Tomi Valkeinen wrote: > On 13/02/2024 13:39, Daniel Stone wrote: > > Specifically, you probably want commits 4cde507be6a1 and 58dde0e0c000. > > I think the window of breakage was small enough that - assuming either > > those commits or an upgrade to Weston 12/13 fixes it - we can just ask > > people to upgrade to a fixed Weston. > > > >>> Presuming this is not related to any TI specific code, I guess it's a > >>> regression in the sense that at some point Weston added the support to use > >>> planes for composition, so previously with only a single plane per display > >>> there was no issue. > > > > That point was 12 years ago, so not that novel. ;) > > Hmm, so do I understand it right, the plane code from 12 years back > supposedly works ok, but somewhere around Weston 10 something broke, but > was fixed with the commits you mention above? We always had plane support but pre-zpos; we added support for zpos a couple/few releases ago, but then massively refactored it ... so it could've always been broken, or could've been broken for as long as we have zpos, or it could've just been a small window in between the refactor.
Re: [PATCH 1/2] drm/tidss: Fix initial plane zpos values
Hi, On Fri, 16 Feb 2024 at 09:00, Tomi Valkeinen wrote: > On 13/02/2024 13:39, Daniel Stone wrote: > > Specifically, you probably want commits 4cde507be6a1 and 58dde0e0c000. > > I think the window of breakage was small enough that - assuming either > > those commits or an upgrade to Weston 12/13 fixes it - we can just ask > > people to upgrade to a fixed Weston. > > > >>> Presuming this is not related to any TI specific code, I guess it's a > >>> regression in the sense that at some point Weston added the support to use > >>> planes for composition, so previously with only a single plane per display > >>> there was no issue. > > > > That point was 12 years ago, so not that novel. ;) > > Hmm, so do I understand it right, the plane code from 12 years back > supposedly works ok, but somewhere around Weston 10 something broke, but > was fixed with the commits you mention above? We always had plane support but pre-zpos; we added support for zpos a couple/few releases ago, but then massively refactored it ... so it could've always been broken, or could've been broken for as long as we have zpos, or it could've just been a small window in between the refactor.
Re: [PATCH 1/2] drm/tidss: Fix initial plane zpos values
Hi, On Tue, 13 Feb 2024 at 10:18, Marius Vlad wrote: > On Tue, Feb 13, 2024 at 11:57:59AM +0200, Tomi Valkeinen wrote: > > I haven't. I'm quite unfamiliar with Weston, and Randolph from TI (cc'd) has > > been working on the Weston side of things. I also don't know if there's > > something TI specific here, as the use case is with non-mainline GPU drivers > > and non-mainline Mesa. I should have been a bit clearer in the patch > > description, as I didn't mean that upstream Weston has a bug (maybe it has, > > maybe it has not). Don't worry about it. We've had bugs in the past and I'm sure we'll have more. :) Either way, it's definitely better to have the kernel expose sensible behaviour rather than weird workarounds, unless they've been around for so long that they're basically baked into ABI. > > The issue seen is that when Weston decides to use DRM planes for > > composition, the plane zpositions are not configured correctly (or at all?). > > Afaics, this leads to e.g. weston showing a window with a DRM "overlay" > > plane that is behind the "primary" root plane, so the window is not visible. > > And as Weston thinks that the area supposedly covered by the overlay plane > > does not need to be rendered on the root plane, there are also artifacts on > > that area. > > > > Also, the Weston I used is a bit older one (10.0.1), as I needed to go back > > in my buildroot versions to get all that non-mainline GPU stuff compiled and > > working. A more recent Weston may behave differently. > > Right after Weston 10, we had a few minor changes related to the > zpos-sorting list of planes and how we parse the plan list without having > a temporary zpos ordered list to pick planes from. > > And there's another fix for missing out to set out the zpos for scanout > to the minimum available - which seems like a good candidate to explain > what happens in the issue described above. So if trying Weston again, > please try with at least Weston 12, which should have those changes > in. Specifically, you probably want commits 4cde507be6a1 and 58dde0e0c000. I think the window of breakage was small enough that - assuming either those commits or an upgrade to Weston 12/13 fixes it - we can just ask people to upgrade to a fixed Weston. > > Presuming this is not related to any TI specific code, I guess it's a > > regression in the sense that at some point Weston added the support to use > > planes for composition, so previously with only a single plane per display > > there was no issue. That point was 12 years ago, so not that novel. ;) Cheers, Daniel
Re: [PATCH 1/2] drm/tidss: Fix initial plane zpos values
Hi, On Tue, 13 Feb 2024 at 10:18, Marius Vlad wrote: > On Tue, Feb 13, 2024 at 11:57:59AM +0200, Tomi Valkeinen wrote: > > I haven't. I'm quite unfamiliar with Weston, and Randolph from TI (cc'd) has > > been working on the Weston side of things. I also don't know if there's > > something TI specific here, as the use case is with non-mainline GPU drivers > > and non-mainline Mesa. I should have been a bit clearer in the patch > > description, as I didn't mean that upstream Weston has a bug (maybe it has, > > maybe it has not). Don't worry about it. We've had bugs in the past and I'm sure we'll have more. :) Either way, it's definitely better to have the kernel expose sensible behaviour rather than weird workarounds, unless they've been around for so long that they're basically baked into ABI. > > The issue seen is that when Weston decides to use DRM planes for > > composition, the plane zpositions are not configured correctly (or at all?). > > Afaics, this leads to e.g. weston showing a window with a DRM "overlay" > > plane that is behind the "primary" root plane, so the window is not visible. > > And as Weston thinks that the area supposedly covered by the overlay plane > > does not need to be rendered on the root plane, there are also artifacts on > > that area. > > > > Also, the Weston I used is a bit older one (10.0.1), as I needed to go back > > in my buildroot versions to get all that non-mainline GPU stuff compiled and > > working. A more recent Weston may behave differently. > > Right after Weston 10, we had a few minor changes related to the > zpos-sorting list of planes and how we parse the plan list without having > a temporary zpos ordered list to pick planes from. > > And there's another fix for missing out to set out the zpos for scanout > to the minimum available - which seems like a good candidate to explain > what happens in the issue described above. So if trying Weston again, > please try with at least Weston 12, which should have those changes > in. Specifically, you probably want commits 4cde507be6a1 and 58dde0e0c000. I think the window of breakage was small enough that - assuming either those commits or an upgrade to Weston 12/13 fixes it - we can just ask people to upgrade to a fixed Weston. > > Presuming this is not related to any TI specific code, I guess it's a > > regression in the sense that at some point Weston added the support to use > > planes for composition, so previously with only a single plane per display > > there was no issue. That point was 12 years ago, so not that novel. ;) Cheers, Daniel
Re: [PATCH] drm/vmwgfx: Filter modes which exceed 3/4 of graphics memory.
On Wed, 31 Jan 2024 at 02:31, Zack Rusin wrote: > On Tue, Jan 30, 2024 at 6:50 PM Daniel Stone wrote: > > The entire model we have is that basis timing flows backwards. The > > 'hardware' gives us a deadline, KMS angles to meet that with a small > > margin, the compositor angles to meet that with a margin again, and it > > lines up client repaints to hit that window too. Everything works on > > that model, so it's not super surprising that using svga is - to quote > > one of Weston's DRM-backend people who uses ESXi - 'a juddery mess'. > > That's very hurtful. Or it would be but of course you didn't believe > them because they're working on Weston so clearly don't make good > choices in general, right? The presentation on esxi is just as smooth > as it is by default on Ubuntu on new hardware... Yeah sorry, that wasn't a 'VMware is bad' dig, that was a 'oh that explains so much if you're deliberately doing the other thing' realisation. I'm not suggesting anyone else use my life choices as a template :) > > Given that the entire ecosystem is based on this model, I don't think > > there's an easy way out where svga just does something wildly > > different. The best way to fix it is to probably work on predictable > > quantisation with updates: pick 5/12/47/60Hz to quantise to based on > > your current throughput, with something similar to hotplug/LINK_STATUS > > and faked EDID to let userspace know when the period changes. If you > > have variability within the cycle, e.g. dropped frames, then just suck > > it up and keep the illusion alive to userspace that it's presenting to > > a fixed period, and if/when you calculate there's a better > > quantisation then let userspace know what it is so it can adjust. > > > > But there's really no future in just doing random presentation rates, > > because that's not the API anyone has written for. > > See, my hope was that with vrr we could layer the weird remote > presentation semantics of virtualized guest on top of the same > infrastructure that would be used on real hardware. If you're saying > that it's not the way userspace will work, then yea, that doesn't > help. My issue, that's general for para-virtualized drivers, is that > any behavior that differs from hw drivers means that it's going to > break at some point, we see that even for basic things like the > update-layout hotplug events that have been largely standardized for > many years. I'm assuming that refresh-rate-changed will result in the > same regressions, but fwiw if I can implement FRR correctly and punt > any issues that arise due to changes in the FRR as issues in userspace > then that does make my life a lot easier, so I'm not going to object > to that. Yeah, I think that's the best way forward ... modelling the full pipeline in all its glory starts to look way less like KMS and way more like something like GStreamer. Trying to encapsulate all that reasonably in the kernel would've required - at the very least - a KMS-side queue with target display times in order to be at all useful, and that seemed like way too much complexity when the majority of hardware could be handled with 'I'll fire an ioctl at you and you update at the next 16ms boundary'. I'd be super happy to review any uAPI extensions which added feedback to userspace to let it know that the optimal presentation cadence had changed. Cheers, Daniel
Re: [PATCH] drm/vmwgfx: Filter modes which exceed 3/4 of graphics memory.
Hi, On Tue, 30 Jan 2024 at 18:39, Zack Rusin wrote: > In general, yes. Of course it's a little more convoluted because we'll > act like OpenGL runtime here (i.e. glXSwapBuffers), i.e. our driver > will fake page-flips because the only memory we'll have is a single > buffer as the actual page-flipping happens in the presentation code on > the host. So the guest is not aware of the actual presentation (it's > also why we don't have any sort of vblank signaling in vmwgfx, the > concept just doesn't exist for us). i.e. on para-virtualized drivers > the actual page-flips will be property of the presentation code that's > outside of the guest. It's definitely one those things that I wanted > to have a good solution for in a while, in particular to have a better > story behind vblank handling, but it's difficult because > "presentation" on vm's is in general difficult to define - it might be > some vnc connected host on the other continent. Having said that > that's basically a wonky VRR display so we should be able to handle > our presentation as VRR and give more control of updates to the guest, > but we haven't done it yet. Please don't. Photon time is _a_ useful metric, but only backwards-informational. It's nice to give userspace a good forward estimate of when pixels will hit retinas, but as it's not fully reliable, the main part is being able to let it know when it did happen so it can adjust. Given that it's not reliable, we can't use it as a basis for preparing submissions though, so we don't, even on bare-metal drivers. As you've noted though, it really falls apart on non-bare-metal cases, especially where latency vastly exceeds throughput, or when either is hugely variable. So we don't ever use it as a basis. VRR is worse though. The FRR model is 'you can display new content every $period, and here's your basis so you can calibrate phase'. The VRR model is 'you can display new content so rapidly it's not worth trying to quantise, just fire it as rapidly as possible'. That's a world away from 'e ... might be 16ms, might be 500? dunno really'. The entire model we have is that basis timing flows backwards. The 'hardware' gives us a deadline, KMS angles to meet that with a small margin, the compositor angles to meet that with a margin again, and it lines up client repaints to hit that window too. Everything works on that model, so it's not super surprising that using svga is - to quote one of Weston's DRM-backend people who uses ESXi - 'a juddery mess'. Given that the entire ecosystem is based on this model, I don't think there's an easy way out where svga just does something wildly different. The best way to fix it is to probably work on predictable quantisation with updates: pick 5/12/47/60Hz to quantise to based on your current throughput, with something similar to hotplug/LINK_STATUS and faked EDID to let userspace know when the period changes. If you have variability within the cycle, e.g. dropped frames, then just suck it up and keep the illusion alive to userspace that it's presenting to a fixed period, and if/when you calculate there's a better quantisation then let userspace know what it is so it can adjust. But there's really no future in just doing random presentation rates, because that's not the API anyone has written for. Cheers, Daniel
Re: [PATCH] drm/etnaviv: fix DMA direction handling for cached read/write buffers
Hi Lucas, On Fri, 26 Jan 2024 at 17:00, Lucas Stach wrote: > The dma sync operation needs to be done with DMA_BIDIRECTIONAL when > the BO is prepared for both read and write operations. With the > current inverted if ladder it would only be synced for DMA_FROM_DEVICE. > > [...] > > static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op) > { > - if (op & ETNA_PREP_READ) > + if (op & (ETNA_PREP_READ | ETNA_PREP_WRITE)) > + return DMA_BIDIRECTIONAL; This test will always be true for _either_ read or write. Cheers, Daniel
Re: Future direction of the Mesa Vulkan runtime (or "should we build a new gallium?")
On Fri, 26 Jan 2024 at 00:22, Faith Ekstrand wrote: > On Thu, Jan 25, 2024 at 5:06 PM Gert Wollny wrote: >> I think with Venus we are more interested in using utility libraries on >> an as-needed basis. Here, most of the time the Vulkan commands are just >> serialized according to the Venus protocol and this is then passed to >> the host because usually it wouldn't make sense to let the guest >> translate the Vulkan commands to something different (e.g. something >> that is commonly used in a runtime), only to then re-encode this in the >> Venus driver to satisfy the host Vulkan driver - just think Spir-V: >> why would we want to have NIR only to then re-encode it to Spir-V? > > I think Venus is an entirely different class of driver. It's not even really > a driver. It's more of a Vulkan layer that has a VM boundary in the middle. > It's attempting to be as thin of a Vulkan -> Vulkan pass-through as possible. > As such, it doesn't use most of the shared stuff anyway. It uses the dispatch > framework and that's really about it. As long as that code stays in-tree > roughly as-is, I think Venus will be fine. The eternal response: you forgot WSI! Cheers, Daniel
[ANNOUNCE] wayland-protocols 1.33
Hi, wayland-protocols 1.33 has been released. This marks the linux-dmabuf protocol - now at v5 - stable, introduces the ext-transient-seat protocol, and has a number of minor fixes and clarifications for other protocols. Thanks to all who have contributed. Andri Yngvason (1): Add the transient seat protocol Daniel Stone (1): build: Bump version to 1.33 Jonas Ådahl (1): xdg-shell: Clarify what a toplevel by default includes Lleyton Gray (1): staging/drm-lease: fix typo in description MaxVerevkin (1): linux-dmabuf: sync changes from unstable to stable Sebastian Wick (3): security-context-v1: Document out of band metadata for flatpak security-context-v1: Document what can be done with the open sockets security-context-v1: Make sandbox engine names use reverse-DNS Simon Ser (12): linux-dmabuf: add note about implicit sync members: remove EFL/Enlightenment build: simplify dict loops build: add version for stable protocols linux-dmabuf: mark as stable xdg-decoration: fix configure event summary xdg-decoration: remove ambiguous wording in configure event presentation-time: stop referring to Linux/glibc readme: version should be included in stable protocol filenames linux-dmabuf: require all planes to use the same modifier readme: make it clear that we are a standards body ci: upgrade ci-templates and Debian Vaxry (2): README: fix typos governance: fix typos git tag: 1.33 https://gitlab.freedesktop.org/wayland/wayland-protocols/-/releases/1.33/downloads/wayland-protocols-1.33.tar.xz SHA256: 94f0c50b090d6e61a03f62048467b19abbe851be4e11ae7b36f65f8b98c3963a wayland-protocols-1.33.tar.xz SHA512: 4584f6ac86367655f9db5d0c0ed0681efa31e73f984e4b620fbe5317df21790927f4f5317ecbbc194ac31eaf88caebc431bcc52c23d9dc0098c71de3cb4a9fef wayland-protocols-1.33.tar.xz PGP: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/releases/1.33/downloads/wayland-protocols-1.33.tar.xz.sig
Re: Right mailing list for mutter/gnome-remote-desktop question?
Hi Matt, On Wed, 17 Jan 2024 at 17:08, Matt Hoosier wrote: > Does anybody know whether there’s a dedicated mailing list suitable for > asking questions about the hardware acceleration in the remote desktop > use-case for those two? > > I did a quick look through both repos’ README and CONTRIBUTING files, but > didn’t find anything. https://discourse.gnome.org is probably your best bet there. Cheers, Daniel
Re: [PATCH v2 2/7] drm/ci: mediatek: Rename exisitng job
Hi Vignesh, On Tue, 16 Jan 2024 at 09:55, Vignesh Raman wrote: > Rename the name of xfail files for mediatek (mt8173 and mt8183), > to include information about the tested driver and update xfails > accordingly. Since the correct driver name is passed from the job to > test gpu and display driver, remove the check to set IGT_FORCE_DRIVER > based on driver name. I think something is still wrong here, because I can see that later xfails updates are setting expectations on kms tests when we're supposed to be using the panfrost driver. I can't tell which branch was used to run this, but you definitely want to look closely at the job logs and results to find out what's going on here. Cheers, Daniel
Re: [PATCH v1 0/8] drm/ci: Add support for GPU and display testing
Hi Vignesh, On Wed, 10 Jan 2024 at 10:47, Vignesh Raman wrote: > On 09/01/24 19:08, Daniel Stone wrote: > > A better sequencing would be something like: > >1. add ANX7625 config > >2. refactor _existing_ MTK display jobs to use YAML includes, change > > the existing job name, and rename the existing xfail set, remove > > IGT_FORCE_DRIVER from the script since it's now set by the job > >3. add MTK Panfrost+PVR GPU jobs with new xfails, add xfail entry to > > MAINTAINERS > >4+5: same as 2+3 but for Amlogic > >6+7: same as 2+3 but for Rockchip > > > > Then the separate rename/update xfail commits just disappear, as does > > the removal of IGT_FORCE_DRIVER, because it's just done incrementally > > as part of the commits which change the related functionality. It's > > extremely important that every change can work standalone, instead of > > introducing intermediate breakage which is only fixed in later commits > > in the series. > > Thank you for reviewing the patches. I agree, will follow this sequence > and send a v2 version. Alongside Rob's patch to add msm-specific tests to the runlist, we'd need to add the Panfrost-specific tests. Whilst we're here, we might as well add the vc4/v3d/etnaviv/lima tests so they can use it in future. Panfrost should also skip kms_.* tests - since it's not a KMS driver, it can't run the KMS tests, so there's no point in trying. Cheers, Daniel
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
Hi, On Wed, 10 Jan 2024 at 10:44, Daniel Vetter wrote: > On Tue, Jan 09, 2024 at 11:12:11PM +, Andri Yngvason wrote: > > ţri., 9. jan. 2024 kl. 22:32 skrifađi Daniel Stone : > > > How does userspace determine what's happened without polling? Will it > > > only change after an `ALLOW_MODESET` commit, and be guaranteed to be > > > updated after the commit has completed and the event being sent? > > > Should it send a HOTPLUG event? Other? > > > > > > > Userspace does not determine what's happened without polling. The purpose > > of this property is not for programmatic verification that the preferred > > property was applied. It is my understanding that it's mostly intended for > > debugging purposes. It should only change as a consequence of modesetting, > > although I didn't actually look into what happens if you set the "preferred > > color format" outside of a modeset. > > This feels a bit irky to me, since we don't have any synchronization and > it kinda breaks how userspace gets to know about stuff. > > For context the current immutable properties are all stuff that's derived > from the sink (like edid, or things like that). Userspace is guaranteed to > get a hotplug event (minus driver bugs as usual) if any of these change, > and we've added infrastructure so that the hotplug event even contains the > specific property so that userspace can avoid re-read (which can cause > some costly re-probing) them all. Right. > [...] > > This thing here works entirely differently, and I think we need somewhat > new semantics for this: > > - I agree it should be read-only for userspace, so immutable sounds right. > > - But I also agree with Daniel Stone that this should be tied more > directly to the modeset state. > > So I think the better approach would be to put the output type into > drm_connector_state, require that drivers compute it in their > ->atomic_check code (which in the future would allow us to report it out > for TEST_ONLY commits too), and so guarantee that the value is updated > right after the kms ioctl returns (and not somewhen later for non-blocking > commits). That's exactly the point. Whether userspace gets an explicit notification or it has to 'know' when to read isn't much of an issue - just as long as it's well defined. I think the suggestion of 'do it in atomic_check and then it's guaranteed to be readable when the commit completes' is a good one. I do still have some reservations - for instance, why do we have the fallback to auto when userspace has explicitly requested a certain type? - but they may have been covered previously. Cheers, Daniel
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
Hi, On Wed, 10 Jan 2024 at 10:44, Daniel Vetter wrote: > On Tue, Jan 09, 2024 at 11:12:11PM +, Andri Yngvason wrote: > > ţri., 9. jan. 2024 kl. 22:32 skrifađi Daniel Stone : > > > How does userspace determine what's happened without polling? Will it > > > only change after an `ALLOW_MODESET` commit, and be guaranteed to be > > > updated after the commit has completed and the event being sent? > > > Should it send a HOTPLUG event? Other? > > > > > > > Userspace does not determine what's happened without polling. The purpose > > of this property is not for programmatic verification that the preferred > > property was applied. It is my understanding that it's mostly intended for > > debugging purposes. It should only change as a consequence of modesetting, > > although I didn't actually look into what happens if you set the "preferred > > color format" outside of a modeset. > > This feels a bit irky to me, since we don't have any synchronization and > it kinda breaks how userspace gets to know about stuff. > > For context the current immutable properties are all stuff that's derived > from the sink (like edid, or things like that). Userspace is guaranteed to > get a hotplug event (minus driver bugs as usual) if any of these change, > and we've added infrastructure so that the hotplug event even contains the > specific property so that userspace can avoid re-read (which can cause > some costly re-probing) them all. Right. > [...] > > This thing here works entirely differently, and I think we need somewhat > new semantics for this: > > - I agree it should be read-only for userspace, so immutable sounds right. > > - But I also agree with Daniel Stone that this should be tied more > directly to the modeset state. > > So I think the better approach would be to put the output type into > drm_connector_state, require that drivers compute it in their > ->atomic_check code (which in the future would allow us to report it out > for TEST_ONLY commits too), and so guarantee that the value is updated > right after the kms ioctl returns (and not somewhen later for non-blocking > commits). That's exactly the point. Whether userspace gets an explicit notification or it has to 'know' when to read isn't much of an issue - just as long as it's well defined. I think the suggestion of 'do it in atomic_check and then it's guaranteed to be readable when the commit completes' is a good one. I do still have some reservations - for instance, why do we have the fallback to auto when userspace has explicitly requested a certain type? - but they may have been covered previously. Cheers, Daniel
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
Hi, On Wed, 10 Jan 2024 at 10:44, Daniel Vetter wrote: > On Tue, Jan 09, 2024 at 11:12:11PM +, Andri Yngvason wrote: > > ţri., 9. jan. 2024 kl. 22:32 skrifađi Daniel Stone : > > > How does userspace determine what's happened without polling? Will it > > > only change after an `ALLOW_MODESET` commit, and be guaranteed to be > > > updated after the commit has completed and the event being sent? > > > Should it send a HOTPLUG event? Other? > > > > > > > Userspace does not determine what's happened without polling. The purpose > > of this property is not for programmatic verification that the preferred > > property was applied. It is my understanding that it's mostly intended for > > debugging purposes. It should only change as a consequence of modesetting, > > although I didn't actually look into what happens if you set the "preferred > > color format" outside of a modeset. > > This feels a bit irky to me, since we don't have any synchronization and > it kinda breaks how userspace gets to know about stuff. > > For context the current immutable properties are all stuff that's derived > from the sink (like edid, or things like that). Userspace is guaranteed to > get a hotplug event (minus driver bugs as usual) if any of these change, > and we've added infrastructure so that the hotplug event even contains the > specific property so that userspace can avoid re-read (which can cause > some costly re-probing) them all. Right. > [...] > > This thing here works entirely differently, and I think we need somewhat > new semantics for this: > > - I agree it should be read-only for userspace, so immutable sounds right. > > - But I also agree with Daniel Stone that this should be tied more > directly to the modeset state. > > So I think the better approach would be to put the output type into > drm_connector_state, require that drivers compute it in their > ->atomic_check code (which in the future would allow us to report it out > for TEST_ONLY commits too), and so guarantee that the value is updated > right after the kms ioctl returns (and not somewhen later for non-blocking > commits). That's exactly the point. Whether userspace gets an explicit notification or it has to 'know' when to read isn't much of an issue - just as long as it's well defined. I think the suggestion of 'do it in atomic_check and then it's guaranteed to be readable when the commit completes' is a good one. I do still have some reservations - for instance, why do we have the fallback to auto when userspace has explicitly requested a certain type? - but they may have been covered previously. Cheers, Daniel
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
Hi, On Tue, 9 Jan 2024 at 18:12, Andri Yngvason wrote: > + * active color format: > + * This read-only property tells userspace the color format actually used > + * by the hardware display engine "on the cable" on a connector. The > chosen > + * value depends on hardware capabilities, both display engine and > + * connected monitor. Drivers shall use > + * drm_connector_attach_active_color_format_property() to install this > + * property. Possible values are "not applicable", "rgb", "ycbcr444", > + * "ycbcr422", and "ycbcr420". How does userspace determine what's happened without polling? Will it only change after an `ALLOW_MODESET` commit, and be guaranteed to be updated after the commit has completed and the event being sent? Should it send a HOTPLUG event? Other? Cheers, Daniel
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
Hi, On Tue, 9 Jan 2024 at 18:12, Andri Yngvason wrote: > + * active color format: > + * This read-only property tells userspace the color format actually used > + * by the hardware display engine "on the cable" on a connector. The > chosen > + * value depends on hardware capabilities, both display engine and > + * connected monitor. Drivers shall use > + * drm_connector_attach_active_color_format_property() to install this > + * property. Possible values are "not applicable", "rgb", "ycbcr444", > + * "ycbcr422", and "ycbcr420". How does userspace determine what's happened without polling? Will it only change after an `ALLOW_MODESET` commit, and be guaranteed to be updated after the commit has completed and the event being sent? Should it send a HOTPLUG event? Other? Cheers, Daniel
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
Hi, On Tue, 9 Jan 2024 at 18:12, Andri Yngvason wrote: > + * active color format: > + * This read-only property tells userspace the color format actually used > + * by the hardware display engine "on the cable" on a connector. The > chosen > + * value depends on hardware capabilities, both display engine and > + * connected monitor. Drivers shall use > + * drm_connector_attach_active_color_format_property() to install this > + * property. Possible values are "not applicable", "rgb", "ycbcr444", > + * "ycbcr422", and "ycbcr420". How does userspace determine what's happened without polling? Will it only change after an `ALLOW_MODESET` commit, and be guaranteed to be updated after the commit has completed and the event being sent? Should it send a HOTPLUG event? Other? Cheers, Daniel
Re: [PATCH v1 0/8] drm/ci: Add support for GPU and display testing
Hi, On Wed, 20 Dec 2023 at 12:11, Vignesh Raman wrote: > Some ARM SOCs have a separate display controller and GPU, each with > different drivers. For mediatek mt8173, the GPU driver is powervr, > and the display driver is mediatek. In the case of mediatek mt8183, > the GPU driver is panfrost, and the display driver is mediatek. > With rockchip rk3288/rk3399, the GPU driver is panfrost, while the > display driver is rockchip. For amlogic meson, the GPU driver is > panfrost, and the display driver is meson. > > IGT tests run various tests with different xfails and can test both > GPU devices and KMS/display devices. Currently, in drm-ci for MediaTek, > Rockchip, and Amlogic Meson platforms, only the GPU driver is tested. > This leads to incomplete coverage since the display is never tested on > these platforms. This commit series adds support in drm-ci to run tests > for both GPU and display drivers for MediaTek, Rockchip, and Amlogic > Meson platforms. Thanks a lot for these. The patches need to be squashed to be bisectable though. For example, patch #2 changes the MTK job names and adds more jobs, but the corresponding xfail updates only come in #7 and #8. This means we have a span of a few patches where we don't have useful test results. A better sequencing would be something like: 1. add ANX7625 config 2. refactor _existing_ MTK display jobs to use YAML includes, change the existing job name, and rename the existing xfail set, remove IGT_FORCE_DRIVER from the script since it's now set by the job 3. add MTK Panfrost+PVR GPU jobs with new xfails, add xfail entry to MAINTAINERS 4+5: same as 2+3 but for Amlogic 6+7: same as 2+3 but for Rockchip Then the separate rename/update xfail commits just disappear, as does the removal of IGT_FORCE_DRIVER, because it's just done incrementally as part of the commits which change the related functionality. It's extremely important that every change can work standalone, instead of introducing intermediate breakage which is only fixed in later commits in the series. Cheers, Daniel
Re: [PATCH v6 06/10] drm: ci: mediatek: Set IGT_FORCE_DRIVER for mt8173
Hi Vignesh, On Wed, 29 Nov 2023 at 12:19, Vignesh Raman wrote: > Expected driver for mt8173 is "mediatek" and for mt8183 > it is "panfrost". Set IGT_FORCE_DRIVER to 'mediatek' as > the expected driver for mt8173. Actually, for mt8183 it's both. And for mt8173 it will probably be mediatek+pvr pretty soon. Each of these SoCs (like most Arm devices) have a separate display controller and GPU, with different drivers for each. They'll run different tests with different xfails. So we should figure out a way to support igt running for both devices on the one system. Cheers, Daniel
Re: [PATCH v2 2/2] drm: introduce CLOSEFB IOCTL
On Fri, 20 Oct 2023 at 11:19, Simon Ser wrote: > This new IOCTL allows callers to close a framebuffer without > disabling planes or CRTCs. This takes inspiration from Rob Clark's > unref_fb IOCTL [1] and DRM_MODE_FB_PERSIST [2]. > > User-space patch for wlroots available at [3]. IGT test available > at [4]. Series is: Reviewed-by: Daniel Stone
Re: [PATCH v5 9/9] drm: ci: Update xfails
Hi Vignesh, On Thu, 19 Oct 2023 at 09:07, Vignesh Raman wrote: > +# Some tests crashes with malloc error and IGT tests floods > +# the CI log with error messages and we end up with a warning message > +# Job's log exceeded limit of 4194304 bytes. > +# Job execution will continue but no more output will be collected. This is just from GitLab warning that we have a huge log, so not related to the actual fails here. > +# Below is the error log: > +# malloc(): corrupted top size > +# Received signal SIGABRT. > +# Stack trace: > +# #0 [fatal_sig_handler+0x17b] > +# #1 [__sigaction+0x40] > +# #2 [pthread_key_delete+0x14c] > +# #3 [gsignal+0x12] > +# #4 [abort+0xd3] > +# #5 [__fsetlocking+0x290] > +# #6 [timer_settime+0x37a] > +# #7 [__default_morecore+0x1f1b] > +# #8 [__libc_calloc+0x161] > +# #9 [drmModeGetPlaneResources+0x44] > +# #10 [igt_display_require+0x194] > +# #11 [__igt_uniquereal_main1356+0x93c] > +# #12 [main+0x3f] > +# #13 [__libc_init_first+0x8a] > +# #14 [__libc_start_main+0x85] > +# #15 [_start+0x21] > +# malloc(): corrupted top size By the time we get this error, it indicates that there was previously memory corruption, but it is only being noticed at a later point. The skip lists here are way too big - stuff like drm_read is common testing not affected by virtio at all - so we really need to isolate the test which is actually breaking things. The way to do this would be to use valgrind to detect where the memory corruption is. VirtIO can be run locally so this is something you can do on your machine. Cheers, Daniel ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 9/9] drm: ci: Update xfails
Hi Vignesh, On Thu, 19 Oct 2023 at 09:07, Vignesh Raman wrote: > +# Some tests crashes with malloc error and IGT tests floods > +# the CI log with error messages and we end up with a warning message > +# Job's log exceeded limit of 4194304 bytes. > +# Job execution will continue but no more output will be collected. This is just from GitLab warning that we have a huge log, so not related to the actual fails here. > +# Below is the error log: > +# malloc(): corrupted top size > +# Received signal SIGABRT. > +# Stack trace: > +# #0 [fatal_sig_handler+0x17b] > +# #1 [__sigaction+0x40] > +# #2 [pthread_key_delete+0x14c] > +# #3 [gsignal+0x12] > +# #4 [abort+0xd3] > +# #5 [__fsetlocking+0x290] > +# #6 [timer_settime+0x37a] > +# #7 [__default_morecore+0x1f1b] > +# #8 [__libc_calloc+0x161] > +# #9 [drmModeGetPlaneResources+0x44] > +# #10 [igt_display_require+0x194] > +# #11 [__igt_uniquereal_main1356+0x93c] > +# #12 [main+0x3f] > +# #13 [__libc_init_first+0x8a] > +# #14 [__libc_start_main+0x85] > +# #15 [_start+0x21] > +# malloc(): corrupted top size By the time we get this error, it indicates that there was previously memory corruption, but it is only being noticed at a later point. The skip lists here are way too big - stuff like drm_read is common testing not affected by virtio at all - so we really need to isolate the test which is actually breaking things. The way to do this would be to use valgrind to detect where the memory corruption is. VirtIO can be run locally so this is something you can do on your machine. Cheers, Daniel
Re: Sub 16ms render but missing swap
Hi Joe, On Wed, 18 Oct 2023 at 02:00, Joe M wrote: > A few questions: > 1. What other avenues of investigation should I pursue for the swap delay? > As in, why when I take 12 ms to render do I not see about 4ms for the swap > call to return? My display is running in at 60hz. Further to Emmanuel's point about GPU rendering being async (you can validate by calling glFinish before eglSwapBuffers, which will wait for everything to complete) - which hardware platform are you using here, and which software stack as well? As in, do your Weston + drivers + etc come from upstream projects or are they provided by a vendor? > 2. Has EGL been optimized to use the available wayland callbacks and > maximize available client drawing time? Yes, very much. > 3. Does EGL leverage "weston_direct_display_v1" when available? What's > required to take advantage of it in the app code? (ie. run fullscreen?) No need. We bypass composition as much as we possibly can. You can try using weston-simple-egl with the flag to use direct-display if you want to satisfy yourself, but it's in no way required to bypass GPU composition and use the display controller to scan out. Cheers, Daniel
Re: [PATCH v7 04/23] dt-bindings: display: mediatek: padding: Add MT8188
Hi Shawn, On Mon, 16 Oct 2023 at 06:23, Shawn Sung (宋孝謙) wrote: > On Fri, 2023-10-13 at 17:26 +0100, Daniel Stone wrote: > > If I understand the driver correctly, padding is automatically > > applied > > to compensate for unaligned dimensions. The first/last rows/columns > > of > > the overlay area will be filled with a specified colour (black?) to > > preserve the area. This is unfortunately not OK to do transparently. > > Userspace must be aware of this policy decision and specifically > > request it. If not, the atomic request check should fail and tell > > userspace that the requested configuration is not possible to > > achieve. > > Yes, Padding works as you described, users can assign background colors > for the filled area in 10bit RGB format, however, the rows and columns > that are filled by Padding will be cropped by the hardware components > after it to avoid situations as you mentioned, so users should not > notice any difference. Thanks for the explanation, I hadn't realised that the added padding later gets cropped. Cheers, Daniel
Re: [PATCH] drm/ci: Default to UART for logging
On Fri, 6 Oct 2023 at 18:32, Rob Clark wrote: > ssh logging is the default for mesa, as it is generally more reliable. > But if there are kernel issues, especially at boot, UART logging is > infinitely more useful. Hmm, we should still be capturing the UART boot logs regardless. Those go into a collapsed 'LAVA boot' section but they don't just disappear ... ?
Re: [PATCH v7 04/23] dt-bindings: display: mediatek: padding: Add MT8188
Hi Shawn, On Fri, 6 Oct 2023 at 08:38, Hsiao Chien Sung wrote: > + Padding provides ability to add pixels to width and height of a layer with > + specified colors. Due to hardware design, Mixer in VDOSYS1 requires > + width of a layer to be 2-pixel-align, or 4-pixel-align when ETHDR is > enabled, > + we need Padding to deal with odd width. > + Please notice that even if the Padding is in bypass mode, settings in > + register must be cleared to 0, or undefined behaviors could happen. If I understand the driver correctly, padding is automatically applied to compensate for unaligned dimensions. The first/last rows/columns of the overlay area will be filled with a specified colour (black?) to preserve the area. This is unfortunately not OK to do transparently. Userspace must be aware of this policy decision and specifically request it. If not, the atomic request check should fail and tell userspace that the requested configuration is not possible to achieve. Cheers, Daniel
Re: drm/vkms: deadlock between dev->event_lock and timer
On Mon, 18 Sept 2023 at 23:02, Helen Koike wrote: > On 14/09/2023 05:12, Daniel Vetter wrote: > > Also yes how that landed without anyone running lockdep is ... not good. I > > guess we need a lockdep enabled drm ci target that runs vkms tests asap > > :-) > > btw, I just executed a draft version of vkms targed on the ci, we do get > the warning: > > https://gitlab.freedesktop.org/helen.fornazier/linux/-/jobs/49156305#L623 > > I'm just not sure if tests would fail (since it is a warning) and it has > a chance to be ignored if people don't look at the logs (unless if igt > already handles that). Hmm, dmesg-warn is already a separate igt test status. I guess it just needs to be handled explicitly. > I still need to do some adjustments (it seems the tests is hanging > somewhere and we got a timeout) but we should have vkms in drm ci soon. Might be due to the locking explosion? The kernels should probably have soft-lockup detection enabled as well as lockdep. Cheers, Daniel
Re: [PATCH 00/10] Add mediate-drm secure flow for SVP
Hi Jason, CK, On Tue, 19 Sept 2023 at 04:04, Jason-JH.Lin wrote: > The patch series provides drm driver support for enabling secure video > path (SVP) playback on MediaiTek hardware in the Linux kernel. > > [...] > > Memory Usage in SVP: > The overall flow of SVP starts with encrypted video coming in from an > outside source into the REE. The REE will then allocate a 'secure > buffer' and send the corresponding 'secure handle' along with the > encrypted, compressed video data to the TEE. The TEE will then decrypt > the video and store the result in the 'secure buffer'. The REE will > then allocate a 'secure surface'. The REE will pass the 'secure > handles' for both the 'secure buffer' and 'secure surface' into the > TEE for video decoding. The video decoder HW will then decode the > contents of the 'secure buffer' and place the result in the 'secure > surface'. The REE will then attach the 'secure surface' to the overlay > plane for rendering of the video. > > Everything relating to ensuring security of the actual contents of the > 'secure buffer' and 'secure surface' is out of scope for the REE and > is the responsibility of the TEE. > > DRM driver handles allocation of gem objects that are backed by a 'secure > surface' and for displaying a 'secure surface' on the overlay plane. > This introduces a new flag for object creation called > DRM_MTK_GEM_CREATE_ENCRYPTED which indicates it should be a 'secure > surface'. All changes here are in MediaTek specific code. To be honest, it seems strange that DRM is being used as the allocator for buffers which will mostly be used by codec hardware which is not mentioned here. I can understand that minigbm and gbm_gralloc make this easy to implement, but it's not really right to add this all to mtk-drm just to support some codec features. NXP posted a patchset a while ago to add secure-memory support to dma-heaps[0]. This would be much cleaner (e.g. avoiding strcmp on allocating name, avoiding mtk-drm being a render node when it can't render) I think, and also allow common secure-path semantics between different vendors. Having common secure-path semantics between different vendors would be very helpful, because the first question when we add new uAPI is 'where is the open-source userspace?'. If there is at least a common interface through e.g. dma-heaps, then we could have some standard cross-vendor userspace code which would work well with the standard interface. Cheers, Daniel [0]: https://lore.kernel.org/lkml/20220805135330.970-2-olivier.ma...@nxp.com/
Re: [PATCH v11] drm: Add initial ci/ subdirectory
Hey, On Thu, 14 Sept 2023 at 10:54, Maxime Ripard wrote: > On Tue, Sep 12, 2023 at 02:16:41PM +0100, Daniel Stone wrote: > > Hopefully less mangled formatting this time: turns out Thunderbird + > > plain text is utterly unreadable, so that's one less MUA that is > > actually usable to send email to kernel lists without getting shouted > > at. > > Sorry if it felt that way, it definitely wasn't my intention to shout at > you. Email is indeed kind of a pain to deal with, and I wanted to keep > the discussion going. My bad - I didn't mean you _at all_. I was thinking of other, much less pleasant, kernel maintainers, and the ongoing struggle of attempting to actually send well-formatted email to kernel lists in 2023. > > I don't quite see the same picture from your side though. For example, > > my reading of what you've said is that flaky tests are utterly > > unacceptable, as are partial runs, and we shouldn't pretend otherwise. > > With your concrete example (which is really helpful, so thanks), what > > happens to the MT8173 hdmi-inject test? Do we skip all MT8173 testing > > until it's perfect, or does MT8173 testing always fail because that > > test does? > > It's not clear to me why that test is even running in the first place? > There's been some confusion on my side here about what we're going to > test with this. You've mentioned Mesa and GPUs before, but that's a KMS > test so there must be more to it. > > Either way, it's a relevant test so I guess why not. It turns out that > the test is indeed flaky, I guess we could add it to the flaky tests > list. > > BUT > > I want to have every opportunity to fix whatever that failure is. Agreed so far! > So: > > - Is the test broken? If so, we should report it to IGT dev and remove > it from the test suite. > - If not, is that test failure have been reported to the driver author? > - If no answer/fix, we can add it to the flaky tests list, but do we > have some way to reproduce the test failure? > > The last part is especially critical. Looking at the list itself, I have > no idea what board, kernel version, configuration, or what the failure > rate was. Assuming I spend some time looking at the infra to find the > board and configuration, how many times do I have to run the tests to > expect to reproduce the failure (and thus consider it fixed if it > doesn't occur anymore). > > Like, with that board and test, if my first 100 runs of the test work > fine, is it reasonable for me to consider it fixed, or is it only > supposed to happen once every 1000 runs? > > So, ideally, having some (mandatory) metadata in the test lists with a > link to the bug report, the board (DT name?) it happened with, the > version and configuration it was first seen with, and an approximation > of the failure rate for every flaky test list. > > I understand that it's probably difficult to get that after the fact on > the tests that were already merged, but I'd really like to get that > enforced for every new test going forward. > > That should hopefully get us in a much better position to fix some of > those tests issues. And failing that, I can't see how that's > sustainable. OK yeah, and we're still agreed here. That is definitely the standard we should be aiming for. It is there for some - see drivers/gpu/drm/ci/xfails/rockchip-rk3288-skips.txt, but should be there for the rest, it's true. (The specific board/DT it was observed on can be easily retconned because we only run on one specific board type per driver, again to make things more predictable; we could go back and retrospectively add those in a header comment?) For flakes, it can be hard to pin them down, because, well, they're flaky. Usually when we add things in Mesa (sorry to keep coming back to Mesa - it's not to say that it's the objective best thing that everything should follow, only that it's the thing we have the most experience with that we know works well), we do a manual bisect and try to pin the blame on a specific merge request which looks like the most likely culprit. If nothing obvious jumps out, we just note when it was first observed and provide some sample job logs. But yeah, it should be more verbose. FWIW, the reason it wasn't done here - not to say that it shouldn't have been done better, but here we are - is that we just hammered a load of test runs, vacuumed up the results with a script, and that's what generated those files. Given the number of tests and devices, it was hard to narrow each down individually, but yeah, it is something which really wants further analysis and drilling into. It's a good to-do, and I agree it should be the standard going forward. > And Mesa does show what I'm talking about: > > $ find -name *-flakes.txt | xargs git di
Re: [PATCH v11] drm: Add initial ci/ subdirectory
Hi Maxime, Hopefully less mangled formatting this time: turns out Thunderbird + plain text is utterly unreadable, so that's one less MUA that is actually usable to send email to kernel lists without getting shouted at. On Mon, 11 Sept 2023 at 15:46, Maxime Ripard wrote: > On Mon, Sep 11, 2023 at 03:30:55PM +0200, Michel Dänzer wrote: > > > There's in 6.6-rc1 around 240 reported flaky tests. None of them have > > > any context. That new series hads a few dozens too, without any context > > > either. And there's no mention about that being a plan, or a patch > > > adding a new policy for all tests going forward. > > > > That does sound bad, would need to be raised in review. > > > > > Any concern I raised were met with a giant "it worked on Mesa" handwave > > > > Lessons learned from years of experience with big real-world CI > > systems like this are hardly "handwaving". > > Your (and others) experience certainly isn't. It is valuable, welcome, > and very much appreciated. > > However, my questions and concerns being ignored time and time again > about things like what is the process is going to be like, what is going > to be tested, who is going to be maintaining that test list, how that > interacts with stable, how we can possibly audit the flaky tests list, > etc. have felt like they were being handwaived away. Sorry it ended up coming across like that. It wasn't the intent. > I'm not saying that because I disagree, I still do on some, but that's > fine to some extent. However, most of these issues are not so much an > infrastructure issue, but a community issue. And I don't even expect a > perfect solution right now, unlike what you seem to think. But I do > expect some kind of plan instead of just ignoring that problem. > > Like, I had to ask the MT8173 question 3 times in order to get an > answer, and I'm still not sure what is going to be done to address that > particular issue. > > So, I'm sorry, but I certainly feel like it here. I don't quite see the same picture from your side though. For example, my reading of what you've said is that flaky tests are utterly unacceptable, as are partial runs, and we shouldn't pretend otherwise. With your concrete example (which is really helpful, so thanks), what happens to the MT8173 hdmi-inject test? Do we skip all MT8173 testing until it's perfect, or does MT8173 testing always fail because that test does? Both have their downsides. Not doing any testing has the obvious downside, and means that the driver can get worse until it gets perfect. Always marking the test as failed means that the test results are useless: if failure is expected, then red is good. I mean, say you're contributing a patch to fix some documentation or add a helper to common code which only v3d uses. The test results come back, and your branch is failing tests on MT8173, specifically the hdmi-inject@4k test. What then? Either as a senior contributor you 'know' that's the case, or as a casual contributor you get told 'oh yeah, don't worry about the test results, they always fail'. Both lead to the same outcome, which is that no-one pays any attention to the results, and they get worse. What we do agree on is that yes, those tests should absolutely be fixed, and not just swept under the rug. Part of this is having maintainers actually meaningfully own their test results. For example, I'm looking at the expectation lists for the Intel gen in my laptop, and I'm seeing a lot of breakage in blending tests, as well as dual-display fails which include the resolution of my external display. I'd expect the Intel driver maintainers to look at them, get them fixed, and gradually prune those xfails/flakes down towards zero. If the maintainers don't own it though, then it's not going to get fixed. And we are exactly where we are today: broken plane blending and 1440p on KBL, broken EDID injection on MT8173, and broken atomic commits on stoney. Without stronger action from the maintainers (e.g. throwing i915 out of the tree until it has 100% pass 100% of the time), adding testing isn't making the situation better or worse in and of itself. What it _is_ doing though, is giving really clear documentation of the status of each driver, and backing that up by verifying it. Only maintainers can actually fix the drivers (or the tests tbf). But doing the testing does let us be really clear to everyone what the actual state is, and that way people can make informed decisions too. And the only way we're going to drive the test rate down is by the subsystem maintainers enforcing it. Does that make sense on where I'm (and I think a lot of others are) coming from? To answer the other question about 'where are the logs?': some of them have the failure data in them, others don't. They all should going forward at least though. Cheers, Daniel
Re: [PATCH v11] drm: Add initial ci/ subdirectory
Hi, On 04/09/2023 09:54, Daniel Vetter wrote: On Wed, 30 Aug 2023 at 17:14, Helen Koike > wrote: >> >> On 30/08/2023 11:57, Maxime Ripard wrote: >>> >>> I agree that we need a baseline, but that baseline should be >>> defined by the tests own merits, not their outcome on a >>> particular platform. >>> >>> In other words, I want all drivers to follow that baseline, and >>> if they don't it's a bug we should fix, and we should be vocal >>> about it. We shouldn't ignore the test because it's broken. >>> >>> Going back to the example I used previously, >>> kms_hdmi_inject@inject-4k shouldn't fail on mt8173, ever. That's >>> a bug. Ignoring it and reporting that "all tests are good" isn't >>> ok. There's something wrong with that driver and we should fix >>> it. >>> >>> Or at the very least, explain in much details what is the >>> breakage, how we noticed it, why we can't fix it, and how to >>> reproduce it. >>> >>> Because in its current state, there's no chance we'll ever go >>> over that test list and remove some of them. Or even know if, if >>> we ever fix a bug somewhere, we should remove a flaky or failing >>> test. >>> >>> [...] >>> we need to have a clear view about which tests are not corresponding to it, so we can start fixing. First we need to be aware of the issues so we can start fixing them, otherwise we will stay in the "no tests no failures" ground :) >>> >>> I think we have somewhat contradicting goals. You want to make >>> regression testing, so whatever test used to work in the past >>> should keep working. That's fine, but it's different from >>> "expectations about what the DRM drivers are supposed to pass in >>> the IGT test suite" which is about validation, ie "all KMS >>> drivers must behave this way". >> >> [...] >> >> >> We could have some policy: if you want to enable a certain device >> in the CI, you need to make sure it passes all tests first to force >> people to go fix the issues, but maybe it would be a big barrier. >> >> I'm afraid that, if a test fail (and it is a clear bug), people >> would just say "work for most of the cases, this is not a priority >> to fix" and just start ignoring the CI, this is why I think >> regression tests is a good way to start with. > > I think eventually we need to get to both goals, but currently > driver and test quality just isn't remotely there. > > I think a good approach would be if CI work focuses on the pure sw > tests first, so kunit and running igt against vgem/vkms. And then we > could use that to polish a set of must-pass igt testcases, which > also drivers in general are supposed to pass. Plus ideally weed out > the bad igts that aren't reliable enough or have bad assumptions. > > For hardware I think it will take a very long time until we get to a > point where CI can work without a test result list, we're nowhere > close to that. But for virtual driver this really should be > achievable, albeit with a huge amount of effort required to get > there I think. Yeah, this is what our experience with Mesa (in particular) has taught us. Having 100% of the tests pass 100% of the time on 100% of the platforms is a great goal that everyone should aim for. But it will also never happen. Firstly, we're just not there yet today. Every single GPU-side DRM driver has userspace-triggerable faults which cause occasional errors in GL/Vulkan tests. Every single one. We deal with these in Mesa by retrying; if we didn't retry, across the breadth of hardware we test, I'd expect 99% of should-succeed merges to fail because of these intermittent bugs in the DRM drivers. We don't have the same figure for KMS - because we don't test it - but I'd be willing to bet no driver is 100% if you run tests often enough. Secondly, we will never be there. If we could pause for five years and sit down making all the current usecases for all the current hardware on the current kernel run perfectly, we'd probably get there. But we can't: there's new hardware, new userspace, and hundreds of new kernel trees. Even without the first two, what happens when the Arm SMMU maintainers (choosing a random target to pick on, sorry Robin) introduce subtle breakage which makes a lot of tests fail some of the time? Do we refuse to backmerge Linus into DRM until it's fixed, or do we disable all testing on Arm until it's fixed? When we've done that, what happens when we re-enable testing, and discover that a bunch of tests get broken because we haven't been testing? Thirdly, hardware is capricious. 'This board doesn't make it to u-boot' is a clear infrastructure error, but if you test at sufficient scale, cold solder or failing caps surface way more often than you might think. And you can't really pick those out by any other means than running at scale, dealing with non-binary results, and looking at the trends over time. (Again this is something we do in Mesa - we graph test failures per
Re: [RFC]: shmem fd for non-DMA buffer sharing cross drivers
Hi, On Fri, 25 Aug 2023 at 08:56, Hsia-Jun Li wrote: > On 8/25/23 15:40, Pekka Paalanen wrote: > > if userspace cannot access things like an image's HDR metadata, then it > > will be impossible for userspace to program KMS to have the correct > > color pipeline, or to send intended HDR metadata to a video sink. > > > > You cannot leave userspace out of HDR metadata handling, because quite > > probably the V4L2 buffer is not the only thing on screen. That means > > there must composition of multiple sources with different image > > properties and metadata, which means it is no longer obvious what HDR > > metadata should be sent to the video sink. > > > > Even if it is a TV-like application rather than a windowed desktop, you > > will still have other contents to composite: OSD (volume indicators, > > channels indicators, program guide, ...), sub-titles, channel logos, > > notifications... These components ideally should not change their > > appearance arbitrarily with the main program content and metadata > > changes. Either the metadata sent to the video sink is kept static and > > the main program adapted on the fly, or main program metadata is sent > > to the video sink and the additional content is adapted on the fly. > > > > There is only one set of HDR metadata and one composited image that can > > be sent to a video sink, so both must be chosen and produced correctly > > at the source side. This cannot be done automatically inside KMS kernel > > drivers. > > There may be some misunderstanding. > Let suppose this HDR data is in a vendor specific format. > Both upstream(decoder) and downstream(DRM) hardware devices are coming > from the same vendor. > Then we just need to delivery the reference to this metadata buffer from > the upstream to downstream, both of drivers know how to handle it. > > Despite the userspace, we just need to extend a wayland protocol that > making wayland compositor know how to receive the reference to the > metadata and set it to the DRM plane. > > If you want a common HDR formats for all HDR variants(HDR10+, DV), I am > not against it. But it won't make the userspace be able to fill the HDR > metadata even the HDR data comes from the bitstream(likes SEI). We must > consider the case of Secure Video Path(Digital Right), the bitstream is > not accessible from (REE) userspace nor linux kernel, the downstream > must take what the upstream feed. To summarise from IRC, so it's properly documented: the community will not accept this. The mechanism (shmem, dmabuf, copy_from_user, whatever) is _not_ the problem. The problem is the concept. There have been incredibly extensive discussions on this list about colour management and HDR, summarised in documentation in the DRM repository, as well as three talks at the last XDC. This design is the result of discussion between many community participants - including hardware vendors - who have all come up with a design which prioritises transparency and explicit operation. What you are suggesting is exactly the opposite of this. A design in which opaque magic blobs are passed around and the kernel does unknown things based on the contents of those blobs, contradicts this design. (This is different to compression, where even if the format is proprietary, the effect is well-understood - for the current compression mechanisms, it is a lossless transform.) The old Android Display Framework (ADF) was based around the same design with blobs of opaque driver-specific data, where generic code - either in the kernel or in userspace - could not understand the effect of these blobs. This design was rejected, and we made a clear choice to follow the DRM design principles instead. Upstream will not accept any design which hides magic away. 'GKI is hard', 'other vendors won't let us', etc, are not good enough reasons to change our mind on this fundamental principle. Cheers, Daniel
Re: [PATCH v2 5/8] drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()
Hi Geert, On Thu, 24 Aug 2023 at 16:09, Geert Uytterhoeven wrote: > struct drm_client_dev *client = buffer->client; > - struct drm_mode_fb_cmd fb_req = { }; > - const struct drm_format_info *info; > + struct drm_mode_fb_cmd2 fb_req = { }; > int ret; > > - info = drm_format_info(format); > - fb_req.bpp = drm_format_info_bpp(info, 0); > - fb_req.depth = info->depth; > fb_req.width = width; > fb_req.height = height; > - fb_req.handle = handle; > - fb_req.pitch = buffer->pitch; > + fb_req.pixel_format = format; > + fb_req.handles[0] = handle; > + fb_req.pitches[0] = buffer->pitch; > > - ret = drm_mode_addfb(client->dev, _req, client->file); > + ret = drm_mode_addfb2(client->dev, _req, client->file); > if (ret) > return ret; This should explicitly set the LINEAR modifier (and the modifier flag) if the driver supports modifiers. Cheers, Daniel
Re: [PATCH v2 13/15] drm/panthor: Allow driver compilation
Hi, On 11/08/2023 17:35, Robin Murphy wrote: On 2023-08-09 17:53, Boris Brezillon wrote: +obj-$(CONFIG_DRM_PANTHOR) += panthor.o FWIW I still think it would be nice to have a minor directory/Kconfig/Makefile reshuffle and a trivial bit of extra registration glue to build both drivers into a single module. It seems like it could be a perpetual source of confusion to end users where Mesa "panfrost" is the right option but kernel "panfrost" is the wrong one. Especially when pretty much every other GPU driver is also just one big top-level module to load for many different generations of hardware. Plus it would mean that if someone did want to have a go at deduplicating the resource-wrangling boilerplate for OPPs etc. in future, there's more chance of being able to do so meaningfully. It might be nice to point it out, but to be fair Intel and AMD both have two (or more) drivers, as does Broadcom/RPi. As does, err ... Mali. I can see the point, but otoh if someone's managed to build all the right regulator/clock/etc modules to get a working system, they'll probably manage to figure teh GPU side out? Cheers, Daniel
[PATCH] drm/rockchip: Don't spam logs in atomic check
Userspace should not be able to trigger DRM_ERROR messages to spam the logs; especially not through atomic commit parameters which are completely legitimate for userspace to attempt. Signed-off-by: Daniel Stone Fixes: 7707f7227f09 ("drm/rockchip: Add support for afbc") --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 86fd9f51c692..14320bc73e5b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -832,12 +832,12 @@ static int vop_plane_atomic_check(struct drm_plane *plane, * need align with 2 pixel. */ if (fb->format->is_yuv && ((new_plane_state->src.x1 >> 16) % 2)) { - DRM_ERROR("Invalid Source: Yuv format not support odd xpos\n"); + DRM_DEBUG_KMS("Invalid Source: Yuv format not support odd xpos\n"); return -EINVAL; } if (fb->format->is_yuv && new_plane_state->rotation & DRM_MODE_REFLECT_Y) { - DRM_ERROR("Invalid Source: Yuv format does not support this rotation\n"); + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support this rotation\n"); return -EINVAL; } @@ -845,7 +845,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane, struct vop *vop = to_vop(crtc); if (!vop->data->afbc) { - DRM_ERROR("vop does not support AFBC\n"); + DRM_DEBUG_KMS("vop does not support AFBC\n"); return -EINVAL; } @@ -854,15 +854,16 @@ static int vop_plane_atomic_check(struct drm_plane *plane, return ret; if (new_plane_state->src.x1 || new_plane_state->src.y1) { - DRM_ERROR("AFBC does not support offset display, xpos=%d, ypos=%d, offset=%d\n", - new_plane_state->src.x1, - new_plane_state->src.y1, fb->offsets[0]); + DRM_DEBUG_KMS("AFBC does not support offset display, " \ + "xpos=%d, ypos=%d, offset=%d\n", + new_plane_state->src.x1, new_plane_state->src.y1, + fb->offsets[0]); return -EINVAL; } if (new_plane_state->rotation && new_plane_state->rotation != DRM_MODE_ROTATE_0) { - DRM_ERROR("No rotation support in AFBC, rotation=%d\n", - new_plane_state->rotation); + DRM_DEBUG_KMS("No rotation support in AFBC, rotation=%d\n", + new_plane_state->rotation); return -EINVAL; } } -- 2.41.0
[PATCH v2 1/2] doc: dma-buf: Rewrite intro section a little
Make it a little bit more clear what's going on and fix some formatting. Signed-off-by: Daniel Stone --- Documentation/driver-api/dma-buf.rst | 24 1 file changed, 16 insertions(+), 8 deletions(-) v2: New. diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index f92a32d095d9..862dbc2759d0 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -5,14 +5,22 @@ The dma-buf subsystem provides the framework for sharing buffers for hardware (DMA) access across multiple device drivers and subsystems, and for synchronizing asynchronous hardware access. -This is used, for example, by drm "prime" multi-GPU support, but is of -course not limited to GPU use cases. - -The three main components of this are: (1) dma-buf, representing a -sg_table and exposed to userspace as a file descriptor to allow passing -between devices, (2) fence, which provides a mechanism to signal when -one device has finished access, and (3) reservation, which manages the -shared or exclusive fence(s) associated with the buffer. +As an example, it is used extensively by the DRM subsystem to exchange +buffers between processes, contexts, library APIs within the same +process, and also to exchange buffers with other subsystems such as +V4L2. + +This document describes the way in which kernel subsystems can use and +interact with the three main primitives offered by dma-buf: + + - dma-buf, representing a sg_table and exposed to userspace as a file + descriptor to allow passing between processes, subsystems, devices, + etc; + - dma-fence, providing a mechanism to signal when an asynchronous + hardware operation has completed; and + - dma-resv, which manages a set of dma-fences for a particular dma-buf + allowing implicit (kernel-ordered) synchronization of work to + preserve the illusion of coherent access Shared DMA Buffers -- -- 2.41.0
[PATCH v2 2/2] doc: uapi: Add document describing dma-buf semantics
Since there's a lot of confusion around this, document both the rules and the best practice around negotiating, allocating, importing, and using buffers when crossing context/process/device/subsystem boundaries. This ties up all of dma-buf, formats and modifiers, and their usage. Signed-off-by: Daniel Stone --- Documentation/driver-api/dma-buf.rst | 8 + Documentation/gpu/drm-uapi.rst| 7 + .../userspace-api/dma-buf-alloc-exchange.rst | 384 ++ Documentation/userspace-api/index.rst | 1 + 4 files changed, 400 insertions(+) create mode 100644 Documentation/userspace-api/dma-buf-alloc-exchange.rst v2: - Moved to general uAPI section, cross-referenced from dma-buf/DRM - Added Pekka's suggested glossary with some small changes - Cleanups and clarifications from Simon and James diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index 862dbc2759d0..0c153d79ccc4 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -22,6 +22,14 @@ interact with the three main primitives offered by dma-buf: allowing implicit (kernel-ordered) synchronization of work to preserve the illusion of coherent access + +Userspace API principles and use + + +For more details on how to design your subsystem's API for dma-buf use, please +see Documentation/userspace-api/dma-buf-alloc-exchange.rst. + + Shared DMA Buffers -- diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 65fb3036a580..eef5fd19bc92 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -486,3 +486,10 @@ and the CRTC index is its position in this array. .. kernel-doc:: include/uapi/drm/drm_mode.h :internal: + + +dma-buf interoperability + + +Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst for +information on how dma-buf is integrated and exposed within DRM. diff --git a/Documentation/userspace-api/dma-buf-alloc-exchange.rst b/Documentation/userspace-api/dma-buf-alloc-exchange.rst new file mode 100644 index ..090453d2ad78 --- /dev/null +++ b/Documentation/userspace-api/dma-buf-alloc-exchange.rst @@ -0,0 +1,384 @@ +.. Copyright 2021-2023 Collabora Ltd. + + +Exchanging pixel buffers + + +As originally designed, the Linux graphics subsystem had extremely limited +support for sharing pixel-buffer allocations between processes, devices, and +subsystems. Modern systems require extensive integration between all three +classes; this document details how applications and kernel subsystems should +approach this sharing for two-dimensional image data. + +It is written with reference to the DRM subsystem for GPU and display devices, +V4L2 for media devices, and also to Vulkan, EGL and Wayland, for userspace +support, however any other subsystems should also follow this design and advice. + + +Glossary of terms += + +.. glossary:: + +image: + Conceptually a two-dimensional array of pixels. The pixels may be stored + in one or more memory buffers. Has width and height in pixels, pixel + format and modifier (implicit or explicit). + +row: + A span along a single y-axis value, e.g. from co-ordinates (0,100) to + (200,100). + +scanline: + Synonym for row. + +column: + A span along a single x-axis value, e.g. from co-ordinates (100,0) to + (100,100). + +memory buffer: + A piece of memory for storing (parts of) pixel data. Has stride and size + in bytes and at least one handle in some API. May contain one or more + planes. + +plane: + A two-dimensional array of some or all of an image's color and alpha + channel values. + +pixel: + A picture element. Has a single color value which is defined by one or + more color channels values, e.g. R, G and B, or Y, Cb and Cr. May also + have an alpha value as an additional channel. + +pixel data: + Bytes or bits that represent some or all of the color/alpha channel values + of a pixel or an image. The data for one pixel may be spread over several + planes or memory buffers depending on format and modifier. + +color value: + A tuple of numbers, representing a color. Each element in the tuple is a + color channel value. + +color channel: + One of the dimensions in a color model. For example, RGB model has + channels R, G, and B. Alpha channel is sometimes counted as a color + channel as well. + +pixel format: + A description of how pixel data represents the pixel's color and alpha + values. + +modifier: + A description of how pixel data is laid out in memory buffers. + +alpha: + A value that denotes the color coverage in a pixel. Sometimes used for + translucency instead
[PATCH v2 0/2] doc: uapi: Document dma-buf interop design & semantics
Hi all, This is v2 to the linked patch series; thanks to everyone for reviewing the initial version. I've moved this out of a pure DRM scope and into the general userspace-API design section. Hopefully it helps others and answers a bunch of questions. I think it'd be great to have input/links/reflections from other subsystems as well here. Cheers, Daniel
Re: [PATCH] doc: gpu: Add document describing buffer exchange
On Thu, 3 Aug 2023 at 16:46, Daniel Stone wrote: > On Tue, 9 Nov 2021 at 09:13, Daniel Vetter wrote: > > Seconded on just landing this without trying to perfect it first, because > > I was just looking for it and didn't find it anywhere :-/ > > Swing and a miss ... > > I've just sent out v2 with - AFAICT - all the changes from all .. all of you in this thread. Thanks a lot for the review! Cheers, Daniel
Re: [PATCH] doc: gpu: Add document describing buffer exchange
On Tue, 9 Nov 2021 at 09:13, Daniel Vetter wrote: > On Mon, Nov 08, 2021 at 04:18:22PM -0800, James Jones wrote: > > On 9/6/21 5:28 AM, Simon Ser wrote: > > > > Since there's a lot of confusion around this, document both the rules > > > > and the best practice around negotiating, allocating, importing, and > > > > using buffers when crossing context/process/device/subsystem boundaries. > > > > > > > > This ties up all of dmabuf, formats and modifiers, and their usage. > > > > > > > > Signed-off-by: Daniel Stone > > > > > > Thanks a lot for this write-up! This looks very good to me, a few comments > > > below. > > > > Agreed, it would be awesome if this were merged somewhere. IMHO, a lot of > > the non-trivial/typo suggestions below could be taken care of as follow-on > > patches, as the content here is better in than out, even if it could be > > clarified a bit. > > Seconded on just landing this without trying to perfect it first, because > I was just looking for it and didn't find it anywhere :-/ Swing and a miss ... I've just sent out v2 with - AFAICT - all the changes from all Cheers, Daniel
Re: [petsc-users] Confusion/failures about the tests involved in including Hypre
This still isn't working, and it looks like the md/mt distinciton might be the culprit. I use the Ninja generator with cmake, on the intel oneapi command line (i.e. windows command line but with the pathing and other things sorted out for the new intel compilers). Looking at the build.ninja file produced, I see rules like: --- build CMakeFiles\HYPRE.dir\blas\dasum.c.obj: C_COMPILER__HYPRE_Debug ..\blas\dasum.c || cmake_object_order_depends_target_HYPRE DEFINES = -D_CRT_SECURE_NO_WARNINGS DEP_FILE = CMakeFiles\HYPRE.dir\blas\dasum.c.obj.d FLAGS = /DWIN32 /D_WINDOWS /Zi /Ob0 /Od /RTC1 -MDd INCLUDES = [etc] -- Note the -MDd flag (which is an -MD flag if I ask for a non debug build). It's odd that it isn't an /MD flag, but I've done some quick checks on the oneapi command line and it looks like -MD is still interpreted as valid input by icx (e.g., >> icx /MD and >> icx -MD both only complain about lack of input files, while >> icx /MDblahblahnotarealflag and >> icx -MDblahblahnotarealflag both give "unknown argument" errors). Thus it really seems like my Hypre library has been building with the MD option, although MT is the default ( https://www.intel.com/content/www/us/en/docs/dpcpp-cpp-compiler/developer-guide-reference/2023-0/md.html and https://www.intel.com/content/www/us/en/docs/dpcpp-cpp-compiler/developer-guide-reference/2023-0/mt.html), as suggested by Satish. There seems to be no hint of the MD flag in any of the cmake files provided with hypre, so it looks like the -MD flag is being added by the cmake Ninja generator as a kind of default (and maybe in a confused way - why the - instead of /?) So the task now seems to be to convince cmake to stop doing this. On Mon, Jul 24, 2023 at 11:21 AM Daniel Stone wrote: > On the hypre versioning - aha. For this project I locked the petsc version > a little while ago (3.19.1), but I've been using a fresh clone of hypre, so > clearly > it's too modern a version. Using the appropriate version of hypre (2.28.0, > according to hypre.py) might fix some things. > I may have other problems in the form of the confilicting compiler options > as Satish mentioned, which I'll have to figure out too. > > Thanks, > > Daniel > > On Fri, Jul 21, 2023 at 9:32 PM Barry Smith wrote: > >> >> >> hypre_Error was changed from an integer to a struct in hypre. PETSc code >> was changed in main to work with that struct and not work if hypre_Error is >> an int. This means main cannot work with previous hypre versions (where >> hypre_Error is an int). Sure, instead of changing the minimum version one >> could potentially change PETSc's main version source code to work with both >> old and new hypre, but that would need to be done and has not been done. >> main is simply broken, it allows configure to succeed with an older version >> of hypre but PETSc main cannot compile with this, thus confusing developers >> and users. This does need to be fixed, one way or another. >> >> >> >> > On Jul 21, 2023, at 1:32 PM, Satish Balay wrote: >> > >> > We don't have a good way to verify if the changes continue to work >> > with the minver of the pkg [and if minor fixes can get this working - >> > without increasing this requirement - sure without access to the new >> > features provided by the new version] >> > >> > Having a single version dependency between pkgs makes the whole >> > ecosystem [where many pkgs with all their dependencies mixed in] >> > brittle.. >> > >> > Satish >> > >> > On Thu, 20 Jul 2023, Barry Smith wrote: >> > >> >> >> >> Satish, >> >> >> >> I think hypre.py in main needs minversion = 2.29 instead of 2.14 >> >> >> >> >> >> >> >>> On Jul 20, 2023, at 11:05 AM, Satish Balay wrote: >> >>> >> >>> Can check config/BuildSystem/config/packages/hypre.py >> >>> >> >>> petsc-3.19 (or release branch) is compatible with hypre 2.28.0, petsc >> 'main' branch with 2.29.0 >> >>> >> >>> Satish >> >>> >> >>> On Thu, 20 Jul 2023, Barry Smith via petsc-users wrote: >> >>> >> >>>> >> >>>> You cannot use this version of PETSc, 3.19, with the version of >> hypre you installed. In hypre they recently changed hypre_Error from an >> integer to a struct which completely breaks compatibility with previous >> versions of hypre (and hence previous versions of PETSc). You must use the >> main git branch of PETSc with the version of hypre you installed. >> >
Re: [PATCH v10] drm: Add initial ci/ subdirectory
On Thu, 27 Jul 2023 at 22:47, Rob Clark wrote: > > I did run into a bit of a chicken vs. egg problem with testing the "in > > tree" version (compared to earlier versions which kept most of the yml > > and scripts in a separate tree), is that it actually requires this > > commit to exist in the branch you want to run CI on. My earlier > > workaround of pulling the drm/ci commit in via > > ${branchname}-external-fixes no longer works. > > After unwinding some more gitlab repo settings that were for the > previous out-of-tree yml setup, I have this working. > > Tested-by: Rob Clark > Acked-by: Rob Clark And it's also: Acked-by: Daniel Stone It's been back and forth a few times by now and reviewed pretty heavily by all the people who are across the CI details. I think the next step is to answer all the workflow questions by actually getting it into trees and using it in anger. There was some discussion about whether this should come in from drm-misc, or the core DRM tree, or a completely separate pull, but I'm not sure what the conclusion was ... maintainers, thoughts? Cheers, Daniel
Re: [PATCH RESEND] drm/mediatek: Add valid modifier check
On Mon, 24 Jul 2023 at 18:58, Justin Green wrote: > + if (cmd->modifier[0] && > This is not DRM_FORMAT_MOD_INVALID. Please either explicitly compare against INVALID if that's what you meant, or against LINEAR if that's what you meant, or both. Cheers, Daniel
[petsc-users] support for mixed block size matrices/AIM in PETSc?
Hello PETSc Users/Developers, A collegue of mine is looking into implementing an adaptive implicit method (AIM) over PETSc in our simulator. This has led to some interesting questions about what can be done with blocked matrices, which I'm not able to answer myself - does anyone have any insight? Apparently it would be ideal if we could find a matrix (and vector) type that supports a kind of mixed block size: "For AIM [...] we will have matrix elements of various shapes: 1x1, 1xN, Nx1 and NxN. [...]. The solution and residual will be a mix of 1 and N variable/cell block" There are ideas for how to implement what we want using the fixed-block-size objects we understand well, but if anything like the above exists it would be very exciting. Thanks, Daniel
Re: [petsc-users] Confusion/failures about the tests involved in including Hypre
On the hypre versioning - aha. For this project I locked the petsc version a little while ago (3.19.1), but I've been using a fresh clone of hypre, so clearly it's too modern a version. Using the appropriate version of hypre (2.28.0, according to hypre.py) might fix some things. I may have other problems in the form of the confilicting compiler options as Satish mentioned, which I'll have to figure out too. Thanks, Daniel On Fri, Jul 21, 2023 at 9:32 PM Barry Smith wrote: > > > hypre_Error was changed from an integer to a struct in hypre. PETSc code > was changed in main to work with that struct and not work if hypre_Error is > an int. This means main cannot work with previous hypre versions (where > hypre_Error is an int). Sure, instead of changing the minimum version one > could potentially change PETSc's main version source code to work with both > old and new hypre, but that would need to be done and has not been done. > main is simply broken, it allows configure to succeed with an older version > of hypre but PETSc main cannot compile with this, thus confusing developers > and users. This does need to be fixed, one way or another. > > > > > On Jul 21, 2023, at 1:32 PM, Satish Balay wrote: > > > > We don't have a good way to verify if the changes continue to work > > with the minver of the pkg [and if minor fixes can get this working - > > without increasing this requirement - sure without access to the new > > features provided by the new version] > > > > Having a single version dependency between pkgs makes the whole > > ecosystem [where many pkgs with all their dependencies mixed in] > > brittle.. > > > > Satish > > > > On Thu, 20 Jul 2023, Barry Smith wrote: > > > >> > >> Satish, > >> > >> I think hypre.py in main needs minversion = 2.29 instead of 2.14 > >> > >> > >> > >>> On Jul 20, 2023, at 11:05 AM, Satish Balay wrote: > >>> > >>> Can check config/BuildSystem/config/packages/hypre.py > >>> > >>> petsc-3.19 (or release branch) is compatible with hypre 2.28.0, petsc > 'main' branch with 2.29.0 > >>> > >>> Satish > >>> > >>> On Thu, 20 Jul 2023, Barry Smith via petsc-users wrote: > >>> > >>>> > >>>> You cannot use this version of PETSc, 3.19, with the version of hypre > you installed. In hypre they recently changed hypre_Error from an integer > to a struct which completely breaks compatibility with previous versions of > hypre (and hence previous versions of PETSc). You must use the main git > branch of PETSc with the version of hypre you installed. > >>>> > >>>> Barry > >>>> > >>>> > >>>>> On Jul 20, 2023, at 5:10 AM, Daniel Stone < > daniel.st...@opengosim.com> wrote: > >>>>> > >>>>> Hi All, > >>>>> > >>>>> Many thanks for the detailed explainations and ideas! > >>>>> > >>>>> I tried skipping the test. When it came time to do the build itself > (make $PETSC_DIR... all) I get some failures, unsurprisingly: > >>>>> > >>>>> > >>>>> > >>>>>FC arch-mswin-c-opt/obj/dm/f90-mod/petscdmplexmod.o > >>>>>CC > arch-mswin-c-opt/obj/ksp/pc/impls/hypre/ftn-custom/zhypref.o > >>>>>CC arch-mswin-c-opt/obj/ksp/pc/impls/hypre/ftn-auto/hypref.o > >>>>>CC arch-mswin-c-opt/obj/ksp/pc/impls/hypre/hypre.o > >>>>> > C:\cygwin64\home\DANIEL~1\PETSC_~1.1\src\ksp\pc\impls\hypre\hypre.c(444,29): > error: assigning to 'hypre_Error' from incompatible type 'int' > >>>>> hypre__global_error = 0; > >>>>> ^ ~ > >>>>> C:\cygwin64\home\DANIEL~1\PETSC_~1.1\include\petscerror.h(1752,7): > note: expanded from macro 'PetscStackCallExternalVoid' > >>>>> __VA_ARGS__; \ > >>>>> ^~~ > >>>>> > C:\cygwin64\home\DANIEL~1\PETSC_~1.1\src\ksp\pc\impls\hypre\hypre.c(634,29): > error: assigning to 'hypre_Error' from incompatible type 'int' > >>>>> hypre__global_error = 0; > >>>>> ^ ~ > >>>>> C:\cygwin64\home\DANIEL~1\PETSC_~1.1\include\petscerror.h(1752,7): > note: expanded from macro 'PetscStackCallExternalVoid' > >>>>> __VA_ARGS__; \ > >>&
Re: [petsc-users] Confusion/failures about the tests involved in including Hypre
ar.gz'] > > >> -self.functions = ['HYPRE_IJMatrixCreate'] > > >> +self.functions = [] > > >> self.includes= ['HYPRE.h'] > > >> self.liblist = [['libHYPRE.a']] > > >> self.buildLanguages = ['C','Cxx'] > > >> > > >> > > >> Satish > > >> > > >> > > >> On Wed, 19 Jul 2023, Barry Smith wrote: > > >> > > >>> > > >>> You don't indicate what type of libraries you built hypre with; > static or shared. My guess is you ended up with shared > > >>> > > >>> I think the answer to your difficulty is hidden in __cdecl (Satish > will know much better than me). When you are looking for symbols in Windows > shared libraries you have to prepend something to the function prototype to > have it successfully found. For example the PETSc include files have these > things __declspec(dllimport) The configure test fails because it does not > provide the needed prototype. Likely you built PTScotch with static > libraries so no problem. > > >>> > > >>> The simplest fix would be to build static hypre libraries. I think > it is a major project to get PETSc configure and macro system to work > properly with external packages that are in Windows shared libraries since > more use of __declspec would be needed. > > >>> > > >>> Barry > > >>> > > >>> The PETSc installation instructions should probably say something > about external packages with Windows shared libraries. > > >>> > > >>> > > >>> > > >>> > > >>> > > >>> > > >>> > > >>>> On Jul 19, 2023, at 10:52 AM, Daniel Stone < > daniel.st...@opengosim.com> wrote: > > >>>> > > >>>> Hello, > > >>>> > > >>>> I'm working on getting a petsc build running on windows. One > necessary package to include is Hypre. I've been able to build Hypre > seperately using cmake, and confirmed that the library works > > >>>> by setting up a VS project to run some of the example programs. > > >>>> > > >>>> My attempted petsc build is being done through cygwin. I've been > able to (with varying degrees of difficulty), build a fairly plain petsc, > and one that downloads and builds ptscotch (after some modifications > > >>>> to both ptscotch and the config script). I am now attempting to > include Hypre (using the --hypre-iclude and --hypre-lib flags, etc). Note > that the same compilers are being used for both Hypre and for petsc > > >>>> through cygwin - the new intel oneapi compilers (icx and ifx, after > again varying amounts of pain to work around their awkwardness with the > config script). > > >>>> > > >>>> I'm seeing a problem when the config script does some tests on the > included hypre lib. The source code looks like: > > >>>> > > >>>> #include "confdefs.h" > > >>>> #include "conffix.h" > > >>>> /* Override any gcc2 internal prototype to avoid an error. */ > > >>>> > > >>>> #include "HYPRE.h" > > >>>> > > >>>> char HYPRE_IJMatrixCreate(); > > >>>> static void _check_HYPRE_IJMatrixCreate() { HYPRE_IJMatrixCreate(); > } > > >>>> > > >>>> int main() { > > >>>> _check_HYPRE_IJMatrixCreate();; > > >>>> return 0; > > >>>> } > > >>>> > > >>>> > > >>>> As I understand this is a fairly standard type of stub program used > by the config script to check that it is able to link to certain symbols in > given libraries. Tests like this have succeeded in my builds that > > >>>> include PTScotch. > > >>>> > > >>>> I keep getting a linker error with the above test, including if I > seperate it out and try to build it seperately: > > >>>> > > >>>> unresolved external symbol "char __cdel HYPRE_IJMatrixCreate(void)" > > > >>>> > > >>>> Ok, it looks like a problem with either the library or linker > commands. But here's the interesting thing - If I transplant this code into > VS, with the same project setting that allows it to build the much more > > >>>> nontrivial Hypre example programs, I get the same error: > > >>>> > > >>>> Error LNK2001 unresolved external symbol "char __cdecl > HYPRE_IJMatrixCreate(void)" (?HYPRE_IJMatrixCreate@@YADXZ) hypretry1 > C:\Users\DanielOGS\source\repos\hypretry1\hypretry1\Source.obj 1 > > >>>> > > >>>> So it seems like there might be something about this type of stub > program that is not working with my Hypre library. I don't fully understand > this program - it's able to call the function with no arguments, but > > >>>> it also needs to be linked against a library containing the > function, apparently by wrapping it in a static void function? Not > something I've seen before. > > >>>> > > >>>> Does anyone have any insight into what might be going wrong - or > really just any explaination of how the stub program works so I can figure > out why it isn't in this case? > > >>>> > > >>>> Many thanks, > > >>>> > > >>>> Daniel > > >>> > > >>> > > >> > > > > > > >
Re: [petsc-users] Confusion/failures about the tests involved in including Hypre
gt; the configure tests will fail). > > > > Barry > > > > > > > On Jul 19, 2023, at 11:40 AM, Satish Balay wrote: > > > > > > BTW: Some explanation of configure: > > > > > > It attempts the following on linux: > > > > > >>>>>>> > > > Source: > > > #include "confdefs.h" > > > #include "conffix.h" > > > /* Override any gcc2 internal prototype to avoid an error. */ > > > char HYPRE_IJMatrixCreate(); > > > static void _check_HYPRE_IJMatrixCreate() { HYPRE_IJMatrixCreate(); } > > > > > > int main(void) { > > > _check_HYPRE_IJMatrixCreate(); > > > return 0; > > > } > > > <<<<<<< > > > > > > Note - it does not include 'HYPRE.h' here - but redefines the > prototype as 'char HYPRE_IJMatrixCreate(); > > > > > > Compiling it manually: > > > > > >>>>> > > > [balay@pj01 petsc]$ cat conftest.c > > > char HYPRE_IJMatrixCreate(); > > > static void _check_HYPRE_IJMatrixCreate() { HYPRE_IJMatrixCreate(); } > > > > > > int main(void) { > > > _check_HYPRE_IJMatrixCreate(); > > > return 0; > > > } > > > [balay@pj01 petsc]$ gcc -c conftest.c > > > [balay@pj01 petsc]$ nm -Ao conftest.o |grep HYPRE_IJMatrixCreate > > > conftest.o: t _check_HYPRE_IJMatrixCreate > > > conftest.o: U HYPRE_IJMatrixCreate > > > [balay@pj01 petsc]$ nm -Ao arch-linux-c-debug/lib/libHYPRE.so |grep > HYPRE_IJMatrixCreate > > > arch-linux-c-debug/lib/libHYPRE.so:0007f2c2 T > HYPRE_IJMatrixCreate > > > [balay@pj01 petsc]$ > > > <<<< > > > > > > Here the "U HYPRE_IJMatrixCreate" in conftest.o matches "T > HYPRE_IJMatrixCreate" in libHYPRE.so - so the "link" test in configure > succeeds! > > > > > >>>>>>> > > > [balay@pj01 petsc]$ gcc -o conftest conftest.o > arch-linux-c-debug/lib/libHYPRE.so > > > [balay@pj01 petsc]$ echo $? > > > 0 > > > <<<<< > > > > > > On windows - [due to name mangling by cdecl/stdcall, (/MT vs /MD) > etc..] - this might not match - resulting in link failures. > > > > > > Satish > > > > > > > > > On Wed, 19 Jul 2023, Satish Balay via petsc-users wrote: > > > > > >> You could try skipping this test [and assume --with-hypre-include and > --with-hypre-lib options are correct] - and see if this works. > > >> > > >> diff --git a/config/BuildSystem/config/packages/hypre.py > b/config/BuildSystem/config/packages/hypre.py > > >> index 5bc88322aa2..2d6c7932e17 100644 > > >> --- a/config/BuildSystem/config/packages/hypre.py > > >> +++ b/config/BuildSystem/config/packages/hypre.py > > >> @@ -11,7 +11,7 @@ class Configure(config.package.GNUPackage): > > >> self.requiresversion = 1 > > >> self.gitcommit = 'v'+self.version > > >> self.download= ['git:// > https://github.com/hypre-space/hypre',' > https://github.com/hypre-space/hypre/archive/'+self.gitcommit+'.tar.gz'] > > >> -self.functions = ['HYPRE_IJMatrixCreate'] > > >> +self.functions = [] > > >> self.includes= ['HYPRE.h'] > > >> self.liblist = [['libHYPRE.a']] > > >> self.buildLanguages = ['C','Cxx'] > > >> > > >> > > >> Satish > > >> > > >> > > >> On Wed, 19 Jul 2023, Barry Smith wrote: > > >> > > >>> > > >>> You don't indicate what type of libraries you built hypre with; > static or shared. My guess is you ended up with shared > > >>> > > >>> I think the answer to your difficulty is hidden in __cdecl (Satish > will know much better than me). When you are looking for symbols in Windows > shared libraries you have to prepend something to the function prototype to > have it successfully found. For example the PETSc include files have these > things __declspec(dllimport) The configure test fails because it does not > provide the needed prototype. Likely you built PTScotch with static > libraries so no problem. > > >>> > > >>> The simplest fix would be to build static hypre libraries. I think > it is a major project to get PETSc configure and macro system to work > properly with external pa
[petsc-users] Confusion/failures about the tests involved in including Hypre
Hello, I'm working on getting a petsc build running on windows. One necessary package to include is Hypre. I've been able to build Hypre seperately using cmake, and confirmed that the library works by setting up a VS project to run some of the example programs. My attempted petsc build is being done through cygwin. I've been able to (with varying degrees of difficulty), build a fairly plain petsc, and one that downloads and builds ptscotch (after some modifications to both ptscotch and the config script). I am now attempting to include Hypre (using the --hypre-iclude and --hypre-lib flags, etc). Note that the same compilers are being used for both Hypre and for petsc through cygwin - the new intel oneapi compilers (icx and ifx, after again varying amounts of pain to work around their awkwardness with the config script). I'm seeing a problem when the config script does some tests on the included hypre lib. The source code looks like: #include "confdefs.h" #include "conffix.h" /* Override any gcc2 internal prototype to avoid an error. */ #include "HYPRE.h" char HYPRE_IJMatrixCreate(); static void _check_HYPRE_IJMatrixCreate() { HYPRE_IJMatrixCreate(); } int main() { _check_HYPRE_IJMatrixCreate();; return 0; } As I understand this is a fairly standard type of stub program used by the config script to check that it is able to link to certain symbols in given libraries. Tests like this have succeeded in my builds that include PTScotch. I keep getting a linker error with the above test, including if I seperate it out and try to build it seperately: unresolved external symbol "char __cdel HYPRE_IJMatrixCreate(void)" Ok, it looks like a problem with either the library or linker commands. But here's the interesting thing - If I transplant this code into VS, with the same project setting that allows it to build the much more nontrivial Hypre example programs, I get the same error: Error LNK2001 unresolved external symbol "char __cdecl HYPRE_IJMatrixCreate(void)" (?HYPRE_IJMatrixCreate@@YADXZ) hypretry1 C:\Users\DanielOGS\source\repos\hypretry1\hypretry1\Source.obj 1 So it seems like there might be something about this type of stub program that is not working with my Hypre library. I don't fully understand this program - it's able to call the function with no arguments, but it also needs to be linked against a library containing the function, apparently by wrapping it in a static void function? Not something I've seen before. Does anyone have any insight into what might be going wrong - or really just any explaination of how the stub program works so I can figure out why it isn't in this case? Many thanks, Daniel
Re: Need support to display application at (0, 0) position on Weston desktop
Hi Huy, On Wed, 12 Jul 2023 at 16:15, huy nguyen wrote: > I have a Linux system based on weston wayland. I run MPV player and expect > it displays a video window at (0,0) position on the screen (top left corner > of the display). I already use x11egl backend option to MPV to support a > fixed position to application but the video window of MPV is displayed at > an offset (X offset, Y offset) from (0,0) position as shown by the picture > below: > You probably want to make mpv be fullscreen, and then it will take up the whole area of the screen. kiosk-shell does this well, by telling all applications to be fullscreen. Cheers, Daniel
Re: Need support to have weston randr release
Hi Huy, On Sat, 8 Jul 2023 at 08:39, huy nguyen wrote: > I have a Linux system based on weston wayland and I need to get the > current setting of the display resolution. > Unfortunately, xrandr command does not work on Wayland. > After much searching, I came to this information which is about adding > weston-randr support for Weston compositor: > > https://lists.freedesktop.org/archives/wayland-devel/2014-February/013480.html > > However, I could not find a link to download the patch to apply in order > to have weston-randr command > Please advise if the patch is available for the community to use. > The current resolution is provided as an event on the wl_output interfaces. >From the command line, you can get this from wayland-info. Cheers, Daniel
Re: Weston mirror/clone to 2 different displays
Hi Dawn, On Thu, 22 Jun 2023 at 18:09, Dawn HOWE wrote: > I am developing an (embedded) medical device which is required to have a > touchscreen display and also mirror the output to a monitor connected via > HDMI. The device is using Wayland/Weston on TorizonCore (based on a yocto > kirkstone). I am able to get the display extended from HDMI to LVDS, but > not have the output mirrored to both displays. I posted a query on the > Toradex community, and received a response that Weston may not be capable > of doing this. ( > https://community.toradex.com/t/apalis-imx8-hdmi-and-lvds-display-not-mirroring-cloning/19869 > ). > > > > I have searched and found some old posts from several years ago indicating > that it was not supported, but may be with a patch. I understand that > “same-as” configuration in weston.ini does not work for my scenario. > > > > What is the current state of cloning/mirroring to two different outputs, > but on the same card. E.g (card1-HDMI-A-1 and card1-LVDS-1): > > ls /sys/class/drm > > card0 card1 card1-HDMI-A-1 card1-LVDS-1 renderD128 > renderD129 version > Weston can currently mirror it if the display driver directly supports it. You can use the same-as configuration option (see man weston-drm) to enable this. If your display driver doesn't support this in the kernel, then Weston won't do it for now, but we are actively working on this and expect to have a branch capable of this within the next couple of weeks or so. Cheers, Daniel
Re: Weston 12 compatibility with Yocto Kirkstone
Hi Namit, On Thu, 15 Jun 2023 at 16:37, Namit Solanki (QUIC) < quic_nsola...@quicinc.com> wrote: > > As we all know Weston 10 has bitbakes files available for Yocto kirkstone > version. Can Weston 12 work with Kirkstone as well? > > > > Is Weston 12 compatible with Kirkstone? > > > > Do we need to write our own bitbake files for Weston 12 to compile with > Kirkstone? > > > > Please help on these queries. > OpenEmbedded does have a 11.0.1 build definition for Weston. You should be able to reuse this whilst bumping the version to 12.0.0, as long as you also pull the other dependencies from OE such as libseat and probably a newer version of Meson. Cheers, Daniel
Re: Refresh rates with multiple monitors
Hi Joe, On Wed, 14 Jun 2023 at 21:33, Joe M wrote: > Thanks Daniel. Do you know if wl_output instances are decoupled from each > other, when it comes to display refresh? > Yep, absolutely. > The wl_output geometry info hints that each output can be thought of as a > region in a larger compositor canvas, given the logical x/y fields in the > geometry. Is the compositor able to handle the repaint scheduling in a > refresh-aware way? > Yes. > I'm trying to get a better understanding of how these pieces interact to > maximize draw time but still hit the glass every frame. The various blog > posts and documentation out there are pretty clear when it comes to drawing > to a single physical display, but less so when multiple displays are in use. > Per-output repaint cycles are taken as a given. You can assume that every compositor does this, and any compositor which doesn't do this is so hopelessly broken as to not be worth considering. Cheers, Daniel
Re: Refresh rates with multiple monitors
Hi, On Tue, 13 Jun 2023 at 10:20, Pekka Paalanen wrote: > On Tue, 13 Jun 2023 01:11:44 + (UTC) > Joe M wrote: > > As I understand, there is one global wl_display. Is there always one > > wl_compositor too? > > That is inconsequential. > Yeah, I think the really consequential thing is that a wl_display really just represents a connection to a Wayland server (aka compositor). Display targets (e.g. 'the HDMI connector on the left', 'the DSI panel') are represented by wl_output objects. There is one of those for each output. Cheers, Daniel
Re: Why does Java (XWayland / Weston) resize a Window to 1x1 pixel when HDMI is unplugged (and does not resize back when HDMI is plugged)
On Thu, 8 Jun 2023 at 16:54, Martin Petzold wrote: > Am 08.06.23 um 16:58 schrieb Daniel Stone: > > On Thu, 8 Jun 2023 at 14:28, Pekka Paalanen wrote: > >> On Thu, 8 Jun 2023 14:49:37 +0200 >> Martin Petzold wrote: >> > btw. we are using a Weston 9 package from NXP and there may be >> important >> > fixes for our i.MX8 platform in there. >> >> Oh. We cannot support modified Weston, sorry. Significant vendor >> modifications tend to break things, and we have no idea what they do or >> why. Maybe this problem is not because of that, maybe it is, hard to >> guess. > > > The good news is that mainline Linux runs very well on all i.MX6, and most > i.MX8 platforms. You can ditch the NXP BSP and just use a vanilla Yocto > build for your machine. This will have upstream Weston which should solve > your problem. > > Do you mean Linux mainline or Yocto mainline? > > Because we are building from Debian and not from Yocto, for several > reasons. We have a more complex system setup. > Ah, I wasn't aware they also had Debian distributions. Nice. Yes, I mean mainline of upstream Linux + Mesa + Weston (plus GStreamer etc if you want to use that). That's worked very well out of the box for a few years now with no vendor trees required. Cheers, Daniel
Re: Why does Java (XWayland / Weston) resize a Window to 1x1 pixel when HDMI is unplugged (and does not resize back when HDMI is plugged)
Hi, On Thu, 8 Jun 2023 at 14:28, Pekka Paalanen wrote: > On Thu, 8 Jun 2023 14:49:37 +0200 > Martin Petzold wrote: > > btw. we are using a Weston 9 package from NXP and there may be important > > fixes for our i.MX8 platform in there. > > Oh. We cannot support modified Weston, sorry. Significant vendor > modifications tend to break things, and we have no idea what they do or > why. Maybe this problem is not because of that, maybe it is, hard to > guess. The good news is that mainline Linux runs very well on all i.MX6, and most i.MX8 platforms. You can ditch the NXP BSP and just use a vanilla Yocto build for your machine. This will have upstream Weston which should solve your problem. Cheers, Daniel
Re: wayland: wrong tiling with wl_drm on Vivante GC2000
Hi Christian, On Thu, 30 Mar 2023 at 20:15, Christian Gudrian wrote: > We're running a custom Wayland compositor based on Qt Wayland on an i.MX6 > Quad system with a Vivante GC2000 GPU using the Mesa Gallium driver. While > the compositor itself displays correctly, client buffers are displayed with > wrong tiling. > > So far I've found out, that the client's __DRIimage is created by > etna_resource_create with a tiled layout. This image is later passed to > create_wl_buffer which ignores the tiling information and passes the > image's FD to wl_drm_create_prime_buffer. > > If I patch etna_resource_create to always use a linear layout the client's > buffer is displayed correctly. That's certainly not how to do it correctly. > Is there a way to ensure a linear layout for this use case? > You need to force QtWayland to use the wp-linux-dmabuf-v1 platform integration for the compositor. All the others do this automatically but Qt requires an environment variable for this. Cheers, Daniel
Re: display band (display area vs real visible area)
Hi, On Tue, 21 Mar 2023 at 12:08, Jani Nikula wrote: > On Tue, 21 Mar 2023, Daniel Stone wrote: > > There have been some threads - mostly motivated by MacBooks and the > > Asahi team - about creating a KMS property to express invisible areas. > > This would be the same thing, and the userspace ecosystem will pick it > > up when the kernel exposes it. > > In my case the kernel handled it completely internally, and the > userspace didn't even know. But I guess it depends on the display being > able to take a smaller framebuffer, otherwise I don't think it's > feasible. > > I haven't checked the threads you mention but I assume it covers the > more general case of having rounded corners or holes for the camera, not > just the frame covering the edges or something like that. That couldn't > possibly be handled by kernel alone, but it's also a bunch of > infrastructure work both in kernel and userspace to make it happen. Yeah, exactly. Just a connector property, which could come from DT or ACPI or manual overrides or whatever. Userspace would still allocate a full-size framebuffer, but look at that property and not render anything useful into those areas. In the camera/notch case, it's a matter of not putting useful content there. In the letterbox/pillarbox case, it's about shrinking the reported screen size so that window management, clients, etc, all believe that the screen is smaller than it is. Cheers, Daniel
Re: display band (display area vs real visible area)
Hi, On Tue, 21 Mar 2023 at 11:24, Jani Nikula wrote: > On Tue, 21 Mar 2023, Michael Nazzareno Trimarchi > wrote: > > On Tue, Mar 21, 2023 at 11:43 AM Jani Nikula > > wrote: > >> On Tue, 21 Mar 2023, Michael Nazzareno Trimarchi > >> wrote: > >> > I would like to know the best approach in the graphics subsystem how > >> > deal with panels where the display area is different from the visible > >> > area because the display has a band left and right. I have already > >> > done the drm driver for the panel but from userspace point of view > >> > it's a pain to deal in wayland for input device and output device. Do > >> > you have any suggestions? > >> > >> Do you have the EDID for the panel? > > > > mipi->panel so should not have edid > > That's the kind of information you'd expect in the original question. ;) > > I've done that sort of thing in the past, but not sure if it would fly > upstream. Basically the kernel driver would lie about the resolution to > userspace, and handle the centering and the bands internally. In my > case, the DSI command mode panel in question had commands to set the > visible area, so the driver didn't have to do all that much extra to > make it happen. There have been some threads - mostly motivated by MacBooks and the Asahi team - about creating a KMS property to express invisible areas. This would be the same thing, and the userspace ecosystem will pick it up when the kernel exposes it. Cheers, Daniel
Re: Weston 10+ and GLES2 compatibility
Hi Daniel, On Fri, 10 Mar 2023 at 14:28, Levin, Daniel wrote: > We are currently attempting to update from Weston 9.0.0 to Weston 10+ and > facing issues with GLES2 compatibility at both build time and run time. > > For instance, gl_renderer_setup() exits with error if GL_EXT_unpack_subimage > is not present. Other code explicitly includes GLES3/gl3.h and uses pixel > formats from GL_EXT_texture_storage. > > We are using Mali 400 with proprietary Arm userspace GL drivers, which > supports only GLES2 without extensions above. > > Could you please clarify whether for Weston 10+ GLES3 is now mandatory > dependency? Was this highlighted in any release notes? > > If so, then we have to freeze Weston on version 9.0.0. That did indeed change. We require GLES3 headers at build time (no requirement for ES3 runtime contexts), and we do require GL_EXT_unpack_subimage. It's safe to use newer header sets than your driver supports; you could just take the headers directly from Khronos, or from Mesa, and build against those whilst using your other driver at runtime. GL_EXT_unpack_subimage also has no hardware dependency. It's literally about two lines to implement in software. If your proprietary driver can't support this, I strongly recommend switching to the Lima driver shipped as part of the Linux kernel and Mesa. > Could you please explain is it safe to keep updating all other Wayland > components (client, protocols, xwayland), and keep only Weston compositor > downgraded to 9.0.0? I tested and see that such combination works properly. > Though I am not sure if that is the correct approach, or it might cause > issues. And instead we have to downgrade all the Wayland components to the > same older version (in our case: client 1.19, protocols 1.21, weston 9.0.0). It's completely safe to use newer wayland + wayland-protocols + etc with an older Weston. Cheers, Daniel
Re: Weston does not start with "Failed to open device: No such file or directory, Try again..."
Hi Martin, On Fri, 17 Feb 2023 at 11:27, Martin Petzold wrote: > Feb 17 12:16:24 tavla DISPLAY Wayland[957]: [12:16:24.624] Loading module > '/usr/lib/aarch64-linux-gnu/libweston-9/g2d-renderer.so' > Feb 17 12:16:25 tavla DISPLAY Wayland[957]: [ 1] Failed to open device: > No such file or directory, Try again... > Feb 17 12:16:26 tavla DISPLAY Wayland[957]: [ 2] Failed to open device: > No such file or directory, Try again... > Feb 17 12:16:27 tavla DISPLAY Wayland[957]: [ 3] Failed to open device: > No such file or directory, Try again... > Feb 17 12:16:28 tavla DISPLAY Wayland[957]: [ 4] Failed to open device: > No such file or directory, Try again... > Feb 17 12:16:28 tavla DISPLAY Wayland[957]: [ 5] _OpenDevice(1249): > FATAL: Failed to open device, errno=No such file or directory. g2d-renderer comes from the NXP fork of Weston, customised to work on their downstream kernels with their libraries. It's presumably looking for some kind of G2D device node which it can't see for some reason. If you're using an upstream kernel then vanilla Weston 9.0.0 (with no NXP patches) works great there on i.MX devices. If you're using a downstream kernel/GLES/Weston/etc from NXP, then I'm afraid you need to contact them for support. Cheers, Daniel
Re: [PATCH] drm: document expectations for GETFB2 handles
Hi, On Thu, 16 Feb 2023 at 09:25, Simon Ser wrote: > On Thursday, February 16th, 2023 at 10:11, Pekka Paalanen > wrote: > > Btw. does this also mean that if you use GETFB2 to get handle A, you > > export that as dmabuf and import in the same open device instance, you > > again get handle A? > > I haven't tested it, but I believe that is correct. > > > IOW, you should never ever export a dmabuf of what you got with > > GETFB2. If one did, one might import it oneself via GBM, breaking all > > reference counting. But you also cannot "just leak" the handle A, > > because if GBM happens to run on a different DRM device opened > > instance, GBM would get a different handle to own. > > > > That's... err. How is a compositor supposed to do transition animation > > from an old FB to its own thing? I guess mmap + glTexImage2D equivalent > > to make a copy of the old FB so one can use it as a texture? > > I think the compositor can export the handle as DMA-BUF, then close the > handle immediately. Then go about its business as usual. Yeah, I think either of those two are the most sensible. We did go back and forth over the semantics a few times - part over mail and part over IRC - and the eventual conclusion was to match GetFB to make it easier for users to transition, but to de-duplicate the handles _within_ the call for consistency with the rest of everything else. It's not the most pleasant compromise, but eh. Cheers, Daniel
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Wed, 15 Feb 2023 at 20:54, Harry Wentland wrote: > On 2/15/23 06:46, Daniel Stone wrote: > > On Tue, 14 Feb 2023 at 16:57, Harry Wentland wrote: > >> On 2/14/23 10:49, Sebastian Wick wrote: > >> From what I've seen recently I am inclined to favor an incremental > >> approach more. The reason is that any API, or portion thereof, is > >> useless unless it's enabled full stack. When it isn't it becomes > >> dead code quickly, or never really works because we overlooked > >> one thing. The colorspace debacle shows how even something as > >> simple as extra enum values in KMS APIs shouldn't be added unless > >> someone in a canonical upstream project actually uses them. I > >> would argue that such a canonical upstream project actually has > >> to be a production environment and not something like Weston. > > > > Just to chime in as well that it is a real production environment; > > it's probably actually shipped the most of any compositor by a long > > way. It doesn't have much place on the desktop, but it does live in > > planes, trains, automobiles, digital signage, kiosks, STBs/TVs, and > > about a billion other places you might not have expected. > > > > Understood. > > Curious if there's a list of some concrete examples. If I was allowed to name them, I'd definitely be doing a much better job of promoting it ... but if you've bought a car in the last 7-8 years, it's much more likely than not that its console display is using Weston. Probably about 50% odds that you've flown on a plane whose IFE is driven by Weston. You've definitely walked past a lot of digital signage advertisements and display walls which are driven by Weston. There are a huge number of consumer products (and other modes of transport, would you believe?) that are too, but I can't name them because it gets too specific. The cars are probably using a 10+ year old (and frankly awful) SoC. The display walls are probably using a 6ish-year-old SoC with notoriously poor memory bandwidth. Or TVs trying to make 4K UIs fly on an ancient (pre-unified-shader) GPU. The hits go on. We do ship things on nice and capable new hardware as well, but keeping old hardware working with new software stacks is non-negotiable for us, and we have to bend over backwards to make that happen. > >> We should look at this from a use-case angle, similar to what > >> the gamescope guys are doing. Small steps, like: > >> 1) Add HDR10 output (PQ, BT.2020) to the display > >> 2) Add ability to do sRGB linear blending > >> 3) Add ability to do sRGB and PQ linear blending > >> 4) Post-blending 3D LUT > >> 5) Pre-blending 3D LUT > >> > >> At each stage the whole stack needs to work together in production. > > > > Personally, I do think at this stage we probably have enough of an > > understanding to be able to work with an intermediate solution. We > > just need to think hard about what that intermediate solution is - > > making sure that we don't end up in the same tangle of impossible > > semantics like the old 'broadcast RGB' / colorspace / HDR properties > > which were never thought through - so that it is something we can > > build on rather than something we have to work around. But it would be > > really good to make HDR10/HDR10+ media and HDR games work on HDR > > displays, yeah. > > I have a feeling we'll make some progress here this year. I definitely > think the whole HDR/Colour work is on the right track in Weston and > Wayland which will hopefully give us a good base to work with over > many years. Yep! Coming to the point you were making in the other mail - Weston was traditionally used as _the_ enablement vehicle for KMS, because we cared about using the depth of hardware much more than anyone else (e.g. being years ahead on planes), and the vendor who wanted to enable it either wanted to enable Weston specifically or just didn't have an open userspace stack for it. The other compositors couldn't be that vehicle, either because they were more focused on desktop UI, or they could just afford to throw the GPU at it and suck up the occasional frame hitch / thermal burn / etc. I like to think we had a reputation for being pretty thoughtful and careful with our review as well, and didn't give it lightly to misguided ideas which caused long-term problems. But we've got a greater diversity in userspace these days, and that's no bad thing. If the best vehicle to demonstrate HDR GPU rendering is gamescope, then use gamescope as that vehicle. We'll be there if we can, and if it makes sense for us, but it's not a requirement. Cheers, Daniel
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Wed, 15 Feb 2023 at 20:54, Harry Wentland wrote: > On 2/15/23 06:46, Daniel Stone wrote: > > On Tue, 14 Feb 2023 at 16:57, Harry Wentland wrote: > >> On 2/14/23 10:49, Sebastian Wick wrote: > >> From what I've seen recently I am inclined to favor an incremental > >> approach more. The reason is that any API, or portion thereof, is > >> useless unless it's enabled full stack. When it isn't it becomes > >> dead code quickly, or never really works because we overlooked > >> one thing. The colorspace debacle shows how even something as > >> simple as extra enum values in KMS APIs shouldn't be added unless > >> someone in a canonical upstream project actually uses them. I > >> would argue that such a canonical upstream project actually has > >> to be a production environment and not something like Weston. > > > > Just to chime in as well that it is a real production environment; > > it's probably actually shipped the most of any compositor by a long > > way. It doesn't have much place on the desktop, but it does live in > > planes, trains, automobiles, digital signage, kiosks, STBs/TVs, and > > about a billion other places you might not have expected. > > > > Understood. > > Curious if there's a list of some concrete examples. If I was allowed to name them, I'd definitely be doing a much better job of promoting it ... but if you've bought a car in the last 7-8 years, it's much more likely than not that its console display is using Weston. Probably about 50% odds that you've flown on a plane whose IFE is driven by Weston. You've definitely walked past a lot of digital signage advertisements and display walls which are driven by Weston. There are a huge number of consumer products (and other modes of transport, would you believe?) that are too, but I can't name them because it gets too specific. The cars are probably using a 10+ year old (and frankly awful) SoC. The display walls are probably using a 6ish-year-old SoC with notoriously poor memory bandwidth. Or TVs trying to make 4K UIs fly on an ancient (pre-unified-shader) GPU. The hits go on. We do ship things on nice and capable new hardware as well, but keeping old hardware working with new software stacks is non-negotiable for us, and we have to bend over backwards to make that happen. > >> We should look at this from a use-case angle, similar to what > >> the gamescope guys are doing. Small steps, like: > >> 1) Add HDR10 output (PQ, BT.2020) to the display > >> 2) Add ability to do sRGB linear blending > >> 3) Add ability to do sRGB and PQ linear blending > >> 4) Post-blending 3D LUT > >> 5) Pre-blending 3D LUT > >> > >> At each stage the whole stack needs to work together in production. > > > > Personally, I do think at this stage we probably have enough of an > > understanding to be able to work with an intermediate solution. We > > just need to think hard about what that intermediate solution is - > > making sure that we don't end up in the same tangle of impossible > > semantics like the old 'broadcast RGB' / colorspace / HDR properties > > which were never thought through - so that it is something we can > > build on rather than something we have to work around. But it would be > > really good to make HDR10/HDR10+ media and HDR games work on HDR > > displays, yeah. > > I have a feeling we'll make some progress here this year. I definitely > think the whole HDR/Colour work is on the right track in Weston and > Wayland which will hopefully give us a good base to work with over > many years. Yep! Coming to the point you were making in the other mail - Weston was traditionally used as _the_ enablement vehicle for KMS, because we cared about using the depth of hardware much more than anyone else (e.g. being years ahead on planes), and the vendor who wanted to enable it either wanted to enable Weston specifically or just didn't have an open userspace stack for it. The other compositors couldn't be that vehicle, either because they were more focused on desktop UI, or they could just afford to throw the GPU at it and suck up the occasional frame hitch / thermal burn / etc. I like to think we had a reputation for being pretty thoughtful and careful with our review as well, and didn't give it lightly to misguided ideas which caused long-term problems. But we've got a greater diversity in userspace these days, and that's no bad thing. If the best vehicle to demonstrate HDR GPU rendering is gamescope, then use gamescope as that vehicle. We'll be there if we can, and if it makes sense for us, but it's not a requirement. Cheers, Daniel
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
Hi, On Tue, 14 Feb 2023 at 16:57, Harry Wentland wrote: > On 2/14/23 10:49, Sebastian Wick wrote: > From what I've seen recently I am inclined to favor an incremental > approach more. The reason is that any API, or portion thereof, is > useless unless it's enabled full stack. When it isn't it becomes > dead code quickly, or never really works because we overlooked > one thing. The colorspace debacle shows how even something as > simple as extra enum values in KMS APIs shouldn't be added unless > someone in a canonical upstream project actually uses them. I > would argue that such a canonical upstream project actually has > to be a production environment and not something like Weston. Just to chime in as well that it is a real production environment; it's probably actually shipped the most of any compositor by a long way. It doesn't have much place on the desktop, but it does live in planes, trains, automobiles, digital signage, kiosks, STBs/TVs, and about a billion other places you might not have expected. Probably the main factor that joins all these together - apart from not having much desktop-style click-and-drag reconfigurable UI - is that we need to use the hardware pipeline as efficiently as possible, because either we don't have the memory bandwidth to burn like desktops, or we need to minimise it for power/thermal reasons. Given that, we don't really want to paint ourselves into a corner with incremental solutions that mean we can't do fully efficient things later. We're also somewhat undermanned, and we've been using our effort to try to make sure that the full solution - including full colour-managed pathways for things like movie and TV post-prod composition, design, etc - is possible at some point through the full Wayland ecosystem at some point. The X11 experience was so horribly botched that it wasn't really possible without a complete professional setup, and that's something I personally don't want to see. However ... > I could see us getting to a fully new color pipeline API but > the only way to do that is with a development model that supports > it. While upstream needs to be our ultimate goal, a good way > to bring in new APIs and ensure a full-stack implementation is > to develop them in a downstream production kernel, alongside > userspace that makes use of it. Once the implementation is > proven in the downstream repos it can then go upstream. This > brings new challenges, though, as things don't get wide > testing and get out of sync with upstream quickly. The > alternative is the incremental approach. > > We should look at this from a use-case angle, similar to what > the gamescope guys are doing. Small steps, like: > 1) Add HDR10 output (PQ, BT.2020) to the display > 2) Add ability to do sRGB linear blending > 3) Add ability to do sRGB and PQ linear blending > 4) Post-blending 3D LUT > 5) Pre-blending 3D LUT > > At each stage the whole stack needs to work together in production. Personally, I do think at this stage we probably have enough of an understanding to be able to work with an intermediate solution. We just need to think hard about what that intermediate solution is - making sure that we don't end up in the same tangle of impossible semantics like the old 'broadcast RGB' / colorspace / HDR properties which were never thought through - so that it is something we can build on rather than something we have to work around. But it would be really good to make HDR10/HDR10+ media and HDR games work on HDR displays, yeah. Cheers, Daniel
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
Hi, On Tue, 14 Feb 2023 at 16:57, Harry Wentland wrote: > On 2/14/23 10:49, Sebastian Wick wrote: > From what I've seen recently I am inclined to favor an incremental > approach more. The reason is that any API, or portion thereof, is > useless unless it's enabled full stack. When it isn't it becomes > dead code quickly, or never really works because we overlooked > one thing. The colorspace debacle shows how even something as > simple as extra enum values in KMS APIs shouldn't be added unless > someone in a canonical upstream project actually uses them. I > would argue that such a canonical upstream project actually has > to be a production environment and not something like Weston. Just to chime in as well that it is a real production environment; it's probably actually shipped the most of any compositor by a long way. It doesn't have much place on the desktop, but it does live in planes, trains, automobiles, digital signage, kiosks, STBs/TVs, and about a billion other places you might not have expected. Probably the main factor that joins all these together - apart from not having much desktop-style click-and-drag reconfigurable UI - is that we need to use the hardware pipeline as efficiently as possible, because either we don't have the memory bandwidth to burn like desktops, or we need to minimise it for power/thermal reasons. Given that, we don't really want to paint ourselves into a corner with incremental solutions that mean we can't do fully efficient things later. We're also somewhat undermanned, and we've been using our effort to try to make sure that the full solution - including full colour-managed pathways for things like movie and TV post-prod composition, design, etc - is possible at some point through the full Wayland ecosystem at some point. The X11 experience was so horribly botched that it wasn't really possible without a complete professional setup, and that's something I personally don't want to see. However ... > I could see us getting to a fully new color pipeline API but > the only way to do that is with a development model that supports > it. While upstream needs to be our ultimate goal, a good way > to bring in new APIs and ensure a full-stack implementation is > to develop them in a downstream production kernel, alongside > userspace that makes use of it. Once the implementation is > proven in the downstream repos it can then go upstream. This > brings new challenges, though, as things don't get wide > testing and get out of sync with upstream quickly. The > alternative is the incremental approach. > > We should look at this from a use-case angle, similar to what > the gamescope guys are doing. Small steps, like: > 1) Add HDR10 output (PQ, BT.2020) to the display > 2) Add ability to do sRGB linear blending > 3) Add ability to do sRGB and PQ linear blending > 4) Post-blending 3D LUT > 5) Pre-blending 3D LUT > > At each stage the whole stack needs to work together in production. Personally, I do think at this stage we probably have enough of an understanding to be able to work with an intermediate solution. We just need to think hard about what that intermediate solution is - making sure that we don't end up in the same tangle of impossible semantics like the old 'broadcast RGB' / colorspace / HDR properties which were never thought through - so that it is something we can build on rather than something we have to work around. But it would be really good to make HDR10/HDR10+ media and HDR games work on HDR displays, yeah. Cheers, Daniel
Re: [PATCH v3 0/3] drm/rockchip: dw_hdmi: Add 4k@30 support
Hi Sascha, On Mon, 6 Feb 2023 at 15:49, Sascha Hauer wrote: > On Mon, Feb 06, 2023 at 03:04:48PM +0100, Sascha Hauer wrote: > > I guess a first step would be to limit the maximum resolution of vopl > > to what the hardware can do. We would likely end up with 1080p by > > default then for the applications. > > I did that, but the result is not what I expected. Discarding a mode in > the connector means it won't show up in the connectors list of modes. > Discarding it in the CRTC though means the mode is still exposed by the > connector, but actually trying to use it then fails. > > This means when discarding the mode in the CRTC the screen stays black. > > I am not sure where I should go from here. You've done the right thing. Userspace should detect this and try with alternative CRTC routing. The kernel shouldn't be trying to solve this problem. Cheers, Daniel
[petsc-users] win32fe?
Hello all, I am currently having to figure out a way to get petsc working on windows, using compilers from the intel oneAPI package. This means new compiler names, such as "icx" for the c compiler and "ifx" for the fortran one. I see from the installation instructions, and from old notes from a collegue, how to set up the cygwin environment and to use, e.g., --with-cc="win32fe icl" (if using an older intel compiler) when configuring. Unfortunately,win32fe only workes with a number of fixed (older) compilers, and simply cannot be made to work with icx or ifx, as far as I can see. When I try to not use the compilers without win32fe (e.g. --withcc=icx"), I get a slew of problems when the configure script trys to compile and link test c programs. It seems that icx works fine, but gets extemely confused when having to target output files to, e.g., /tmp or /anywhere/non/trivial, when running under cygwin. Is this the sort of problem that win32fe is intended to solve? I can't find any source code for win32fe, nor really any explainations of what it's supposed to be doing or how it works. Since it looks like I have to find a way of making it work with icx and ifx, I'm a bit stuck. Can anyone provide any insight as to what/how win32 does, or where I can look for some? Many thanks, Daniel
Re: [RFC] drm/fourcc: Add a modifier for contiguous memory
Hi Randy, On Tue, 29 Nov 2022 at 10:11, Hsia-Jun Li wrote: > Currently, we assume all the pixel formats are multiple planes, devices > could support each component has its own memory plane. > But that may not apply for any device in the world. We could have a > device without IOMMU then this is not impossible. > > Besides, when we export an handle through the PRIME, the upstream > device(likes a capture card or camera) may not support non-contiguous > memory. It would be better to allocate the handle in contiguous memory > at the first time. > > We may think the memory allocation is done in user space, we could do > the trick there. But the dumb_create() sometimes is not the right API > for that. > > "Note that userspace is not allowed to use such objects for render > acceleration - drivers must create their own private ioctls for such a > use case." > "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 a hardware-specific ioctl to allocate suitable buffer objects." > > We need to relay on those device custom APIs then. It would be helpful > for their library to calculate the right size for contiguous memory. It > would be useful for the driver supports rendering dumb buffer as well. As a buffer can only have a single modifier, this isn't practical. Contiguous needs to be negotiated separately and out of band. See e.g. dma-heaps for this. Cheers, Daniel
Re: [PATCH v4 0/4] new subsystem for compute accelerator devices
Hi Oded, On Sat, 19 Nov 2022 at 20:44, Oded Gabbay wrote: > This is the fourth (and hopefully last) version of the patch-set to add the > new subsystem for compute accelerators. I removed the RFC headline as > I believe it is now ready for merging. > > Compare to v3, this patch-set contains one additional patch that adds > documentation regarding the accel subsystem. I hope it's good enough for > this stage. In addition, there were few very minor fixes according to > comments received on v3. > > The patches are in the following repo: > https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4 Series is: Acked-by: Daniel Stone Cheers, Daniel
Re: [PATCH] drm/fourcc: Document open source user waiver
On Wed, 23 Nov 2022 at 19:24, Daniel Vetter wrote: > It's a bit a FAQ, and we really can't claim to be the authoritative > source for allocating these numbers used in many standard extensions > if we tell closed source or vendor stacks in general to go away. > > Iirc this was already clarified in some vulkan discussions, but I > can't find that anywhere anymore. At least not in a public link. I seem to recall the policy being set in an IRC discussion at some point (or perhaps during the AFBC merge?). This is a good clarification of what we already do in practice. Acked-by: Daniel Stone
Re: Try to address the DMA-buf coherency problem
Hi Christian, On Fri, 28 Oct 2022 at 18:50, Christian König wrote: > Am 28.10.22 um 17:46 schrieb Nicolas Dufresne: > > Though, its not generically possible to reverse these roles. If you want to > > do > > so, you endup having to do like Android (gralloc) and ChromeOS (minigbm), > > because you will have to allocate DRM buffers that knows about importer > > specific > > requirements. See link [1] for what it looks like for RK3399, with Motion > > Vector > > size calculation copied from the kernel driver into a userspace lib > > (arguably > > that was available from V4L2 sizeimage, but this is technically difficult to > > communicate within the software layers). If you could let the decoder export > > (with proper cache management) the non-generic code would not be needed. > > Yeah, but I can also reverse the argument: > > Getting the parameters for V4L right so that we can share the image is > tricky, but getting the parameters so that the stuff is actually > directly displayable by GPUs is even trickier. > > Essentially you need to look at both sides and interference to get to a > common ground, e.g. alignment, pitch, width/height, padding, etc. > > Deciding from which side to allocate from is just one step in this > process. For example most dGPUs can't display directly from system > memory altogether, but it is possible to allocate the DMA-buf through > the GPU driver and then write into device memory with P2P PCI transfers. > > So as far as I can see switching importer and exporter roles and even > having performant extra fallbacks should be a standard feature of userspace. > > > Another case where reversing the role is difficult is for case where you > > need to > > multiplex the streams (let's use a camera to illustrate) and share that with > > multiple processes. In these uses case, the DRM importers are volatile, > > which > > one do you abuse to do allocation from ? In multimedia server like > > PipeWire, you > > are not really aware if the camera will be used by DRM or not, and if > > something > > "special" is needed in term of role inversion. It is relatively easy to deal > > with matching modifiers, but using downstream (display/gpu) as an exporter > > is > > always difficult (and require some level of abuse and guessing). > > Oh, very good point! Yeah we do have use cases for this where an input > buffer is both displayed as well as encoded. This is the main issue, yeah. For a standard media player, they would try to allocate through V4L2 and decode through that into locally-allocated buffers. All they know is that there's a Wayland server at the other end of a socket somewhere which will want to import the FD. The server does give you some hints along the way: it will tell you that importing into a particular GPU target device is necessary as the ultimate fallback, and importing into a particular KMS device is preferable as the optimal path to hit an overlay. So let's say that the V4L2 client does what you're proposing: it allocates a buffer chain, schedules a decode into that buffer, and passes it along to the server to import. The server fails to import the buffer into the GPU, and tells the client this. The client then ... well, it doesn't know that it needs to allocate within the GPU instead, but it knows that doing so might be one thing which would make the request succeed. But the client is just a video player. It doesn't understand how to allocate BOs for Panfrost or AMD or etnaviv. So without a universal allocator (again ...), 'just allocate on the GPU' isn't a useful response to the client. I fully understand your point about APIs like Vulkan not sensibly allowing bracketing, and that's fine. On the other hand, a lot of extant usecases (camera/codec -> GPU/display, GPU -> codec, etc) on Arm just cannot fulfill complete coherency. On a lot of these platforms, despite what you might think about the CPU/GPU capabilities, the bottleneck is _always_ memory bandwidth, so mandating extra copies is an absolute non-starter, and would instantly cripple billions of devices. Lucas has been pretty gentle, but to be more clear, this is not an option and won't be for at least the next decade. So we obviously need a third way at this point, because 'all devices must always be coherent' vs. 'cache must be an unknown' can't work. How about this as a suggestion: we have some unused flags in the PRIME ioctls. Can we add a flag for 'import must be coherent'? That flag wouldn't be set for the existing ecosystem Lucas/Nicolas/myself are talking about, where we have explicit handover points and users are fully able to perform cache maintenance. For newer APIs where it's not possible to properly express that bracketing, they would always set that flag (unless we add an API carve-out where the client promises to do whatever is required to maintain that). Would that be viable? Cheers, Daniel
Re: [PATCH v8] drm: Add initial ci/ subdirectory
Hi all, On Tue, 25 Oct 2022 at 08:32, Daniel Vetter wrote: > On Fri, 9 Sept 2022 at 19:18, Daniel Stone wrote: > > But equally - and sorry for not jumping on the IRC (?) discussion as I was > > in the middle of other stuff when it came up - I'm don't think this is the > > right plan. > > > > Mesa having all its CI in-tree makes sense, because merges happen rapidly > > to a single canonical tree. If the scripts need to be changed for whatever > > reason, we can merge something in under an hour and everyone immediately > > gets it. DRM is quite different though, given the forest of trees we have > > and the long merge paths between them. I worry that merging the CI scripts > > in-tree - especially for our initial attempt at it, when we're likely to > > need to make quite a lot of changes before it settles down to become a > > stable system that works for everyone - is shooting ourselves in the foot > > by limiting our flexibility. > > So the entire "we have multiple trees" is why I want at least the > gitlab-ci.yaml in tree, to force people to collaborate more on one > thing instead of everyone rolling their own fun and hacking stuff up. > And there's still tons of stuff outside in the separate repo, like the > lab status so Linus doesn't get a silly history of lab flapping. > > Also wrt "developers don't get the update right away due to > backmerge/pull delays", that's why integration trees like drm-tip or > linux-next exist. So maybe initially we should make sure the ci > patches go in through drm-misc, to maximize how many people see it. > And even for mesa it's not fully automatic, you still have the rebase > your branch if you picked a bad one for development (but yeah marge > does that if the MR is ready). If you're doing kernel development on a > linear tree instead of an integration tree, you're doing it very > wrong. > > What I think everyone agrees on is that we probably get the split > wrong and need to shuffle some files back, and that's something > we need to warn Linus about I guess. But somewhere we do need a split > between internal and external stuff, and personally I'd like if at > least the pure sw testing (build and virtual stuff) could be all in > upstream. > > > Given that it's currently very dependent on fd.o infrastructure (published > > ci-templates, the registry, the specific-tag runners for Collabora/CrOS > > DUTs, etc), there isn't much of a portability gain in bringing the scripts > > into the tree either. It's a good goal, but not where we are today. > > I don't think there's huge chances for any non-fdo gitlab anytime > soon. Once we get there we might need to figure out how to standardize > the hw lab interfacing, and if we have all the sw targets in upstream > that should help with shuffling stuff around for a hypothetical LF > gitlab CI (or whatever it is). Yep, having talked through things on IRC, I'm happy with where we are; let's give it a go and find out. Acked-by: Daniel Stone Cheers, Daniel
Bug#1021735: libffi upgrade breaks Wayland on aarch64
Hi, This libffi upgrade also completely breaks all use of Wayland on aarch64. We use libffi to dispatch protocol messages (requests received by the server and events received by the client) to native-code handlers, and we are now getting completely nonsensical values from it. Can this upgrade please be rolled back until it can be root-caused and fixed? Cheers, Daniel
Bug#1021735: libffi upgrade breaks Wayland on aarch64
Hi, This libffi upgrade also completely breaks all use of Wayland on aarch64. We use libffi to dispatch protocol messages (requests received by the server and events received by the client) to native-code handlers, and we are now getting completely nonsensical values from it. Can this upgrade please be rolled back until it can be root-caused and fixed? Cheers, Daniel
Bug#1021735: libffi upgrade breaks Wayland on aarch64
Hi, This libffi upgrade also completely breaks all use of Wayland on aarch64. We use libffi to dispatch protocol messages (requests received by the server and events received by the client) to native-code handlers, and we are now getting completely nonsensical values from it. Can this upgrade please be rolled back until it can be root-caused and fixed? Cheers, Daniel
Re: [PATCH v4] drm/mediatek: Add AFBC support to Mediatek DRM driver
Hi, On Fri, 14 Oct 2022 at 08:46, AngeloGioacchino Del Regno < angelogioacchino.delre...@collabora.com> wrote: > Il 13/10/22 21:31, Justin Green ha scritto: > > Signed-off-by: Justin Green > > Reviewed-by: AngeloGioacchino Del Regno < > angelogioacchino.delre...@collabora.com> > And also: Acked-by: Daniel Stone I was worried about INVALID being passed through, but for the most part it seems like it magically turns into LINEAR through either zero extension or explicit initialisation to zero. Cheers, Daniel
Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
Hi Justin, On Wed, 12 Oct 2022 at 20:12, Justin Green wrote: > @@ -226,6 +249,32 @@ int mtk_ovl_layer_check(struct device *dev, unsigned > int idx, > if (state->fb->format->is_yuv && rotation != 0) > return -EINVAL; > > + if (state->fb->modifier) { > Please spell this out explicitly as DRM_FORMAT_MOD_LINEAR. For some reason, we specify that 0 is explicitly block-linear, whilst INVALID ('unknown layout, figure it out by magic') is a non-zero value. So != 0 isn't a check for 'was a modifier explicitly specified', even if != 0 is almost always 'is this buffer non-linear'. + unsigned long long modifier; > + unsigned int fourcc; > u64, u32, but ... > + if (!ovl->data->supports_afbc) > + return -EINVAL; > + > + modifier = state->fb->modifier; > + > + if (modifier != > DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 | > + > AFBC_FORMAT_MOD_SPLIT | > + > AFBC_FORMAT_MOD_SPARSE)) > + return -EINVAL; > + > + fourcc = state->fb->format->format; > + if (fourcc != DRM_FORMAT_BGRA && > + fourcc != DRM_FORMAT_ABGR && > + fourcc != DRM_FORMAT_ARGB && > + fourcc != DRM_FORMAT_XRGB && > + fourcc != DRM_FORMAT_XBGR && > + fourcc != DRM_FORMAT_RGB888 && > + fourcc != DRM_FORMAT_BGR888) > + return -EINVAL; The core shouldn't allow a framebuffer to be created with a format/modifier pair which wasn't advertised, so these checks can be dropped. Except that it's never advertised. mtk_plane_init() passes a set of formats and modifiers to drm_universal_plane_init(); the AFBC modifier needs to be added here, as well as LINEAR and INVALID. Then you'll need to set the format_mod_supported() hook on the plane to filter out format/modifier pairs. After that, you should see (e.g. with drm_info) that you get an IN_FORMATS blob which advertises DRM_FORMAT_MOD_ARM_AFBC(...) as well as linear for DRM_FORMAT_XRGB, but only linear for DRM_FORMAT_RGB565. After doing that, you should see framebuffer creation fail for unsupported pairings, e.g. RGB565 + AFBC. > + bool is_afbc = pending->modifier; ... != DRM_FORMAT_MOD_LINEAR > @@ -52,6 +53,7 @@ static void mtk_plane_reset(struct drm_plane *plane) > > state->base.plane = plane; > state->pending.format = DRM_FORMAT_RGB565; > + state->pending.modifier = 0; > = DRM_FORMAT_MOD_LINEAR > @@ -120,21 +122,52 @@ static void mtk_plane_update_new_state(struct > drm_plane_state *new_state, > struct drm_gem_object *gem; > struct mtk_drm_gem_obj *mtk_gem; > unsigned int pitch, format; > + unsigned long long modifier; > u64 > + if (!modifier) { > modifier == DRM_FORMAT_MOD_LINEAR Cheers, Daniel
Re: [PATCH 4/5] drm/damage-helper: Do partial updates if framebuffer has not been changed
Hi, On Tue, 20 Sept 2022 at 15:31, Ville Syrjälä wrote: > On Tue, Sep 20, 2022 at 03:56:18PM +0200, Thomas Zimmermann wrote: > > Set partial updates on a plane if the framebuffer has not been changed > > on an atomic commit. If such a plane has damage clips, the driver will > > use them; otherwise the update is effectively empty. Planes that change > > their framebuffer still perform a full update. > > I have a feeling you're sort of papering over some inefficient > userspace that's asking updates on planes that don't actually > need them. I'm not sure adding more code for that is a particularly > great idea. Wouldn't it be better to just fix the userspace code? > I'm not sure why it would need to be 'fixed', when it's never been a property of the atomic API that you must minimise updates. Weston does this (dumps the full state in every time), and I know we're not the only ones. 'Fixing' it would require doing a bunch of diffing in userspace, because maintaining a delta and trying to unwind that delta across multiple TEST_ONLY runs, isn't much fun. It's certainly a far bigger diff than this patch. > Any property change on the plane could need a full plane > update as well (eg. some color mangement stuff etc.). So you'd > have to keep adding exceptions to that list whenever new > properties are added. > Eh, it's just a blob ID comparison. > And I think the semantics of the ioctl(s) has so far been that > basically any page flip (whether or not it actually changes > the fb) still ends up doing whatever flushing is needed to > guarantee the fb contents are up to date on the screen (if > someone sneaked in some front buffer rendering in between). > Ie. you could even use basically a nop page flip in place > of dirtyfb. > > Another thing the ioctls have always done is actually perform > the commit whether anything changed or nor. That is, they > still do all the same the same vblanky stuff and the commit > takes the same amount of time. Not sure if your idea is > to potentially short circuit the entire thing and make it > take no time at all? > I would expect it to always perform a commit, though. Cheers, Daniel
Re: [PATCH v8] drm: Add initial ci/ subdirectory
Hi, On Fri, 9 Sept 2022 at 15:15, Tomeu Vizoso wrote: > Also include a configuration file that points to the out-of-tree CI > scripts. > I think this para is outdated given ... v8: > - Move all files specific to testing the kernel into the kernel tree > (thus I have dropped the r-bs I had collected so far) > - Uprev Gitlab CI infrastructure scripts to the latest from Mesa But equally - and sorry for not jumping on the IRC (?) discussion as I was in the middle of other stuff when it came up - I'm don't think this is the right plan. Mesa having all its CI in-tree makes sense, because merges happen rapidly to a single canonical tree. If the scripts need to be changed for whatever reason, we can merge something in under an hour and everyone immediately gets it. DRM is quite different though, given the forest of trees we have and the long merge paths between them. I worry that merging the CI scripts in-tree - especially for our initial attempt at it, when we're likely to need to make quite a lot of changes before it settles down to become a stable system that works for everyone - is shooting ourselves in the foot by limiting our flexibility. Given that it's currently very dependent on fd.o infrastructure (published ci-templates, the registry, the specific-tag runners for Collabora/CrOS DUTs, etc), there isn't much of a portability gain in bringing the scripts into the tree either. It's a good goal, but not where we are today. Cheers, Daniel
Re: Meeting (BOF) at Plumbers Dublin to discuss backlight brightness as connector object property RFC?
On Fri, 9 Sept 2022 at 12:50, Simon Ser wrote: > On Friday, September 9th, 2022 at 12:23, Hans de Goede < > hdego...@redhat.com> wrote: > > "people using > > non fully integrated desktop environments like e.g. sway often use custom > > scripts binded to hotkeys to get functionality like the brightness > > up/down keyboard hotkeys changing the brightness. This typically involves > > e.g. the xbacklight utility. > > > > Even if the xbacklight utility is ported to use kms with the new > connector > > object brightness properties then this still will not work because > > changing the properties will require drm-master rights and e.g. sway will > > already hold those." > > I don't think this is a good argument. Sway (which I'm a maintainer of) > can add a command to change the backlight, and then users can bind their > keybinding to that command. This is not very different from e.g. a > keybind to switch on/off a monitor. > > We can also standardize a protocol to change the backlight across all > non-fully-integrated desktop environments (would be a simple addition > to output-power-management [1]), so that a single tool can work for > multiple compositors. Yeah, I mean, as one of the main people arguing that non-fully-integrated desktops are not the design we want, I agree with Simon. Cheers, Daniel