Hi all, On 11 June 2016 at 01:21, Alex Goins <[email protected]> wrote: > Reverse PRIME seems to be designed with discrete graphics as a sink in > mind, designed to do an extra copy from sysmem to vidmem to prevent a > discrete chip from needing to scan out from sysmem. > > The criteria it used to detect this case is if we are a GPU screen and > Glamor accelerated. It's possible for i915 to fulfill these conditions, > despite the fact that the additional copy doesn't make sense for i915. > > Normally, you could just set AccelMethod = none as an option for the device > and call it a day. However, when running with modesetting as both the sink > and the source, Glamor must be enabled. > > Ideally, you would be able to set AccelMethod individually for devices > using the same driver, but there seems to be a bug in X option parsing that > makes all devices on a driver inherit the options from the first detected > device. Thus, glamor needs to be enabled for all or for none until that bug > (if it's even a bug) is fixed. > > Nonetheless, it probably doesn't make sense to do the extra copy on i915 > even if Glamor is enabled for the device, so this is more user friendly by > not requiring users to disable acceleration for i915. > IMHO the proposed patch does not sound like a reasonable long-term solution. Ideally the driver will expose a flag, based on which Xorg will enable/disable reverse prime. That said, as a short-term fix this is fine, barring the issues mentioned below.
> v1: N/A > v2: N/A > v3: N/A > v4: Initial commit > v5: Unchanged > v6: Rebase onto ToT > > Signed-off-by: Alex Goins <[email protected]> > --- > hw/xfree86/drivers/modesetting/driver.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/hw/xfree86/drivers/modesetting/driver.c > b/hw/xfree86/drivers/modesetting/driver.c > index f7abd1e..403cb96 100644 > --- a/hw/xfree86/drivers/modesetting/driver.c > +++ b/hw/xfree86/drivers/modesetting/driver.c > @@ -1378,9 +1378,13 @@ ScreenInit(ScreenPtr pScreen, int argc, char **argv) > xf86DrvMsg(pScrn->scrnIndex, X_ERROR, > "Failed to initialize the Present extension.\n"); > } > - /* enable reverse prime if we are a GPU screen, and accelerated */ > - if (pScreen->isGPU) > - ms->drmmode.reverse_prime_offload_mode = TRUE; > + /* enable reverse prime if we are a GPU screen, and accelerated, and > not > + * i915. i915 is happy scanning out from sysmem. */ > + if (pScreen->isGPU) { > + drmVersionPtr version = drmGetVersion(ms->drmmode.fd); > + if (strncmp("i915", version->name, version->name_len)) 'version' can be null. > + ms->drmmode.reverse_prime_offload_mode = TRUE; > + } We're leaking 'version'. Same two apply for the UDL patch. Regards, Emil _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
