[Bug 75226] Dark rendering of War for the Overworld
https://bugs.freedesktop.org/show_bug.cgi?id=75226 Emil Velikov changed: What|Removed |Added Attachment #94458|text/plain |application/octet-stream mime type|| -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/f5ca5d15/attachment.html>
AMD/AMD hybrid graphics
On Don, 2014-02-20 at 08:44 +0100, Boszormenyi Zoltan wrote: > 2014-02-20 06:47 keltez?ssel, Michel D?nzer ?rta: > > On Don, 2014-02-20 at 06:09 +0100, Boszormenyi Zoltan wrote: > >> 2014-02-20 04:20 keltez?ssel, Michel D?nzer ?rta: > >>> On Mit, 2014-02-19 at 11:56 +0100, Boszormenyi Zoltan wrote: > 2014-02-19 10:59 keltez?ssel, Michel D?nzer ?rta: > > On Mit, 2014-02-19 at 09:11 +0100, Boszormenyi Zoltan wrote: > > > >> Can Mesa/Xorg use both r600g and radeonsi at the same time? > > Yes, that seems to work fine for others. You may need Mesa 10.1 or newer > > though. > Do you mean mean with Mesa 9.2.5 and Xorg server 1.14.4 in > Fedora 20 at this time, it's not possible unless I compile my own > llvm-3.5 SVN, Mesa 10.1 or 10.2 GIT and Xorg 1.15 GIT? > >>> I don't think Xorg 1.15 is necessary, but it shouldn't hurt either. > >>> > >>> > Attached is the log from both 1.14.4 (FC20) and 1.15.0 (rawhide), [...] > >>> The log files end abruptly, so we need to see the X server stderr > >>> output. Assuming you're using gdm, it should be captured > >>> in /var/log/gdm*/:0.log . > >> FC20 has lightdm by default, here's /var/log/lightdm/x-0.log > > [...] > > > >> X: ../../../include/privates.h:122: dixGetPrivateAddr: Assertion > >> `key->initialized' failed. > > Can you get a backtrace for this assertion failure with gdb? See > > http://wiki.x.org/wiki/Development/Documentation/ServerDebugging/ [...] > #0 0x7fadc165f1c9 in __GI_raise (sig=sig at entry=6) at > ../nptl/sysdeps/unix/sysv/linux/raise.c:56 > #1 0x7fadc16608d8 in __GI_abort () at abort.c:89 > #2 0x7fadc1658126 in __assert_fail_base (fmt=0x7fadc17a98a0 "%s%s%s:%u: > %s%sAssertion > `%s' failed.\n%n", assertion=assertion at entry=0x5a4c53 "key->initialized", > file=file at entry=0x5a8dfc "../../../include/privates.h", line=line at > entry=122, > function=function at entry=0x5aaa90 <__PRETTY_FUNCTION__.8544> > "dixGetPrivateAddr") at > assert.c:92 > #3 0x7fadc16581d2 in __GI___assert_fail (assertion=assertion at > entry=0x5a4c53 > "key->initialized", file=file at entry=0x5a8dfc > "../../../include/privates.h", > line=line at entry=122, > function=function at entry=0x5aaa90 <__PRETTY_FUNCTION__.8544> > "dixGetPrivateAddr") at > assert.c:101 > #4 0x00424f70 in dixGetPrivateAddr (key=, > key=, > privates=0x169d558) at ../../../include/privates.h:122 > #5 0x004810eb in dixGetPrivateAddr (key=, > key=, > privates=0x169d558) at xf86cmap.c:239 > #6 dixSetPrivate (val=, key=0x822f80 , > privates=0x169d558) at ../../../include/privates.h:148 > #7 xf86HandleColormaps (pScreen=pScreen at entry=0x169d180, > maxColors=maxColors at entry=256, > sigRGBbits=10, loadPalette=loadPalette at entry=0x7fadbcd37ea0 > , > setOverscan=setOverscan at entry=0x0, flags=flags at entry=3) at > xf86cmap.c:184 > #8 0x7fadbcd3b32c in drmmode_setup_colormap (pScreen=pScreen at > entry=0x169d180, > pScrn=pScrn at entry=0x169c930) at drmmode_display.c:1990 > #9 0x7fadbcd370ef in RADEONScreenInit_KMS (pScreen=pScreen at > entry=0x169d180, > argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at radeon_kms.c:1366 > #10 0x00438b4d in AddGPUScreen (pfnInit=0x7fadbcd36c60 > , > argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at dispatch.c:3874 > #11 0x0047aa9f in InitOutput (pScreenInfo=pScreenInfo at > entry=0x82dd60 , > argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at xf86Init.c:886 Hmm, looks like there's confusion about how colormaps are supposed to be handled. Dave, any ideas? -- Earthling Michel D?nzer| http://www.amd.com Libre software enthusiast |Mesa and X developer
AMD/AMD hybrid graphics
On Fri, Feb 21, 2014 at 12:25 PM, Michel D?nzer wrote: > On Don, 2014-02-20 at 08:44 +0100, Boszormenyi Zoltan wrote: >> 2014-02-20 06:47 keltez?ssel, Michel D?nzer ?rta: >> > On Don, 2014-02-20 at 06:09 +0100, Boszormenyi Zoltan wrote: >> >> 2014-02-20 04:20 keltez?ssel, Michel D?nzer ?rta: >> >>> On Mit, 2014-02-19 at 11:56 +0100, Boszormenyi Zoltan wrote: >> 2014-02-19 10:59 keltez?ssel, Michel D?nzer ?rta: >> > On Mit, 2014-02-19 at 09:11 +0100, Boszormenyi Zoltan wrote: >> > >> >> Can Mesa/Xorg use both r600g and radeonsi at the same time? >> > Yes, that seems to work fine for others. You may need Mesa 10.1 or >> > newer >> > though. >> Do you mean mean with Mesa 9.2.5 and Xorg server 1.14.4 in >> Fedora 20 at this time, it's not possible unless I compile my own >> llvm-3.5 SVN, Mesa 10.1 or 10.2 GIT and Xorg 1.15 GIT? >> >>> I don't think Xorg 1.15 is necessary, but it shouldn't hurt either. >> >>> >> >>> >> Attached is the log from both 1.14.4 (FC20) and 1.15.0 (rawhide), [...] >> >>> The log files end abruptly, so we need to see the X server stderr >> >>> output. Assuming you're using gdm, it should be captured >> >>> in /var/log/gdm*/:0.log . >> >> FC20 has lightdm by default, here's /var/log/lightdm/x-0.log >> > [...] >> > >> >> X: ../../../include/privates.h:122: dixGetPrivateAddr: Assertion >> >> `key->initialized' failed. >> > Can you get a backtrace for this assertion failure with gdb? See >> > http://wiki.x.org/wiki/Development/Documentation/ServerDebugging/ > > [...] > >> #0 0x7fadc165f1c9 in __GI_raise (sig=sig at entry=6) at >> ../nptl/sysdeps/unix/sysv/linux/raise.c:56 >> #1 0x7fadc16608d8 in __GI_abort () at abort.c:89 >> #2 0x7fadc1658126 in __assert_fail_base (fmt=0x7fadc17a98a0 "%s%s%s:%u: >> %s%sAssertion >> `%s' failed.\n%n", assertion=assertion at entry=0x5a4c53 "key->initialized", >> file=file at entry=0x5a8dfc "../../../include/privates.h", line=line at >> entry=122, >> function=function at entry=0x5aaa90 <__PRETTY_FUNCTION__.8544> >> "dixGetPrivateAddr") at >> assert.c:92 >> #3 0x7fadc16581d2 in __GI___assert_fail (assertion=assertion at >> entry=0x5a4c53 >> "key->initialized", file=file at entry=0x5a8dfc >> "../../../include/privates.h", >> line=line at entry=122, >> function=function at entry=0x5aaa90 <__PRETTY_FUNCTION__.8544> >> "dixGetPrivateAddr") at >> assert.c:101 >> #4 0x00424f70 in dixGetPrivateAddr (key=, >> key=, >> privates=0x169d558) at ../../../include/privates.h:122 >> #5 0x004810eb in dixGetPrivateAddr (key=, >> key=, >> privates=0x169d558) at xf86cmap.c:239 >> #6 dixSetPrivate (val=, key=0x822f80 , >> privates=0x169d558) at ../../../include/privates.h:148 >> #7 xf86HandleColormaps (pScreen=pScreen at entry=0x169d180, >> maxColors=maxColors at entry=256, >> sigRGBbits=10, loadPalette=loadPalette at entry=0x7fadbcd37ea0 >> , >> setOverscan=setOverscan at entry=0x0, flags=flags at entry=3) at >> xf86cmap.c:184 >> #8 0x7fadbcd3b32c in drmmode_setup_colormap (pScreen=pScreen at >> entry=0x169d180, >> pScrn=pScrn at entry=0x169c930) at drmmode_display.c:1990 >> #9 0x7fadbcd370ef in RADEONScreenInit_KMS (pScreen=pScreen at >> entry=0x169d180, >> argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at radeon_kms.c:1366 >> #10 0x00438b4d in AddGPUScreen (pfnInit=0x7fadbcd36c60 >> , >> argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at dispatch.c:3874 >> #11 0x0047aa9f in InitOutput (pScreenInfo=pScreenInfo at >> entry=0x82dd60 , >> argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at xf86Init.c:886 > > Hmm, looks like there's confusion about how colormaps are supposed to be > handled. > > > Dave, any ideas? xf86_crtc_supports_gamma probably stops us from ever getting into that function, but in the case where we have 0 crtcs I could see fallout. Either fix the DDX to not call colormap handling if we have 0 crtcs and/or fix the server to not install bother with colormaps if we have 0 crtcs. Dave.
[Bug 74868] r600g: Diablo III Crashes After a few minutes
https://bugs.freedesktop.org/show_bug.cgi?id=74868 Nick Tenney changed: What|Removed |Added Attachment #94142|0 |1 is obsolete|| --- Comment #11 from Nick Tenney --- Created attachment 94468 --> https://bugs.freedesktop.org/attachment.cgi?id=94468&action=edit D3 Valgrind output with debugging symbols Recompiled and ran Valgrind again. The resulting Valgrind looks more informative than previous attempts. Let me know if anything else would be helpful. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/4f1d8f76/attachment.html>
[PATCH] nouveau, ACPI: fix regression caused by b072e53
On Fri, Feb 21, 2014 at 6:27 AM, Rafael J. Wysocki wrote: > On 2/20/2014 10:23 AM, Jiang Liu wrote: >> >> Fix regression caused by commit b072e53, which breaks loading nouveau >> driver on optimus laptops. >> >> On some platforms, ACPI _DSM method (nouveau_op_dsm_muid, function 0) >> has special requirements on the fourth parameter, which is different >> from ACPI specifications. So revert to the private implementation >> to check availability of _DSM functions instead of using common >> acpi_check_dsm() interface. >> >> Reported-and-Tested-by: Maarten Lankhorst >> >> Signed-off-by: Jiang Liu > > > I'm taking this, because the commit that introduced the regression went in > through my tree. > > In the future I'll appreciate CCing ACPI-related patches to linux-acpi, > however. Thanks, Acked-by: Dave Airlie Dave.
[PATCH 05/13] drm: provide device-refcount
On Wed, Jan 29, 2014 at 03:01:52PM +0100, David Herrmann wrote: [...] > diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c [...] > @@ -486,12 +490,10 @@ EXPORT_SYMBOL(drm_dev_alloc); > * @dev: DRM device to free > * > * Free a DRM device that has previously been allocated via drm_dev_alloc(). > - * You must not use kfree() instead or you will leak memory. > - * > - * This must not be called once the device got registered. Use drm_put_dev() > - * instead, which then calls drm_dev_free(). > + * This may not be called directly, you must use drm_dev_ref() and > + * drm_dev_unref() to gain and drop references to the object. > */ > -void drm_dev_free(struct drm_device *dev) > +static void drm_dev_free(struct drm_device *dev) With the function turning static it can't be called from anywhere else but this file, so perhaps the last sentence of the comment can now go away as well? Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/ff910e25/attachment.pgp>
[PATCH 06/13] drm: add minor-lookup/release helpers
On Wed, Jan 29, 2014 at 03:01:53PM +0100, David Herrmann wrote: [...] > diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c > index c51333e..d3232b6 100644 > --- a/drivers/gpu/drm/drm_stub.c > +++ b/drivers/gpu/drm/drm_stub.c > @@ -356,6 +356,45 @@ static void drm_unplug_minor(struct drm_minor *minor) > } > > /** > + * drm_minor_acquire - Acquire a DRM minor > + * @minor_id: Minor ID of the DRM-minor > + * > + * Looks up the given minor-ID and returns the respective DRM-minor object. > The > + * refence-count of the underlying device is increased so you must release > this > + * object with drm_minor_release(). > + * > + * As long as you hold this minor, it is guaranteed that the object and the > + * minor->dev pointer will stay valid! However, the device may get unplugged > and > + * unregistered while you hold the minor. > + * > + * Returns: > + * Pointer to minor-object with increased device-refcount, or PTR_ERR on > + * failure. > + */ > +struct drm_minor *drm_minor_acquire(unsigned int minor_id) > +{ > + struct drm_minor *minor; > + > + minor = idr_find(&drm_minors_idr, minor_id); > + if (!minor) > + return ERR_PTR(-ENODEV); > + > + drm_dev_ref(minor->dev); Is it possible that somebody would drop the last reference on the device right between the idr_find() call and drm_dev_ref()? In which case both the device and the minor will have become invalid when drm_dev_ref() is called. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/c0ae5d93/attachment.pgp>
[PATCH 09/13] drm: rename drm_unplug/get_minor() to drm_minor_register/unregister()
On Wed, Jan 29, 2014 at 03:01:56PM +0100, David Herrmann wrote: > drm_get_minor() no longer allocates objects, and drm_unplug_minor() is now > the exact reverse of it. Rename it to _register/unregister() so their > name actually says what they do. > > Furthermore, remove the direct minor-ptr and instead pass the minor-type. > This way we know the actualy slot of the minor and can reset it if Nit: "actualy" -> "actual". Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/5c0820ef/attachment.pgp>
[PATCH 11/13] drm: remove redundant minor->device field
On Wed, Feb 12, 2014 at 02:36:24PM +0100, Daniel Vetter wrote: > On Wed, Jan 29, 2014 at 03:01:58PM +0100, David Herrmann wrote: > > Whenever we access minor->device, we are in a minor->kdev->...->fops > > callback so the minor->kdev pointer *must* be valid. Thus, simply use > > minor->kdev->devt instead of minor->device and remove the redundant field. > > > > Signed-off-by: David Herrmann > > I think this is simply compat cruft from the days when the drm core was > still shared with the *bsds. With the one patch I've commented on all > patches up to this one are > > Reviewed-by: Daniel Vetter > > As discussed on irc I think we don't want to have stable minor ids really, > userspace simply needs to inquire udev to get at the right > render/control/legacy node it wants. Does that mean we should go all the way and don't keep the +64 (for control) and +128 (for render nodes) offsets either? Should it be possible to have a /dev/dri directory that looks somewhat like this: /dev/dri/card0(GPU#0, legacy) /dev/dri/card1(GPU#1, legacy) /dev/dri/render0 (GPU#1, render) ? Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/8e711f5c/attachment.pgp>
[PATCH 00/13] DRM Reliable Minor-IDs
On Wed, Jan 29, 2014 at 03:01:47PM +0100, David Herrmann wrote: > Hi > > I was looking into our minor-allocation code and it has one major drawback: > char-dev minor-IDs are unreliable. The +128 hacks we use in user-space to > calculate render-node IDs based on the original card just does not work. > > Instead of allocating dummy IDs for each driver, I went ahead and tried to fix > it properly. So this series changes the minor-ID management to just allocate a > single dev->minor_base ID instead of one ID per "drm_minor". Now we can use > this > base to calculate the correct offset minor-ID for each existing DRM-minor. > > While at it, I introduced drm-refcounts to make minor-lookup independent of > drm_global_mutex. This is still not finished (and dev->open_count still > exists) > but I already have the next patches waiting here. > > Comments welcome! > > Branch is also available here: > http://cgit.freedesktop.org/~dvdhrm/linux/log/?h=minor > If someone could pull this into their tree and push to Fengguang's > test-framework, I'd appreciate it a lot! I'm still waiting for a reply from > him. The series: Tested-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/fa247aa0/attachment-0001.pgp>
[PATCH 06/13] drm: add minor-lookup/release helpers
Hi On Fri, Feb 21, 2014 at 8:09 AM, Thierry Reding wrote: > On Wed, Jan 29, 2014 at 03:01:53PM +0100, David Herrmann wrote: > [...] >> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c >> index c51333e..d3232b6 100644 >> --- a/drivers/gpu/drm/drm_stub.c >> +++ b/drivers/gpu/drm/drm_stub.c >> @@ -356,6 +356,45 @@ static void drm_unplug_minor(struct drm_minor *minor) >> } >> >> /** >> + * drm_minor_acquire - Acquire a DRM minor >> + * @minor_id: Minor ID of the DRM-minor >> + * >> + * Looks up the given minor-ID and returns the respective DRM-minor object. >> The >> + * refence-count of the underlying device is increased so you must release >> this >> + * object with drm_minor_release(). >> + * >> + * As long as you hold this minor, it is guaranteed that the object and the >> + * minor->dev pointer will stay valid! However, the device may get >> unplugged and >> + * unregistered while you hold the minor. >> + * >> + * Returns: >> + * Pointer to minor-object with increased device-refcount, or PTR_ERR on >> + * failure. >> + */ >> +struct drm_minor *drm_minor_acquire(unsigned int minor_id) >> +{ >> + struct drm_minor *minor; >> + >> + minor = idr_find(&drm_minors_idr, minor_id); >> + if (!minor) >> + return ERR_PTR(-ENODEV); >> + >> + drm_dev_ref(minor->dev); > > Is it possible that somebody would drop the last reference on the device > right between the idr_find() call and drm_dev_ref()? In which case both > the device and the minor will have become invalid when drm_dev_ref() is > called. No, it's locked via the drm_global_mutex. And later I introduce a spinlock here that protects this in case we remove the global lock. Thanks David
[PATCH 06/13] drm: add minor-lookup/release helpers
On Fri, Feb 21, 2014 at 08:34:15AM +0100, David Herrmann wrote: > Hi > > On Fri, Feb 21, 2014 at 8:09 AM, Thierry Reding > wrote: > > On Wed, Jan 29, 2014 at 03:01:53PM +0100, David Herrmann wrote: > > [...] > >> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c > >> index c51333e..d3232b6 100644 > >> --- a/drivers/gpu/drm/drm_stub.c > >> +++ b/drivers/gpu/drm/drm_stub.c > >> @@ -356,6 +356,45 @@ static void drm_unplug_minor(struct drm_minor *minor) > >> } > >> > >> /** > >> + * drm_minor_acquire - Acquire a DRM minor > >> + * @minor_id: Minor ID of the DRM-minor > >> + * > >> + * Looks up the given minor-ID and returns the respective DRM-minor > >> object. The > >> + * refence-count of the underlying device is increased so you must > >> release this > >> + * object with drm_minor_release(). > >> + * > >> + * As long as you hold this minor, it is guaranteed that the object and > >> the > >> + * minor->dev pointer will stay valid! However, the device may get > >> unplugged and > >> + * unregistered while you hold the minor. > >> + * > >> + * Returns: > >> + * Pointer to minor-object with increased device-refcount, or PTR_ERR on > >> + * failure. > >> + */ > >> +struct drm_minor *drm_minor_acquire(unsigned int minor_id) > >> +{ > >> + struct drm_minor *minor; > >> + > >> + minor = idr_find(&drm_minors_idr, minor_id); > >> + if (!minor) > >> + return ERR_PTR(-ENODEV); > >> + > >> + drm_dev_ref(minor->dev); > > > > Is it possible that somebody would drop the last reference on the device > > right between the idr_find() call and drm_dev_ref()? In which case both > > the device and the minor will have become invalid when drm_dev_ref() is > > called. > > No, it's locked via the drm_global_mutex. And later I introduce a > spinlock here that protects this in case we remove the global lock. Indeed. Thanks for clarifying. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/17b1780a/attachment.pgp>
[RFC 00/12] Support render-node only drivers
From: Thierry Reding This series builds on top of David's reliable DRM minor series: [PATCH 00/13] DRM Reliable Minor-IDs Tegra K1 has a Kepler-type GPU without any display engine. Instead it reuses the Tegra display engine. That means that effectively the GPU becomes a render-node only device. In order to reflect that, it would be preferable for the associated /dev/dri/cardX node not to show up. To achieve that, the DRIVER_MODESET feature needs to be removed from the GPU driver, but that unfortunately implies activating a bunch of legacy behaviour for pre-KMS drivers. To allow for drivers that don't support modesetting IOCTLs (because they drive no output) but which aren't legacy either, the meaning of the DRIVER_MODESET needs to be redefined. This series attempts to do so by first renaming DRM_MINOR_LEGACY to DRM_MINOR_PRIMARY to more accurately reflect its purpose. Legacy and modesetting are then decoupled by introducing a DRIVER_LEGACY driver feature that can be set by truly legacy drivers. This allows the old DRIVER_MODESET feature to advertise support only for modesetting functionality, without implying that it is a non-legacy driver. After all the drivers have been updated, the core can be modified to create the primary minor only when DRIVER_MODESET is available. The remainder of the series cleans up some drm_core_check_feature() usage and drop some unused code related to that. Thierry Thierry Reding (12): drm: Rename DRM_MINOR_LEGACY to DRM_MINOR_PRIMARY drm: Introduce DRIVER_LEGACY feature drm/i915: Mark as legacy if KMS is disabled drm: Separate DRIVER_MODESET and DRIVER_LEGACY drm: Create primary minor only if mode-setting is supported drm: Remove gratuituous blank line drm: Use drm_core_check_feature() where possible drm/exynos: Remove dead code drm/gma500: Remove dead code drm/i810: Remove dead code drm/i915: Remove dead code drm/qxl: Remove dead code drivers/gpu/drm/drm_bufs.c | 12 +-- drivers/gpu/drm/drm_crtc.c | 1 - drivers/gpu/drm/drm_dma.c | 4 ++-- drivers/gpu/drm/drm_fops.c | 14 ++-- drivers/gpu/drm/drm_gem.c | 6 +++--- drivers/gpu/drm/drm_irq.c | 12 +-- drivers/gpu/drm/drm_pci.c | 12 +-- drivers/gpu/drm/drm_scatter.c | 6 +++--- drivers/gpu/drm/drm_stub.c | 24 +++-- drivers/gpu/drm/drm_sysfs.c | 8 +++ drivers/gpu/drm/exynos/exynos_drm_gem.c | 10 - drivers/gpu/drm/gma500/gem.c| 3 --- drivers/gpu/drm/i810/i810_dma.c | 7 -- drivers/gpu/drm/i810/i810_drv.c | 3 ++- drivers/gpu/drm/i915/i915_dma.c | 38 - drivers/gpu/drm/i915/i915_drv.c | 18 ++-- drivers/gpu/drm/i915/i915_gem.c | 17 +++ drivers/gpu/drm/i915/i915_gem_context.c | 6 -- drivers/gpu/drm/i915/i915_irq.c | 6 +++--- drivers/gpu/drm/i915/i915_suspend.c | 15 ++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- drivers/gpu/drm/mga/mga_drv.c | 3 ++- drivers/gpu/drm/qxl/qxl_kms.c | 4 drivers/gpu/drm/radeon/radeon_drv.c | 3 ++- drivers/gpu/drm/savage/savage_drv.c | 2 +- drivers/gpu/drm/sis/sis_drv.c | 2 +- drivers/gpu/drm/tdfx/tdfx_drv.c | 1 + drivers/gpu/drm/via/via_drv.c | 2 +- include/drm/drmP.h | 3 ++- 29 files changed, 112 insertions(+), 132 deletions(-) -- 1.8.4.2
[RFC 01/12] drm: Rename DRM_MINOR_LEGACY to DRM_MINOR_PRIMARY
From: Thierry Reding The term "legacy" is overloaded in the context of DRM. DRM_MINOR_LEGACY doesn't accurately describe the use of the minor. The associated minor is the primary minor for a device, as reflected by the .primary field of struct drm_device. For consistency, rename the enumeration value to DRM_MINOR_PRIMARY. Signed-off-by: Thierry Reding --- drivers/gpu/drm/drm_stub.c | 16 drivers/gpu/drm/drm_sysfs.c | 4 ++-- include/drm/drmP.h | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 2567ecb5c574..fd2f1758366d 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -319,7 +319,7 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev, unsigned int type) { switch (type) { - case DRM_MINOR_LEGACY: + case DRM_MINOR_PRIMARY: return &dev->primary; case DRM_MINOR_RENDER: return &dev->render; @@ -493,7 +493,7 @@ EXPORT_SYMBOL(drm_put_dev); void drm_unplug_dev(struct drm_device *dev) { /* for a USB device */ - drm_minor_unregister(dev, DRM_MINOR_LEGACY); + drm_minor_unregister(dev, DRM_MINOR_PRIMARY); drm_minor_unregister(dev, DRM_MINOR_RENDER); drm_minor_unregister(dev, DRM_MINOR_CONTROL); @@ -564,7 +564,7 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver, goto err_minors; } - ret = drm_minor_alloc(dev, DRM_MINOR_LEGACY); + ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY); if (ret) goto err_minors; @@ -592,7 +592,7 @@ err_ctxbitmap: err_ht: drm_ht_remove(&dev->map_hash); err_minors: - drm_minor_free(dev, DRM_MINOR_LEGACY); + drm_minor_free(dev, DRM_MINOR_PRIMARY); drm_minor_free(dev, DRM_MINOR_RENDER); drm_minor_free(dev, DRM_MINOR_CONTROL); drm_minor_free_base(dev); @@ -618,7 +618,7 @@ static void drm_dev_free(struct drm_device *dev) drm_ctxbitmap_cleanup(dev); drm_ht_remove(&dev->map_hash); - drm_minor_free(dev, DRM_MINOR_LEGACY); + drm_minor_free(dev, DRM_MINOR_PRIMARY); drm_minor_free(dev, DRM_MINOR_RENDER); drm_minor_free(dev, DRM_MINOR_CONTROL); drm_minor_free_base(dev); @@ -692,7 +692,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) if (ret) goto err_minors; - ret = drm_minor_register(dev, DRM_MINOR_LEGACY); + ret = drm_minor_register(dev, DRM_MINOR_PRIMARY); if (ret) goto err_minors; @@ -718,7 +718,7 @@ err_unload: if (dev->driver->unload) dev->driver->unload(dev); err_minors: - drm_minor_unregister(dev, DRM_MINOR_LEGACY); + drm_minor_unregister(dev, DRM_MINOR_PRIMARY); drm_minor_unregister(dev, DRM_MINOR_RENDER); drm_minor_unregister(dev, DRM_MINOR_CONTROL); out_unlock: @@ -752,7 +752,7 @@ void drm_dev_unregister(struct drm_device *dev) list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) drm_rmmap(dev, r_list->map); - drm_minor_unregister(dev, DRM_MINOR_LEGACY); + drm_minor_unregister(dev, DRM_MINOR_PRIMARY); drm_minor_unregister(dev, DRM_MINOR_RENDER); drm_minor_unregister(dev, DRM_MINOR_CONTROL); } diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index 9be02d9a78e8..c3f3d0b6a8fe 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -43,7 +43,7 @@ static int __drm_class_suspend(struct device *dev, pm_message_t state) struct drm_minor *drm_minor = to_drm_minor(dev); struct drm_device *drm_dev = drm_minor->dev; - if (drm_minor->type == DRM_MINOR_LEGACY && + if (drm_minor->type == DRM_MINOR_PRIMARY && !drm_core_check_feature(drm_dev, DRIVER_MODESET) && drm_dev->driver->suspend) return drm_dev->driver->suspend(drm_dev, state); @@ -84,7 +84,7 @@ static int drm_class_resume(struct device *dev) struct drm_minor *drm_minor = to_drm_minor(dev); struct drm_device *drm_dev = drm_minor->dev; - if (drm_minor->type == DRM_MINOR_LEGACY && + if (drm_minor->type == DRM_MINOR_PRIMARY && !drm_core_check_feature(drm_dev, DRIVER_MODESET) && drm_dev->driver->resume) return drm_dev->driver->resume(drm_dev); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index e5f6732f4757..92604c435ecc 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1010,7 +1010,7 @@ struct drm_driver { }; enum drm_minor_type { - DRM_MINOR_LEGACY, + DRM_MINOR_PRIMARY, DRM_MINOR_CONTROL, DRM_MINOR_RENDER, DRM_MINOR_CNT, -- 1.8.4.2
[RFC 02/12] drm: Introduce DRIVER_LEGACY feature
From: Thierry Reding Currently drivers that set the DRIVER_MODESET feature are considered to be non-legacy drivers. At the same time DRIVER_MODESET implies that the mode-setting IOCTLs are available. It is therefore not possible to distinguish between a non-legacy driver with full mode-setting support and a non-legacy driver without mode-setting functionality. To separate the meaning of "legacy" and "modeset", a new driver feature is introduced: DRIVER_LEGACY. The meaning of DRIVER_MODESET can then be changed to apply to the mode-setting functionality only, irrespective of whether it is legacy or not. Mark all legacy drivers appropriately. Signed-off-by: Thierry Reding --- drivers/gpu/drm/i810/i810_drv.c | 3 ++- drivers/gpu/drm/mga/mga_drv.c | 3 ++- drivers/gpu/drm/radeon/radeon_drv.c | 3 ++- drivers/gpu/drm/savage/savage_drv.c | 2 +- drivers/gpu/drm/sis/sis_drv.c | 2 +- drivers/gpu/drm/tdfx/tdfx_drv.c | 1 + drivers/gpu/drm/via/via_drv.c | 2 +- include/drm/drmP.h | 1 + 8 files changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c index 441ccf8f5bdc..fb21abfba414 100644 --- a/drivers/gpu/drm/i810/i810_drv.c +++ b/drivers/gpu/drm/i810/i810_drv.c @@ -58,7 +58,8 @@ static const struct file_operations i810_driver_fops = { static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | - DRIVER_HAVE_DMA, + DRIVER_HAVE_DMA | + DRIVER_LEGACY, .dev_priv_size = sizeof(drm_i810_buf_priv_t), .load = i810_driver_load, .lastclose = i810_driver_lastclose, diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c index 6b1a87c8aac5..0bd929e38d99 100644 --- a/drivers/gpu/drm/mga/mga_drv.c +++ b/drivers/gpu/drm/mga/mga_drv.c @@ -59,7 +59,8 @@ static const struct file_operations mga_driver_fops = { static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | - DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED, + DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | + DRIVER_LEGACY, .dev_priv_size = sizeof(drm_mga_buf_priv_t), .load = mga_driver_load, .unload = mga_driver_unload, diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 84a1bbb75f91..5afa997bab00 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -292,7 +292,8 @@ static const struct file_operations radeon_driver_old_fops = { static struct drm_driver driver_old = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | - DRIVER_HAVE_IRQ | DRIVER_HAVE_DMA | DRIVER_IRQ_SHARED, + DRIVER_HAVE_IRQ | DRIVER_HAVE_DMA | DRIVER_IRQ_SHARED | + DRIVER_LEGACY, .dev_priv_size = sizeof(drm_radeon_buf_priv_t), .load = radeon_driver_load, .firstopen = radeon_driver_firstopen, diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c index 3c030216e888..bedb800eb7d0 100644 --- a/drivers/gpu/drm/savage/savage_drv.c +++ b/drivers/gpu/drm/savage/savage_drv.c @@ -50,7 +50,7 @@ static const struct file_operations savage_driver_fops = { static struct drm_driver driver = { .driver_features = - DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA, + DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, .dev_priv_size = sizeof(drm_savage_buf_priv_t), .load = savage_driver_load, .firstopen = savage_driver_firstopen, diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index 756f787b7143..402f10b58d57 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -102,7 +102,7 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) } static struct drm_driver driver = { - .driver_features = DRIVER_USE_AGP, + .driver_features = DRIVER_USE_AGP | DRIVER_LEGACY, .load = sis_driver_load, .unload = sis_driver_unload, .open = sis_driver_open, diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index 3492ca5c46d3..fc61c476ff49 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -55,6 +55,7 @@ static const struct file_operations tdfx_driver_fops = { }; static struct drm_driver driver = { + .features = DRIVER_LEGACY, .fops = &tdfx_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c index 50abc2adfaee..c16ca40b8e07 100644 --- a/drivers/gpu/drm/via/via_drv.c +++ b/drivers/gpu/drm/via/via_drv.c @@ -73,7 +73,7 @@ static const struct file_operations via_driver_fops = { static struct drm_driver driver = { .driver_features = DR
[RFC 07/12] drm: Use drm_core_check_feature() where possible
From: Thierry Reding Wherever possible, use drm_core_check_feature() for consistency. Signed-off-by: Thierry Reding --- drivers/gpu/drm/drm_fops.c | 8 drivers/gpu/drm/drm_gem.c | 6 +++--- drivers/gpu/drm/drm_stub.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_gem.c | 4 ++-- drivers/gpu/drm/gma500/gem.c| 2 +- drivers/gpu/drm/i915/i915_gem_context.c | 4 ++-- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 46eee1da4ad7..b78ce8eaae81 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -242,7 +242,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp, init_waitqueue_head(&priv->event_wait); priv->event_space = 4096; /* set aside 4k for event buffer */ - if (dev->driver->driver_features & DRIVER_GEM) + if (drm_core_check_feature(dev, DRIVER_GEM)) drm_gem_open(dev, priv); if (drm_core_check_feature(dev, DRIVER_PRIME)) @@ -333,7 +333,7 @@ out_close: out_prime_destroy: if (drm_core_check_feature(dev, DRIVER_PRIME)) drm_prime_destroy_file_private(&priv->prime); - if (dev->driver->driver_features & DRIVER_GEM) + if (drm_core_check_feature(dev, DRIVER_GEM)) drm_gem_release(dev, priv); put_pid(priv->pid); kfree(priv); @@ -490,10 +490,10 @@ int drm_release(struct inode *inode, struct file *filp) drm_events_release(file_priv); - if (dev->driver->driver_features & DRIVER_MODESET) + if (drm_core_check_feature(dev, DRIVER_MODESET)) drm_fb_release(file_priv); - if (dev->driver->driver_features & DRIVER_GEM) + if (drm_core_check_feature(dev, DRIVER_GEM)) drm_gem_release(dev, file_priv); mutex_lock(&dev->ctxlist_mutex); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 5bbad873c798..44ca82f6a211 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -545,7 +545,7 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, struct drm_gem_close *args = data; int ret; - if (!(dev->driver->driver_features & DRIVER_GEM)) + if (!drm_core_check_feature(dev, DRIVER_GEM)) return -ENODEV; ret = drm_gem_handle_delete(file_priv, args->handle); @@ -567,7 +567,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, struct drm_gem_object *obj; int ret; - if (!(dev->driver->driver_features & DRIVER_GEM)) + if (!drm_core_check_feature(dev, DRIVER_GEM)) return -ENODEV; obj = drm_gem_object_lookup(dev, file_priv, args->handle); @@ -615,7 +615,7 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data, int ret; u32 handle; - if (!(dev->driver->driver_features & DRIVER_GEM)) + if (!drm_core_check_feature(dev, DRIVER_GEM)) return -ENODEV; mutex_lock(&dev->object_name_lock); diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 839460b774c5..9079a9f90e7b 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -614,7 +614,7 @@ EXPORT_SYMBOL(drm_dev_alloc); */ static void drm_dev_free(struct drm_device *dev) { - if (dev->driver->driver_features & DRIVER_GEM) + if (drm_core_check_feature(dev, DRIVER_GEM)) drm_gem_destroy(dev); drm_ctxbitmap_cleanup(dev); diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index 42d2904d88c7..07a4d0f92722 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -329,7 +329,7 @@ int exynos_drm_gem_map_offset_ioctl(struct drm_device *dev, void *data, DRM_DEBUG_KMS("handle = 0x%x, offset = 0x%lx\n", args->handle, (unsigned long)args->offset); - if (!(dev->driver->driver_features & DRIVER_GEM)) { + if (!drm_core_check_feature(dev, DRIVER_GEM)) { DRM_ERROR("does not support GEM.\n"); return -ENODEV; } @@ -396,7 +396,7 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data, struct file *anon_filp; unsigned long addr; - if (!(dev->driver->driver_features & DRIVER_GEM)) { + if (!drm_core_check_feature(dev, DRIVER_GEM)) { DRM_ERROR("does not support GEM.\n"); return -ENODEV; } diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index e2db48a81ed0..d8426bc31b09 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -62,7 +62,7 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, int ret = 0; struct drm_gem_object *obj; - if (!(dev->driver->driver_features & DRIVER_GEM)) + if (!drm_core_check_feature(dev, DRIVER_GEM)
[RFC 03/12] drm/i915: Mark as legacy if KMS is disabled
From: Thierry Reding When kernel mode-setting is disabled, mark the driver as legacy to pick up the special semantics required for userspace mode-setting. Signed-off-by: Thierry Reding --- drivers/gpu/drm/i915/i915_drv.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 2d05d7ce4c29..ea916e117f01 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -985,13 +985,19 @@ static int __init i915_init(void) #if defined(CONFIG_DRM_I915_KMS) if (i915.modeset != 0) driver.driver_features |= DRIVER_MODESET; + else + driver.driver_features |= DRIVER_LEGACY; #endif if (i915.modeset == 1) driver.driver_features |= DRIVER_MODESET; + else + driver.driver_features |= DRIVER_LEGACY; #ifdef CONFIG_VGA_CONSOLE - if (vgacon_text_force() && i915.modeset == -1) + if (vgacon_text_force() && i915.modeset == -1) { driver.driver_features &= ~DRIVER_MODESET; + driver.driver_features |= DRIVER_LEGACY; + } #endif if (!(driver.driver_features & DRIVER_MODESET)) { -- 1.8.4.2
[RFC 04/12] drm: Separate DRIVER_MODESET and DRIVER_LEGACY
From: Thierry Reding Support non-legacy drivers without mode-setting functionality by using the new DRIVER_LEGACY feature to separate out legacy code, rather than relying on DRIVER_MODESET not being advertised. Signed-off-by: Thierry Reding --- drivers/gpu/drm/drm_bufs.c | 12 +-- drivers/gpu/drm/drm_dma.c | 4 ++-- drivers/gpu/drm/drm_fops.c | 6 +++--- drivers/gpu/drm/drm_irq.c | 12 +-- drivers/gpu/drm/drm_pci.c | 12 +-- drivers/gpu/drm/drm_scatter.c | 6 +++--- drivers/gpu/drm/drm_sysfs.c | 4 ++-- drivers/gpu/drm/i915/i915_dma.c | 38 - drivers/gpu/drm/i915/i915_drv.c | 10 - drivers/gpu/drm/i915/i915_gem.c | 17 +++ drivers/gpu/drm/i915/i915_irq.c | 6 +++--- drivers/gpu/drm/i915/i915_suspend.c | 15 ++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 13 files changed, 71 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index edec31fe3fed..362ad21bd0e4 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -1141,7 +1141,7 @@ int drm_addbufs(struct drm_device *dev, void *data, struct drm_buf_desc *request = data; int ret; - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return -EINVAL; if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA)) @@ -1187,7 +1187,7 @@ int drm_infobufs(struct drm_device *dev, void *data, int i; int count; - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return -EINVAL; if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA)) @@ -1269,7 +1269,7 @@ int drm_markbufs(struct drm_device *dev, void *data, int order; struct drm_buf_entry *entry; - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return -EINVAL; if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA)) @@ -1317,7 +1317,7 @@ int drm_freebufs(struct drm_device *dev, void *data, int idx; struct drm_buf *buf; - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return -EINVAL; if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA)) @@ -1372,7 +1372,7 @@ int drm_mapbufs(struct drm_device *dev, void *data, struct drm_buf_map *request = data; int i; - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return -EINVAL; if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA)) @@ -1452,7 +1452,7 @@ int drm_mapbufs(struct drm_device *dev, void *data, int drm_dma_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return -EINVAL; if (dev->driver->dma_ioctl) diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c index 8a140a953754..bce37007c9c3 100644 --- a/drivers/gpu/drm/drm_dma.c +++ b/drivers/gpu/drm/drm_dma.c @@ -49,7 +49,7 @@ int drm_legacy_dma_setup(struct drm_device *dev) int i; if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA) || - drm_core_check_feature(dev, DRIVER_MODESET)) { + !drm_core_check_feature(dev, DRIVER_LEGACY)) { return 0; } @@ -80,7 +80,7 @@ void drm_legacy_dma_takedown(struct drm_device *dev) int i, j; if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA) || - drm_core_check_feature(dev, DRIVER_MODESET)) { + !drm_core_check_feature(dev, DRIVER_LEGACY)) { return; } diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 80e2a6c5ff8a..46eee1da4ad7 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -50,8 +50,8 @@ static int drm_setup(struct drm_device * dev) { int ret; - if (dev->driver->firstopen && - !drm_core_check_feature(dev, DRIVER_MODESET)) { + if (drm_core_check_feature(dev, DRIVER_LEGACY) && + dev->driver->firstopen) { ret = dev->driver->firstopen(dev); if (ret != 0) return ret; @@ -386,7 +386,7 @@ static void drm_events_release(struct drm_file *file_priv) */ static void drm_legacy_dev_reinit(struct drm_device *dev) { - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return; dev->sigdata.lock = NULL; diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c2676b5908d9..70
[RFC 06/12] drm: Remove gratuituous blank line
From: Thierry Reding Signed-off-by: Thierry Reding --- drivers/gpu/drm/drm_crtc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 35ea15d5..9f9044a0a3ee 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1426,7 +1426,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; - mutex_lock(&file_priv->fbs_lock); /* * For the non-control nodes we need to limit the list of resources -- 1.8.4.2
[RFC 05/12] drm: Create primary minor only if mode-setting is supported
From: Thierry Reding Non-legacy devices may not always support mode-setting functionality, so create the primary minor conditionally. One setup where this happens is the Tegra K1, where the Tegra DRM driver exposes the display engine via standard KMS IOCTLs, and nouveau drives the Kepler-type GPU that has no display capabilities. Signed-off-by: Thierry Reding --- drivers/gpu/drm/drm_stub.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index fd2f1758366d..839460b774c5 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -564,9 +564,11 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver, goto err_minors; } - ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY); - if (ret) - goto err_minors; + if (drm_core_check_feature(dev, DRIVER_MODESET)) { + ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY); + if (ret) + goto err_minors; + } if (drm_ht_create(&dev->map_hash, 12)) goto err_minors; -- 1.8.4.2
[RFC 08/12] drm/exynos: Remove dead code
From: Thierry Reding The Exynos driver always sets DRIVER_GEM, so testing for the absence of the feature will always fail. Signed-off-by: Thierry Reding --- drivers/gpu/drm/exynos/exynos_drm_gem.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index 07a4d0f92722..9230d6f7f964 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -329,11 +329,6 @@ int exynos_drm_gem_map_offset_ioctl(struct drm_device *dev, void *data, DRM_DEBUG_KMS("handle = 0x%x, offset = 0x%lx\n", args->handle, (unsigned long)args->offset); - if (!drm_core_check_feature(dev, DRIVER_GEM)) { - DRM_ERROR("does not support GEM.\n"); - return -ENODEV; - } - return exynos_drm_gem_dumb_map_offset(file_priv, dev, args->handle, &args->offset); } @@ -396,11 +391,6 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data, struct file *anon_filp; unsigned long addr; - if (!drm_core_check_feature(dev, DRIVER_GEM)) { - DRM_ERROR("does not support GEM.\n"); - return -ENODEV; - } - mutex_lock(&dev->struct_mutex); obj = drm_gem_object_lookup(dev, file_priv, args->handle); -- 1.8.4.2
[RFC 09/12] drm/gma500: Remove dead code
From: Thierry Reding The gma500 driver sets DRIVER_GEM unconditionally, so testing for the absence of the feature will always fail. Signed-off-by: Thierry Reding --- drivers/gpu/drm/gma500/gem.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index d8426bc31b09..424dda95fbc6 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -62,9 +62,6 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, int ret = 0; struct drm_gem_object *obj; - if (!drm_core_check_feature(dev, DRIVER_GEM)) - return -ENODEV; - mutex_lock(&dev->struct_mutex); /* GEM does all our handle to object mapping */ -- 1.8.4.2
[RFC 10/12] drm/i810: Remove dead code
From: Thierry Reding The i810 driver never sets DRIVER_HAVE_IRQ, so testing for the presence of the feature will always fail. Signed-off-by: Thierry Reding --- drivers/gpu/drm/i810/i810_dma.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c index aeace37415aa..d12e5132c4bc 100644 --- a/drivers/gpu/drm/i810/i810_dma.c +++ b/drivers/gpu/drm/i810/i810_dma.c @@ -200,13 +200,6 @@ static int i810_dma_cleanup(struct drm_device *dev) { struct drm_device_dma *dma = dev->dma; - /* Make sure interrupts are disabled here because the uninstall ioctl -* may not have been called from userspace and after dev_private -* is freed, it's too late. -*/ - if (drm_core_check_feature(dev, DRIVER_HAVE_IRQ) && dev->irq_enabled) - drm_irq_uninstall(dev); - if (dev->dev_private) { int i; drm_i810_private_t *dev_priv = -- 1.8.4.2
[RFC 11/12] drm/i915: Remove dead code
From: Thierry Reding The i915 driver sets DRIVER_GEM unconditionally, so testing for the feature will always fail. Signed-off-by: Thierry Reding --- drivers/gpu/drm/i915/i915_gem_context.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index e9bba3c29ec6..da74522f377d 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -746,9 +746,6 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, struct i915_hw_context *ctx; int ret; - if (!drm_core_check_feature(dev, DRIVER_GEM)) - return -ENODEV; - if (!HAS_HW_CONTEXTS(dev)) return -ENODEV; @@ -775,9 +772,6 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, struct i915_hw_context *ctx; int ret; - if (!drm_core_check_feature(dev, DRIVER_GEM)) - return -ENODEV; - if (args->ctx_id == DEFAULT_CONTEXT_ID) return -ENOENT; -- 1.8.4.2
[RFC 12/12] drm/qxl: Remove dead code
From: Thierry Reding The QXL driver sets DRIVER_MODESET unconditionally, so testing for the absence of the feature will always fail. Signed-off-by: Thierry Reding --- drivers/gpu/drm/qxl/qxl_kms.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index fd88eb4a3f79..eec2e983519b 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -310,10 +310,6 @@ int qxl_driver_load(struct drm_device *dev, unsigned long flags) struct qxl_device *qdev; int r; - /* require kms */ - if (!drm_core_check_feature(dev, DRIVER_MODESET)) - return -ENODEV; - qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL); if (qdev == NULL) return -ENOMEM; -- 1.8.4.2
[RFC 03/12] drm/i915: Mark as legacy if KMS is disabled
On Fri, Feb 21, 2014 at 2:55 AM, Thierry Reding wrote: > From: Thierry Reding > > When kernel mode-setting is disabled, mark the driver as legacy to pick > up the special semantics required for userspace mode-setting. > > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/i915/i915_drv.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 2d05d7ce4c29..ea916e117f01 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -985,13 +985,19 @@ static int __init i915_init(void) > #if defined(CONFIG_DRM_I915_KMS) > if (i915.modeset != 0) > driver.driver_features |= DRIVER_MODESET; > + else > + driver.driver_features |= DRIVER_LEGACY; > #endif > if (i915.modeset == 1) > driver.driver_features |= DRIVER_MODESET; > + else > + driver.driver_features |= DRIVER_LEGACY; If i915.modeset == -1 (which seems like a legal value as per the hunk below, and I'm guessing the default), it'll end up with both MODESET and LEGACY. Is that a legal combination? > > #ifdef CONFIG_VGA_CONSOLE > - if (vgacon_text_force() && i915.modeset == -1) > + if (vgacon_text_force() && i915.modeset == -1) { > driver.driver_features &= ~DRIVER_MODESET; > + driver.driver_features |= DRIVER_LEGACY; > + } > #endif > > if (!(driver.driver_features & DRIVER_MODESET)) { > -- > 1.8.4.2 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: silence GCC warning on 32 bit
On Thu, Feb 20, 2014 at 10:24:53PM +0100, Paul Bolle wrote: > On Thu, 2014-02-20 at 16:07 -0500, Ilia Mirkin wrote: > > On Thu, Feb 20, 2014 at 4:02 PM, Paul Bolle wrote: > > > @@ -935,7 +935,7 @@ static ssize_t radeon_ttm_gtt_read(struct file *f, > > > char __user *buf, > > > while (size) { > > > loff_t p = *pos / PAGE_SIZE; > > > unsigned off = *pos & ~PAGE_MASK; > > > - ssize_t cur_size = min(size, PAGE_SIZE - off); > > > + ssize_t cur_size = min(size, (size_t)(PAGE_SIZE - off)); > > > > Isn't the usual way of dealing with these to do something like > > > > ssize_t cur_size = min_t(ssize_t, size, PAGE_SIZE - off) > > I wouldn't know. I did > $ git grep -n "(size_t)(PAGE_SIZE" > arch/powerpc/mm/dma-noncoherent.c:357: size_t seg_size = > min((size_t)(PAGE_SIZE - offset), size); > arch/tile/kernel/pci-dma.c:176: size_t bytes = min(size, > (size_t)(PAGE_SIZE - offset)); > arch/tile/kernel/pci-dma.c:192: size_t bytes = min(size, > (size_t)(PAGE_SIZE - offset)); > drivers/net/wireless/ti/wlcore/main.c:806: len = min(maxlen, > (size_t)(PAGE_SIZE - wl->fwlog_size)); > drivers/scsi/libfc/fc_libfc.c:143: > (size_t)(PAGE_SIZE - (off & ~PAGE_MASK))); > drivers/target/tcm_fc/tfc_io.c:160: tlen = min(tlen, > (size_t)(PAGE_SIZE - > drivers/target/tcm_fc/tfc_io.c:311: tlen = min(tlen, > (size_t)(PAGE_SIZE - > > and concluded my solution was acceptable. Is your alternative considered > to be better? Yes, min_t() is specifically meant for this type of situation. On a side note, I think cur_size should be size_t rather than ssize_t. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/08d60f63/attachment-0001.pgp>
[RFC 03/12] drm/i915: Mark as legacy if KMS is disabled
On Fri, Feb 21, 2014 at 03:17:28AM -0500, Ilia Mirkin wrote: > On Fri, Feb 21, 2014 at 2:55 AM, Thierry Reding > wrote: > > From: Thierry Reding > > > > When kernel mode-setting is disabled, mark the driver as legacy to pick > > up the special semantics required for userspace mode-setting. > > > > Signed-off-by: Thierry Reding > > --- > > drivers/gpu/drm/i915/i915_drv.c | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 2d05d7ce4c29..ea916e117f01 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -985,13 +985,19 @@ static int __init i915_init(void) > > #if defined(CONFIG_DRM_I915_KMS) > > if (i915.modeset != 0) > > driver.driver_features |= DRIVER_MODESET; > > + else > > + driver.driver_features |= DRIVER_LEGACY; > > #endif > > if (i915.modeset == 1) > > driver.driver_features |= DRIVER_MODESET; > > + else > > + driver.driver_features |= DRIVER_LEGACY; > > If i915.modeset == -1 (which seems like a legal value as per the hunk > below, and I'm guessing the default), it'll end up with both MODESET > and LEGACY. Is that a legal combination? I don't see a reason why the combination would be illegal, but it doesn't make a lot of sense either. So I think the above hunk should be turned into this instead: + if (i915.modeset == 0) + driver.driver_features |= DRIVER_LEGACY; Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/90250735/attachment.pgp>
[Intel-gfx] [PATCH 0/6] Intel Color Manager Framework
On Fri, Feb 21, 2014 at 03:34:43AM +, Sharma, Shashank wrote: > Hi Ville/All, > > We gave a presentation on design on this framework, few months ago, in one of > our common forum with OTC folks. > We discussed, took review comments, and re-designed the framework, as per > the feedbacks. Apparently I wasn't there. And in any case it would be better to discuss it on dri-devel where people outside Intel can give their opinion. > > We also discussed the benefits of providing the controls directly from /sysfs > over going for a UI manager based property settings. > So I don't understand where are we going wrong, can you please elaborate a > bit ? The most important issue from my POV is that it can't be part of the atomic modeset. Another issue is that it make the whole API inconsistent. Some stuff through ioctl, some stuff through sysfs, some stuff through whatever the next guy thinks of. It's not pretty. I've worked in the past with a driver where I had to poke at various standardish ioctls, custom ioctls, and sysfs to make it do anything useful, and I have no interest in repeating that experience. sysfs is especially painful since you have do the string<->binary conversions all over the place, and also you en up doing open+read/write+close cycles for every little thing. It also adds more entrypoints into the driver for us to worry about. That means extra worries about the power management stuff and locking at the very least. Also the rules of sysfs say "one item per file". The only allowed exception to this rule I know of is hardware provided blobs (like EDID, PCI ROM etc.). Your current implementation breaks this rule blatantly. > > This is just a basic design, and once go ahead with this, we can always work > on making hardware agnostic, as you recommended. > > IMHO, controls from /sysfs would be a very generic interface for all > linux/drm based platform, where any userspace can read/write and control > properties. > We don't even need a UI manager or a minimum executable to play around, just > a small script can do. But we can always write something on top of this, > to be included in any UI framework or property. If there's a real need to get at properties through sysfs, then we could think about exposing them all. But that may presents some issues where the current master suddenly gets confused about its state since someone else went behind its back and changed a bunch of stuff. > > Regards > Shashank > > -Original Message- > From: Ville Syrj?l? [mailto:ville.syrjala at linux.intel.com] > Sent: Thursday, February 20, 2014 6:41 PM > To: Sharma, Shashank > Cc: intel-gfx at lists.freedesktop.org; Shankar, Uma; dri-devel at > lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 0/6] Intel Color Manager Framework > > On Thu, Feb 20, 2014 at 06:07:21PM +0530, Shashank Sharma wrote: > > Color manager is a new framework in i915 driver, which provides a > > unified interface for various color correction methods supported by > > intel hardwares. The high level overview of this change is: > > Would have been good to discuss this idea before implementing it. The plan is > to use kms properties for this kind of stuff which allows us to hook it up > with the upcoming atomic modeset API. Just yesterday there was some > discussion on #dri-devel about exposing user settable blob properties even > before the atomic modeset API lands (it was always the plan for the atomic > modeset API anyway). So based on a cursory glance, this looks like it's going > in the wrong direction. > > Also ideally the properties should be hardware agnostic, so a generic > userspace could use them regardless of the hardware/driver. Obviously that > might not be possible in all cases, but we should at least spend a bit of > effort on trying to make that happen for most properties. > > -- > Ville Syrj?l? > Intel OTC -- Ville Syrj?l? Intel OTC
[Bug 75226] Dark rendering of War for the Overworld
https://bugs.freedesktop.org/show_bug.cgi?id=75226 Michel D?nzer changed: What|Removed |Added Assignee|dri-devel at lists.freedesktop |mesa-dev at lists.freedesktop. |.org|org Component|Drivers/Gallium/r600|Mesa core --- Comment #4 from Michel D?nzer --- Playing back the traces looks the same using llvmpipe, which indicates that the problem is in a component shared at least by Gallium drivers (which includes the nouveau drivers for nVidia cards). Did you get any specific reports of users not being affected by this using nouveau or intel Mesa drivers? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/54f78c03/attachment.html>
[PATCH 00/11] SimpleDRM & Sysfb
On Mon, Jan 27, 2014 at 11:18:51PM +0100, David Herrmann wrote: > Hi > > On Thu, Jan 23, 2014 at 3:14 PM, David Herrmann > wrote: > > Hi > > > > Another round of SimpleDRM patches. I somehow lost track of the last ones > > and as > > this is a major rewrite, I'll just start at v1 again. > > > > Some comments up-front: > > > > - @Ingo: Patch #1 and #2 are unchanged from the previous ML discussions. I > >included them in this series as the other patches depend on them. Could > > you > >pick them up for the x86 tree? The other 9 patches won't make it in 3.14 > > so > >no reason to put them through the DRM tree. > >All mentioned issues should be addressed. If there's still sth missing, > >please let me know. > > > > - The DRM patches depend on my "DRM Anonymous Inode" patches. But it > > should be > >trivial to apply them on drm-next (I think only one line needs to be > > changed: > >i_mapping => dev_mapping). > > > > - I tested the SimpleDRM fbdev fallback with linux-console+Xorg and it > > works > >fine. The DRM backend is only tested with some DRM tests I have locally. > > I > >have no idea how to make Xorg pick up a specific /dev/dri/card0 card. It > >always tells me "no screens found" (as the underlying device is not > > marked as > >boot_vga..). If someone knows how to tell Xorg to use card0, I'd gladly > > test > >this. But I'm no longer used to writing xorg.confs.. > > For completeness, I tested this with Xorg+xf86-video-modesetting and > it works just fine. The xorg.conf I used can be found below. If this > driver gets upstreamed, I will try to make the X11 auto-loader detect > it just like any other platform-device. I recently posted patches[0] to the xorg-devel mailing list that I think should solve that issue. Thierry [0]: http://lists.x.org/archives/xorg-devel/2014-February/040568.html -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/009a3a1e/attachment.pgp>
[Bug 74764] Steam overlay not working
https://bugs.freedesktop.org/show_bug.cgi?id=74764 --- Comment #11 from darkbasic --- I definitely have this problem with latest git, didn't try early version. With STEAM_OVERLAY=0 it works flawlessly, unfortunately this is not a solution. With Catalyst it works flawlessly even with STEAM_OVERLAY=1. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/8ea0d4ea/attachment.html>
AMD/AMD hybrid graphics
On Thu, Feb 20, 2014 at 9:37 PM, Dave Airlie wrote: > On Fri, Feb 21, 2014 at 12:25 PM, Michel D?nzer wrote: >> On Don, 2014-02-20 at 08:44 +0100, Boszormenyi Zoltan wrote: >>> 2014-02-20 06:47 keltez?ssel, Michel D?nzer ?rta: >>> > On Don, 2014-02-20 at 06:09 +0100, Boszormenyi Zoltan wrote: >>> >> 2014-02-20 04:20 keltez?ssel, Michel D?nzer ?rta: >>> >>> On Mit, 2014-02-19 at 11:56 +0100, Boszormenyi Zoltan wrote: >>> >>>> 2014-02-19 10:59 keltez?ssel, Michel D?nzer ?rta: >>> >>>>> On Mit, 2014-02-19 at 09:11 +0100, Boszormenyi Zoltan wrote: >>> >>>>> >>> >>>>>> Can Mesa/Xorg use both r600g and radeonsi at the same time? >>> >>>>> Yes, that seems to work fine for others. You may need Mesa 10.1 or >>> >>>>> newer >>> >>>>> though. >>> >>>> Do you mean mean with Mesa 9.2.5 and Xorg server 1.14.4 in >>> >>>> Fedora 20 at this time, it's not possible unless I compile my own >>> >>>> llvm-3.5 SVN, Mesa 10.1 or 10.2 GIT and Xorg 1.15 GIT? >>> >>> I don't think Xorg 1.15 is necessary, but it shouldn't hurt either. >>> >>> >>> >>> >>> >>>> Attached is the log from both 1.14.4 (FC20) and 1.15.0 (rawhide), [...] >>> >>> The log files end abruptly, so we need to see the X server stderr >>> >>> output. Assuming you're using gdm, it should be captured >>> >>> in /var/log/gdm*/:0.log . >>> >> FC20 has lightdm by default, here's /var/log/lightdm/x-0.log >>> > [...] >>> > >>> >> X: ../../../include/privates.h:122: dixGetPrivateAddr: Assertion >>> >> `key->initialized' failed. >>> > Can you get a backtrace for this assertion failure with gdb? See >>> > http://wiki.x.org/wiki/Development/Documentation/ServerDebugging/ >> >> [...] >> >>> #0 0x7fadc165f1c9 in __GI_raise (sig=sig at entry=6) at >>> ../nptl/sysdeps/unix/sysv/linux/raise.c:56 >>> #1 0x7fadc16608d8 in __GI_abort () at abort.c:89 >>> #2 0x7fadc1658126 in __assert_fail_base (fmt=0x7fadc17a98a0 >>> "%s%s%s:%u: %s%sAssertion >>> `%s' failed.\n%n", assertion=assertion at entry=0x5a4c53 "key->initialized", >>> file=file at entry=0x5a8dfc "../../../include/privates.h", line=line >>> at entry=122, >>> function=function at entry=0x5aaa90 <__PRETTY_FUNCTION__.8544> >>> "dixGetPrivateAddr") at >>> assert.c:92 >>> #3 0x7fadc16581d2 in __GI___assert_fail (assertion=assertion at >>> entry=0x5a4c53 >>> "key->initialized", file=file at entry=0x5a8dfc >>> "../../../include/privates.h", >>> line=line at entry=122, >>> function=function at entry=0x5aaa90 <__PRETTY_FUNCTION__.8544> >>> "dixGetPrivateAddr") at >>> assert.c:101 >>> #4 0x00424f70 in dixGetPrivateAddr (key=, >>> key=, >>> privates=0x169d558) at ../../../include/privates.h:122 >>> #5 0x004810eb in dixGetPrivateAddr (key=, >>> key=, >>> privates=0x169d558) at xf86cmap.c:239 >>> #6 dixSetPrivate (val=, key=0x822f80 , >>> privates=0x169d558) at ../../../include/privates.h:148 >>> #7 xf86HandleColormaps (pScreen=pScreen at entry=0x169d180, >>> maxColors=maxColors at entry=256, >>> sigRGBbits=10, loadPalette=loadPalette at entry=0x7fadbcd37ea0 >>> , >>> setOverscan=setOverscan at entry=0x0, flags=flags at entry=3) at >>> xf86cmap.c:184 >>> #8 0x7fadbcd3b32c in drmmode_setup_colormap (pScreen=pScreen at >>> entry=0x169d180, >>> pScrn=pScrn at entry=0x169c930) at drmmode_display.c:1990 >>> #9 0x7fadbcd370ef in RADEONScreenInit_KMS (pScreen=pScreen at >>> entry=0x169d180, >>> argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at >>> radeon_kms.c:1366 >>> #10 0x00438b4d in AddGPUScreen (pfnInit=0x7fadbcd36c60 >>> , >>> argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at dispatch.c:3874 >>> #11 0x0047aa9f in InitOutput (pScreenInfo=pScreenInfo at >>> entry=0x82dd60 , >>> argc=argc at entry=1, argv=argv at entry=0x7fff83c684c8) at xf86Init.c:886 >> >> Hmm, looks like there's confusion about how colormaps are supposed to be >> handled. >> >> >> Dave, any ideas? > > xf86_crtc_supports_gamma probably stops us from ever getting into that > function, but in the case where we have 0 crtcs I could see fallout. > > Either fix the DDX to not call colormap handling if we have 0 crtcs > and/or fix the server to not install bother with colormaps if we have > 0 crtcs. > Does the attached patch help? Alex > Dave. > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- next part -- A non-text attachment was scrubbed... Name: 0001-radeon-don-t-install-colormap-handling-if-there-are-.patch Type: text/x-diff Size: 1792 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/940a29b9/attachment-0001.patch>
[Bug 42960] Display does not work when resuming from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=42960 --- Comment #8 from leon_ti at mail.ru --- I can confirm similar behaviour for A6 "ARUBA" APU without any discrete GPU, at Samsubg NP535U3C laptop. leon_ti at leonid-samsung:~$ uname -a Linux leonid-samsung 3.12.0-031200-generic #201311031935 SMP Mon Nov 4 00:36:54 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/06fac8ac/attachment.html>
AMD/AMD hybrid graphics
2014-02-21 14:37 keltez?ssel, Alex Deucher ?rta: > > Does the attached patch help? > > Alex Yes, it helped, thank you very much. The compressed Xorg.0.log is attached as proof. To others: the patch is against xf86-video-ati. Two notes, though: 1. Is it normal that "xrandr --listproviders" lists 3 providers? [zozo at localhost ~]$ cat xrandr-providers Providers: number : 3 Provider 0: id: 0x86 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 4 outputs: 3 associated providers: 2 name:radeon Provider 1: id: 0x4f cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 0 outputs: 0 associated providers: 2 name:radeon Provider 2: id: 0x4f cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 0 outputs: 0 associated providers: 2 name:radeon 2. I tried glxgears with and without DRI_PRIME=1 as below: [zozo at localhost ~]$ glxgears Running synchronized to the vertical refresh. The framerate should be approximately the same as the monitor refresh rate. 299 frames in 5.0 seconds = 59.558 FPS [zozo at localhost ~]$ DRI_PRIME=1 glxgears Running synchronized to the vertical refresh. The framerate should be approximately the same as the monitor refresh rate. 5589 frames in 5.0 seconds = 1117.730 FPS 5369 frames in 5.0 seconds = 1073.715 FPS 5699 frames in 5.0 seconds = 1139.670 FPS Obviously, it doesn't sync to the framerate with PRIME. On the other hand, nothing is displayed in the glxgears window. I have llvm 3.4-4 and Mesa 10.1-rc1 from Fedora 21 rawhide. I will go back to plain Fedora 20 (Xorg 1.14, llvm 3.3 and Mesa 9.2.5) and try the same patch to see if there's any difference. Best regards, Zolt?n B?sz?rm?nyi -- next part -- A non-text attachment was scrubbed... Name: Xorg.0.log.fixed.gz Type: application/x-tar Size: 9901 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/e05c9936/attachment.tar>
[Bug 70651] cannot write to power_dpm_force_performance_level: invalid argument (radeon 7730m)
https://bugzilla.kernel.org/show_bug.cgi?id=70651 --- Comment #2 from Fabio Sangiovanni --- hi, any advice on this, please? -- You are receiving this mail because: You are watching the assignee of the bug.
[Intel-gfx] [PATCH 0/6] Intel Color Manager Framework
On Fri, Feb 21, 2014 at 02:20:24PM +, Sharma, Shashank wrote: > Hi Ville, > > Thanks for your time and comments. > I can understand two basic problems what you see in this implementation: > > 1. The most important issue from my POV is that it can't be part of the > atomic modeset. > 2. it make the whole API inconsistent. > > I am not sure if its good to block all current implementation because we have > thought something for this in atomic modeset. > I think even in atomic modeset we need the core implementation like this, but > the interface would be different, which might come in from of a DRM property. > So at that time we can use this core implementation as it is, only the > interfaces/framework needs to be changed. > > In this way we can always go ahead with a current implementation, and can > just change the interfaces to fit in to the final interface like DRM property > in atomic modeset. > Or you can suggest us the expected interface, and we can work on modifying > that as per expectation. The exptected interface will be range properties for stuff like brightness, contrast etc. controls. There are already such things as connector properties, but we're going to want something similar as plane or crtc properties. One thing that worries me about such properties though is whether we can make them hardware agnostic and yet allow userspace precise control over the final image. That is, if we map some fixed input range to a hardware specific output range, userspace can't know how the actual output will change when the input changes. On the other hand if the input is hardware specific, userspace can't know what value to put in there to get the expected change on the output side. For bigger stuff like CSC matrices and gamma ramps we will want to use some reasonably well defined blobs. Ie. the internal strucuture of the blob has to be documented and it shouldn't contain more than necessary. Ie. just the CSC matrix coefficients for one matrix, or just the entries for a single gamma ramp. Again ideally we should make the blobs hardware agnostic, but still allow precise control over the output data. I think this is going to involve first going over our hardware features, trying to find the common patterns between different generations. If there's a way to make something that works across the board for us, or at least across a wide range, then we should also ask for some input on dri-devel whether the proposed property would work for other people. We may need to define new property types to more precisely define what the value of the property actually means. > > Please correct me if any of my assumptions are not right, or not feasible, or > if I am just a moron :) . The implementation itself has to be tied into the pipe config (and eventual plane config) stuff, which are the structures meant to house the full device state, which will then be applied in one go. At the moment it looks like you're writing a bunch of registers from various places w/o much thought into how those things interact with anything else. For instance, what's the story with your use of the pipe CSC unit vs. the already existing "Broadcast RGB" property? > > Regards > Shashank > -Original Message- > From: Ville Syrj?l? [mailto:ville.syrjala at linux.intel.com] > Sent: Friday, February 21, 2014 2:47 PM > To: Sharma, Shashank > Cc: intel-gfx at lists.freedesktop.org; Shankar, Uma; dri-devel at > lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 0/6] Intel Color Manager Framework > > On Fri, Feb 21, 2014 at 03:34:43AM +, Sharma, Shashank wrote: > > Hi Ville/All, > > > > We gave a presentation on design on this framework, few months ago, in one > > of our common forum with OTC folks. > > We discussed, took review comments, and re-designed the framework, as per > > the feedbacks. > > Apparently I wasn't there. And in any case it would be better to discuss it > on dri-devel where people outside Intel can give their opinion. > > > > > We also discussed the benefits of providing the controls directly from > > /sysfs over going for a UI manager based property settings. > > So I don't understand where are we going wrong, can you please elaborate a > > bit ? > > The most important issue from my POV is that it can't be part of the atomic > modeset. > > Another issue is that it make the whole API inconsistent. Some stuff through > ioctl, some stuff through sysfs, some stuff through whatever the next guy > thinks of. It's not pretty. I've worked in the past with a driver where I had > to poke at various standardish ioctls, custom ioctls, and sysfs to make it do > anything useful, and I have no interest in repeating that experience. sysfs > is especially painful since you have do the string<->binary conversions all > over the place, and also you en up doing open+read/write+close cycles for > every little thing. > > It also adds more entrypoints into the driver for us to
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 darkbasic changed: What|Removed |Added Summary|[UVD] vdpau hangs the |[UVD] vdpau has terrible |system on radeonsi (HD |performance on radeonsi (HD |7950) |7950) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/11b7f342/attachment.html>
[Intel-gfx] [PATCH 0/6] Intel Color Manager Framework
On Fri, Feb 21, 2014 at 9:20 AM, Sharma, Shashank wrote: > Hi Ville, > > Thanks for your time and comments. > I can understand two basic problems what you see in this implementation: > > 1. The most important issue from my POV is that it can't be part of the > atomic modeset. > 2. it make the whole API inconsistent. > > I am not sure if its good to block all current implementation because we have > thought something for this in atomic modeset. > I think even in atomic modeset we need the core implementation like this, but > the interface would be different, which might come in from of a DRM property. > So at that time we can use this core implementation as it is, only the > interfaces/framework needs to be changed. What I would suggest is re-form the userspace facing API to be properties.. if needed we can add a setblobprop ioctl (Sean Paul was, I think, adding that already). Then userspace and use setprop ioctls for now, and optionally atomic ioctl later when it is in place. No reason for it to be blocked waiting for atomic. btw, I didn't look into the patches yet, but full-nak on idea of exposing via sysfs. This should be the sort of thing that is set by the process that has mastership on the drm device, which we can't enforce via sysfs. Using properties seems like the way to go. BR, -R > In this way we can always go ahead with a current implementation, and can > just change the interfaces to fit in to the final interface like DRM property > in atomic modeset. > Or you can suggest us the expected interface, and we can work on modifying > that as per expectation.
AMD/AMD hybrid graphics
2014-02-21 15:32 keltez?ssel, Boszormenyi Zoltan ?rta: > 2014-02-21 14:37 keltez?ssel, Alex Deucher ?rta: >> >> Does the attached patch help? >> >> Alex > > Yes, it helped, thank you very much. > The compressed Xorg.0.log is attached as proof. > > To others: the patch is against xf86-video-ati. > > Two notes, though: > > 1. Is it normal that "xrandr --listproviders" lists 3 providers? > > [zozo at localhost ~]$ cat xrandr-providers > Providers: number : 3 > Provider 0: id: 0x86 cap: 0xf, Source Output, Sink Output, Source Offload, > Sink Offload > crtcs: 4 outputs: 3 associated providers: 2 name:radeon > Provider 1: id: 0x4f cap: 0xf, Source Output, Sink Output, Source Offload, > Sink Offload > crtcs: 0 outputs: 0 associated providers: 2 name:radeon > Provider 2: id: 0x4f cap: 0xf, Source Output, Sink Output, Source Offload, > Sink Offload > crtcs: 0 outputs: 0 associated providers: 2 name:radeon > > 2. I tried glxgears with and without DRI_PRIME=1 as below: > > [zozo at localhost ~]$ glxgears > Running synchronized to the vertical refresh. The framerate should be > approximately the same as the monitor refresh rate. > 299 frames in 5.0 seconds = 59.558 FPS > [zozo at localhost ~]$ DRI_PRIME=1 glxgears > Running synchronized to the vertical refresh. The framerate should be > approximately the same as the monitor refresh rate. > 5589 frames in 5.0 seconds = 1117.730 FPS > 5369 frames in 5.0 seconds = 1073.715 FPS > 5699 frames in 5.0 seconds = 1139.670 FPS > > Obviously, it doesn't sync to the framerate with PRIME. > On the other hand, nothing is displayed in the glxgears window. > I have llvm 3.4-4 and Mesa 10.1-rc1 from Fedora 21 rawhide. > > I will go back to plain Fedora 20 (Xorg 1.14, llvm 3.3 and Mesa 9.2.5) > and try the same patch to see if there's any difference. With Fedora 20 packages X doesn't come up. The last message on the screen is: [ 19.016881] pci_pm_runtime_suspend(): radeon_pmops_runtime_suspend+0x0/0xa0 [radeon] return -22 Xorg crashes and Xorg.0.log now contains the backtrace, attached. Best regards, Zolt?n B?sz?rm?nyi -- next part -- A non-text attachment was scrubbed... Name: Xorg.0.log-1.14-segfault.gz Type: application/x-tar Size: 7917 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/8bd36ad0/attachment.tar>
AMD/AMD hybrid graphics
On Fri, Feb 21, 2014 at 9:32 AM, Boszormenyi Zoltan wrote: > 2014-02-21 14:37 keltez?ssel, Alex Deucher ?rta: > >> >> Does the attached patch help? >> >> Alex > > > Yes, it helped, thank you very much. > The compressed Xorg.0.log is attached as proof. > > To others: the patch is against xf86-video-ati. > > Two notes, though: > > 1. Is it normal that "xrandr --listproviders" lists 3 providers? > > [zozo at localhost ~]$ cat xrandr-providers > Providers: number : 3 > Provider 0: id: 0x86 cap: 0xf, Source Output, Sink Output, Source Offload, > Sink Offload crtcs: 4 outputs: 3 associated providers: 2 name:radeon > Provider 1: id: 0x4f cap: 0xf, Source Output, Sink Output, Source Offload, > Sink Offload crtcs: 0 outputs: 0 associated providers: 2 name:radeon > Provider 2: id: 0x4f cap: 0xf, Source Output, Sink Output, Source Offload, > Sink Offload crtcs: 0 outputs: 0 associated providers: 2 name:radeon > > 2. I tried glxgears with and without DRI_PRIME=1 as below: > > [zozo at localhost ~]$ glxgears > Running synchronized to the vertical refresh. The framerate should be > approximately the same as the monitor refresh rate. > 299 frames in 5.0 seconds = 59.558 FPS > [zozo at localhost ~]$ DRI_PRIME=1 glxgears > Running synchronized to the vertical refresh. The framerate should be > approximately the same as the monitor refresh rate. > 5589 frames in 5.0 seconds = 1117.730 FPS > 5369 frames in 5.0 seconds = 1073.715 FPS > 5699 frames in 5.0 seconds = 1139.670 FPS > > Obviously, it doesn't sync to the framerate with PRIME. > On the other hand, nothing is displayed in the glxgears window. > I have llvm 3.4-4 and Mesa 10.1-rc1 from Fedora 21 rawhide. > Make sure you are running a compositor. Alex
[Bug 70651] cannot write to power_dpm_force_performance_level: invalid argument (radeon 7730m)
https://bugzilla.kernel.org/show_bug.cgi?id=70651 Alex Deucher changed: What|Removed |Added CC||alexdeucher at gmail.com --- Comment #3 from Alex Deucher --- Does it work properly if you disable runtime pm? Boot with radeon.runpm=0 on the kernel command line in grub. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 --- Comment #3 from darkbasic --- I switched from libav/mplayer2 to ffmpeg/mplayer git and the crash seems gone. Bye bye libav. Being stable I did some more tests and unfortunately performance sucks so I renamed the bug report. Benchmarking the video output: ffh264 software decoder + gl/xv/vdpau ~ $ mplayer -benchmark -nosound -lavdopts threads=16 PlanetEarthBirds.mkv -vo gl BENCHMARKs: VC: 3.403s VO: 7.530s A: 0.000s Sys: 0.672s = 11.605s BENCHMARK%: VC: 29.3238% VO: 64.8892% A: 0.% Sys: 5.7870% = 100.% ~ $ mplayer -benchmark -nosound -lavdopts threads=16 PlanetEarthBirds.mkv -vo xv BENCHMARKs: VC: 7.878s VO: 2.162s A: 0.000s Sys: 1.040s = 11.080s BENCHMARK%: VC: 71.0951% VO: 19.5154% A: 0.% Sys: 9.3896% = 100.% ~ $ mplayer -benchmark -nosound -lavdopts threads=16 PlanetEarthBirds.mkv -vo vdpau BENCHMARKs: VC: 0.863s VO: 43.888s A: 0.000s Sys: 0.550s = 45.300s BENCHMARK%: VC: 1.9042% VO: 96.8823% A: 0.% Sys: 1.2135% = 100.% With both -vo gl and -vo xv it took ~11.080s while with -vo vdpau it took 45.300s. -vo vdpau is *4x* slower. Benchmarking the video decoder: ffh264/ffh264vdpau + vo null ~ $ mplayer -benchmark -nosound -lavdopts threads=16 PlanetEarthBirds.mkv -vo null BENCHMARKs: VC: 8.855s VO: 0.002s A: 0.000s Sys: 0.493s =9.350s BENCHMARK%: VC: 94.7024% VO: 0.0262% A: 0.% Sys: 5.2714% = 100.% ~ $ mplayer -benchmark -nosound PlanetEarthBirds.mkv -vo null -vc ffh264vdpau It seems I can't use the null video output with the hardware decoder ffh264vdpau: I get tons of "Too many buffered pts". It does support only -vo vdpau, not even xv or gl. Such a pity. (is it a known limitation/bug?) Let's do a more complete benchmark: ffh264+xv vs ffh264vdpau+vdpau We already saw ffh264+xv took 11.080s, let's see how ffh264vdpau+vdpau performs. ~ $ mplayer -benchmark -nosound PlanetEarthBirds.mkv -vo vdpau -vc ffh264vdpau BENCHMARKs: VC: 0.869s VO: 44.581s A: 0.000s Sys: 0.374s = 45.824s BENCHMARK%: VC: 1.8955% VO: 97.2876% A: 0.% Sys: 0.8169% = 100.% As expected it's still 4 times slower because the bottleneck is obviously the vdpau video output and not the ffh264vdpau decoder. Being 4x slower vdpau is completely useless right now :( Please note that I disabled desktop compositing before doing the tests. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/01dd5b72/attachment.html>
[Intel-gfx] [PATCH 0/6] Intel Color Manager Framework
On Fri, Feb 21, 2014 at 9:46 AM, Ville Syrj?l? wrote: > On Fri, Feb 21, 2014 at 02:20:24PM +, Sharma, Shashank wrote: >> Hi Ville, >> >> Thanks for your time and comments. >> I can understand two basic problems what you see in this implementation: >> >> 1. The most important issue from my POV is that it can't be part of the >> atomic modeset. >> 2. it make the whole API inconsistent. >> >> I am not sure if its good to block all current implementation because we >> have thought something for this in atomic modeset. >> I think even in atomic modeset we need the core implementation like this, >> but the interface would be different, which might come in from of a DRM >> property. >> So at that time we can use this core implementation as it is, only the >> interfaces/framework needs to be changed. >> >> In this way we can always go ahead with a current implementation, and can >> just change the interfaces to fit in to the final interface like DRM >> property in atomic modeset. >> Or you can suggest us the expected interface, and we can work on modifying >> that as per expectation. > > The exptected interface will be range properties for stuff like > brightness, contrast etc. controls. There are already such things as > connector properties, but we're going to want something similar as > plane or crtc properties. One thing that worries me about such > properties though is whether we can make them hardware agnostic and > yet allow userspace precise control over the final image. That is, if we > map some fixed input range to a hardware specific output range, userspace > can't know how the actual output will change when the input changes. On > the other hand if the input is hardware specific, userspace can't know > what value to put in there to get the expected change on the output side. > > For bigger stuff like CSC matrices and gamma ramps we will want to use > some reasonably well defined blobs. Ie. the internal strucuture of the > blob has to be documented and it shouldn't contain more than necessary. > Ie. just the CSC matrix coefficients for one matrix, or just the entries > for a single gamma ramp. Again ideally we should make the blobs hardware > agnostic, but still allow precise control over the output data. > > I think this is going to involve first going over our hardware features, > trying to find the common patterns between different generations. If > there's a way to make something that works across the board for us, or > at least across a wide range, then we should also ask for some input on > dri-devel whether the proposed property would work for other people. We > may need to define new property types to more precisely define what the > value of the property actually means. > Our hardware has similar features, so I'm sure there will be quite a bit of common ground. I also vote for properties. Alex
[Bug 70651] cannot write to power_dpm_force_performance_level: invalid argument (radeon 7730m)
https://bugzilla.kernel.org/show_bug.cgi?id=70651 --- Comment #4 from Alex Deucher --- Created attachment 126951 --> https://bugzilla.kernel.org/attachment.cgi?id=126951&action=edit possible fix for SI Starting with 3.13, the driver automatically powers down the dGPU on hybrid laptops when it's not in use. Accessing the sysfs and debufgs information when it's powered down is invalid since the gpu is powered down. This patch returns sensible values when you try to get/set this information when the GPU is powered off. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] drm/radeon/pm: handle powered down GPUs properly in sysfs/debugfs
When we power down the dGPU on a PX system, bail if the user tried to adjust the power state or check the temperature. The GPU is powered down, so it doesn't make sense to actually do anything. We could power up the dGPU to complete the operation, but it would just be undone again as soon as it was completed as the card would be powered down again. Return 0 for temperature and return -EINVAL for other interfaces. bug: https://bugzilla.kernel.org/show_bug.cgi?id=70651 Signed-off-by: Alex Deucher Cc: stable at vger.kernel.org --- drivers/gpu/drm/radeon/radeon_pm.c | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 8e8153e..6f20bb0 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -67,6 +67,11 @@ int radeon_pm_get_type_index(struct radeon_device *rdev, void radeon_pm_acpi_event_handler(struct radeon_device *rdev) { + struct drm_device *ddev = rdev->ddev; + + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) + return; + if ((rdev->pm.pm_method == PM_METHOD_DPM) && rdev->pm.dpm_enabled) { mutex_lock(&rdev->pm.mutex); if (power_supply_is_system_supplied() > 0) @@ -361,6 +366,9 @@ static ssize_t radeon_set_pm_profile(struct device *dev, struct drm_device *ddev = dev_get_drvdata(dev); struct radeon_device *rdev = ddev->dev_private; + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) + return -EINVAL; + mutex_lock(&rdev->pm.mutex); if (rdev->pm.pm_method == PM_METHOD_PROFILE) { if (strncmp("default", buf, strlen("default")) == 0) @@ -410,7 +418,8 @@ static ssize_t radeon_set_pm_method(struct device *dev, struct radeon_device *rdev = ddev->dev_private; /* we don't support the legacy modes with dpm */ - if (rdev->pm.pm_method == PM_METHOD_DPM) { + if ((rdev->pm.pm_method == PM_METHOD_DPM) || + (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)) { count = -EINVAL; goto fail; } @@ -459,6 +468,11 @@ static ssize_t radeon_set_dpm_state(struct device *dev, struct drm_device *ddev = dev_get_drvdata(dev); struct radeon_device *rdev = ddev->dev_private; + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) { + count = -EINVAL; + goto fail; + } + mutex_lock(&rdev->pm.mutex); if (strncmp("battery", buf, strlen("battery")) == 0) rdev->pm.dpm.user_state = POWER_STATE_TYPE_BATTERY; @@ -500,6 +514,9 @@ static ssize_t radeon_set_dpm_forced_performance_level(struct device *dev, enum radeon_dpm_forced_level level; int ret = 0; + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) + return -EINVAL; + mutex_lock(&rdev->pm.mutex); if (strncmp("low", buf, strlen("low")) == 0) { level = RADEON_DPM_FORCED_LEVEL_LOW; @@ -538,9 +555,13 @@ static ssize_t radeon_hwmon_show_temp(struct device *dev, char *buf) { struct radeon_device *rdev = dev_get_drvdata(dev); + struct drm_device *ddev = rdev->ddev; int temp; - if (rdev->asic->pm.get_temperature) + /* return 0 if PX card is off */ + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) + temp = 0; + else if (rdev->asic->pm.get_temperature) temp = radeon_get_temperature(rdev); else temp = 0; @@ -1579,7 +1600,9 @@ static int radeon_debugfs_pm_info(struct seq_file *m, void *data) struct drm_device *dev = node->minor->dev; struct radeon_device *rdev = dev->dev_private; - if (rdev->pm.dpm_enabled) { + if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) { + seq_printf(m, "Chip powered off\n"); + } else if (rdev->pm.dpm_enabled) { mutex_lock(&rdev->pm.mutex); if (rdev->asic->dpm.debugfs_print_current_performance_level) radeon_dpm_debugfs_print_current_performance_level(rdev, m); -- 1.8.3.1
[PATCH] drm/radeon/pm: handle powered down GPUs properly in sysfs/debugfs
Am 21.02.2014 16:50, schrieb Alex Deucher: > When we power down the dGPU on a PX system, bail if the > user tried to adjust the power state or check the temperature. > The GPU is powered down, so it doesn't make sense to actually > do anything. We could power up the dGPU to complete the > operation, but it would just be undone again as soon as it was > completed as the card would be powered down again. Return 0 > for temperature and return -EINVAL for other interfaces. > > bug: > https://bugzilla.kernel.org/show_bug.cgi?id=70651 > > Signed-off-by: Alex Deucher > Cc: stable at vger.kernel.org Reviewed-by: Christian K?nig > --- > drivers/gpu/drm/radeon/radeon_pm.c | 29 ++--- > 1 file changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_pm.c > b/drivers/gpu/drm/radeon/radeon_pm.c > index 8e8153e..6f20bb0 100644 > --- a/drivers/gpu/drm/radeon/radeon_pm.c > +++ b/drivers/gpu/drm/radeon/radeon_pm.c > @@ -67,6 +67,11 @@ int radeon_pm_get_type_index(struct radeon_device *rdev, > > void radeon_pm_acpi_event_handler(struct radeon_device *rdev) > { > + struct drm_device *ddev = rdev->ddev; > + > + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) > + return; > + > if ((rdev->pm.pm_method == PM_METHOD_DPM) && rdev->pm.dpm_enabled) { > mutex_lock(&rdev->pm.mutex); > if (power_supply_is_system_supplied() > 0) > @@ -361,6 +366,9 @@ static ssize_t radeon_set_pm_profile(struct device *dev, > struct drm_device *ddev = dev_get_drvdata(dev); > struct radeon_device *rdev = ddev->dev_private; > > + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) > + return -EINVAL; > + > mutex_lock(&rdev->pm.mutex); > if (rdev->pm.pm_method == PM_METHOD_PROFILE) { > if (strncmp("default", buf, strlen("default")) == 0) > @@ -410,7 +418,8 @@ static ssize_t radeon_set_pm_method(struct device *dev, > struct radeon_device *rdev = ddev->dev_private; > > /* we don't support the legacy modes with dpm */ > - if (rdev->pm.pm_method == PM_METHOD_DPM) { > + if ((rdev->pm.pm_method == PM_METHOD_DPM) || > + (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)) { > count = -EINVAL; > goto fail; > } > @@ -459,6 +468,11 @@ static ssize_t radeon_set_dpm_state(struct device *dev, > struct drm_device *ddev = dev_get_drvdata(dev); > struct radeon_device *rdev = ddev->dev_private; > > + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) { > + count = -EINVAL; > + goto fail; > + } > + > mutex_lock(&rdev->pm.mutex); > if (strncmp("battery", buf, strlen("battery")) == 0) > rdev->pm.dpm.user_state = POWER_STATE_TYPE_BATTERY; > @@ -500,6 +514,9 @@ static ssize_t > radeon_set_dpm_forced_performance_level(struct device *dev, > enum radeon_dpm_forced_level level; > int ret = 0; > > + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) > + return -EINVAL; > + > mutex_lock(&rdev->pm.mutex); > if (strncmp("low", buf, strlen("low")) == 0) { > level = RADEON_DPM_FORCED_LEVEL_LOW; > @@ -538,9 +555,13 @@ static ssize_t radeon_hwmon_show_temp(struct device *dev, > char *buf) > { > struct radeon_device *rdev = dev_get_drvdata(dev); > + struct drm_device *ddev = rdev->ddev; > int temp; > > - if (rdev->asic->pm.get_temperature) > + /* return 0 if PX card is off */ > + if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) > + temp = 0; > + else if (rdev->asic->pm.get_temperature) > temp = radeon_get_temperature(rdev); > else > temp = 0; > @@ -1579,7 +1600,9 @@ static int radeon_debugfs_pm_info(struct seq_file *m, > void *data) > struct drm_device *dev = node->minor->dev; > struct radeon_device *rdev = dev->dev_private; > > - if (rdev->pm.dpm_enabled) { > + if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) { > + seq_printf(m, "Chip powered off\n"); > + } else if (rdev->pm.dpm_enabled) { > mutex_lock(&rdev->pm.mutex); > if (rdev->asic->dpm.debugfs_print_current_performance_level) > > radeon_dpm_debugfs_print_current_performance_level(rdev, m);
[PATCH] drm/radeon: use variable UVD clocks
Now that Christian fixed the performance problems with the feedback buffer in mesa, we can enable variable UVD clocks. There are multiple UVD power states associated with different types and numbers of streams. This uses the appropriate state based on that information rather than always using the fastest UVD clocks which saves some power. One possible downside is that this may adversely affect decode benchmarks since these power states target specific playback requirements rather than maximum performance. If that becomes an issue, we can add a sysfs attribute to force the max UVD state. Signed-off-by: Alex Deucher --- drivers/gpu/drm/radeon/radeon_pm.c | 3 --- drivers/gpu/drm/radeon/radeon_uvd.c | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 6f20bb0..2cb2fb8 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -956,8 +956,6 @@ void radeon_dpm_enable_uvd(struct radeon_device *rdev, bool enable) if (enable) { mutex_lock(&rdev->pm.mutex); rdev->pm.dpm.uvd_active = true; - /* disable this for now */ -#if 0 if ((rdev->pm.dpm.sd == 1) && (rdev->pm.dpm.hd == 0)) dpm_state = POWER_STATE_TYPE_INTERNAL_UVD_SD; else if ((rdev->pm.dpm.sd == 2) && (rdev->pm.dpm.hd == 0)) @@ -967,7 +965,6 @@ void radeon_dpm_enable_uvd(struct radeon_device *rdev, bool enable) else if ((rdev->pm.dpm.sd == 0) && (rdev->pm.dpm.hd == 2)) dpm_state = POWER_STATE_TYPE_INTERNAL_UVD_HD2; else -#endif dpm_state = POWER_STATE_TYPE_INTERNAL_UVD; rdev->pm.dpm.state = dpm_state; mutex_unlock(&rdev->pm.mutex); diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c index 6781fee..ceb7b28 100644 --- a/drivers/gpu/drm/radeon/radeon_uvd.c +++ b/drivers/gpu/drm/radeon/radeon_uvd.c @@ -805,8 +805,7 @@ void radeon_uvd_note_usage(struct radeon_device *rdev) (rdev->pm.dpm.hd != hd)) { rdev->pm.dpm.sd = sd; rdev->pm.dpm.hd = hd; - /* disable this for now */ - /*streams_changed = true;*/ + streams_changed = true; } } -- 1.8.3.1
[PATCH 1/5] drm/radeon: separate gart and vm functions
On Thu, Feb 20, 2014 at 3:20 PM, Christian K?nig wrote: > From: Christian K?nig > > Both are complex enough on their own. > > Signed-off-by: Christian K?nig For the series: Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/Makefile | 2 +- > drivers/gpu/drm/radeon/radeon_gart.c | 958 -- > drivers/gpu/drm/radeon/radeon_vm.c | 981 > +++ > 3 files changed, 982 insertions(+), 959 deletions(-) > create mode 100644 drivers/gpu/drm/radeon/radeon_vm.c > > diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile > index ed60caa..0943353 100644 > --- a/drivers/gpu/drm/radeon/Makefile > +++ b/drivers/gpu/drm/radeon/Makefile > @@ -80,7 +80,7 @@ radeon-y += radeon_device.o radeon_asic.o radeon_kms.o \ > r600_dpm.o rs780_dpm.o rv6xx_dpm.o rv770_dpm.o rv730_dpm.o > rv740_dpm.o \ > rv770_smc.o cypress_dpm.o btc_dpm.o sumo_dpm.o sumo_smc.o > trinity_dpm.o \ > trinity_smc.o ni_dpm.o si_smc.o si_dpm.o kv_smc.o kv_dpm.o ci_smc.o \ > - ci_dpm.o dce6_afmt.o > + ci_dpm.o dce6_afmt.o radeon_vm.o > > # add async DMA block > radeon-y += \ > diff --git a/drivers/gpu/drm/radeon/radeon_gart.c > b/drivers/gpu/drm/radeon/radeon_gart.c > index a8f9b46..2e72365 100644 > --- a/drivers/gpu/drm/radeon/radeon_gart.c > +++ b/drivers/gpu/drm/radeon/radeon_gart.c > @@ -28,8 +28,6 @@ > #include > #include > #include "radeon.h" > -#include "radeon_reg.h" > -#include "radeon_trace.h" > > /* > * GART > @@ -394,959 +392,3 @@ void radeon_gart_fini(struct radeon_device *rdev) > > radeon_dummy_page_fini(rdev); > } > - > -/* > - * GPUVM > - * GPUVM is similar to the legacy gart on older asics, however > - * rather than there being a single global gart table > - * for the entire GPU, there are multiple VM page tables active > - * at any given time. The VM page tables can contain a mix > - * vram pages and system memory pages and system memory pages > - * can be mapped as snooped (cached system pages) or unsnooped > - * (uncached system pages). > - * Each VM has an ID associated with it and there is a page table > - * associated with each VMID. When execting a command buffer, > - * the kernel tells the the ring what VMID to use for that command > - * buffer. VMIDs are allocated dynamically as commands are submitted. > - * The userspace drivers maintain their own address space and the kernel > - * sets up their pages tables accordingly when they submit their > - * command buffers and a VMID is assigned. > - * Cayman/Trinity support up to 8 active VMs at any given time; > - * SI supports 16. > - */ > - > -/* > - * vm helpers > - * > - * TODO bind a default page at vm initialization for default address > - */ > - > -/** > - * radeon_vm_num_pde - return the number of page directory entries > - * > - * @rdev: radeon_device pointer > - * > - * Calculate the number of page directory entries (cayman+). > - */ > -static unsigned radeon_vm_num_pdes(struct radeon_device *rdev) > -{ > - return rdev->vm_manager.max_pfn >> RADEON_VM_BLOCK_SIZE; > -} > - > -/** > - * radeon_vm_directory_size - returns the size of the page directory in bytes > - * > - * @rdev: radeon_device pointer > - * > - * Calculate the size of the page directory in bytes (cayman+). > - */ > -static unsigned radeon_vm_directory_size(struct radeon_device *rdev) > -{ > - return RADEON_GPU_PAGE_ALIGN(radeon_vm_num_pdes(rdev) * 8); > -} > - > -/** > - * radeon_vm_manager_init - init the vm manager > - * > - * @rdev: radeon_device pointer > - * > - * Init the vm manager (cayman+). > - * Returns 0 for success, error for failure. > - */ > -int radeon_vm_manager_init(struct radeon_device *rdev) > -{ > - struct radeon_vm *vm; > - struct radeon_bo_va *bo_va; > - int r; > - unsigned size; > - > - if (!rdev->vm_manager.enabled) { > - /* allocate enough for 2 full VM pts */ > - size = radeon_vm_directory_size(rdev); > - size += rdev->vm_manager.max_pfn * 8; > - size *= 2; > - r = radeon_sa_bo_manager_init(rdev, > &rdev->vm_manager.sa_manager, > - RADEON_GPU_PAGE_ALIGN(size), > - RADEON_VM_PTB_ALIGN_SIZE, > - RADEON_GEM_DOMAIN_VRAM); > - if (r) { > - dev_err(rdev->dev, "failed to allocate vm bo > (%dKB)\n", > - (rdev->vm_manager.max_pfn * 8) >> 10); > - return r; > - } > - > - r = radeon_asic_vm_init(rdev); > - if (r) > - return r; > - > - rdev->vm_manager.enabled = true; > - > - r = radeon_sa_bo_manager_start(rdev, > &rdev->vm_manager.sa_manager); > - if (r) > - return r; > - } > -
[PATCH] drm/radeon: use variable UVD clocks
Am 21.02.2014 17:34, schrieb Alex Deucher: > Now that Christian fixed the performance problems with > the feedback buffer in mesa, we can enable variable UVD > clocks. There are multiple UVD power states associated > with different types and numbers of streams. This uses > the appropriate state based on that information rather > than always using the fastest UVD clocks which saves some > power. One possible downside is that this may adversely > affect decode benchmarks since these power states target > specific playback requirements rather than maximum > performance. If that becomes an issue, we can add a > sysfs attribute to force the max UVD state. > > Signed-off-by: Alex Deucher Reviewed-by: Christian K?nig Additional to that we should also count the number of frames per second submitted to choose a power state, but that's not so urgent right now. Do you want to pull that in through drm-fixes or should I apply it to the drm-next-3.15 tree? For me it sounds more like drm-next. Christian. > --- > drivers/gpu/drm/radeon/radeon_pm.c | 3 --- > drivers/gpu/drm/radeon/radeon_uvd.c | 3 +-- > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_pm.c > b/drivers/gpu/drm/radeon/radeon_pm.c > index 6f20bb0..2cb2fb8 100644 > --- a/drivers/gpu/drm/radeon/radeon_pm.c > +++ b/drivers/gpu/drm/radeon/radeon_pm.c > @@ -956,8 +956,6 @@ void radeon_dpm_enable_uvd(struct radeon_device *rdev, > bool enable) > if (enable) { > mutex_lock(&rdev->pm.mutex); > rdev->pm.dpm.uvd_active = true; > - /* disable this for now */ > -#if 0 > if ((rdev->pm.dpm.sd == 1) && (rdev->pm.dpm.hd == 0)) > dpm_state = POWER_STATE_TYPE_INTERNAL_UVD_SD; > else if ((rdev->pm.dpm.sd == 2) && (rdev->pm.dpm.hd == > 0)) > @@ -967,7 +965,6 @@ void radeon_dpm_enable_uvd(struct radeon_device *rdev, > bool enable) > else if ((rdev->pm.dpm.sd == 0) && (rdev->pm.dpm.hd == > 2)) > dpm_state = POWER_STATE_TYPE_INTERNAL_UVD_HD2; > else > -#endif > dpm_state = POWER_STATE_TYPE_INTERNAL_UVD; > rdev->pm.dpm.state = dpm_state; > mutex_unlock(&rdev->pm.mutex); > diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c > b/drivers/gpu/drm/radeon/radeon_uvd.c > index 6781fee..ceb7b28 100644 > --- a/drivers/gpu/drm/radeon/radeon_uvd.c > +++ b/drivers/gpu/drm/radeon/radeon_uvd.c > @@ -805,8 +805,7 @@ void radeon_uvd_note_usage(struct radeon_device *rdev) > (rdev->pm.dpm.hd != hd)) { > rdev->pm.dpm.sd = sd; > rdev->pm.dpm.hd = hd; > - /* disable this for now */ > - /*streams_changed = true;*/ > + streams_changed = true; > } > } >
[PATCH] drm/radeon: use variable UVD clocks
On Fri, Feb 21, 2014 at 12:01 PM, Christian K?nig wrote: > Am 21.02.2014 17:34, schrieb Alex Deucher: > >> Now that Christian fixed the performance problems with >> the feedback buffer in mesa, we can enable variable UVD >> clocks. There are multiple UVD power states associated >> with different types and numbers of streams. This uses >> the appropriate state based on that information rather >> than always using the fastest UVD clocks which saves some >> power. One possible downside is that this may adversely >> affect decode benchmarks since these power states target >> specific playback requirements rather than maximum >> performance. If that becomes an issue, we can add a >> sysfs attribute to force the max UVD state. >> >> Signed-off-by: Alex Deucher > > > Reviewed-by: Christian K?nig > > Additional to that we should also count the number of frames per second > submitted to choose a power state, but that's not so urgent right now. > > Do you want to pull that in through drm-fixes or should I apply it to the > drm-next-3.15 tree? For me it sounds more like drm-next. Yes, this is drm-next material. thanks! Alex > > Christian. > > >> --- >> drivers/gpu/drm/radeon/radeon_pm.c | 3 --- >> drivers/gpu/drm/radeon/radeon_uvd.c | 3 +-- >> 2 files changed, 1 insertion(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c >> b/drivers/gpu/drm/radeon/radeon_pm.c >> index 6f20bb0..2cb2fb8 100644 >> --- a/drivers/gpu/drm/radeon/radeon_pm.c >> +++ b/drivers/gpu/drm/radeon/radeon_pm.c >> @@ -956,8 +956,6 @@ void radeon_dpm_enable_uvd(struct radeon_device *rdev, >> bool enable) >> if (enable) { >> mutex_lock(&rdev->pm.mutex); >> rdev->pm.dpm.uvd_active = true; >> - /* disable this for now */ >> -#if 0 >> if ((rdev->pm.dpm.sd == 1) && (rdev->pm.dpm.hd == >> 0)) >> dpm_state = >> POWER_STATE_TYPE_INTERNAL_UVD_SD; >> else if ((rdev->pm.dpm.sd == 2) && >> (rdev->pm.dpm.hd == 0)) >> @@ -967,7 +965,6 @@ void radeon_dpm_enable_uvd(struct radeon_device *rdev, >> bool enable) >> else if ((rdev->pm.dpm.sd == 0) && >> (rdev->pm.dpm.hd == 2)) >> dpm_state = >> POWER_STATE_TYPE_INTERNAL_UVD_HD2; >> else >> -#endif >> dpm_state = POWER_STATE_TYPE_INTERNAL_UVD; >> rdev->pm.dpm.state = dpm_state; >> mutex_unlock(&rdev->pm.mutex); >> diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c >> b/drivers/gpu/drm/radeon/radeon_uvd.c >> index 6781fee..ceb7b28 100644 >> --- a/drivers/gpu/drm/radeon/radeon_uvd.c >> +++ b/drivers/gpu/drm/radeon/radeon_uvd.c >> @@ -805,8 +805,7 @@ void radeon_uvd_note_usage(struct radeon_device *rdev) >> (rdev->pm.dpm.hd != hd)) { >> rdev->pm.dpm.sd = sd; >> rdev->pm.dpm.hd = hd; >> - /* disable this for now */ >> - /*streams_changed = true;*/ >> + streams_changed = true; >> } >> } >> > >
[Bug 74764] Steam overlay not working
https://bugs.freedesktop.org/show_bug.cgi?id=74764 --- Comment #12 from Kertesz Laszlo --- (In reply to comment #11) > I definitely have this problem with latest git, didn't try early version. > With STEAM_OVERLAY=0 it works flawlessly, unfortunately this is not a > solution. > With Catalyst it works flawlessly even with STEAM_OVERLAY=1. I use mesa+kernel from git with a A8-6500 APU/Radeon HD 8570D IGP ( that uses r600g and steam overlay works at least with Day of Defeat and L4D2. Debian Testing 64 bit, xfce 4.10. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/725ea60a/attachment.html>
[Intel-gfx] [PATCH 0/6] Intel Color Manager Framework
Hi Ville/All, We gave a presentation on design on this framework, few months ago, in one of our common forum with OTC folks. We discussed, took review comments, and re-designed the framework, as per the feedbacks. We also discussed the benefits of providing the controls directly from /sysfs over going for a UI manager based property settings. So I don't understand where are we going wrong, can you please elaborate a bit ? This is just a basic design, and once go ahead with this, we can always work on making hardware agnostic, as you recommended. IMHO, controls from /sysfs would be a very generic interface for all linux/drm based platform, where any userspace can read/write and control properties. We don't even need a UI manager or a minimum executable to play around, just a small script can do. But we can always write something on top of this, to be included in any UI framework or property. Regards Shashank -Original Message- From: Ville Syrj?l? [mailto:ville.syrj...@linux.intel.com] Sent: Thursday, February 20, 2014 6:41 PM To: Sharma, Shashank Cc: intel-gfx at lists.freedesktop.org; Shankar, Uma; dri-devel at lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 0/6] Intel Color Manager Framework On Thu, Feb 20, 2014 at 06:07:21PM +0530, Shashank Sharma wrote: > Color manager is a new framework in i915 driver, which provides a > unified interface for various color correction methods supported by > intel hardwares. The high level overview of this change is: Would have been good to discuss this idea before implementing it. The plan is to use kms properties for this kind of stuff which allows us to hook it up with the upcoming atomic modeset API. Just yesterday there was some discussion on #dri-devel about exposing user settable blob properties even before the atomic modeset API lands (it was always the plan for the atomic modeset API anyway). So based on a cursory glance, this looks like it's going in the wrong direction. Also ideally the properties should be hardware agnostic, so a generic userspace could use them regardless of the hardware/driver. Obviously that might not be possible in all cases, but we should at least spend a bit of effort on trying to make that happen for most properties. -- Ville Syrj?l? Intel OTC
[PATCH] nouveau, ACPI: fix regression caused by b072e53
Thanks, Rafael. Will cc ACPI maillist next time. On 2014/2/21 4:27, Rafael J. Wysocki wrote: > On 2/20/2014 10:23 AM, Jiang Liu wrote: >> Fix regression caused by commit b072e53, which breaks loading nouveau >> driver on optimus laptops. >> >> On some platforms, ACPI _DSM method (nouveau_op_dsm_muid, function 0) >> has special requirements on the fourth parameter, which is different >> from ACPI specifications. So revert to the private implementation >> to check availability of _DSM functions instead of using common >> acpi_check_dsm() interface. >> >> Reported-and-Tested-by: Maarten Lankhorst >> >> Signed-off-by: Jiang Liu > > I'm taking this, because the commit that introduced the regression went > in through my tree. > > In the future I'll appreciate CCing ACPI-related patches to linux-acpi, > however. > > Thanks, > Rafael > > >> --- >> drivers/gpu/drm/nouveau/nouveau_acpi.c | 26 >> -- >> 1 file changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c >> b/drivers/gpu/drm/nouveau/nouveau_acpi.c >> index 4ef83df..83face3 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c >> @@ -106,6 +106,29 @@ static int nouveau_optimus_dsm(acpi_handle >> handle, int func, int arg, uint32_t * >> return 0; >> } >> +/* >> + * On some platforms, _DSM(nouveau_op_dsm_muid, func0) has special >> + * requirements on the fourth parameter, so a private implementation >> + * instead of using acpi_check_dsm(). >> + */ >> +static int nouveau_check_optimus_dsm(acpi_handle handle) >> +{ >> +int result; >> + >> +/* >> + * Function 0 returns a Buffer containing available functions. >> + * The args parameter is ignored for function 0, so just put 0 in it >> + */ >> +if (nouveau_optimus_dsm(handle, 0, 0, &result)) >> +return 0; >> + >> +/* >> + * ACPI Spec v4 9.14.1: if bit 0 is zero, no function is supported. >> + * If the n-th bit is enabled, function n is supported >> + */ >> +return result & 1 && result & (1 << NOUVEAU_DSM_OPTIMUS_CAPS); >> +} >> + >> static int nouveau_dsm(acpi_handle handle, int func, int arg) >> { >> int ret = 0; >> @@ -207,8 +230,7 @@ static int nouveau_dsm_pci_probe(struct pci_dev >> *pdev) >> 1 << NOUVEAU_DSM_POWER)) >> retval |= NOUVEAU_DSM_HAS_MUX; >> -if (acpi_check_dsm(dhandle, nouveau_op_dsm_muid, 0x0100, >> - 1 << NOUVEAU_DSM_OPTIMUS_CAPS)) >> +if (nouveau_check_optimus_dsm(dhandle)) >> retval |= NOUVEAU_DSM_HAS_OPT; >> if (retval & NOUVEAU_DSM_HAS_OPT) { > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
[Intel-gfx] [PATCH 0/6] Intel Color Manager Framework
Hi Ville, Thanks for your time and comments. I can understand two basic problems what you see in this implementation: 1. The most important issue from my POV is that it can't be part of the atomic modeset. 2. it make the whole API inconsistent. I am not sure if its good to block all current implementation because we have thought something for this in atomic modeset. I think even in atomic modeset we need the core implementation like this, but the interface would be different, which might come in from of a DRM property. So at that time we can use this core implementation as it is, only the interfaces/framework needs to be changed. In this way we can always go ahead with a current implementation, and can just change the interfaces to fit in to the final interface like DRM property in atomic modeset. Or you can suggest us the expected interface, and we can work on modifying that as per expectation. Please correct me if any of my assumptions are not right, or not feasible, or if I am just a moron :) . Regards Shashank -Original Message- From: Ville Syrj?l? [mailto:ville.syrj...@linux.intel.com] Sent: Friday, February 21, 2014 2:47 PM To: Sharma, Shashank Cc: intel-gfx at lists.freedesktop.org; Shankar, Uma; dri-devel at lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 0/6] Intel Color Manager Framework On Fri, Feb 21, 2014 at 03:34:43AM +, Sharma, Shashank wrote: > Hi Ville/All, > > We gave a presentation on design on this framework, few months ago, in one of > our common forum with OTC folks. > We discussed, took review comments, and re-designed the framework, as per > the feedbacks. Apparently I wasn't there. And in any case it would be better to discuss it on dri-devel where people outside Intel can give their opinion. > > We also discussed the benefits of providing the controls directly from /sysfs > over going for a UI manager based property settings. > So I don't understand where are we going wrong, can you please elaborate a > bit ? The most important issue from my POV is that it can't be part of the atomic modeset. Another issue is that it make the whole API inconsistent. Some stuff through ioctl, some stuff through sysfs, some stuff through whatever the next guy thinks of. It's not pretty. I've worked in the past with a driver where I had to poke at various standardish ioctls, custom ioctls, and sysfs to make it do anything useful, and I have no interest in repeating that experience. sysfs is especially painful since you have do the string<->binary conversions all over the place, and also you en up doing open+read/write+close cycles for every little thing. It also adds more entrypoints into the driver for us to worry about. That means extra worries about the power management stuff and locking at the very least. Also the rules of sysfs say "one item per file". The only allowed exception to this rule I know of is hardware provided blobs (like EDID, PCI ROM etc.). Your current implementation breaks this rule blatantly. > > This is just a basic design, and once go ahead with this, we can always work > on making hardware agnostic, as you recommended. > > IMHO, controls from /sysfs would be a very generic interface for all > linux/drm based platform, where any userspace can read/write and control > properties. > We don't even need a UI manager or a minimum executable to play > around, just a small script can do. But we can always write something on top > of this, to be included in any UI framework or property. If there's a real need to get at properties through sysfs, then we could think about exposing them all. But that may presents some issues where the current master suddenly gets confused about its state since someone else went behind its back and changed a bunch of stuff. > > Regards > Shashank > > -Original Message- > From: Ville Syrj?l? [mailto:ville.syrjala at linux.intel.com] > Sent: Thursday, February 20, 2014 6:41 PM > To: Sharma, Shashank > Cc: intel-gfx at lists.freedesktop.org; Shankar, Uma; > dri-devel at lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 0/6] Intel Color Manager Framework > > On Thu, Feb 20, 2014 at 06:07:21PM +0530, Shashank Sharma wrote: > > Color manager is a new framework in i915 driver, which provides a > > unified interface for various color correction methods supported by > > intel hardwares. The high level overview of this change is: > > Would have been good to discuss this idea before implementing it. The plan is > to use kms properties for this kind of stuff which allows us to hook it up > with the upcoming atomic modeset API. Just yesterday there was some > discussion on #dri-devel about exposing user settable blob properties even > before the atomic modeset API lands (it was always the plan for the atomic > modeset API anyway). So based on a cursory glance, this looks like it's going > in the wrong direction. > > Also ideally the properties
[GIT PULL] TDA998x I2C updates
David, Please incorporate the latest TDA998x I2C updates, which can be found at: git://ftp.arm.linux.org.uk/~rmk/linux-cubox.git tda998x-devel with SHA1 5e7fe2fef4347d7a09bb15588d8bbe3cb83b6ed4. Updates from Jean-Fracois for the TDA998x driver, which are on top of the fixes you have previously pulled, except these changes aren't intended for -rc, but the next merge window. Several of these are issues of correctness - passing more correct HDMI info packets, not reading registers in older chips documented as write only (despite appearing to be read/write in later chips). Others are code cleanups (using definitions rather than constants where we have them already in the kernel). Additional functionality is also added by way of optional support for the IRQ from the TDA998x, which allows us to avoid busy-waiting for the EDID reads. This will update the following files: .../devicetree/bindings/drm/i2c/tda998x.txt| 27 + drivers/gpu/drm/i2c/tda998x_drv.c | 602 + 2 files changed, 410 insertions(+), 219 deletions(-) create mode 100644 Documentation/devicetree/bindings/drm/i2c/tda998x.txt through these changes: Jean-Francois Moine (17): drm/i2c: tda998x: use HDMI constants drm/i2c: tda998x: add the active aspect in HDMI AVI frame drm/i2c: tda998x: use ALSA IEC958 definitions and update audio frequency drm/i2c: tda998x: simplify the i2c read/write functions drm/i2c: tda998x: check more I/O errors drm/i2c: tda998x: code cleanup drm/i2c: tda998x: change probe message origin drm/i2c: tda998x: don't freeze the system at audio startup time drm/i2c: tda998x: don't read write-only registers drm/i2c: tda998x: add DT support drm/i2c: tda998x: add DT documentation drm/i2c: tda998x: always enable EDID read IRQ drm/i2c: tda998x: use irq for connection status and EDID read drm/i2c: tda998x: make the audio code more readable drm/i2c: tda998x: remove the unused variable ca_i2s drm/i2c: tda998x: code optimization drm/i2c: tda998x: adjust the audio clock divider for S/PDIF Russell King (2): drm/i2c: tda998x: clean up error chip version checking drm/i2c: tda998x: always use the same device for all kernel messages Thanks.
[Intel-gfx] [PATCH 0/6] Intel Color Manager Framework
On Fri, Feb 21, 2014 at 9:49 AM, Rob Clark wrote: > On Fri, Feb 21, 2014 at 9:20 AM, Sharma, Shashank > wrote: >> Hi Ville, >> >> Thanks for your time and comments. >> I can understand two basic problems what you see in this implementation: >> >> 1. The most important issue from my POV is that it can't be part of the >> atomic modeset. >> 2. it make the whole API inconsistent. >> >> I am not sure if its good to block all current implementation because we >> have thought something for this in atomic modeset. >> I think even in atomic modeset we need the core implementation like this, >> but the interface would be different, which might come in from of a DRM >> property. >> So at that time we can use this core implementation as it is, only the >> interfaces/framework needs to be changed. > > What I would suggest is re-form the userspace facing API to be > properties.. if needed we can add a setblobprop ioctl (Sean Paul was, > I think, adding that already). This is news to me ;-) I'm pulling the atomic stuff into the CrOS tree, but I think Rahul Sharma is the guy you're looking for wrt setblobprop (http://www.spinics.net/lists/dri-devel/msg54010.html). Sean > Then userspace and use setprop ioctls > for now, and optionally atomic ioctl later when it is in place. No > reason for it to be blocked waiting for atomic. > > btw, I didn't look into the patches yet, but full-nak on idea of > exposing via sysfs. This should be the sort of thing that is set by > the process that has mastership on the drm device, which we can't > enforce via sysfs. Using properties seems like the way to go. > > BR, > -R > >> In this way we can always go ahead with a current implementation, and can >> just change the interfaces to fit in to the final interface like DRM >> property in atomic modeset. >> Or you can suggest us the expected interface, and we can work on modifying >> that as per expectation.
[Intel-gfx] [PATCH 0/6] Intel Color Manager Framework
On Thu, Feb 20, 2014 at 5:11 AM, Ville Syrj?l? < ville.syrjala at linux.intel.com> wrote: > On Thu, Feb 20, 2014 at 06:07:21PM +0530, Shashank Sharma wrote: > > Color manager is a new framework in i915 driver, which provides > > a unified interface for various color correction methods supported > > by intel hardwares. The high level overview of this change is: > > Would have been good to discuss this idea before implementing it. The > plan is to use kms properties for this kind of stuff which allows us > to hook it up with the upcoming atomic modeset API. Just yesterday there > was some discussion on #dri-devel about exposing user settable blob > properties even before the atomic modeset API lands (it was always the > plan for the atomic modeset API anyway). So based on a cursory glance, > this looks like it's going in the wrong direction. > +1. We'e looking into hooking up color correction controls, and if the interface isn't standard our user space won't be portable across drivers. There are multiple reasons for using drm properties: - the KMS interface already provides a way to set the gamma ramp, which this code seems to replicate. - the KMS interface allows us to name properties independently and enumerate them. It seems like right now you can't enumerate properties or guess what "property 0" is. I'd rather set the "Color conversion matrix" than remember to set "property 0" (and even then, I'm not really sure it exists). - you can reuse the get/set infrastructure which is already in place Another thing that came out of the discussion on irc is that we should standardize the properties. For example we could use a text file describing the name of the controls and the format of the data (something similar to the device tree bindings). That way user space can expect "color conversion matrix" to mean the same thing everywhere, to get the same data as input, and to work the same way on all platforms. St?phane -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140221/afa7ddd4/attachment.html>
[PATCH 0/5] drm: Allow vblank interrupts during modeset and make the code less racy
From: Ville Syrj?l? I'm going to require vblank interrupts during modeset in i915 soon, however the drm vblank code won't currently allow it. It simply refuses to re-enable the vblank interrupts whenever they were disable and the refcount is already elevated. That means anyone holding a vblank reference across a modeset/dpms cycle can prevent the driver from operating correctly. The same problem can also cause wait for vblank ioctls issued soon after a modeset to fail. I've cooked up a few i-g-t/kms_flip subtests to excercise this. I don't want to be responsible for potentially breaking every other drm driver, so I've made the new behaviour opt in. If the driver uses the new way of doing things, vblank interrupts (and ioctls) are allowed everywhere except between drm_vblank_off() and drm_vblanl_on(). Any wait for vblank ioctl currently pending will get completed when drm_vblank_off() is called, including blocking waits which previously could have just sat there until the timeout expired. I also fixed the vblank disable timer to act predictably when multiple crtcs are involved , and I hoovered up an old patch from Peter Hurley to skip the double irqsave in drm_vblank_get(). Peter Hurley (1): drm: Use correct spinlock flavor in drm_vblank_get() Ville Syrj?l? (4): drm: Make the vblank disable timer per-crtc drm: Allow the driver to reject vblank requests only when it really has the vblank interrupts disabled drm: Allow reenabling of vblank interrupts even if refcount>0 drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races drivers/gpu/drm/armada/armada_crtc.c | 2 +- drivers/gpu/drm/drm_irq.c| 78 ++-- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +- drivers/gpu/drm/gma500/gma_display.c | 2 +- drivers/gpu/drm/i915/i915_dma.c | 6 +++ drivers/gpu/drm/i915/intel_display.c | 23 ++ drivers/gpu/drm/tegra/dc.c | 2 +- include/drm/drmP.h | 14 +- 8 files changed, 91 insertions(+), 38 deletions(-) -- 1.8.3.2
[PATCH 1/5] drm: Use correct spinlock flavor in drm_vblank_get()
From: Peter Hurley The irq flags state is already established by the outer spin_lock_irqsave(); re-disabling irqs is redundant. Signed-off-by: Peter Hurley --- drivers/gpu/drm/drm_irq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c2676b5..baa12e7 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -882,13 +882,13 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) */ int drm_vblank_get(struct drm_device *dev, int crtc) { - unsigned long irqflags, irqflags2; + unsigned long irqflags; int ret = 0; spin_lock_irqsave(&dev->vbl_lock, irqflags); /* Going from 0->1 means we have to enable interrupts again */ if (atomic_add_return(1, &dev->vblank[crtc].refcount) == 1) { - spin_lock_irqsave(&dev->vblank_time_lock, irqflags2); + spin_lock(&dev->vblank_time_lock); if (!dev->vblank[crtc].enabled) { /* Enable vblank irqs under vblank_time_lock protection. * All vblank count & timestamp updates are held off @@ -906,7 +906,7 @@ int drm_vblank_get(struct drm_device *dev, int crtc) drm_update_vblank_count(dev, crtc); } } - spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags2); + spin_unlock(&dev->vblank_time_lock); } else { if (!dev->vblank[crtc].enabled) { atomic_dec(&dev->vblank[crtc].refcount); -- 1.8.3.2
[PATCH 2/5] drm: Make the vblank disable timer per-crtc
From: Ville Syrj?l? Currently there's one per-device vblank disable timer, and it gets reset wheneven the vblank refcount for any crtc drops to zero. That means that one crtc could accidentally be keeping the vblank interrupts for other crtcs enabled even if there are no users for them. Make the disable timer per-crtc to avoid this issue. Signed-off-by: Ville Syrj?l? --- drivers/gpu/drm/drm_irq.c | 40 +--- include/drm/drmP.h| 4 +++- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index baa12e7..3211158 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -167,33 +167,34 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) static void vblank_disable_fn(unsigned long arg) { - struct drm_device *dev = (struct drm_device *)arg; + struct drm_vblank_crtc *vblank = (void *)arg; + struct drm_device *dev = vblank->dev; unsigned long irqflags; - int i; + int crtc = vblank->crtc; if (!dev->vblank_disable_allowed) return; - for (i = 0; i < dev->num_crtcs; i++) { - spin_lock_irqsave(&dev->vbl_lock, irqflags); - if (atomic_read(&dev->vblank[i].refcount) == 0 && - dev->vblank[i].enabled) { - DRM_DEBUG("disabling vblank on crtc %d\n", i); - vblank_disable_and_save(dev, i); - } - spin_unlock_irqrestore(&dev->vbl_lock, irqflags); + spin_lock_irqsave(&dev->vbl_lock, irqflags); + if (atomic_read(&vblank->refcount) == 0 && vblank->enabled) { + DRM_DEBUG("disabling vblank on crtc %d\n", crtc); + vblank_disable_and_save(dev, crtc); } + spin_unlock_irqrestore(&dev->vbl_lock, irqflags); } void drm_vblank_cleanup(struct drm_device *dev) { + int crtc; + /* Bail if the driver didn't call drm_vblank_init() */ if (dev->num_crtcs == 0) return; - del_timer_sync(&dev->vblank_disable_timer); - - vblank_disable_fn((unsigned long)dev); + for (crtc = 0; crtc < dev->num_crtcs; crtc++) { + del_timer_sync(&dev->vblank[crtc].disable_timer); + vblank_disable_fn((unsigned long)&dev->vblank[crtc]); + } kfree(dev->vblank); @@ -205,8 +206,6 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) { int i, ret = -ENOMEM; - setup_timer(&dev->vblank_disable_timer, vblank_disable_fn, - (unsigned long)dev); spin_lock_init(&dev->vbl_lock); spin_lock_init(&dev->vblank_time_lock); @@ -216,8 +215,13 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) if (!dev->vblank) goto err; - for (i = 0; i < num_crtcs; i++) + for (i = 0; i < num_crtcs; i++) { + dev->vblank[i].dev = dev; + dev->vblank[i].crtc = i; init_waitqueue_head(&dev->vblank[i].queue); + setup_timer(&dev->vblank[i].disable_timer, vblank_disable_fn, + (unsigned long)&dev->vblank[i]); + } DRM_INFO("Supports vblank timestamp caching Rev 2 (21.10.2013).\n"); @@ -934,7 +938,7 @@ void drm_vblank_put(struct drm_device *dev, int crtc) /* Last user schedules interrupt disable */ if (atomic_dec_and_test(&dev->vblank[crtc].refcount) && (drm_vblank_offdelay > 0)) - mod_timer(&dev->vblank_disable_timer, + mod_timer(&dev->vblank[crtc].disable_timer, jiffies + ((drm_vblank_offdelay * HZ)/1000)); } EXPORT_SYMBOL(drm_vblank_put); @@ -943,8 +947,6 @@ EXPORT_SYMBOL(drm_vblank_put); * drm_vblank_off - disable vblank events on a CRTC * @dev: DRM device * @crtc: CRTC in question - * - * Caller must hold event lock. */ void drm_vblank_off(struct drm_device *dev, int crtc) { diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 04a7f31..f974da9 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1077,14 +1077,17 @@ struct drm_pending_vblank_event { }; struct drm_vblank_crtc { + struct drm_device *dev; /* pointer to the drm_device */ wait_queue_head_t queue;/**< VBLANK wait queue */ struct timeval time[DRM_VBLANKTIME_RBSIZE]; /**< timestamp of current count */ + struct timer_list disable_timer;/* delayed disable timer */ atomic_t count; /**< number of VBLANK interrupts */ atomic_t refcount; /* number of users of vblank interruptsper crtc */ u32 last; /* protected by dev->vbl_lock, used */ /* for wraparound handling */ u32 last_wait; /* Last vblank seqno waited per CRTC */ unsigned int inmodeset;
[PATCH 4/5] drm: Allow reenabling of vblank interrupts even if refcount>0
From: Ville Syrj?l? If someone holds a vblank reference across the modeset, and after/during the modeset someone tries to grab a vblank reference, the current code won't re-enable the vblank interrupts. That's not good, so instead allow the driver to choose whether drm_vblank_get() should always enable the interrupts regardless of the refcount. Combined with the drm_vblank_off/drm_vblank_on reject mechanism, this can also be used to allow drivers to use vblank interrupts during modeset, whether or not someone is currently holding a vblank reference. Signed-off-by: Ville Syrj?l? --- drivers/gpu/drm/drm_irq.c | 3 ++- include/drm/drmP.h| 6 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 6e5d820..d613b6f 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -897,7 +897,8 @@ int drm_vblank_get(struct drm_device *dev, int crtc) } /* Going from 0->1 means we have to enable interrupts again */ - if (atomic_add_return(1, &dev->vblank[crtc].refcount) == 1) { + if (atomic_add_return(1, &dev->vblank[crtc].refcount) == 1 || + dev->vblank_always_enable_on_get) { spin_lock(&dev->vblank_time_lock); if (!dev->vblank[crtc].enabled) { /* Enable vblank irqs under vblank_time_lock protection. diff --git a/include/drm/drmP.h b/include/drm/drmP.h index ee40483..3eca0ee 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1156,6 +1156,12 @@ struct drm_device { */ bool vblank_disable_allowed; + /* +* Should a non-rejected drm_vblank_get() always enable the +* vblank interrupt regardless of the current refcount? +*/ + bool vblank_always_enable_on_get; + /* array of size num_crtcs */ struct drm_vblank_crtc *vblank; -- 1.8.3.2
[PATCH 5/5] drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races
From: Ville Syrj?l? Tell the drm core vblank code to reject drm_vblank_get()s only between drm_vblank_off() and drm_vblank_on() calls, and sprinkle the appropriate drm_vblank_on() calls to the .crtc_enable() hooks. At this time I kept the off calls in their current position, and added the on calls to the end of .crtc_enable(). Later on these will be moved inwards a bit to allow vblank interrupts during plane enable/disable steps. We can kill of the drm_vblank_{pre,post}_modeset() calls since those are there simply to make drm_vblank_get() fail during a modeset. The way they do it is by grabbing a vblank reference, and after drm_vblank_off() gets called this will results in drm_vblank_get() failing due to the elevated refcount while vblank interrupts are disabled. Unfortunately this means there's no point during modeset where the behaviour can be restored back to the normal state until the vblank refcount drops to 0. There's no gurantee of that happening even after the modeset has completed, so simply dropping the drm_vblank_{pre,post}_modeset() calls is the best option. The new reject mechanism will take care of things in a much more consistent and race free manner. Testcase: igt/kms_flip/{dpms,modeset}-vs-vblank-race Signed-off-by: Ville Syrj?l? --- drivers/gpu/drm/i915/i915_dma.c | 6 ++ drivers/gpu/drm/i915/intel_display.c | 23 +++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 7688abc..d5e27bb 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1293,6 +1293,12 @@ static int i915_load_modeset_init(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; int ret; + /* +* Allow the use of vblank interrupts during modeset, +* and make the vblank code behaviour more consistent +*/ + dev->vblank_always_enable_on_get = true; + ret = intel_parse_bios(dev); if (ret) DRM_INFO("failed to find VBIOS tables\n"); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bab0d08..2933540 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3607,6 +3607,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) * happening. */ intel_wait_for_vblank(dev, intel_crtc->pipe); + + drm_vblank_on(dev, pipe); } /* IPS only exists on ULT machines and is tied to pipe A. */ @@ -3632,6 +3634,8 @@ static void haswell_crtc_enable_planes(struct drm_crtc *crtc) mutex_lock(&dev->struct_mutex); intel_update_fbc(dev); mutex_unlock(&dev->struct_mutex); + + drm_vblank_on(dev, pipe); } static void haswell_crtc_disable_planes(struct drm_crtc *crtc) @@ -3643,7 +3647,7 @@ static void haswell_crtc_disable_planes(struct drm_crtc *crtc) int plane = intel_crtc->plane; intel_crtc_wait_for_pending_flips(crtc); - drm_vblank_off(dev, pipe, false); + drm_vblank_off(dev, pipe, true); /* FBC must be disabled before disabling the plane on HSW. */ if (dev_priv->fbc.plane == plane) @@ -3774,7 +3778,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) encoder->disable(encoder); intel_crtc_wait_for_pending_flips(crtc); - drm_vblank_off(dev, pipe, false); + drm_vblank_off(dev, pipe, true); if (dev_priv->fbc.plane == plane) intel_disable_fbc(dev); @@ -4160,6 +4164,8 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) for_each_encoder_on_crtc(dev, crtc, encoder) encoder->enable(encoder); + + drm_vblank_on(dev, pipe); } static void i9xx_crtc_enable(struct drm_crtc *crtc) @@ -4205,6 +4211,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) for_each_encoder_on_crtc(dev, crtc, encoder) encoder->enable(encoder); + + drm_vblank_on(dev, pipe); } static void i9xx_pfit_disable(struct intel_crtc *crtc) @@ -4239,7 +4247,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) /* Give the overlay scaler a chance to disable if it's on this pipe */ intel_crtc_wait_for_pending_flips(crtc); - drm_vblank_off(dev, pipe, false); + drm_vblank_off(dev, pipe, true); if (dev_priv->fbc.plane == plane) intel_disable_fbc(dev); @@ -7035,15 +7043,10 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, struct intel_encoder *encoder; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_display_mode *mode = &intel_crtc->config.requested_mode; - int pipe = intel_crtc->pipe; int ret; - drm_vblank_pre_modeset(dev, pipe); - ret = dev_priv->display.crtc_mode_set(crtc, x, y, fb); - drm_vblank_post_modeset(dev, pipe); - if (ret != 0) retur
[PATCH 3/5] drm: Allow the driver to reject vblank requests only when it really has the vblank interrupts disabled
From: Ville Syrj?l? Allow the driver to specify whether all new vblank requests after drm_vblank_off() should be rejected. And add a counterpart called drm_vblank_on() which will again allow vblank requests to come in. Signed-off-by: Ville Syrj?l? --- drivers/gpu/drm/armada/armada_crtc.c | 2 +- drivers/gpu/drm/drm_irq.c| 29 - drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +- drivers/gpu/drm/gma500/gma_display.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 6 +++--- drivers/gpu/drm/tegra/dc.c | 2 +- include/drm/drmP.h | 4 +++- 7 files changed, 38 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index d8e3982..74317b2 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -257,7 +257,7 @@ static void armada_drm_vblank_off(struct armada_crtc *dcrtc) * Tell the DRM core that vblank IRQs aren't going to happen for * a while. This cleans up any pending vblank events for us. */ - drm_vblank_off(dev, dcrtc->num); + drm_vblank_off(dev, dcrtc->num, false); /* Handle any pending flip event. */ spin_lock_irq(&dev->event_lock); diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 3211158..6e5d820 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -890,6 +890,12 @@ int drm_vblank_get(struct drm_device *dev, int crtc) int ret = 0; spin_lock_irqsave(&dev->vbl_lock, irqflags); + + if (dev->vblank[crtc].reject) { + ret = -EINVAL; + goto out; + } + /* Going from 0->1 means we have to enable interrupts again */ if (atomic_add_return(1, &dev->vblank[crtc].refcount) == 1) { spin_lock(&dev->vblank_time_lock); @@ -917,6 +923,8 @@ int drm_vblank_get(struct drm_device *dev, int crtc) ret = -EINVAL; } } + + out: spin_unlock_irqrestore(&dev->vbl_lock, irqflags); return ret; @@ -947,8 +955,9 @@ EXPORT_SYMBOL(drm_vblank_put); * drm_vblank_off - disable vblank events on a CRTC * @dev: DRM device * @crtc: CRTC in question + * @reject: reject drm_vblank_get() until drm_vblank_on() has been called? */ -void drm_vblank_off(struct drm_device *dev, int crtc) +void drm_vblank_off(struct drm_device *dev, int crtc, bool reject) { struct drm_pending_vblank_event *e, *t; struct timeval now; @@ -956,6 +965,7 @@ void drm_vblank_off(struct drm_device *dev, int crtc) unsigned int seq; spin_lock_irqsave(&dev->vbl_lock, irqflags); + dev->vblank[crtc].reject = reject; vblank_disable_and_save(dev, crtc); wake_up(&dev->vblank[crtc].queue); @@ -979,6 +989,22 @@ void drm_vblank_off(struct drm_device *dev, int crtc) } EXPORT_SYMBOL(drm_vblank_off); + +/** + * drm_vblank_on - enable vblank events on a CRTC + * @dev: DRM device + * @crtc: CRTC in question + */ +void drm_vblank_on(struct drm_device *dev, int crtc) +{ + unsigned long irqflags; + + spin_lock_irqsave(&dev->vbl_lock, irqflags); + dev->vblank[crtc].reject = false; + spin_unlock_irqrestore(&dev->vbl_lock, irqflags); +} +EXPORT_SYMBOL(drm_vblank_on); + /** * drm_vblank_pre_modeset - account for vblanks across mode sets * @dev: DRM device @@ -1224,6 +1250,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data, DRM_WAIT_ON(ret, dev->vblank[crtc].queue, 3 * HZ, (((drm_vblank_count(dev, crtc) - vblwait->request.sequence) <= (1 << 23)) || +dev->vblank[crtc].reject || !dev->irq_enabled)); if (ret != -EINTR) { diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index ebc0150..e2d6b9d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -68,7 +68,7 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) /* wait for the completion of page flip. */ wait_event(exynos_crtc->pending_flip_queue, atomic_read(&exynos_crtc->pending_flip) == 0); - drm_vblank_off(crtc->dev, exynos_crtc->pipe); + drm_vblank_off(crtc->dev, exynos_crtc->pipe, false); } exynos_drm_fn_encoder(crtc, &mode, exynos_drm_encoder_crtc_dpms); diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c index 386de2c..ff18220 100644 --- a/drivers/gpu/drm/gma500/gma_display.c +++ b/drivers/gpu/drm/gma500/gma_display.c @@ -281,7 +281,7 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode) REG_WRITE(VGACNTRL, VGA_DISP_DISABLE); /* Turn off vblank interrupts */ - drm_vblank_off(dev, pipe); +
AMD/AMD hybrid graphics
2014-02-21 16:12 keltez?ssel, Alex Deucher ?rta: > On Fri, Feb 21, 2014 at 9:32 AM, Boszormenyi Zoltan wrote: >> [zozo at localhost ~]$ DRI_PRIME=1 glxgears >> Running synchronized to the vertical refresh. The framerate should be >> approximately the same as the monitor refresh rate. >> 5589 frames in 5.0 seconds = 1117.730 FPS >> 5369 frames in 5.0 seconds = 1073.715 FPS >> 5699 frames in 5.0 seconds = 1139.670 FPS >> >> Obviously, it doesn't sync to the framerate with PRIME. >> On the other hand, nothing is displayed in the glxgears window. >> I have llvm 3.4-4 and Mesa 10.1-rc1 from Fedora 21 rawhide. >> > Make sure you are running a compositor. Turning on compositing in MATE fixed it. Thank you very much. It seems I will have to keep Rawhide on this notebook. :-) Best regards, Zolt?n B?sz?rm?nyi
[PATCH 11/13] drm: remove redundant minor->device field
Hi On Fri, Feb 21, 2014 at 8:30 AM, Thierry Reding wrote: > On Wed, Feb 12, 2014 at 02:36:24PM +0100, Daniel Vetter wrote: >> On Wed, Jan 29, 2014 at 03:01:58PM +0100, David Herrmann wrote: >> > Whenever we access minor->device, we are in a minor->kdev->...->fops >> > callback so the minor->kdev pointer *must* be valid. Thus, simply use >> > minor->kdev->devt instead of minor->device and remove the redundant field. >> > >> > Signed-off-by: David Herrmann >> >> I think this is simply compat cruft from the days when the drm core was >> still shared with the *bsds. With the one patch I've commented on all >> patches up to this one are >> >> Reviewed-by: Daniel Vetter >> >> As discussed on irc I think we don't want to have stable minor ids really, >> userspace simply needs to inquire udev to get at the right >> render/control/legacy node it wants. > > Does that mean we should go all the way and don't keep the +64 (for > control) and +128 (for render nodes) offsets either? Should it be > possible to have a /dev/dri directory that looks somewhat like this: > > /dev/dri/card0(GPU#0, legacy) > /dev/dri/card1(GPU#1, legacy) > /dev/dri/render0 (GPU#1, render) That might break backwards compat, but may be worth it. However, we *have* to keep the +64 / +128 offsets for minor numbers. There's already user-space using that for dev-type testing (which is fine!). Thanks David
[PATCH] drm/exynos: power up HDMI before mixer
Testing on Exynos4412, when changing screen resolution under GNOME/X11, first DPMS is set to OFF via drm_mode_obj_set_property_ioctl. This is done via drm_hdmi_dpms which powers down the mixer then the HDMI component. Then the mode change happens. We then see this call chain, powering things back on: exynos_drm_crtc_commit exynos_drm_crtc_dpms exynos_drm_encoder_crtc_dpms drm_hdmi_dpms And at this point, drm_hdmi_dpms first powers on the mixer, then the HDMI component. Strangely enough, this works fine on the first resolution change, but on the second it hangs in mixer_poweron() in: mixer_reg_write(res, MXR_INT_EN, ctx->int_en); Through some experiments I determined that this register write will hang the machine here unless the hdmi_resources.hdmi clock is running. I can't explain why there is a difference between the first and second time; I did check that the underlying clock gating register has the same value in both cases. Anyway, the mixer clearly has some kind of dependency on the HDMI component, so lets make sure we power that up first. Signed-off-by: Daniel Drake --- drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c index 8548b97..3bfd9d6 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c @@ -261,10 +261,17 @@ static void drm_hdmi_dpms(struct device *subdrv_dev, int mode) { struct drm_hdmi_context *ctx = to_context(subdrv_dev); + /* When powering up, we must first power up the HDMI component, as +* otherwise mixer register accesses will sometimes hang. +* When powering down, we do the opposite: mixer off, HDMI off. */ + + if (mode == DRM_MODE_DPMS_ON && hdmi_ops && hdmi_ops->dpms) + hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode); + if (mixer_ops && mixer_ops->dpms) mixer_ops->dpms(ctx->mixer_ctx->ctx, mode); - if (hdmi_ops && hdmi_ops->dpms) + if (mode != DRM_MODE_DPMS_ON && hdmi_ops && hdmi_ops->dpms) hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode); } -- 1.8.3.2
[PATCH] drm/radeon: fix missing bo reservation
Alex, shouldn't you send a pull request, that this land in 3.14-rc4? Just in case... Thanks, Dieter Am 20.02.2014 18:47, schrieb Christian K?nig: > From: Christian K?nig > > Otherwise we might get a crash here. > > Signed-off-by: Christian K?nig > Cc: stable at vger.kernel.org > --- > drivers/gpu/drm/radeon/radeon_kms.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon_kms.c > b/drivers/gpu/drm/radeon/radeon_kms.c > index 114d167..2aecd6d 100644 > --- a/drivers/gpu/drm/radeon/radeon_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_kms.c > @@ -537,6 +537,10 @@ int radeon_driver_open_kms(struct drm_device > *dev, struct drm_file *file_priv) > > radeon_vm_init(rdev, &fpriv->vm); > > + r = radeon_bo_reserve(rdev->ring_tmp_bo.bo, false); > + if (r) > + return r; > + > /* map the ib pool buffer read only into >* virtual address space */ > bo_va = radeon_vm_bo_add(rdev, &fpriv->vm, > @@ -544,6 +548,8 @@ int radeon_driver_open_kms(struct drm_device *dev, > struct drm_file *file_priv) > r = radeon_vm_bo_set_addr(rdev, bo_va, RADEON_VA_IB_OFFSET, > RADEON_VM_PAGE_READABLE | > RADEON_VM_PAGE_SNOOPED); > + > + radeon_bo_unreserve(rdev->ring_tmp_bo.bo); > if (r) { > radeon_vm_fini(rdev, &fpriv->vm); > kfree(fpriv);
[PATCH] drm/radeon: fix missing bo reservation
On Fri, Feb 21, 2014 at 4:26 PM, Dieter N?tzel wrote: > Alex, > > shouldn't you send a pull request, that this land in 3.14-rc4? > Just in case... I'll send a pull request next week along with any other patches I've accumulated since the last pull. I usually try and send a -fixes pull once a week assuming there are fixes that need to go in. Alex > > Thanks, > Dieter > > Am 20.02.2014 18:47, schrieb Christian K?nig: >> >> From: Christian K?nig >> >> Otherwise we might get a crash here. >> >> Signed-off-by: Christian K?nig >> Cc: stable at vger.kernel.org >> --- >> drivers/gpu/drm/radeon/radeon_kms.c | 6 ++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c >> b/drivers/gpu/drm/radeon/radeon_kms.c >> index 114d167..2aecd6d 100644 >> --- a/drivers/gpu/drm/radeon/radeon_kms.c >> +++ b/drivers/gpu/drm/radeon/radeon_kms.c >> @@ -537,6 +537,10 @@ int radeon_driver_open_kms(struct drm_device >> *dev, struct drm_file *file_priv) >> >> radeon_vm_init(rdev, &fpriv->vm); >> >> + r = radeon_bo_reserve(rdev->ring_tmp_bo.bo, false); >> + if (r) >> + return r; >> + >> /* map the ib pool buffer read only into >> * virtual address space */ >> bo_va = radeon_vm_bo_add(rdev, &fpriv->vm, >> @@ -544,6 +548,8 @@ int radeon_driver_open_kms(struct drm_device *dev, >> struct drm_file *file_priv) >> r = radeon_vm_bo_set_addr(rdev, bo_va, >> RADEON_VA_IB_OFFSET, >> RADEON_VM_PAGE_READABLE | >> RADEON_VM_PAGE_SNOOPED); >> + >> + radeon_bo_unreserve(rdev->ring_tmp_bo.bo); >> if (r) { >> radeon_vm_fini(rdev, &fpriv->vm); >> kfree(fpriv);