Re: Fw: [Intel-gfx] [PATCH] intel: add a timed wait function
On Thu, May 31, 2012 at 12:35 AM, Eric Anholt wrote: > > Did you want pointer for timeout in the userspace api? I don't feel > strongly about it, I just didn't see a use. The equivalent API I could > think of was select(), where apparently linux returns time unwaited, > while "everyone else" doesn't. I don't see a strong recommendation > either way from that. I wanted the kernel to return the unwaited time (or at least a upper bound to it, with coarse clocks that's the best we can do) so that ioctl restarting after a signal does the right thing. Exposing that any further than libdrm doesn't make much sense imo. -Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Add downclock quirk for Samsung LTN121AT10-301
On Wed, May 30, 2012 at 11:21:32AM -0400, Sean Paul wrote: > On Wed, May 30, 2012 at 10:16 AM, Adam Jackson wrote: > > On 5/30/12 8:05 AM, Sean Paul wrote: > > > >> Yes, definitely. The reason I can't set it via xrandr (easily) is > >> because we look for lvds downclock modes (in i915) on the driver init. > >> Since the driver initializes way before we have a chance to add a new > >> mode via xrandr, the driver won't have a downclock mode. > >> > >> I suppose the other option is to hack the i915 driver to allow a > >> downclocked mode to be added after it's been initialized. I haven't > >> looked into this solution, it might be worth investigating. > > > > > > Just so I'm clear, is what you're looking for "I want this pair of timings, > > with the driver magically switching between them"? If all you wanted was > > the lower clock speed all the time you could just, you know, do that, so I > > assume that's not what you're after. > > > > Yep, you got it. > > > If binding two timings together like that is what you want, then that seems > > like a pretty reasonable device-specific ioctl at first glance. I think the > > only thing to be careful of would be copying the slower timings into the > > driver private of the faster, rather than keeping a pointer or copy of the > > object id, since modes aren't refcounted. > > > > How big of a power savings do you see with this? Wondering if it's worth > > trying to make some common tooling for finding downclocked modes, if it's > > going to be worthwhile on multiple panels. > > > > I saw about 200mW savings with downclocking enabled. It's hard to say > how many panels this might apply to, some panels which report > downclocked states still flicker, and others that don't have it in the > EDID work just fine (like this Samsung panel). > > LVDS downclocking is enabled by default in Fedora and ChromiumOS, so > making this more accessible might be useful for users of these > distros. I think adding lvds connector properties for downclocking would make sense. I guess we need and enable knob and a frequency value. At boot time we'd fill these with the detected values, but then userspace would have full control (i.e. the enable knob should overwrite the i915 module options). That way downclocking would also be much easier to test (and maybe we could try to enable it by default eventually). -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Lots of i915/drm spew on 3.4
On Wed, 30 May 2012 18:26:51 -0400, Dave Jones wrote: > On Wed, May 30, 2012 at 05:58:48PM -0400, Dave Jones wrote: > > On Wed, May 30, 2012 at 11:51:54PM +0200, Daniel Vetter wrote: > > > On Wed, May 30, 2012 at 11:31 PM, Dave Jones wrote: > > > > On this hardware: > > > > > > > > 00:02.0 VGA compatible controller: Intel Corporation 82945G/GZ > Integrated Graphics Controller (rev 02) > > > > > > > > I get this every boot with Linus current tree (up to > af56e0aa35f3ae2a4c1a6d1000702df1dd78cb76) > > > > > > Just a quick question, is this a regression? > > > > seems so, I don't see it on 3.3 The WARN is new, the problem is old. commit c543188afb7a83e66161c026dc6fd5eb38dc0b63 Author: Paulo Zanoni Date: Tue May 15 18:09:02 2012 -0300 drm: add generic ioctls to get/set properties on any object Useless for connector properties (since they already have their own ioctls), but useful when we add properties to CRTCs, planes and other objects. Reviewed-by: Eugeni Dodonov Reviewed-by: Rob Clark Tested-by: Rob Clark Signed-off-by: Paulo Zanoni Signed-off-by: Dave Airlie Before that commit we had no idea that we had run out of property slots. I think the WARN is genuine, but maybe we should just bump the count set it to WARN_ONCE and hope the conversion to lists arrives sooner rather than latter. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] DRM: add drm gem cma helper
Many embedded drm devices do not have a IOMMU and no dedicated memory for graphics. These devices use cma (Contiguous Memory Allocator) backed graphics memory. This patch provides helper functions to be able to share the code. Signed-off-by: Sascha Hauer --- changes since v1: - map whole buffer at mmap time, not page by page at fault time - several cleanups as suggested by Lars-Peter Clausen and Laurent Pinchart I currently do a remap_pfn_range after drm_gem_mmap, so we release dev->struct_mutex in between. I don't know if this is correct or if we have to hold the lock over the whole process. drivers/gpu/drm/Kconfig |6 + drivers/gpu/drm/Makefile |1 + drivers/gpu/drm/drm_gem_cma_helper.c | 243 ++ include/drm/drm_gem_cma_helper.h | 52 4 files changed, 302 insertions(+) create mode 100644 drivers/gpu/drm/drm_gem_cma_helper.c create mode 100644 include/drm/drm_gem_cma_helper.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index e354bc0..f62717e 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -53,6 +53,12 @@ config DRM_TTM GPU memory types. Will be enabled automatically if a device driver uses it. +config DRM_GEM_CMA_HELPER + tristate + depends on DRM + help + Choose this if you need the GEM cma helper functions + config DRM_TDFX tristate "3dfx Banshee/Voodoo3+" depends on DRM && PCI diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index c20da5b..9a0d98a 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -15,6 +15,7 @@ drm-y :=drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \ drm_trace_points.o drm_global.o drm_prime.o drm-$(CONFIG_COMPAT) += drm_ioc32.o +drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm-usb-y := drm_usb.o diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c new file mode 100644 index 000..d8c0dc7 --- /dev/null +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -0,0 +1,243 @@ +/* + * drm gem cma (contiguous memory allocator) helper functions + * + * Copyright (C) 2012 Sascha Hauer, Pengutronix + * + * Based on Samsung Exynos code + * + * Copyright (c) 2011 Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#include +#include +#include + +static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) +{ + return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; +} + +static void drm_gem_cma_buf_destroy(struct drm_device *drm, + struct drm_gem_cma_object *cma_obj) +{ + dma_free_writecombine(drm->dev, cma_obj->base.size, cma_obj->vaddr, + cma_obj->paddr); +} + +/* + * drm_gem_cma_create - allocate an object with the given size + * + * returns a struct drm_gem_cma_object* on success or ERR_PTR values + * on failure. + */ +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, + unsigned int size) +{ + struct drm_gem_cma_object *cma_obj; + struct drm_gem_object *gem_obj; + int ret; + + size = round_up(size, PAGE_SIZE); + + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL); + if (!cma_obj) + return ERR_PTR(-ENOMEM); + + cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size, + &cma_obj->paddr, GFP_KERNEL | __GFP_NOWARN); + if (!cma_obj->vaddr) { + dev_err(drm->dev, "failed to allocate buffer with size %d\n", size); + ret = -ENOMEM; + goto err_dma_alloc; + } + + gem_obj = &cma_obj->base; + + ret = drm_gem_object_init(drm, gem_obj, size); + if (ret) + goto err_obj_init; + + ret = drm_gem_create_mmap_offset(gem_obj); + if (ret) + goto err_create_mmap_offset; + + return cma_obj; + +err_create_mmap_offset: + drm_gem_object_release(gem_obj); + +err_obj_init: + drm_gem_cma_buf_destroy(drm, cma_obj); + +err_dma_alloc: + kfree(cma_obj); + + return ERR_PTR(ret); +} +EXPORT_SYMBOL_GPL(drm_gem_cma_create); + +/* + * drm_gem_cma_create_with_handle - allocate an object with the given + * size and create a gem handle on it + * + * returns a struct drm_gem_cma_object* on success or ERR_PTR values + * on failure. + */ +struct drm_gem_cma_object *drm_gem_cma_create_with_handle( + struct drm_file *file
Re: [PATCH v2] DRM: Add DRM kms/fb cma helper
On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote: > This patch introduces a set of helper function for implementing the KMS > framebuffer layer for drivers which use the drm gem CMA helper function. > > Signed-off-by: Lars-Peter Clausen > > --- > Changes since v1: > * Some spelling fixes > * Add missing kfree in drm_fb_cma_alloc error path > * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second > parameter to specify the plane. > --- > drivers/gpu/drm/Kconfig | 11 + > drivers/gpu/drm/Makefile|1 + > drivers/gpu/drm/drm_fb_cma_helper.c | 384 > +++ > include/drm/drm_fb_cma_helper.h | 27 +++ > 4 files changed, 423 insertions(+) > create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c > create mode 100644 include/drm/drm_fb_cma_helper.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index a78dfbe..e7c0a3d 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER > help > Choose this if you need the GEM cma helper functions > > +config DRM_KMS_CMA_HELPER > + tristate > + select DRM_GEM_CMA_HELPER > + select DRM_KMS_HELPER > + select FB_SYS_FILLRECT > + select FB_SYS_COPYAREA > + select FB_SYS_IMAGEBLIT > + help > + Choose this if you need the KMS cma helper functions > + > + > config DRM_TDFX > tristate "3dfx Banshee/Voodoo3+" > depends on DRM && PCI > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 6e9e948..5dcb1a5 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o > drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o > > drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o > +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c > b/drivers/gpu/drm/drm_fb_cma_helper.c > new file mode 100644 > index 000..8821a98 > --- /dev/null > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -0,0 +1,384 @@ > +/* > + * drm kms/fb cma (contiguous memory allocator) helper functions > + * > + * Copyright (C) 2012 Analog Device Inc. > + * Author: Lars-Peter Clausen > + * > + * Based on udl_fbdev.c > + * Copyright (C) 2012 Red Hat > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +struct drm_fb_cma { > + struct drm_framebuffer fb; > + struct drm_gem_cma_obj *obj[4]; Can we have a define for this magic '4'? It is used several times in this patch. > +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, > + struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj, > + unsigned int num_planes) > +{ > + struct drm_fb_cma *fb_cma; > + int ret; > + int i; > + > + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); > + if (!fb_cma) > + return ERR_PTR(-ENOMEM); > + > + ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); > + if (ret) { > + dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret); > + kfree(fb_cma); > + return ERR_PTR(ret); > + } > + > + drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); > + > + for (i = 0; i < num_planes; i++) > + fb_cma->obj[i] = obj[i]; Check for valid num_planes before this loop? Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object in the patch I just sent. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function
On Thu, May 24, 2012 at 09:26:49PM +0200, Daniel Vetter wrote: > A 30 ms delay is simply way too big to waste cpu cycles on. > > Signed-off-by: Daniel Vetter I've queued this patch here for -next with Chris' irc ack added. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] DRM: add drm gem cma helper
Hi Sascha, Thanks for the patch. Here's a bit more extensive review. On Thursday 31 May 2012 10:08:54 Sascha Hauer wrote: > Many embedded drm devices do not have a IOMMU and no dedicated > memory for graphics. These devices use cma (Contiguous Memory > Allocator) backed graphics memory. This patch provides helper > functions to be able to share the code. > > Signed-off-by: Sascha Hauer > --- > > changes since v1: > > - map whole buffer at mmap time, not page by page at fault time > - several cleanups as suggested by Lars-Peter Clausen and Laurent Pinchart > > I currently do a remap_pfn_range after drm_gem_mmap, so we release > dev->struct_mutex in between. I don't know if this is correct or if we have > to hold the lock over the whole process. drm_gem_mmap() takes a reference on the GEM object so I don't think we'll have any issue there, but please don't take my word for it. dev->struct_mutex is the large mutex I still need to document, I don't know what it protects exactly. > drivers/gpu/drm/Kconfig |6 + > drivers/gpu/drm/Makefile |1 + > drivers/gpu/drm/drm_gem_cma_helper.c | 243 +++ > include/drm/drm_gem_cma_helper.h | 52 > 4 files changed, 302 insertions(+) > create mode 100644 drivers/gpu/drm/drm_gem_cma_helper.c > create mode 100644 include/drm/drm_gem_cma_helper.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index e354bc0..f62717e 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -53,6 +53,12 @@ config DRM_TTM > GPU memory types. Will be enabled automatically if a device driver > uses it. > > +config DRM_GEM_CMA_HELPER > + tristate > + depends on DRM > + help > + Choose this if you need the GEM cma helper functions I would put CMA in uppercase, but that's just nitpicking. BTW this helper is not strictly dedicated to CMA. It uses the DMA API to allocate memory, without caring about the underlying allocator. > + > config DRM_TDFX > tristate "3dfx Banshee/Voodoo3+" > depends on DRM && PCI [snip] > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c > b/drivers/gpu/drm/drm_gem_cma_helper.c new file mode 100644 > index 000..d8c0dc7 > --- /dev/null > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -0,0 +1,243 @@ > +/* > + * drm gem cma (contiguous memory allocator) helper functions > + * > + * Copyright (C) 2012 Sascha Hauer, Pengutronix > + * > + * Based on Samsung Exynos code > + * > + * Copyright (c) 2011 Samsung Electronics Co., Ltd. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#include > +#include > +#include > + > +static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) > +{ > + return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; > +} > + > +static void drm_gem_cma_buf_destroy(struct drm_device *drm, > + struct drm_gem_cma_object *cma_obj) > +{ > + dma_free_writecombine(drm->dev, cma_obj->base.size, cma_obj->vaddr, > + cma_obj->paddr); > +} > + > +/* > + * drm_gem_cma_create - allocate an object with the given size > + * > + * returns a struct drm_gem_cma_object* on success or ERR_PTR values > + * on failure. > + */ > +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, > + unsigned int size) > +{ > + struct drm_gem_cma_object *cma_obj; > + struct drm_gem_object *gem_obj; > + int ret; > + > + size = round_up(size, PAGE_SIZE); > + > + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL); > + if (!cma_obj) > + return ERR_PTR(-ENOMEM); > + > + cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size, > + &cma_obj->paddr, GFP_KERNEL | __GFP_NOWARN); > + if (!cma_obj->vaddr) { > + dev_err(drm->dev, "failed to allocate buffer with size %d\n", > size); > + ret = -ENOMEM; > + goto err_dma_alloc; > + } > + > + gem_obj = &cma_obj->base; > + > + ret = drm_gem_object_init(drm, gem_obj, size); > + if (ret) > + goto err_obj_init; > + > + ret = drm_gem_create_mmap_offset(gem_obj); > + if (ret) > + goto err_create_mmap_offset; > + > + return cma_obj; > + > +err_create_mmap_offset: > + drm_gem_object_release(gem_obj); > + > +err_obj_init: > + drm_gem_cma_buf_destroy(drm, cma_obj); > + > +err_dma_alloc: > + kfree(cma_obj); > + > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(drm_gem_c
Re: [PATCH v2] DRM: Add DRM kms/fb cma helper
On 05/31/2012 10:13 AM, Sascha Hauer wrote: > On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote: >> This patch introduces a set of helper function for implementing the KMS >> framebuffer layer for drivers which use the drm gem CMA helper function. >> >> Signed-off-by: Lars-Peter Clausen >> >> --- >> Changes since v1: >> * Some spelling fixes >> * Add missing kfree in drm_fb_cma_alloc error path >> * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second >>parameter to specify the plane. >> --- >> drivers/gpu/drm/Kconfig | 11 + >> drivers/gpu/drm/Makefile|1 + >> drivers/gpu/drm/drm_fb_cma_helper.c | 384 >> +++ >> include/drm/drm_fb_cma_helper.h | 27 +++ >> 4 files changed, 423 insertions(+) >> create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c >> create mode 100644 include/drm/drm_fb_cma_helper.h >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index a78dfbe..e7c0a3d 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER >> help >>Choose this if you need the GEM cma helper functions >> >> +config DRM_KMS_CMA_HELPER >> +tristate >> +select DRM_GEM_CMA_HELPER >> +select DRM_KMS_HELPER >> +select FB_SYS_FILLRECT >> +select FB_SYS_COPYAREA >> +select FB_SYS_IMAGEBLIT >> +help >> + Choose this if you need the KMS cma helper functions >> + >> + >> config DRM_TDFX >> tristate "3dfx Banshee/Voodoo3+" >> depends on DRM && PCI >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >> index 6e9e948..5dcb1a5 100644 >> --- a/drivers/gpu/drm/Makefile >> +++ b/drivers/gpu/drm/Makefile >> @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o >> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o >> >> drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o >> +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o >> >> obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o >> >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c >> b/drivers/gpu/drm/drm_fb_cma_helper.c >> new file mode 100644 >> index 000..8821a98 >> --- /dev/null >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -0,0 +1,384 @@ >> +/* >> + * drm kms/fb cma (contiguous memory allocator) helper functions >> + * >> + * Copyright (C) 2012 Analog Device Inc. >> + * Author: Lars-Peter Clausen >> + * >> + * Based on udl_fbdev.c >> + * Copyright (C) 2012 Red Hat >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * as published by the Free Software Foundation; either version 2 >> + * of the License, or (at your option) any later version. >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +struct drm_fb_cma { >> +struct drm_framebuffer fb; >> +struct drm_gem_cma_obj *obj[4]; > > Can we have a define for this magic '4'? It is used several times in > this patch. Yes, had the same though. The magic 4 is actually used all through all of DRM. Something like '#define DRM_MAX_PLANES 4' probably makes sense. > >> +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, >> +struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj, >> +unsigned int num_planes) >> +{ >> +struct drm_fb_cma *fb_cma; >> +int ret; >> +int i; >> + >> +fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); >> +if (!fb_cma) >> +return ERR_PTR(-ENOMEM); >> + >> +ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); >> +if (ret) { >> +dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret); >> +kfree(fb_cma); >> +return ERR_PTR(ret); >> +} >> + >> +drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); >> + >> +for (i = 0; i < num_planes; i++) >> +fb_cma->obj[i] = obj[i]; > > Check for valid num_planes before this loop? > Hm, I think the callers already take care of this. drm_format_num_planes will always return a valid number and the other caller passes 1 unconditionally. > > Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object > in the patch I just sent. > Do you think it makes sense for you to carry this patch as part of your iMX DRM series? Thanks, - Lars ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] DRM: add drm gem CMA helper
Hi Sascha, On Wednesday 30 May 2012 18:28:12 Sascha Hauer wrote: > On Wed, May 30, 2012 at 05:40:13PM +0200, Laurent Pinchart wrote: > > Hi Sascha, > > > > Thank you for the patch. I've successfully tested the helper with the new > > SH Mobile DRM driver. Just a couple of comments below in addition to > > Lars' comments (this is not a full review, just details that caught my > > attention when comparing the code with my implementation, and trying to > > use it). > > > +/* > > > + * drm_gem_cma_mmap - (struct file_operation)->mmap callback function > > > + */ > > > +int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) > > > +{ > > > + int ret; > > > + > > > + ret = drm_gem_mmap(filp, vma); > > > + if (ret) > > > + return ret; > > > + > > > + vma->vm_flags &= ~VM_PFNMAP; > > > + vma->vm_flags |= VM_MIXEDMAP; > > > > Why is this a mixed map ? > > Honestly, I don't know. This is copied from the exynos driver. I don't > know much about these flags, so if you think something else is better > here than you're probably right ;) I wouldn't claim to be an expert on this matter :-) As far as I know, PFNMAP means that the mapping refers to physical memory with no struct page associated with it (that could be I/O memory, DRAM reserved at boot time, ... basically any memory outside of the system memory resources controlled by the kernel page allocator). MIXEDMAP means that the mapping refers to memory that contains both PFNMAP regions and non-PFNMAP regions. I would be surprised if dma_alloc_writecombine() returned such a mix. On the other hand the memory it returns might be PFNMAP or non-PFNMAP depending on the underneath allocator, maybe that's why VM_MIXEDMAP was used. I'd appreciate an expert's opinion on this :-) > > > + > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(drm_gem_cma_mmap); > > > > My implementation maps the whole buffer in one go at mmap time, not page > > by page at page fault time. Isn't that more efficient when mapping frame > > buffer memory ? > > I remember Alan has mentioned this in an earlier review. I'll update the > patch accordingly. > > I will fix this and the other things you mentioned and repost. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm: Renesas SH Mobile DRM driver
Hi Sascha, On Wednesday 30 May 2012 18:40:56 Sascha Hauer wrote: > On Wed, May 30, 2012 at 02:32:59PM +0200, Laurent Pinchart wrote: > > The SH Mobile LCD controller (LCDC) DRM driver supports the main > > graphics plane in RGB and YUV formats, as well as the overlay planes (in > > alpha-blending mode only). > > > > Only flat panel outputs using the parallel interface are supported. > > Support for SYS panels, HDMI and DSI is currently not implemented. > > > > Signed-off-by: Laurent Pinchart > > --- > > [...] > > > +int shmob_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) > > +{ > > + struct drm_file *priv = filp->private_data; > > + struct drm_device *dev = priv->minor->dev; > > + struct drm_gem_mm *mm = dev->mm_private; > > + struct shmob_drm_gem_object *sobj; > > + struct drm_local_map *map; > > + struct drm_gem_object *obj; > > + struct drm_hash_item *hash; > > + pgprot_t prot; > > + int ret; > > + > > + if (drm_device_is_unplugged(dev)) > > + return -ENODEV; > > + > > + mutex_lock(&dev->struct_mutex); > > + > > + if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) { > > + ret = -EINVAL; > > + goto out_unlock; > > + } > > + > > + map = drm_hash_entry(hash, struct drm_map_list, hash)->map; > > + if (!map || > > + ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) { > > + ret = -EPERM; > > + goto out_unlock; > > + } > > + > > + /* Check for valid size. */ > > + if (map->size < vma->vm_end - vma->vm_start) { > > + ret = -EINVAL; > > + goto out_unlock; > > + } > > + > > + obj = map->handle; > > + if (!obj->dev->driver->gem_vm_ops) { > > + ret = -EINVAL; > > + goto out_unlock; > > + } > > + > > + sobj = to_shmob_gem_object(obj); > > + prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); > > + ret = remap_pfn_range(vma, vma->vm_start, sobj->dma >> PAGE_SHIFT, > > + vma->vm_end - vma->vm_start, prot); > > + if (ret < 0) > > + goto out_unlock; > > + > > + vma->vm_flags |= VM_RESERVED | VM_IO | VM_PFNMAP | VM_DONTEXPAND; > > + vma->vm_ops = obj->dev->driver->gem_vm_ops; > > + vma->vm_private_data = obj; > > + vma->vm_page_prot = prot; > > + > > + /* Take a ref for this mapping of the object, so that the fault > > +* handler can dereference the mmap offset's pointer to the object. > > +* This reference is cleaned up by the corresponding vm_close > > +* (which should happen whether the vma was created by this call, or > > +* by a vm_open due to mremap or partial unmap or whatever). > > +*/ > > + drm_gem_object_reference(obj); > > + > > + drm_vm_open_locked(dev, vma); > > + > > +out_unlock: > > + mutex_unlock(&dev->struct_mutex); > > + > > + return ret; > > +} > > I just noticed this function is nearly a copy of drm_gem_mmap except for > the the additional remap_pfn_range here. Would it make sense to turn > drm_gem_mmap into a wrapper around a drm_gem_mmap_locked, call this one > from here and add the remap_pfn_range? > > If yes, I would do so in my cma gem helper patch. Seems you've already done so :-) I don't think locking is an issue, but as I mentioned in my review I don't know with 100% certainty what dev->struct_mutex covers. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Lots of i915/drm spew on 3.4
2012/5/31 Chris Wilson : > Before that commit we had no idea that we had run out of property slots. > I think the WARN is genuine, but maybe we should just bump the count set > it to WARN_ONCE and hope the conversion to lists arrives sooner rather > than latter. > -Chris > Chris is right: this is not a regression. Before that patch, no one checked if property creation really worked. I chose not to use WARN_ONCE because we need to increase the variable once for each time you see the message. Assuming this message appears on your log less than 8 times, does this patch fix your problem? diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 73e4560..bac55c2 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -54,7 +54,7 @@ struct drm_mode_object { struct drm_object_properties *properties; }; -#define DRM_OBJECT_MAX_PROPERTY 16 +#define DRM_OBJECT_MAX_PROPERTY 24 struct drm_object_properties { int count; uint32_t ids[DRM_OBJECT_MAX_PROPERTY]; -- Paulo Zanoni ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Add downclock quirk for Samsung LTN121AT10-301
On Thu, May 31, 2012 at 3:09 AM, Daniel Vetter wrote: > On Wed, May 30, 2012 at 11:21:32AM -0400, Sean Paul wrote: >> On Wed, May 30, 2012 at 10:16 AM, Adam Jackson wrote: >> > On 5/30/12 8:05 AM, Sean Paul wrote: >> > >> >> Yes, definitely. The reason I can't set it via xrandr (easily) is >> >> because we look for lvds downclock modes (in i915) on the driver init. >> >> Since the driver initializes way before we have a chance to add a new >> >> mode via xrandr, the driver won't have a downclock mode. >> >> >> >> I suppose the other option is to hack the i915 driver to allow a >> >> downclocked mode to be added after it's been initialized. I haven't >> >> looked into this solution, it might be worth investigating. >> > >> > >> > Just so I'm clear, is what you're looking for "I want this pair of timings, >> > with the driver magically switching between them"? If all you wanted was >> > the lower clock speed all the time you could just, you know, do that, so I >> > assume that's not what you're after. >> > >> >> Yep, you got it. >> >> > If binding two timings together like that is what you want, then that seems >> > like a pretty reasonable device-specific ioctl at first glance. I think >> > the >> > only thing to be careful of would be copying the slower timings into the >> > driver private of the faster, rather than keeping a pointer or copy of the >> > object id, since modes aren't refcounted. >> > >> > How big of a power savings do you see with this? Wondering if it's worth >> > trying to make some common tooling for finding downclocked modes, if it's >> > going to be worthwhile on multiple panels. >> > >> >> I saw about 200mW savings with downclocking enabled. It's hard to say >> how many panels this might apply to, some panels which report >> downclocked states still flicker, and others that don't have it in the >> EDID work just fine (like this Samsung panel). >> >> LVDS downclocking is enabled by default in Fedora and ChromiumOS, so >> making this more accessible might be useful for users of these >> distros. > > I think adding lvds connector properties for downclocking would make > sense. I guess we need and enable knob and a frequency value. At boot time > we'd fill these with the detected values, but then userspace would have > full control (i.e. the enable knob should overwrite the i915 module > options). That way downclocking would also be much easier to test (and > maybe we could try to enable it by default eventually). Excellent, thanks to everyone for the feedback! I'll put together a patch when I get some free cycles. Sean > -Daniel > -- > Daniel Vetter > Mail: dan...@ffwll.ch > Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm prime mmap/vmap
Hi Linus, as mentioned previously these are the extra bits of drm that relied on the dma-buf pull to work, the first three just stub out the mmap interface, and the next set provide vmap export to i915/radeon/nouveau and vmap import to udl. Dave. The following changes since commit af56e0aa35f3ae2a4c1a6d1000702df1dd78cb76: Merge git://git.kernel.org/pub/scm/linux/kernel/git/sage/ceph-client (2012-05-30 11:17:19 -0700) are available in the git repository at: git://people.freedesktop.org/~airlied/linux drm-prime-vmap for you to fetch changes up to 63bc620b45af8c743ac291c8725933278c712692: radeon: add radeon prime vmap support. (2012-05-31 14:14:01 +0100) Dave Airlie (7): i915: add stub dma-buf mmap callback. nouveau: add stub dma-buf mmap functionality. radeon: add stub dma-buf mmap functionality i915: add dma-buf vmap support for exporting vmapped buffer udl: support vmapping imported dma-bufs nouveau: add vmap support to nouveau prime support radeon: add radeon prime vmap support. drivers/gpu/drm/i915/i915_drv.h |3 ++ drivers/gpu/drm/i915/i915_gem_dmabuf.c | 61 +++ drivers/gpu/drm/nouveau/nouveau_drv.h |3 ++ drivers/gpu/drm/nouveau/nouveau_prime.c | 45 +++ drivers/gpu/drm/radeon/radeon.h |3 ++ drivers/gpu/drm/radeon/radeon_prime.c | 44 ++ drivers/gpu/drm/udl/udl_fb.c| 13 ++- drivers/gpu/drm/udl/udl_gem.c | 25 +++-- 8 files changed, 192 insertions(+), 5 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Lots of i915/drm spew on 3.4
2012/5/30 Dave Jones : > On Wed, May 30, 2012 at 05:58:48PM -0400, Dave Jones wrote: > > On Wed, May 30, 2012 at 11:51:54PM +0200, Daniel Vetter wrote: > > > On Wed, May 30, 2012 at 11:31 PM, Dave Jones wrote: > > > > On this hardware: > > > > > > > > 00:02.0 VGA compatible controller: Intel Corporation 82945G/GZ > Integrated Graphics Controller (rev 02) > > > > > > > > I get this every boot with Linus current tree (up to > af56e0aa35f3ae2a4c1a6d1000702df1dd78cb76) > > > > > > Just a quick question, is this a regression? > > > > seems so, I don't see it on 3.3 > > > > > If so, can you please > > > attach the output of xrandr --verbose from a noisy and a quite kernel > > > (otherwise just please attach it from this noisy kernel). > > > > this machine runs headless, so has no X installed right now, I'll get it > in a while. > > Attached. > Just a little more information: you have a lot of connector properties because for some reason the driver thinks you have TV1, TV2 and TV3. Each TV connector has a lot of properties... With kernel 3.3 you have only TV1 and TV2. Maybe instead of increasing the maximum property count we should try to investigate why there's a new TV connector in the new kernel (and maybe this is also not a bug/regression...). -- Paulo Zanoni ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] DRM: add drm gem cma helper
Hi Sascha, it's good try to avoid demand paging way by page fault handler. actually, we don't need demand paging way on non-iommu system. I looked into your previous patch and I realized that patch was based on old exynos driver. now patch looks almostly good to me and below is my comments minor. Thanks, Inki Dae 2012/5/31 Laurent Pinchart : > Hi Sascha, > > Thanks for the patch. Here's a bit more extensive review. > > On Thursday 31 May 2012 10:08:54 Sascha Hauer wrote: >> Many embedded drm devices do not have a IOMMU and no dedicated >> memory for graphics. These devices use cma (Contiguous Memory >> Allocator) backed graphics memory. This patch provides helper >> functions to be able to share the code. >> >> Signed-off-by: Sascha Hauer >> --- >> >> changes since v1: >> >> - map whole buffer at mmap time, not page by page at fault time >> - several cleanups as suggested by Lars-Peter Clausen and Laurent Pinchart >> >> I currently do a remap_pfn_range after drm_gem_mmap, so we release >> dev->struct_mutex in between. I don't know if this is correct or if we have >> to hold the lock over the whole process. > > drm_gem_mmap() takes a reference on the GEM object so I don't think we'll have > any issue there, but please don't take my word for it. dev->struct_mutex is > the large mutex I still need to document, I don't know what it protects > exactly. > >> drivers/gpu/drm/Kconfig | 6 + >> drivers/gpu/drm/Makefile | 1 + >> drivers/gpu/drm/drm_gem_cma_helper.c | 243 +++ >> include/drm/drm_gem_cma_helper.h | 52 >> 4 files changed, 302 insertions(+) >> create mode 100644 drivers/gpu/drm/drm_gem_cma_helper.c >> create mode 100644 include/drm/drm_gem_cma_helper.h >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index e354bc0..f62717e 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -53,6 +53,12 @@ config DRM_TTM >> GPU memory types. Will be enabled automatically if a device driver >> uses it. >> >> +config DRM_GEM_CMA_HELPER >> + tristate >> + depends on DRM >> + help >> + Choose this if you need the GEM cma helper functions > > I would put CMA in uppercase, but that's just nitpicking. > > BTW this helper is not strictly dedicated to CMA. It uses the DMA API to > allocate memory, without caring about the underlying allocator. > >> + >> config DRM_TDFX >> tristate "3dfx Banshee/Voodoo3+" >> depends on DRM && PCI > > [snip] > >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c >> b/drivers/gpu/drm/drm_gem_cma_helper.c new file mode 100644 >> index 000..d8c0dc7 >> --- /dev/null >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c >> @@ -0,0 +1,243 @@ >> +/* >> + * drm gem cma (contiguous memory allocator) helper functions >> + * >> + * Copyright (C) 2012 Sascha Hauer, Pengutronix >> + * >> + * Based on Samsung Exynos code >> + * >> + * Copyright (c) 2011 Samsung Electronics Co., Ltd. >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * as published by the Free Software Foundation; either version 2 >> + * of the License, or (at your option) any later version. >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> +#include >> +#include >> +#include >> + >> +static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) >> +{ >> + return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; >> +} >> + >> +static void drm_gem_cma_buf_destroy(struct drm_device *drm, >> + struct drm_gem_cma_object *cma_obj) >> +{ >> + dma_free_writecombine(drm->dev, cma_obj->base.size, cma_obj->vaddr, >> + cma_obj->paddr); >> +} >> + >> +/* >> + * drm_gem_cma_create - allocate an object with the given size >> + * >> + * returns a struct drm_gem_cma_object* on success or ERR_PTR values >> + * on failure. >> + */ >> +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, >> + unsigned int size) >> +{ >> + struct drm_gem_cma_object *cma_obj; >> + struct drm_gem_object *gem_obj; >> + int ret; >> + >> + size = round_up(size, PAGE_SIZE); >> + >> + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL); >> + if (!cma_obj) >> + return ERR_PTR(-ENOMEM); >> + >> + cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size, >> + &cma_obj->paddr, GFP_KERNEL | __GFP_NOWARN); how about calling drm_gem_cma_buf_allocate() instead of dma_alloc_wirtecombine() for consistency in using dma api so its call flow would be "dma_gem_cma_buf_allocate() -> dma_alloc_writecombine()". >> + if (!cma_obj->vaddr) { >> + dev_err(drm->dev, "faile
[PATCH 0/2] Miscellaneous mode set and page flip fixes
Hi everybody, Here are two small fixes that disallow format changes in page flip operations, and perform a full mode set instead of a mode set base in the CRTC helper set config handler if the pixel format changed. Laurent Pinchart (2): drm: Don't allow page flip to change pixel format drm: Perform a full mode set when the pixel format changed drivers/gpu/drm/drm_crtc.c|8 drivers/gpu/drm/drm_crtc_helper.c |3 +++ 2 files changed, 11 insertions(+), 0 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: Don't allow page flip to change pixel format
A page flip is not a mode set, changing the frame buffer pixel format doesn't make sense and isn't handled by most drivers anyway. Disallow it. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/drm_crtc.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 92cea9d..0d15b56 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3530,6 +3530,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, goto out; } + if (crtc->fb->pixel_format != fb->pixel_format || + crtc->fb->bits_per_pixel != crtc->fb->bits_per_pixel || + crtc->fb->depth != fb->depth) { + DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n"); + ret = -EINVAL; + goto out; + } + if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) { ret = -ENOMEM; spin_lock_irqsave(&dev->event_lock, flags); -- 1.7.3.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm: Perform a full mode set when the pixel format changed
Test whether the pixel format changes in the mode set handler, and perform a full mode set instead of a mode set base if it does. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/drm_crtc_helper.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 3252e70..0e8cd89 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -609,6 +609,9 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) } else if (set->fb->bits_per_pixel != set->crtc->fb->bits_per_pixel) { mode_changed = true; + } else if (set->fb->pixel_format != + set->crtc->fb->pixel_format) { + mode_changed = true; } else fb_changed = true; } -- 1.7.3.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] DRM: Add DRM kms/fb cma helper
On Thu, May 31, 2012 at 11:34:37AM +0200, Lars-Peter Clausen wrote: > >> + drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); > >> + > >> + for (i = 0; i < num_planes; i++) > >> + fb_cma->obj[i] = obj[i]; > > > > Check for valid num_planes before this loop? > > > > Hm, I think the callers already take care of this. drm_format_num_planes will > always return a valid number and the other caller passes 1 unconditionally. As long as the array always is big enough to hold the maximum number of planes I think it's fine. However, if there is some format with 5 planes (don't know if there is, I only know yuv with multiple planes) it's not obvious that this code does not support this. > > > > > Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object > > in the patch I just sent. > > > > Do you think it makes sense for you to carry this patch as part of your iMX > DRM > series? I can carry this, but it could be that Laurent has his driver in an acceptable state earlier. Let's see. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code
On Thu, May 24, 2012 at 11:35 AM, Alex Deucher wrote: > On Thu, May 24, 2012 at 3:49 AM, Christian König > wrote: >> From: Christian Koenig >> >> 1. It is really dangerous to have more than one >> spinlock protecting the same information. >> >> 2. radeon_irq_set sometimes wasn't called with lock >> protection, so it can happen that more than one >> CPU would tamper with the irq regs at the same >> time. >> >> 3. The pm.gui_idle variable was assuming that the 3D >> engine wasn't becoming idle between testing the >> register and setting the variable. So just remove >> it and test the register directly. >> >> Signed-off-by: Christian Koenig >> --- >> drivers/gpu/drm/radeon/evergreen.c | 1 - >> drivers/gpu/drm/radeon/r100.c | 1 - >> drivers/gpu/drm/radeon/r600.c | 1 - >> drivers/gpu/drm/radeon/r600_hdmi.c | 6 +-- >> drivers/gpu/drm/radeon/radeon.h | 33 +++--- >> drivers/gpu/drm/radeon/radeon_irq_kms.c | 72 >> +-- >> drivers/gpu/drm/radeon/radeon_kms.c | 12 -- >> drivers/gpu/drm/radeon/radeon_pm.c | 12 +- >> drivers/gpu/drm/radeon/rs600.c | 1 - >> drivers/gpu/drm/radeon/si.c | 1 - >> 10 files changed, 90 insertions(+), 50 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/evergreen.c >> b/drivers/gpu/drm/radeon/evergreen.c >> index bfcb39e..9e9b3bb 100644 >> --- a/drivers/gpu/drm/radeon/evergreen.c >> +++ b/drivers/gpu/drm/radeon/evergreen.c >> @@ -3254,7 +3254,6 @@ restart_ih: >> break; >> case 233: /* GUI IDLE */ >> DRM_DEBUG("IH: GUI idle\n"); >> - rdev->pm.gui_idle = true; >> wake_up(&rdev->irq.idle_queue); >> break; >> default: >> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c >> index 415b7d8..2587426 100644 >> --- a/drivers/gpu/drm/radeon/r100.c >> +++ b/drivers/gpu/drm/radeon/r100.c >> @@ -782,7 +782,6 @@ int r100_irq_process(struct radeon_device *rdev) >> /* gui idle interrupt */ >> if (status & RADEON_GUI_IDLE_STAT) { >> rdev->irq.gui_idle_acked = true; >> - rdev->pm.gui_idle = true; >> wake_up(&rdev->irq.idle_queue); >> } >> /* Vertical blank interrupts */ >> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c >> index eadbb06..90c6639 100644 >> --- a/drivers/gpu/drm/radeon/r600.c >> +++ b/drivers/gpu/drm/radeon/r600.c >> @@ -3542,7 +3542,6 @@ restart_ih: >> break; >> case 233: /* GUI IDLE */ >> DRM_DEBUG("IH: GUI idle\n"); >> - rdev->pm.gui_idle = true; >> wake_up(&rdev->irq.idle_queue); >> break; >> default: >> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c >> b/drivers/gpu/drm/radeon/r600_hdmi.c >> index 226379e..b76c0f2 100644 >> --- a/drivers/gpu/drm/radeon/r600_hdmi.c >> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c >> @@ -523,8 +523,7 @@ void r600_hdmi_enable(struct drm_encoder *encoder) >> >> if (rdev->irq.installed) { >> /* if irq is available use it */ >> - rdev->irq.afmt[dig->afmt->id] = true; >> - radeon_irq_set(rdev); >> + radeon_irq_kms_enable_afmt(rdev, dig->afmt->id); >> } >> >> dig->afmt->enabled = true; >> @@ -560,8 +559,7 @@ void r600_hdmi_disable(struct drm_encoder *encoder) >> offset, radeon_encoder->encoder_id); >> >> /* disable irq */ >> - rdev->irq.afmt[dig->afmt->id] = false; >> - radeon_irq_set(rdev); >> + radeon_irq_kms_disable_afmt(rdev, dig->afmt->id); >> >> /* Older chipsets not handled by AtomBIOS */ >> if (rdev->family >= CHIP_R600 && !ASIC_IS_DCE3(rdev)) { >> diff --git a/drivers/gpu/drm/radeon/radeon.h >> b/drivers/gpu/drm/radeon/radeon.h >> index 8479096..23552b4 100644 >> --- a/drivers/gpu/drm/radeon/radeon.h >> +++ b/drivers/gpu/drm/radeon/radeon.h >> @@ -610,21 +610,20 @@ union radeon_irq_stat_regs { >> #define RADEON_MAX_AFMT_BLOCKS 6 >> >> struct radeon_irq { >> - bool installed; >> - bool sw_int[RADEON_NUM_RINGS]; >> - bool crtc_vblank_int[RADEON_MAX_CRTCS]; >> - bool pflip[RADEON_MAX_CRTCS]; >> - wait_queue_head_t vblank_queue; >> - bool hpd[RADEON_MAX_HPD_PINS]; >> - bool gui_idle; >> - bool gui_idle_acked; >> - wait_queue_head_t idle_queue; >> - bool afmt[RADEON_MAX_AFMT_BLOCKS]; >> - spinlock_t sw_lock; >> - int sw_refcount[RADEON_NUM_RINGS]; >> - union radeon_irq_stat_regs stat_regs; >> - spinlock_t pflip_lock[RADEO
Re: [PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code
On Thu, May 31, 2012 at 2:15 PM, Alex Deucher wrote: > On Thu, May 24, 2012 at 11:35 AM, Alex Deucher wrote: >> On Thu, May 24, 2012 at 3:49 AM, Christian König >> wrote: >>> From: Christian Koenig >>> >>> 1. It is really dangerous to have more than one >>> spinlock protecting the same information. >>> >>> 2. radeon_irq_set sometimes wasn't called with lock >>> protection, so it can happen that more than one >>> CPU would tamper with the irq regs at the same >>> time. >>> >>> 3. The pm.gui_idle variable was assuming that the 3D >>> engine wasn't becoming idle between testing the >>> register and setting the variable. So just remove >>> it and test the register directly. >>> >>> Signed-off-by: Christian Koenig >>> --- >>> drivers/gpu/drm/radeon/evergreen.c | 1 - >>> drivers/gpu/drm/radeon/r100.c | 1 - >>> drivers/gpu/drm/radeon/r600.c | 1 - >>> drivers/gpu/drm/radeon/r600_hdmi.c | 6 +-- >>> drivers/gpu/drm/radeon/radeon.h | 33 +++--- >>> drivers/gpu/drm/radeon/radeon_irq_kms.c | 72 >>> +-- >>> drivers/gpu/drm/radeon/radeon_kms.c | 12 -- >>> drivers/gpu/drm/radeon/radeon_pm.c | 12 +- >>> drivers/gpu/drm/radeon/rs600.c | 1 - >>> drivers/gpu/drm/radeon/si.c | 1 - >>> 10 files changed, 90 insertions(+), 50 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/radeon/evergreen.c >>> b/drivers/gpu/drm/radeon/evergreen.c >>> index bfcb39e..9e9b3bb 100644 >>> --- a/drivers/gpu/drm/radeon/evergreen.c >>> +++ b/drivers/gpu/drm/radeon/evergreen.c >>> @@ -3254,7 +3254,6 @@ restart_ih: >>> break; >>> case 233: /* GUI IDLE */ >>> DRM_DEBUG("IH: GUI idle\n"); >>> - rdev->pm.gui_idle = true; >>> wake_up(&rdev->irq.idle_queue); >>> break; >>> default: >>> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c >>> index 415b7d8..2587426 100644 >>> --- a/drivers/gpu/drm/radeon/r100.c >>> +++ b/drivers/gpu/drm/radeon/r100.c >>> @@ -782,7 +782,6 @@ int r100_irq_process(struct radeon_device *rdev) >>> /* gui idle interrupt */ >>> if (status & RADEON_GUI_IDLE_STAT) { >>> rdev->irq.gui_idle_acked = true; >>> - rdev->pm.gui_idle = true; >>> wake_up(&rdev->irq.idle_queue); >>> } >>> /* Vertical blank interrupts */ >>> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c >>> index eadbb06..90c6639 100644 >>> --- a/drivers/gpu/drm/radeon/r600.c >>> +++ b/drivers/gpu/drm/radeon/r600.c >>> @@ -3542,7 +3542,6 @@ restart_ih: >>> break; >>> case 233: /* GUI IDLE */ >>> DRM_DEBUG("IH: GUI idle\n"); >>> - rdev->pm.gui_idle = true; >>> wake_up(&rdev->irq.idle_queue); >>> break; >>> default: >>> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c >>> b/drivers/gpu/drm/radeon/r600_hdmi.c >>> index 226379e..b76c0f2 100644 >>> --- a/drivers/gpu/drm/radeon/r600_hdmi.c >>> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c >>> @@ -523,8 +523,7 @@ void r600_hdmi_enable(struct drm_encoder *encoder) >>> >>> if (rdev->irq.installed) { >>> /* if irq is available use it */ >>> - rdev->irq.afmt[dig->afmt->id] = true; >>> - radeon_irq_set(rdev); >>> + radeon_irq_kms_enable_afmt(rdev, dig->afmt->id); >>> } >>> >>> dig->afmt->enabled = true; >>> @@ -560,8 +559,7 @@ void r600_hdmi_disable(struct drm_encoder *encoder) >>> offset, radeon_encoder->encoder_id); >>> >>> /* disable irq */ >>> - rdev->irq.afmt[dig->afmt->id] = false; >>> - radeon_irq_set(rdev); >>> + radeon_irq_kms_disable_afmt(rdev, dig->afmt->id); >>> >>> /* Older chipsets not handled by AtomBIOS */ >>> if (rdev->family >= CHIP_R600 && !ASIC_IS_DCE3(rdev)) { >>> diff --git a/drivers/gpu/drm/radeon/radeon.h >>> b/drivers/gpu/drm/radeon/radeon.h >>> index 8479096..23552b4 100644 >>> --- a/drivers/gpu/drm/radeon/radeon.h >>> +++ b/drivers/gpu/drm/radeon/radeon.h >>> @@ -610,21 +610,20 @@ union radeon_irq_stat_regs { >>> #define RADEON_MAX_AFMT_BLOCKS 6 >>> >>> struct radeon_irq { >>> - bool installed; >>> - bool sw_int[RADEON_NUM_RINGS]; >>> - bool crtc_vblank_int[RADEON_MAX_CRTCS]; >>> - bool pflip[RADEON_MAX_CRTCS]; >>> - wait_queue_head_t vblank_queue; >>> - bool hpd[RADEON_MAX_HPD_PINS]; >>> - bool gui_idle; >>> - bool gui_idle_acked; >>> - wait_queue_head_t idle_queue; >>> - bool afmt[RADEON_MAX_AFMT_BLOC
RE: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function
Is there a reason for the 30 ms delay in the first place? -Satyeshwar -Original Message- From: dri-devel-bounces+satyeshwar.singh=intel@lists.freedesktop.org [mailto:dri-devel-bounces+satyeshwar.singh=intel@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Thursday, May 31, 2012 1:29 AM To: Intel Graphics Development; DRI Development Cc: Daniel Vetter Subject: Re: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function On Thu, May 24, 2012 at 09:26:49PM +0200, Daniel Vetter wrote: > A 30 ms delay is simply way too big to waste cpu cycles on. > > Signed-off-by: Daniel Vetter I've queued this patch here for -next with Chris' irc ack added. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function
On Thu, May 31, 2012 at 9:24 PM, Singh, Satyeshwar wrote: > Is there a reason for the 30 ms delay in the first place? git blame says there is, it supposedely fixes a bug with tv detection. -Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/2] drm: Don't allow page flip to change pixel format
Does this by extension mean that stride changes should also not be allowed while page flipping? Thanks, Satyeshwar -Original Message- From: dri-devel-bounces+satyeshwar.singh=intel@lists.freedesktop.org [mailto:dri-devel-bounces+satyeshwar.singh=intel@lists.freedesktop.org] On Behalf Of Laurent Pinchart Sent: Thursday, May 31, 2012 9:26 AM To: dri-devel@lists.freedesktop.org Subject: [PATCH 1/2] drm: Don't allow page flip to change pixel format A page flip is not a mode set, changing the frame buffer pixel format doesn't make sense and isn't handled by most drivers anyway. Disallow it. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/drm_crtc.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 92cea9d..0d15b56 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3530,6 +3530,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, goto out; } + if (crtc->fb->pixel_format != fb->pixel_format || + crtc->fb->bits_per_pixel != crtc->fb->bits_per_pixel || + crtc->fb->depth != fb->depth) { + DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n"); + ret = -EINVAL; + goto out; + } + if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) { ret = -ENOMEM; spin_lock_irqsave(&dev->event_lock, flags); -- 1.7.3.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: Don't allow page flip to change pixel format
On Thu, May 31, 2012 at 3:44 PM, Singh, Satyeshwar wrote: > Does this by extension mean that stride changes should also not be allowed > while page flipping? Everything beyond a crtc base address change should require a full modeset. Alex > Thanks, > Satyeshwar > > -Original Message- > From: dri-devel-bounces+satyeshwar.singh=intel@lists.freedesktop.org > [mailto:dri-devel-bounces+satyeshwar.singh=intel@lists.freedesktop.org] > On Behalf Of Laurent Pinchart > Sent: Thursday, May 31, 2012 9:26 AM > To: dri-devel@lists.freedesktop.org > Subject: [PATCH 1/2] drm: Don't allow page flip to change pixel format > > A page flip is not a mode set, changing the frame buffer pixel format doesn't > make sense and isn't handled by most drivers anyway. Disallow it. > > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/drm_crtc.c | 8 > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index > 92cea9d..0d15b56 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -3530,6 +3530,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, > goto out; > } > > + if (crtc->fb->pixel_format != fb->pixel_format || > + crtc->fb->bits_per_pixel != crtc->fb->bits_per_pixel || > + crtc->fb->depth != fb->depth) { > + DRM_DEBUG_KMS("Page flip is not allowed to change frame > buffer format.\n"); > + ret = -EINVAL; > + goto out; > + } > + > if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) { > ret = -ENOMEM; > spin_lock_irqsave(&dev->event_lock, flags); > -- > 1.7.3.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code
On 31.05.2012 20:15, Alex Deucher wrote: On Thu, May 24, 2012 at 11:35 AM, Alex Deucher wrote: On Thu, May 24, 2012 at 3:49 AM, Christian König wrote: From: Christian Koenig 1. It is really dangerous to have more than one spinlock protecting the same information. 2. radeon_irq_set sometimes wasn't called with lock protection, so it can happen that more than one CPU would tamper with the irq regs at the same time. 3. The pm.gui_idle variable was assuming that the 3D engine wasn't becoming idle between testing the register and setting the variable. So just remove it and test the register directly. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/evergreen.c |1 - drivers/gpu/drm/radeon/r100.c |1 - drivers/gpu/drm/radeon/r600.c |1 - drivers/gpu/drm/radeon/r600_hdmi.c |6 +-- drivers/gpu/drm/radeon/radeon.h | 33 +++--- drivers/gpu/drm/radeon/radeon_irq_kms.c | 72 +-- drivers/gpu/drm/radeon/radeon_kms.c | 12 -- drivers/gpu/drm/radeon/radeon_pm.c | 12 +- drivers/gpu/drm/radeon/rs600.c |1 - drivers/gpu/drm/radeon/si.c |1 - 10 files changed, 90 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index bfcb39e..9e9b3bb 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -3254,7 +3254,6 @@ restart_ih: break; case 233: /* GUI IDLE */ DRM_DEBUG("IH: GUI idle\n"); - rdev->pm.gui_idle = true; wake_up(&rdev->irq.idle_queue); break; default: diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100..c index 415b7d8..2587426 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -782,7 +782,6 @@ int r100_irq_process(struct radeon_device *rdev) /* gui idle interrupt */ if (status& RADEON_GUI_IDLE_STAT) { rdev->irq.gui_idle_acked = true; - rdev->pm.gui_idle = true; wake_up(&rdev->irq.idle_queue); } /* Vertical blank interrupts */ diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600..c index eadbb06..90c6639 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -3542,7 +3542,6 @@ restart_ih: break; case 233: /* GUI IDLE */ DRM_DEBUG("IH: GUI idle\n"); - rdev->pm.gui_idle = true; wake_up(&rdev->irq.idle_queue); break; default: diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c index 226379e..b76c0f2 100644 --- a/drivers/gpu/drm/radeon/r600_hdmi.c +++ b/drivers/gpu/drm/radeon/r600_hdmi.c @@ -523,8 +523,7 @@ void r600_hdmi_enable(struct drm_encoder *encoder) if (rdev->irq.installed) { /* if irq is available use it */ - rdev->irq.afmt[dig->afmt->id] = true; - radeon_irq_set(rdev); + radeon_irq_kms_enable_afmt(rdev, dig->afmt->id); } dig->afmt->enabled = true; @@ -560,8 +559,7 @@ void r600_hdmi_disable(struct drm_encoder *encoder) offset, radeon_encoder->encoder_id); /* disable irq */ - rdev->irq.afmt[dig->afmt->id] = false; - radeon_irq_set(rdev); + radeon_irq_kms_disable_afmt(rdev, dig->afmt->id); /* Older chipsets not handled by AtomBIOS */ if (rdev->family>= CHIP_R600&& !ASIC_IS_DCE3(rdev)) { diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 8479096..23552b4 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -610,21 +610,20 @@ union radeon_irq_stat_regs { #define RADEON_MAX_AFMT_BLOCKS 6 struct radeon_irq { - boolinstalled; - boolsw_int[RADEON_NUM_RINGS]; - boolcrtc_vblank_int[RADEON_MAX_CRTCS]; - boolpflip[RADEON_MAX_CRTCS]; - wait_queue_head_t vblank_queue; - boolhpd[RADEON_MAX_HPD_PINS]; - boolgui_idle; - boolgui_idle_acked; - wait_queue_head_t idle_queue; - boolafmt[RADEON_MAX_AFMT_BLOCKS]; - spinlock_t sw_lock; - int sw_refcount[RADEON_NUM_RINGS]; - union radeon_irq_stat_regs stat_regs; - spinlock_t pflip_lock[RADEON_MAX_CRTCS]; - int pflip_refcount[RADEON_MAX_CRTCS]; + boolinstalled; + spinlock_t lock; + boolsw_int[RADEON_NUM_RINGS]; + int
[PATCH 02/10] drm/radeon: add infrastructure for advanced ring synchronization
Signed-off-by: Christian König --- drivers/gpu/drm/radeon/radeon.h | 23 ++- drivers/gpu/drm/radeon/radeon_fence.c | 73 + 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 5e259b4..4e232c3 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -257,8 +257,8 @@ struct radeon_fence_driver { uint32_tscratch_reg; uint64_tgpu_addr; volatile uint32_t *cpu_addr; - /* seq is protected by ring emission lock */ - uint64_tseq; + /* sync_seq is protected by ring emission lock */ + uint64_tsync_seq[RADEON_NUM_RINGS]; atomic64_t last_seq; unsigned long last_activity; boolinitialized; @@ -288,6 +288,25 @@ int radeon_fence_wait_any(struct radeon_device *rdev, struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence); void radeon_fence_unref(struct radeon_fence **fence); unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring); +bool radeon_fence_need_sync(struct radeon_fence *fence, int ring); +void radeon_fence_note_sync(struct radeon_fence *fence, int ring); +static inline struct radeon_fence *radeon_fence_later(struct radeon_fence *a, + struct radeon_fence *b) +{ + if (!a) { + return b; + } + + if (!b) { + return a; + } + + if (a->seq > b->seq) { + return a; + } else { + return b; + } +} /* * Tiling registers diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index 401d346..7b55625 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -72,7 +72,7 @@ int radeon_fence_emit(struct radeon_device *rdev, } kref_init(&((*fence)->kref)); (*fence)->rdev = rdev; - (*fence)->seq = ++rdev->fence_drv[ring].seq; + (*fence)->seq = ++rdev->fence_drv[ring].sync_seq[ring]; (*fence)->ring = ring; radeon_fence_ring_emit(rdev, ring, *fence); trace_radeon_fence_emit(rdev->ddev, (*fence)->seq); @@ -449,7 +449,7 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring) * wait. */ seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL; - if (seq >= rdev->fence_drv[ring].seq) { + if (seq >= rdev->fence_drv[ring].sync_seq[ring]) { /* nothing to wait for, last_seq is already the last emited fence */ return -ENOENT; @@ -464,7 +464,7 @@ int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring) * activity can be scheduled so there won't be concurrent access * to seq value. */ - return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].seq, + return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].sync_seq[ring], ring, false, false); } @@ -492,7 +492,8 @@ unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring) * but it's ok to report slightly wrong fence count here. */ radeon_fence_process(rdev, ring); - emitted = rdev->fence_drv[ring].seq - atomic64_read(&rdev->fence_drv[ring].last_seq); + emitted = rdev->fence_drv[ring].sync_seq[ring] + - atomic64_read(&rdev->fence_drv[ring].last_seq); /* to avoid 32bits warp around */ if (emitted > 0x1000) { emitted = 0x1000; @@ -500,6 +501,51 @@ unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring) return (unsigned)emitted; } +bool radeon_fence_need_sync(struct radeon_fence *fence, int dst_ring) +{ + struct radeon_fence_driver *fdrv; + + if (!fence) { + return false; + } + + if (fence->ring == dst_ring) { + return false; + } + + /* we are protected by the ring mutex */ + fdrv = &fence->rdev->fence_drv[dst_ring]; + if (fence->seq <= fdrv->sync_seq[fence->ring]) { + return false; + } + + return true; +} + +void radeon_fence_note_sync(struct radeon_fence *fence, int dst_ring) +{ + struct radeon_fence_driver *dst, *src; + unsigned i; + + if (!fence) { + return; + } + + if (fence->ring == dst_ring) { + return; + } + + /* we are protected by the ring mutex */ + src = &fence->rdev->fence_drv[fence->ring]; + dst = &fence->rdev->fence_drv[dst_ring]; + for (i = 0; i < RADEON_NUM_RINGS; ++i) { + if (i == dst_ring) { + contin
[PATCH 01/10] drm/radeon: remove radeon_fence_create
It is completely unnecessary to create fences before they are emitted, so remove it and a bunch of checks if fences are emitted or not. Signed-off-by: Christian König --- drivers/gpu/drm/radeon/evergreen.c|2 +- drivers/gpu/drm/radeon/ni.c |2 +- drivers/gpu/drm/radeon/r100.c |4 +- drivers/gpu/drm/radeon/r200.c |4 +- drivers/gpu/drm/radeon/r600.c |4 +- drivers/gpu/drm/radeon/r600_blit_kms.c|6 +-- drivers/gpu/drm/radeon/radeon.h | 11 +++-- drivers/gpu/drm/radeon/radeon_asic.h |8 ++-- drivers/gpu/drm/radeon/radeon_benchmark.c | 10 + drivers/gpu/drm/radeon/radeon_fence.c | 42 ++ drivers/gpu/drm/radeon/radeon_ring.c | 19 + drivers/gpu/drm/radeon/radeon_sa.c|2 +- drivers/gpu/drm/radeon/radeon_test.c | 66 - drivers/gpu/drm/radeon/radeon_ttm.c | 30 + drivers/gpu/drm/radeon/si.c |6 +-- 15 files changed, 86 insertions(+), 130 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 58991af..dd3cea4 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -1366,7 +1366,7 @@ void evergreen_mc_program(struct radeon_device *rdev) */ void evergreen_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { - struct radeon_ring *ring = &rdev->ring[ib->fence->ring]; + struct radeon_ring *ring = &rdev->ring[ib->ring]; /* set to DX10/11 mode */ radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0)); diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index b01c2dd..9d9f5ac 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -1127,7 +1127,7 @@ void cayman_fence_ring_emit(struct radeon_device *rdev, void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { - struct radeon_ring *ring = &rdev->ring[ib->fence->ring]; + struct radeon_ring *ring = &rdev->ring[ib->ring]; /* set to DX10/11 mode */ radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0)); diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c index fb44e7e..415b7d8 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -883,7 +883,7 @@ int r100_copy_blit(struct radeon_device *rdev, uint64_t src_offset, uint64_t dst_offset, unsigned num_gpu_pages, - struct radeon_fence *fence) + struct radeon_fence **fence) { struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; uint32_t cur_pages; @@ -947,7 +947,7 @@ int r100_copy_blit(struct radeon_device *rdev, RADEON_WAIT_HOST_IDLECLEAN | RADEON_WAIT_DMA_GUI_IDLE); if (fence) { - r = radeon_fence_emit(rdev, fence); + r = radeon_fence_emit(rdev, fence, RADEON_RING_TYPE_GFX_INDEX); } radeon_ring_unlock_commit(rdev, ring); return r; diff --git a/drivers/gpu/drm/radeon/r200.c b/drivers/gpu/drm/radeon/r200.c index a26144d..f088925 100644 --- a/drivers/gpu/drm/radeon/r200.c +++ b/drivers/gpu/drm/radeon/r200.c @@ -85,7 +85,7 @@ int r200_copy_dma(struct radeon_device *rdev, uint64_t src_offset, uint64_t dst_offset, unsigned num_gpu_pages, - struct radeon_fence *fence) + struct radeon_fence **fence) { struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; uint32_t size; @@ -120,7 +120,7 @@ int r200_copy_dma(struct radeon_device *rdev, radeon_ring_write(ring, PACKET0(RADEON_WAIT_UNTIL, 0)); radeon_ring_write(ring, RADEON_WAIT_DMA_GUI_IDLE); if (fence) { - r = radeon_fence_emit(rdev, fence); + r = radeon_fence_emit(rdev, fence, RADEON_RING_TYPE_GFX_INDEX); } radeon_ring_unlock_commit(rdev, ring); return r; diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index f388a1d..e5279f9 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2369,7 +2369,7 @@ int r600_copy_blit(struct radeon_device *rdev, uint64_t src_offset, uint64_t dst_offset, unsigned num_gpu_pages, - struct radeon_fence *fence) + struct radeon_fence **fence) { struct radeon_sa_bo *vb = NULL; int r; @@ -2670,7 +2670,7 @@ void r600_fini(struct radeon_device *rdev) */ void r600_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { - struct radeon_ring *ring = &rdev->ring[ib->fence->ring]; + struct radeon_ring *ring = &rdev->ring[ib->ring]; /* FIXM
[PATCH 05/10] drm/radeon: remove some unneeded structure members
From: Christian Koenig Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/radeon.h |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 3e83480..618df9a 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -733,9 +733,7 @@ struct r600_ih { struct radeon_bo*ring_obj; volatile uint32_t *ring; unsignedrptr; - unsignedrptr_offs; unsignedwptr; - unsignedwptr_old; unsignedring_size; uint64_tgpu_addr; uint32_tptr_mask; -- 1.7.9.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/10] drm/radeon: rework ring syncing code
Move inter ring syncing with semaphores into the existing ring allocations, with that we need to lock the ring mutex only once. Signed-off-by: Christian König --- drivers/gpu/drm/radeon/evergreen_blit_kms.c |3 +- drivers/gpu/drm/radeon/r600.c |5 +- drivers/gpu/drm/radeon/r600_blit_kms.c | 24 +++-- drivers/gpu/drm/radeon/radeon.h |6 +-- drivers/gpu/drm/radeon/radeon_asic.h|5 +- drivers/gpu/drm/radeon/radeon_cs.c | 38 +++--- drivers/gpu/drm/radeon/radeon_ring.c| 30 +-- drivers/gpu/drm/radeon/radeon_semaphore.c | 71 ++- drivers/gpu/drm/radeon/radeon_test.c|6 +-- drivers/gpu/drm/radeon/radeon_ttm.c | 20 10 files changed, 92 insertions(+), 116 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen_blit_kms.c b/drivers/gpu/drm/radeon/evergreen_blit_kms.c index 1e96bd4..e512560 100644 --- a/drivers/gpu/drm/radeon/evergreen_blit_kms.c +++ b/drivers/gpu/drm/radeon/evergreen_blit_kms.c @@ -622,7 +622,8 @@ int evergreen_blit_init(struct radeon_device *rdev) rdev->r600_blit.primitives.draw_auto = draw_auto; rdev->r600_blit.primitives.set_default_state = set_default_state; - rdev->r600_blit.ring_size_common = 55; /* shaders + def state */ + rdev->r600_blit.ring_size_common = 8; /* sync semaphore */ + rdev->r600_blit.ring_size_common += 55; /* shaders + def state */ rdev->r600_blit.ring_size_common += 16; /* fence emit for VB IB */ rdev->r600_blit.ring_size_common += 5; /* done copy */ rdev->r600_blit.ring_size_common += 16; /* fence emit for done copy */ diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index e5279f9..a8d8c44 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2371,15 +2371,16 @@ int r600_copy_blit(struct radeon_device *rdev, unsigned num_gpu_pages, struct radeon_fence **fence) { + struct radeon_semaphore *sem = NULL; struct radeon_sa_bo *vb = NULL; int r; - r = r600_blit_prepare_copy(rdev, num_gpu_pages, &vb); + r = r600_blit_prepare_copy(rdev, num_gpu_pages, fence, &vb, &sem); if (r) { return r; } r600_kms_blit_copy(rdev, src_offset, dst_offset, num_gpu_pages, vb); - r600_blit_done_copy(rdev, fence, vb); + r600_blit_done_copy(rdev, fence, vb, sem); return 0; } diff --git a/drivers/gpu/drm/radeon/r600_blit_kms.c b/drivers/gpu/drm/radeon/r600_blit_kms.c index 02f4eeb..2b8d641 100644 --- a/drivers/gpu/drm/radeon/r600_blit_kms.c +++ b/drivers/gpu/drm/radeon/r600_blit_kms.c @@ -512,7 +512,8 @@ int r600_blit_init(struct radeon_device *rdev) rdev->r600_blit.primitives.draw_auto = draw_auto; rdev->r600_blit.primitives.set_default_state = set_default_state; - rdev->r600_blit.ring_size_common = 40; /* shaders + def state */ + rdev->r600_blit.ring_size_common = 8; /* sync semaphore */ + rdev->r600_blit.ring_size_common += 40; /* shaders + def state */ rdev->r600_blit.ring_size_common += 5; /* done copy */ rdev->r600_blit.ring_size_common += 16; /* fence emit for done copy */ @@ -666,7 +667,8 @@ static unsigned r600_blit_create_rect(unsigned num_gpu_pages, int r600_blit_prepare_copy(struct radeon_device *rdev, unsigned num_gpu_pages, - struct radeon_sa_bo **vb) + struct radeon_fence **fence, struct radeon_sa_bo **vb, + struct radeon_semaphore **sem) { struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; int r; @@ -689,22 +691,37 @@ int r600_blit_prepare_copy(struct radeon_device *rdev, unsigned num_gpu_pages, return r; } + r = radeon_semaphore_create(rdev, sem); + if (r) { + radeon_sa_bo_free(rdev, vb, NULL); + return r; + } + /* calculate number of loops correctly */ ring_size = num_loops * dwords_per_loop; ring_size += rdev->r600_blit.ring_size_common; r = radeon_ring_lock(rdev, ring, ring_size); if (r) { radeon_sa_bo_free(rdev, vb, NULL); + radeon_semaphore_free(rdev, sem, NULL); return r; } + if (radeon_fence_need_sync(*fence, RADEON_RING_TYPE_GFX_INDEX)) { + radeon_semaphore_sync_rings(rdev, *sem, (*fence)->ring, + RADEON_RING_TYPE_GFX_INDEX); + radeon_fence_note_sync(*fence, RADEON_RING_TYPE_GFX_INDEX); + } else { + radeon_semaphore_free(rdev, sem, NULL); + } + rdev->r600_blit.primitives.set_default_state(rdev); rdev->r600_blit.primitives.set_shaders(rdev); return 0; } void r600_blit_done_copy(struct radeon_device *rdev, stru
[PATCH 06/10] drm/radeon: fix ih ring handling v2
From: Christian Koenig The spinlock was actually there to protect the rptr, but rptr was read outside of the locked area. v2: Keep the spinlock, looks like there is still something in the ih that doesn't like irqs enabled. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/evergreen.c |5 ++--- drivers/gpu/drm/radeon/r600.c |7 ++- drivers/gpu/drm/radeon/radeon.h|1 - drivers/gpu/drm/radeon/si.c|6 ++ 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index dd3cea4..e4c3321 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2950,11 +2950,11 @@ int evergreen_irq_process(struct radeon_device *rdev) if (!rdev->ih.enabled || rdev->shutdown) return IRQ_NONE; + spin_lock_irqsave(&rdev->ih.lock, flags); wptr = evergreen_get_ih_wptr(rdev); rptr = rdev->ih.rptr; DRM_DEBUG("r600_irq_process start: rptr %d, wptr %d\n", rptr, wptr); - spin_lock_irqsave(&rdev->ih.lock, flags); if (rptr == wptr) { spin_unlock_irqrestore(&rdev->ih.lock, flags); return IRQ_NONE; @@ -2966,7 +2966,6 @@ restart_ih: /* display interrupts */ evergreen_irq_ack(rdev); - rdev->ih.wptr = wptr; while (rptr != wptr) { /* wptr/rptr are in bytes! */ ring_index = rptr / 4; @@ -3267,7 +3266,7 @@ restart_ih: } /* make sure wptr hasn't changed while processing */ wptr = evergreen_get_ih_wptr(rdev); - if (wptr != rdev->ih.wptr) + if (wptr != rptr) goto restart_ih; if (queue_hotplug) schedule_work(&rdev->hotplug_work); diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index a8d8c44..e1861ac 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2921,7 +2921,6 @@ void r600_disable_interrupts(struct radeon_device *rdev) WREG32(IH_RB_RPTR, 0); WREG32(IH_RB_WPTR, 0); rdev->ih.enabled = false; - rdev->ih.wptr = 0; rdev->ih.rptr = 0; } @@ -3384,12 +3383,11 @@ int r600_irq_process(struct radeon_device *rdev) if (!rdev->msi_enabled) RREG32(IH_RB_WPTR); + spin_lock_irqsave(&rdev->ih.lock, flags); wptr = r600_get_ih_wptr(rdev); rptr = rdev->ih.rptr; DRM_DEBUG("r600_irq_process start: rptr %d, wptr %d\n", rptr, wptr); - spin_lock_irqsave(&rdev->ih.lock, flags); - if (rptr == wptr) { spin_unlock_irqrestore(&rdev->ih.lock, flags); return IRQ_NONE; @@ -3402,7 +3400,6 @@ restart_ih: /* display interrupts */ r600_irq_ack(rdev); - rdev->ih.wptr = wptr; while (rptr != wptr) { /* wptr/rptr are in bytes! */ ring_index = rptr / 4; @@ -3558,7 +3555,7 @@ restart_ih: } /* make sure wptr hasn't changed while processing */ wptr = r600_get_ih_wptr(rdev); - if (wptr != rdev->ih.wptr) + if (wptr != rptr) goto restart_ih; if (queue_hotplug) schedule_work(&rdev->hotplug_work); diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 618df9a..378d43b 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -733,7 +733,6 @@ struct r600_ih { struct radeon_bo*ring_obj; volatile uint32_t *ring; unsignedrptr; - unsignedwptr; unsignedring_size; uint64_tgpu_addr; uint32_tptr_mask; diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 5ca8ef5..93da01c 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -3095,7 +3095,6 @@ static void si_disable_interrupts(struct radeon_device *rdev) WREG32(IH_RB_RPTR, 0); WREG32(IH_RB_WPTR, 0); rdev->ih.enabled = false; - rdev->ih.wptr = 0; rdev->ih.rptr = 0; } @@ -3518,11 +3517,11 @@ int si_irq_process(struct radeon_device *rdev) if (!rdev->ih.enabled || rdev->shutdown) return IRQ_NONE; + spin_lock_irqsave(&rdev->ih.lock, flags); wptr = si_get_ih_wptr(rdev); rptr = rdev->ih.rptr; DRM_DEBUG("si_irq_process start: rptr %d, wptr %d\n", rptr, wptr); - spin_lock_irqsave(&rdev->ih.lock, flags); if (rptr == wptr) { spin_unlock_irqrestore(&rdev->ih.lock, flags); return IRQ_NONE; @@ -3534,7 +3533,6 @@ restart_ih: /* display interrupts */ si_irq_ack(rdev); - rdev->ih.wptr = wptr; while (rptr != wptr) { /* wptr/rptr are in bytes! */ ring_index = rptr / 4; @@ -3787,
[PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code v2
From: Christian Koenig 1. It is really dangerous to have more than one spinlock protecting the same information. 2. radeon_irq_set sometimes wasn't called with lock protection, so it can happen that more than one CPU would tamper with the irq regs at the same time. 3. The pm.gui_idle variable was assuming that the 3D engine wasn't becoming idle between testing the register and setting the variable. So just remove it and test the register directly. v2: Also handle the hpd irq code the same way. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/evergreen.c | 21 ++- drivers/gpu/drm/radeon/r100.c | 29 ++ drivers/gpu/drm/radeon/r600.c | 38 drivers/gpu/drm/radeon/r600_hdmi.c |6 +- drivers/gpu/drm/radeon/radeon.h | 35 +-- drivers/gpu/drm/radeon/radeon_irq_kms.c | 96 +++ drivers/gpu/drm/radeon/radeon_kms.c | 12 +++- drivers/gpu/drm/radeon/radeon_pm.c | 12 +--- drivers/gpu/drm/radeon/rs600.c | 13 ++--- drivers/gpu/drm/radeon/si.c |1 - 10 files changed, 144 insertions(+), 119 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index e4c3321..48ec1a0 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -428,6 +428,7 @@ void evergreen_hpd_init(struct radeon_device *rdev) { struct drm_device *dev = rdev->ddev; struct drm_connector *connector; + unsigned enabled = 0; u32 tmp = DC_HPDx_CONNECTION_TIMER(0x9c4) | DC_HPDx_RX_INT_TIMER(0xfa) | DC_HPDx_EN; @@ -436,73 +437,64 @@ void evergreen_hpd_init(struct radeon_device *rdev) switch (radeon_connector->hpd.hpd) { case RADEON_HPD_1: WREG32(DC_HPD1_CONTROL, tmp); - rdev->irq.hpd[0] = true; break; case RADEON_HPD_2: WREG32(DC_HPD2_CONTROL, tmp); - rdev->irq.hpd[1] = true; break; case RADEON_HPD_3: WREG32(DC_HPD3_CONTROL, tmp); - rdev->irq.hpd[2] = true; break; case RADEON_HPD_4: WREG32(DC_HPD4_CONTROL, tmp); - rdev->irq.hpd[3] = true; break; case RADEON_HPD_5: WREG32(DC_HPD5_CONTROL, tmp); - rdev->irq.hpd[4] = true; break; case RADEON_HPD_6: WREG32(DC_HPD6_CONTROL, tmp); - rdev->irq.hpd[5] = true; break; default: break; } radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); + enabled |= 1 << radeon_connector->hpd.hpd; } - if (rdev->irq.installed) - evergreen_irq_set(rdev); + radeon_irq_kms_enable_hpd(rdev, enabled); } void evergreen_hpd_fini(struct radeon_device *rdev) { struct drm_device *dev = rdev->ddev; struct drm_connector *connector; + unsigned disabled = 0; list_for_each_entry(connector, &dev->mode_config.connector_list, head) { struct radeon_connector *radeon_connector = to_radeon_connector(connector); switch (radeon_connector->hpd.hpd) { case RADEON_HPD_1: WREG32(DC_HPD1_CONTROL, 0); - rdev->irq.hpd[0] = false; break; case RADEON_HPD_2: WREG32(DC_HPD2_CONTROL, 0); - rdev->irq.hpd[1] = false; break; case RADEON_HPD_3: WREG32(DC_HPD3_CONTROL, 0); - rdev->irq.hpd[2] = false; break; case RADEON_HPD_4: WREG32(DC_HPD4_CONTROL, 0); - rdev->irq.hpd[3] = false; break; case RADEON_HPD_5: WREG32(DC_HPD5_CONTROL, 0); - rdev->irq.hpd[4] = false; break; case RADEON_HPD_6: WREG32(DC_HPD6_CONTROL, 0); - rdev->irq.hpd[5] = false; break; default: break; } + disabled |= 1 << radeon_connector->hpd.hpd; } + radeon_irq_kms_disable_hpd(rdev, disabled); } /* watermark setup */ @@ -3252,7 +3244,6 @@ restart_ih: break; case 233: /* GUI IDLE */ DRM_DEBUG("IH: GUI idle\n"); -
[PATCH 08/10] drm/radeon: replace pflip and sw_int counters with atomics
From: Christian Koenig So we can skip the looking. Also renames sw_int to ring_int, cause that better matches its purpose. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/evergreen.c | 32 - drivers/gpu/drm/radeon/r100.c | 10 +++--- drivers/gpu/drm/radeon/r600.c | 10 +++--- drivers/gpu/drm/radeon/radeon.h |6 ++-- drivers/gpu/drm/radeon/radeon_irq_kms.c | 59 +++ drivers/gpu/drm/radeon/rs600.c | 10 +++--- drivers/gpu/drm/radeon/si.c | 30 7 files changed, 76 insertions(+), 81 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 48ec1a0..3d3520a 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2607,20 +2607,20 @@ int evergreen_irq_set(struct radeon_device *rdev) if (rdev->family >= CHIP_CAYMAN) { /* enable CP interrupts on all rings */ - if (rdev->irq.sw_int[RADEON_RING_TYPE_GFX_INDEX]) { + if (atomic_read(&rdev->irq.ring_int[RADEON_RING_TYPE_GFX_INDEX])) { DRM_DEBUG("evergreen_irq_set: sw int gfx\n"); cp_int_cntl |= TIME_STAMP_INT_ENABLE; } - if (rdev->irq.sw_int[CAYMAN_RING_TYPE_CP1_INDEX]) { + if (atomic_read(&rdev->irq.ring_int[CAYMAN_RING_TYPE_CP1_INDEX])) { DRM_DEBUG("evergreen_irq_set: sw int cp1\n"); cp_int_cntl1 |= TIME_STAMP_INT_ENABLE; } - if (rdev->irq.sw_int[CAYMAN_RING_TYPE_CP2_INDEX]) { + if (atomic_read(&rdev->irq.ring_int[CAYMAN_RING_TYPE_CP2_INDEX])) { DRM_DEBUG("evergreen_irq_set: sw int cp2\n"); cp_int_cntl2 |= TIME_STAMP_INT_ENABLE; } } else { - if (rdev->irq.sw_int[RADEON_RING_TYPE_GFX_INDEX]) { + if (atomic_read(&rdev->irq.ring_int[RADEON_RING_TYPE_GFX_INDEX])) { DRM_DEBUG("evergreen_irq_set: sw int gfx\n"); cp_int_cntl |= RB_INT_ENABLE; cp_int_cntl |= TIME_STAMP_INT_ENABLE; @@ -2628,32 +2628,32 @@ int evergreen_irq_set(struct radeon_device *rdev) } if (rdev->irq.crtc_vblank_int[0] || - rdev->irq.pflip[0]) { + atomic_read(&rdev->irq.pflip[0])) { DRM_DEBUG("evergreen_irq_set: vblank 0\n"); crtc1 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[1] || - rdev->irq.pflip[1]) { + atomic_read(&rdev->irq.pflip[1])) { DRM_DEBUG("evergreen_irq_set: vblank 1\n"); crtc2 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[2] || - rdev->irq.pflip[2]) { + atomic_read(&rdev->irq.pflip[2])) { DRM_DEBUG("evergreen_irq_set: vblank 2\n"); crtc3 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[3] || - rdev->irq.pflip[3]) { + atomic_read(&rdev->irq.pflip[3])) { DRM_DEBUG("evergreen_irq_set: vblank 3\n"); crtc4 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[4] || - rdev->irq.pflip[4]) { + atomic_read(&rdev->irq.pflip[4])) { DRM_DEBUG("evergreen_irq_set: vblank 4\n"); crtc5 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[5] || - rdev->irq.pflip[5]) { + atomic_read(&rdev->irq.pflip[5])) { DRM_DEBUG("evergreen_irq_set: vblank 5\n"); crtc6 |= VBLANK_INT_MASK; } @@ -2974,7 +2974,7 @@ restart_ih: rdev->pm.vblank_sync = true; wake_up(&rdev->irq.vblank_queue); } - if (rdev->irq.pflip[0]) + if (atomic_read(&rdev->irq.pflip[0])) radeon_crtc_handle_flip(rdev, 0); rdev->irq.stat_regs.evergreen.disp_int &= ~LB_D1_VBLANK_INTERRUPT; DRM_DEBUG("IH: D1 vblank\n"); @@ -3000,7 +3000,7 @@ restart_ih: rdev->pm.vblank_sync = true; wake_up(&rdev->irq.vblank_queue); } - if (rdev->irq.pflip[1]) + if (atomic_read(&rdev->irq.pflip[1])) radeon_crtc_handle_flip(rdev, 1); rdev->irq.stat_regs.evergreen.disp_int_cont &= ~LB_D2_VBLAN
[PATCH 09/10] drm/radeon: replace cs_mutex with vm_mutex
Try to remove or replace the cs_mutex with a vm_mutex where it is still needed. Signed-off-by: Christian König --- drivers/gpu/drm/radeon/radeon.h| 44 +--- drivers/gpu/drm/radeon/radeon_cs.c |7 ++--- drivers/gpu/drm/radeon/radeon_device.c |2 +- drivers/gpu/drm/radeon/radeon_gart.c | 16 ++-- drivers/gpu/drm/radeon/radeon_gem.c|2 -- 5 files changed, 12 insertions(+), 59 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 19e7554..b24877f 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -159,48 +159,6 @@ static inline int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len) #endif bool radeon_get_bios(struct radeon_device *rdev); - -/* - * Mutex which allows recursive locking from the same process. - */ -struct radeon_mutex { - struct mutexmutex; - struct task_struct *owner; - int level; -}; - -static inline void radeon_mutex_init(struct radeon_mutex *mutex) -{ - mutex_init(&mutex->mutex); - mutex->owner = NULL; - mutex->level = 0; -} - -static inline void radeon_mutex_lock(struct radeon_mutex *mutex) -{ - if (mutex_trylock(&mutex->mutex)) { - /* The mutex was unlocked before, so it's ours now */ - mutex->owner = current; - } else if (mutex->owner != current) { - /* Another process locked the mutex, take it */ - mutex_lock(&mutex->mutex); - mutex->owner = current; - } - /* Otherwise the mutex was already locked by this process */ - - mutex->level++; -} - -static inline void radeon_mutex_unlock(struct radeon_mutex *mutex) -{ - if (--mutex->level > 0) - return; - - mutex->owner = NULL; - mutex_unlock(&mutex->mutex); -} - - /* * Dummy page */ @@ -1529,7 +1487,6 @@ struct radeon_device { struct radeon_gem gem; struct radeon_pmpm; uint32_tbios_scratch[RADEON_BIOS_NUM_SCRATCH]; - struct radeon_mutex cs_mutex; struct radeon_wbwb; struct radeon_dummy_pagedummy_page; boolshutdown; @@ -1563,6 +1520,7 @@ struct radeon_device { struct radeon_debugfs debugfs[RADEON_DEBUGFS_MAX_COMPONENTS]; unsigneddebugfs_count; /* virtual memory */ + struct mutexvm_mutex; struct radeon_vm_managervm_manager; }; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index d295821..d1ead9c 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -438,6 +438,7 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, } mutex_lock(&vm->mutex); + mutex_lock(&rdev->vm_mutex); r = radeon_vm_bind(rdev, vm); if (r) { goto out; @@ -474,6 +475,7 @@ out: } vm->fence = radeon_fence_ref(parser->ib.fence); } + mutex_unlock(&rdev->vm_mutex); mutex_unlock(&fpriv->vm.mutex); return r; } @@ -494,9 +496,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct radeon_cs_parser parser; int r; - radeon_mutex_lock(&rdev->cs_mutex); if (!rdev->accel_working) { - radeon_mutex_unlock(&rdev->cs_mutex); return -EBUSY; } /* initialize parser */ @@ -510,7 +510,6 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) DRM_ERROR("Failed to initialize parser !\n"); radeon_cs_parser_fini(&parser, r); r = radeon_cs_handle_lockup(rdev, r); - radeon_mutex_unlock(&rdev->cs_mutex); return r; } r = radeon_cs_parser_relocs(&parser); @@ -519,7 +518,6 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) DRM_ERROR("Failed to parse relocation %d!\n", r); radeon_cs_parser_fini(&parser, r); r = radeon_cs_handle_lockup(rdev, r); - radeon_mutex_unlock(&rdev->cs_mutex); return r; } r = radeon_cs_ib_chunk(rdev, &parser); @@ -533,7 +531,6 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) out: radeon_cs_parser_fini(&parser, r); r = radeon_cs_handle_lockup(rdev, r); - radeon_mutex_unlock(&rdev->cs_mutex); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 7667184..57143dd 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -728,7 +728,6 @@
[PATCH 10/10] drm/radeon: work around bugs in caymans compute rings
From: Christian Koenig The shader preemption on cayman doesn't work correctly with multiple rings. So to be able to still make use of the compute rings we use a semaphore to make sure that only one IB can execute at the same time. This isn't as effective as shader preemption, but also isn't as bad as putting everything on the GFX ring. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/ni.c| 142 ++-- drivers/gpu/drm/radeon/radeon.h|2 + drivers/gpu/drm/radeon/radeon_cs.c |2 +- 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index 9d9f5ac..6a3e8a8 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -1125,13 +1125,75 @@ void cayman_fence_ring_emit(struct radeon_device *rdev, radeon_ring_write(ring, 0); } +/* The shader preemption on cayman doesn't work + * correctly with multiple rings. So to be able to + * still make use of the compute rings we use a + * semaphore to make sure that only one IB can execute + * at the same time + */ +static void cayman_cp_ring_create_workaround(struct radeon_device *rdev) +{ + struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; + int r; + + r = radeon_semaphore_create(rdev, &rdev->cayman_ring_lock); + if (r) { + dev_err(rdev->dev, "Can't allocate " + "cayman_ring_lock (%d)!\n", r); + return; + } + + r = radeon_ring_alloc(rdev, ring, 8); + if (r) { + dev_err(rdev->dev, "Can't initialize " + "cayman_ring_lock (%d)!\n", r); + radeon_semaphore_free(rdev, &rdev->cayman_ring_lock, NULL); + return; + } + + radeon_semaphore_emit_signal(rdev, RADEON_RING_TYPE_GFX_INDEX, +rdev->cayman_ring_lock); + + radeon_ring_commit(rdev, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]); +} + +static void cayman_cp_ring_cleanup_workaround(struct radeon_device *rdev) +{ + struct radeon_fence *fence; + int r; + + r = radeon_fence_emit(rdev, &fence, RADEON_RING_TYPE_GFX_INDEX); + if (r) { + dev_err(rdev->dev, "Can't cleanup " + "cayman_ring_lock (%d)!\n", r); + return; + } + + radeon_semaphore_free(rdev, &rdev->cayman_ring_lock, fence); + radeon_fence_unref(&fence); +} + void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { struct radeon_ring *ring = &rdev->ring[ib->ring]; + if (ib->ring != RADEON_RING_TYPE_GFX_INDEX) { + if (rdev->cayman_ring_lock == NULL) { + cayman_cp_ring_create_workaround(rdev); + } + } else { + if (rdev->cayman_ring_lock != NULL && + !radeon_fence_count_emitted(rdev, CAYMAN_RING_TYPE_CP1_INDEX) && + !radeon_fence_count_emitted(rdev, CAYMAN_RING_TYPE_CP2_INDEX)) { + cayman_cp_ring_cleanup_workaround(rdev); + } + } + /* set to DX10/11 mode */ radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0)); radeon_ring_write(ring, 1); + if (rdev->cayman_ring_lock) + radeon_semaphore_emit_wait(rdev, ib->ring, rdev->cayman_ring_lock); radeon_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2)); radeon_ring_write(ring, #ifdef __BIG_ENDIAN @@ -1140,6 +1202,8 @@ void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) (ib->gpu_addr & 0xFFFC)); radeon_ring_write(ring, upper_32_bits(ib->gpu_addr) & 0xFF); radeon_ring_write(ring, ib->length_dw | (ib->vm_id << 24)); + if (rdev->cayman_ring_lock) + radeon_semaphore_emit_signal(rdev, ib->ring, rdev->cayman_ring_lock); /* flush read cache over gart for this vmid */ radeon_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1)); @@ -1190,6 +1254,25 @@ static int cayman_cp_load_microcode(struct radeon_device *rdev) return 0; } +static int cayman_cp_start_compute(struct radeon_device *rdev, int ridx) +{ + struct radeon_ring *ring = &rdev->ring[ridx]; + int r; + + r = radeon_ring_lock(rdev, ring, 2); + if (r) { + DRM_ERROR("radeon: cp failed to lock ring (%d).\n", r); + return r; + } + + /* clear the compute context state */ + radeon_ring_write(ring, PACKET3(PACKET3_CLEAR_STATE, 0) | 2); + radeon_ring_write(ring, 0); + + radeon_ring_unlock_commit(rdev, ring); + return 0; +} + static int cayman_cp_start(struct radeon_device *rdev) { struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; @@ -1251,7 +1334,17 @@ static int cayman_cp_start(struct radeon_device *rdev) radeon_ring_unlock_commit(
[PATCH 04/10] drm/radeon: replace vmram_mutex with mclk_lock v2
It is a rw_semaphore now and only write locked while changing the clock. Also the lock is renamed to better reflect what it is protecting. v2: Keep the ttm_vm_ops on IGPs Signed-off-by: Christian König --- drivers/gpu/drm/radeon/radeon.h|3 ++- drivers/gpu/drm/radeon/radeon_device.c |2 +- drivers/gpu/drm/radeon/radeon_object.c |8 drivers/gpu/drm/radeon/radeon_pm.c |4 ++-- drivers/gpu/drm/radeon/radeon_ttm.c|4 ++-- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index aebaf28..3e83480 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1055,6 +1055,8 @@ struct radeon_power_state { struct radeon_pm { struct mutexmutex; + /* write locked while reprogramming mclk */ + struct rw_semaphore mclk_lock; u32 active_crtcs; int active_crtc_count; int req_vblank; @@ -1551,7 +1553,6 @@ struct radeon_device { struct work_struct audio_work; int num_crtc; /* number of crtcs */ struct mutex dc_hw_i2c_mutex; /* display controller hw i2c mutex */ - struct mutex vram_mutex; bool audio_enabled; struct r600_audio audio_status; /* audio stuff */ struct notifier_block acpi_nb; diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 066c98b..7667184 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -735,7 +735,7 @@ int radeon_device_init(struct radeon_device *rdev, spin_lock_init(&rdev->ih.lock); mutex_init(&rdev->gem.mutex); mutex_init(&rdev->pm.mutex); - mutex_init(&rdev->vram_mutex); + init_rwsem(&rdev->pm.mclk_lock); init_waitqueue_head(&rdev->irq.vblank_queue); init_waitqueue_head(&rdev->irq.idle_queue); r = radeon_gem_init(rdev); diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 830f1a7..6ecb200 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -154,11 +154,11 @@ retry: INIT_LIST_HEAD(&bo->va); radeon_ttm_placement_from_domain(bo, domain); /* Kernel allocation are uninterruptible */ - mutex_lock(&rdev->vram_mutex); + down_read(&rdev->pm.mclk_lock); r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type, &bo->placement, page_align, 0, !kernel, NULL, acc_size, sg, &radeon_ttm_bo_destroy); - mutex_unlock(&rdev->vram_mutex); + up_read(&rdev->pm.mclk_lock); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) { if (domain == RADEON_GEM_DOMAIN_VRAM) { @@ -219,9 +219,9 @@ void radeon_bo_unref(struct radeon_bo **bo) return; rdev = (*bo)->rdev; tbo = &((*bo)->tbo); - mutex_lock(&rdev->vram_mutex); + down_read(&rdev->pm.mclk_lock); ttm_bo_unref(&tbo); - mutex_unlock(&rdev->vram_mutex); + up_read(&rdev->pm.mclk_lock); if (tbo == NULL) *bo = NULL; } diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 0882554..d13b6ae 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -251,7 +251,7 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev) return; mutex_lock(&rdev->ddev->struct_mutex); - mutex_lock(&rdev->vram_mutex); + down_write(&rdev->pm.mclk_lock); mutex_lock(&rdev->ring_lock); /* gui idle int has issues on older chips it seems */ @@ -303,7 +303,7 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev) rdev->pm.dynpm_planned_action = DYNPM_ACTION_NONE; mutex_unlock(&rdev->ring_lock); - mutex_unlock(&rdev->vram_mutex); + up_write(&rdev->pm.mclk_lock); mutex_unlock(&rdev->ddev->struct_mutex); } diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index c43035c..0881131 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -797,9 +797,9 @@ static int radeon_ttm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) return VM_FAULT_NOPAGE; } rdev = radeon_get_rdev(bo->bdev); - mutex_lock(&rdev->vram_mutex); + down_read(&rdev->pm.mclk_lock); r = ttm_vm_ops->fault(vma, vmf); - mutex_unlock(&rdev->vram_mutex); + up_read(&rdev->pm.mclk_lock); return r; } -- 1.7.9.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code v2
On Thu, May 31, 2012 at 4:16 PM, Christian König wrote: > From: Christian Koenig > > 1. It is really dangerous to have more than one > spinlock protecting the same information. > > 2. radeon_irq_set sometimes wasn't called with lock > protection, so it can happen that more than one > CPU would tamper with the irq regs at the same > time. > > 3. The pm.gui_idle variable was assuming that the 3D > engine wasn't becoming idle between testing the > register and setting the variable. So just remove > it and test the register directly. > > v2: Also handle the hpd irq code the same way. couple of comments below for clarity, other than that: Reviewed-by: Alex Deucher > > Signed-off-by: Christian Koenig > --- > drivers/gpu/drm/radeon/evergreen.c | 21 ++- > drivers/gpu/drm/radeon/r100.c | 29 ++ > drivers/gpu/drm/radeon/r600.c | 38 > drivers/gpu/drm/radeon/r600_hdmi.c | 6 +- > drivers/gpu/drm/radeon/radeon.h | 35 +-- > drivers/gpu/drm/radeon/radeon_irq_kms.c | 96 > +++ > drivers/gpu/drm/radeon/radeon_kms.c | 12 +++- > drivers/gpu/drm/radeon/radeon_pm.c | 12 +--- > drivers/gpu/drm/radeon/rs600.c | 13 ++--- > drivers/gpu/drm/radeon/si.c | 1 - > 10 files changed, 144 insertions(+), 119 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/evergreen.c > b/drivers/gpu/drm/radeon/evergreen.c > index e4c3321..48ec1a0 100644 > --- a/drivers/gpu/drm/radeon/evergreen.c > +++ b/drivers/gpu/drm/radeon/evergreen.c > @@ -428,6 +428,7 @@ void evergreen_hpd_init(struct radeon_device *rdev) > { > struct drm_device *dev = rdev->ddev; > struct drm_connector *connector; > + unsigned enabled = 0; > u32 tmp = DC_HPDx_CONNECTION_TIMER(0x9c4) | > DC_HPDx_RX_INT_TIMER(0xfa) | DC_HPDx_EN; > > @@ -436,73 +437,64 @@ void evergreen_hpd_init(struct radeon_device *rdev) > switch (radeon_connector->hpd.hpd) { > case RADEON_HPD_1: > WREG32(DC_HPD1_CONTROL, tmp); > - rdev->irq.hpd[0] = true; > break; > case RADEON_HPD_2: > WREG32(DC_HPD2_CONTROL, tmp); > - rdev->irq.hpd[1] = true; > break; > case RADEON_HPD_3: > WREG32(DC_HPD3_CONTROL, tmp); > - rdev->irq.hpd[2] = true; > break; > case RADEON_HPD_4: > WREG32(DC_HPD4_CONTROL, tmp); > - rdev->irq.hpd[3] = true; > break; > case RADEON_HPD_5: > WREG32(DC_HPD5_CONTROL, tmp); > - rdev->irq.hpd[4] = true; > break; > case RADEON_HPD_6: > WREG32(DC_HPD6_CONTROL, tmp); > - rdev->irq.hpd[5] = true; > break; > default: > break; > } > radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); > + enabled |= 1 << radeon_connector->hpd.hpd; > } > - if (rdev->irq.installed) > - evergreen_irq_set(rdev); > + radeon_irq_kms_enable_hpd(rdev, enabled); > } > > void evergreen_hpd_fini(struct radeon_device *rdev) > { > struct drm_device *dev = rdev->ddev; > struct drm_connector *connector; > + unsigned disabled = 0; > > list_for_each_entry(connector, &dev->mode_config.connector_list, head) > { > struct radeon_connector *radeon_connector = > to_radeon_connector(connector); > switch (radeon_connector->hpd.hpd) { > case RADEON_HPD_1: > WREG32(DC_HPD1_CONTROL, 0); > - rdev->irq.hpd[0] = false; > break; > case RADEON_HPD_2: > WREG32(DC_HPD2_CONTROL, 0); > - rdev->irq.hpd[1] = false; > break; > case RADEON_HPD_3: > WREG32(DC_HPD3_CONTROL, 0); > - rdev->irq.hpd[2] = false; > break; > case RADEON_HPD_4: > WREG32(DC_HPD4_CONTROL, 0); > - rdev->irq.hpd[3] = false; > break; > case RADEON_HPD_5: > WREG32(DC_HPD5_CONTROL, 0); > - rdev->irq.hpd[4] = false; > break; > case RADEON_HPD_6: > WREG32(DC_HPD6_CONTROL, 0); > - rdev->irq.hpd[5] = false; > break; > default: > break; > } > + dis
Re: [PATCH 02/10] drm/radeon: add infrastructure for advanced ring synchronization
On Thu, May 31, 2012 at 4:15 PM, Christian König wrote: > Signed-off-by: Christian König > --- > drivers/gpu/drm/radeon/radeon.h | 23 ++- > drivers/gpu/drm/radeon/radeon_fence.c | 73 > + > 2 files changed, 85 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index 5e259b4..4e232c3 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -257,8 +257,8 @@ struct radeon_fence_driver { > uint32_t scratch_reg; > uint64_t gpu_addr; > volatile uint32_t *cpu_addr; > - /* seq is protected by ring emission lock */ > - uint64_t seq; > + /* sync_seq is protected by ring emission lock */ > + uint64_t sync_seq[RADEON_NUM_RINGS]; > atomic64_t last_seq; > unsigned long last_activity; > bool initialized; > @@ -288,6 +288,25 @@ int radeon_fence_wait_any(struct radeon_device *rdev, > struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence); > void radeon_fence_unref(struct radeon_fence **fence); > unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring); > +bool radeon_fence_need_sync(struct radeon_fence *fence, int ring); > +void radeon_fence_note_sync(struct radeon_fence *fence, int ring); > +static inline struct radeon_fence *radeon_fence_later(struct radeon_fence *a, > + struct radeon_fence *b) > +{ > + if (!a) { > + return b; > + } > + > + if (!b) { > + return a; > + } Please add : BUG_ON(a->ring != b->ring); So we can catch if someone badly use this function. > + > + if (a->seq > b->seq) { > + return a; > + } else { > + return b; > + } > +} > > /* > * Tiling registers > diff --git a/drivers/gpu/drm/radeon/radeon_fence.c > b/drivers/gpu/drm/radeon/radeon_fence.c > index 401d346..7b55625 100644 > --- a/drivers/gpu/drm/radeon/radeon_fence.c > +++ b/drivers/gpu/drm/radeon/radeon_fence.c > @@ -72,7 +72,7 @@ int radeon_fence_emit(struct radeon_device *rdev, > } > kref_init(&((*fence)->kref)); > (*fence)->rdev = rdev; > - (*fence)->seq = ++rdev->fence_drv[ring].seq; > + (*fence)->seq = ++rdev->fence_drv[ring].sync_seq[ring]; > (*fence)->ring = ring; > radeon_fence_ring_emit(rdev, ring, *fence); > trace_radeon_fence_emit(rdev->ddev, (*fence)->seq); > @@ -449,7 +449,7 @@ int radeon_fence_wait_next_locked(struct radeon_device > *rdev, int ring) > * wait. > */ > seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL; > - if (seq >= rdev->fence_drv[ring].seq) { > + if (seq >= rdev->fence_drv[ring].sync_seq[ring]) { > /* nothing to wait for, last_seq is > already the last emited fence */ > return -ENOENT; > @@ -464,7 +464,7 @@ int radeon_fence_wait_empty_locked(struct radeon_device > *rdev, int ring) > * activity can be scheduled so there won't be concurrent access > * to seq value. > */ > - return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].seq, > + return radeon_fence_wait_seq(rdev, > rdev->fence_drv[ring].sync_seq[ring], > ring, false, false); > } > > @@ -492,7 +492,8 @@ unsigned radeon_fence_count_emitted(struct radeon_device > *rdev, int ring) > * but it's ok to report slightly wrong fence count here. > */ > radeon_fence_process(rdev, ring); > - emitted = rdev->fence_drv[ring].seq - > atomic64_read(&rdev->fence_drv[ring].last_seq); > + emitted = rdev->fence_drv[ring].sync_seq[ring] > + - atomic64_read(&rdev->fence_drv[ring].last_seq); > /* to avoid 32bits warp around */ > if (emitted > 0x1000) { > emitted = 0x1000; > @@ -500,6 +501,51 @@ unsigned radeon_fence_count_emitted(struct radeon_device > *rdev, int ring) > return (unsigned)emitted; > } > > +bool radeon_fence_need_sync(struct radeon_fence *fence, int dst_ring) > +{ > + struct radeon_fence_driver *fdrv; > + > + if (!fence) { > + return false; > + } > + > + if (fence->ring == dst_ring) { > + return false; > + } > + > + /* we are protected by the ring mutex */ > + fdrv = &fence->rdev->fence_drv[dst_ring]; > + if (fence->seq <= fdrv->sync_seq[fence->ring]) { > + return false; > + } > + > + return true; > +} > + > +void radeon_fence_note_sync(struct radeon_fence *fence, int dst_ring) > +{ > + struct radeon_fence_driver *dst, *src; > + unsigned i; > + > + if (!fence) { >
[PATCH] drm/radeon: fix bank information in tiling config
From: Alex Deucher While there are cards with more than 8 mem banks, the max number of banks from a tiling perspective is 8, so cap the tiling config at 8 banks. Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=43448 Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org --- drivers/gpu/drm/radeon/evergreen.c |9 ++--- drivers/gpu/drm/radeon/ni.c|9 ++--- drivers/gpu/drm/radeon/rv770.c |8 ++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 58991af..5d9c2c6 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2136,9 +2136,12 @@ static void evergreen_gpu_init(struct radeon_device *rdev) /* num banks is 8 on all fusion asics. 0 = 4, 1 = 8, 2 = 16 */ if (rdev->flags & RADEON_IS_IGP) rdev->config.evergreen.tile_config |= 1 << 4; - else - rdev->config.evergreen.tile_config |= - ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) << 4; + else { + if ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) + rdev->config.evergreen.tile_config |= 1 << 4; + else + rdev->config.evergreen.tile_config |= 0 << 4; + } rdev->config.evergreen.tile_config |= ((mc_arb_ramcfg & BURSTLENGTH_MASK) >> BURSTLENGTH_SHIFT) << 8; rdev->config.evergreen.tile_config |= diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index ce4e7cc..15f950c 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -866,9 +866,12 @@ static void cayman_gpu_init(struct radeon_device *rdev) /* num banks is 8 on all fusion asics. 0 = 4, 1 = 8, 2 = 16 */ if (rdev->flags & RADEON_IS_IGP) rdev->config.cayman.tile_config |= 1 << 4; - else - rdev->config.cayman.tile_config |= - ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) << 4; + else { + if ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) + rdev->config.cayman.tile_config |= 1 << 4; + else + rdev->config.cayman.tile_config |= 0 << 4; + } rdev->config.cayman.tile_config |= ((gb_addr_config & PIPE_INTERLEAVE_SIZE_MASK) >> PIPE_INTERLEAVE_SIZE_SHIFT) << 8; rdev->config.cayman.tile_config |= diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index c2f473b..c824d49 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -689,8 +689,12 @@ static void rv770_gpu_init(struct radeon_device *rdev) if (rdev->family == CHIP_RV770) gb_tiling_config |= BANK_TILING(1); - else - gb_tiling_config |= BANK_TILING((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT); + else { + if ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) + gb_tiling_config |= BANK_TILING(1); + else + gb_tiling_config |= BANK_TILING(0); + } rdev->config.rv770.tiling_nbanks = 4 << ((gb_tiling_config >> 4) & 0x3); gb_tiling_config |= GROUP_SIZE((mc_arb_ramcfg & BURSTLENGTH_MASK) >> BURSTLENGTH_SHIFT); if ((mc_arb_ramcfg & BURSTLENGTH_MASK) >> BURSTLENGTH_SHIFT) -- 1.7.7.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: properly program gart on rv740, juniper, cypress, barts
From: Alex Deucher Need to program an additional VM register. This doesn't not currently cause any problems, but allows us to program the proper backend map in a subsequent patch which should improve performance on these asics. Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org --- drivers/gpu/drm/radeon/evergreen.c |5 + drivers/gpu/drm/radeon/evergreend.h |1 + drivers/gpu/drm/radeon/rv770.c |2 ++ drivers/gpu/drm/radeon/rv770d.h |1 + 4 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 5d9c2c6..0408ac2 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -1029,6 +1029,11 @@ int evergreen_pcie_gart_enable(struct radeon_device *rdev) WREG32(MC_VM_MD_L1_TLB0_CNTL, tmp); WREG32(MC_VM_MD_L1_TLB1_CNTL, tmp); WREG32(MC_VM_MD_L1_TLB2_CNTL, tmp); + if ((rdev->family == CHIP_JUNIPER) || + (rdev->family == CHIP_CYPRESS) || + (rdev->family == CHIP_HEMLOCK) || + (rdev->family == CHIP_BARTS)) + WREG32(MC_VM_MD_L1_TLB3_CNTL, tmp); } WREG32(MC_VM_MB_L1_TLB0_CNTL, tmp); WREG32(MC_VM_MB_L1_TLB1_CNTL, tmp); diff --git a/drivers/gpu/drm/radeon/evergreend.h b/drivers/gpu/drm/radeon/evergreend.h index 79130bf..3dd43e7 100644 --- a/drivers/gpu/drm/radeon/evergreend.h +++ b/drivers/gpu/drm/radeon/evergreend.h @@ -452,6 +452,7 @@ #defineMC_VM_MD_L1_TLB0_CNTL 0x2654 #defineMC_VM_MD_L1_TLB1_CNTL 0x2658 #defineMC_VM_MD_L1_TLB2_CNTL 0x265C +#defineMC_VM_MD_L1_TLB3_CNTL 0x2698 #defineFUS_MC_VM_MD_L1_TLB0_CNTL 0x265C #defineFUS_MC_VM_MD_L1_TLB1_CNTL 0x2660 diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index c824d49..c12349d 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -151,6 +151,8 @@ int rv770_pcie_gart_enable(struct radeon_device *rdev) WREG32(MC_VM_MD_L1_TLB0_CNTL, tmp); WREG32(MC_VM_MD_L1_TLB1_CNTL, tmp); WREG32(MC_VM_MD_L1_TLB2_CNTL, tmp); + if (rdev->family == CHIP_RV740) + WREG32(MC_VM_MD_L1_TLB3_CNTL, tmp); WREG32(MC_VM_MB_L1_TLB0_CNTL, tmp); WREG32(MC_VM_MB_L1_TLB1_CNTL, tmp); WREG32(MC_VM_MB_L1_TLB2_CNTL, tmp); diff --git a/drivers/gpu/drm/radeon/rv770d.h b/drivers/gpu/drm/radeon/rv770d.h index 9c549f7..7addbef 100644 --- a/drivers/gpu/drm/radeon/rv770d.h +++ b/drivers/gpu/drm/radeon/rv770d.h @@ -174,6 +174,7 @@ #defineMC_VM_MD_L1_TLB0_CNTL 0x2654 #defineMC_VM_MD_L1_TLB1_CNTL 0x2658 #defineMC_VM_MD_L1_TLB2_CNTL 0x265C +#defineMC_VM_MD_L1_TLB3_CNTL 0x2698 #defineMC_VM_SYSTEM_APERTURE_DEFAULT_ADDR 0x203C #defineMC_VM_SYSTEM_APERTURE_HIGH_ADDR 0x2038 #defineMC_VM_SYSTEM_APERTURE_LOW_ADDR 0x2034 -- 1.7.7.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/radeon: fix HD6790, HD6570 backend programming
From: Jerome Glisse Bugs that should be fixed by this patch : https://bugs.freedesktop.org/show_bug.cgi?id=49792 https://bugzilla.kernel.org/show_bug.cgi?id=43207 https://bugs.freedesktop.org/show_bug.cgi?id=39282 Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/evergreen.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 0408ac2..6a57f0d 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2178,9 +2178,9 @@ static void evergreen_gpu_init(struct radeon_device *rdev) WREG32(CC_SYS_RB_BACKEND_DISABLE, rb); WREG32(GC_USER_RB_BACKEND_DISABLE, rb); WREG32(CC_GC_SHADER_PIPE_CONFIG, sp); -} + } - grbm_gfx_index |= SE_BROADCAST_WRITES; + grbm_gfx_index = INSTANCE_BROADCAST_WRITES | SE_BROADCAST_WRITES; WREG32(GRBM_GFX_INDEX, grbm_gfx_index); WREG32(RLC_GFX_INDEX, grbm_gfx_index); -- 1.7.7.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/radeon: fixup tiling group size and backendmap on r6xx-r9xx (v4)
From: Alex Deucher Tiling group size is always 256bits on r6xx/r7xx/r8xx/9xx. Also fix and simplify render backend map. This now properly sets up the backend map on r6xx-9xx which should improve 3D performance. Signed-off-by: Alex Deucher Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/evergreen.c | 368 +-- drivers/gpu/drm/radeon/evergreend.h | 10 + drivers/gpu/drm/radeon/ni.c | 351 +++-- drivers/gpu/drm/radeon/nid.h| 11 + drivers/gpu/drm/radeon/r600.c | 199 +++ drivers/gpu/drm/radeon/r600d.h |2 + drivers/gpu/drm/radeon/radeon.h |5 + drivers/gpu/drm/radeon/rv770.c | 264 + drivers/gpu/drm/radeon/rv770d.h |3 + 9 files changed, 222 insertions(+), 991 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 6a57f0d..01550d0 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -1558,163 +1558,10 @@ int evergreen_cp_resume(struct radeon_device *rdev) /* * Core functions */ -static u32 evergreen_get_tile_pipe_to_backend_map(struct radeon_device *rdev, - u32 num_tile_pipes, - u32 num_backends, - u32 backend_disable_mask) -{ - u32 backend_map = 0; - u32 enabled_backends_mask = 0; - u32 enabled_backends_count = 0; - u32 cur_pipe; - u32 swizzle_pipe[EVERGREEN_MAX_PIPES]; - u32 cur_backend = 0; - u32 i; - bool force_no_swizzle; - - if (num_tile_pipes > EVERGREEN_MAX_PIPES) - num_tile_pipes = EVERGREEN_MAX_PIPES; - if (num_tile_pipes < 1) - num_tile_pipes = 1; - if (num_backends > EVERGREEN_MAX_BACKENDS) - num_backends = EVERGREEN_MAX_BACKENDS; - if (num_backends < 1) - num_backends = 1; - - for (i = 0; i < EVERGREEN_MAX_BACKENDS; ++i) { - if (((backend_disable_mask >> i) & 1) == 0) { - enabled_backends_mask |= (1 << i); - ++enabled_backends_count; - } - if (enabled_backends_count == num_backends) - break; - } - - if (enabled_backends_count == 0) { - enabled_backends_mask = 1; - enabled_backends_count = 1; - } - - if (enabled_backends_count != num_backends) - num_backends = enabled_backends_count; - - memset((uint8_t *)&swizzle_pipe[0], 0, sizeof(u32) * EVERGREEN_MAX_PIPES); - switch (rdev->family) { - case CHIP_CEDAR: - case CHIP_REDWOOD: - case CHIP_PALM: - case CHIP_SUMO: - case CHIP_SUMO2: - case CHIP_TURKS: - case CHIP_CAICOS: - force_no_swizzle = false; - break; - case CHIP_CYPRESS: - case CHIP_HEMLOCK: - case CHIP_JUNIPER: - case CHIP_BARTS: - default: - force_no_swizzle = true; - break; - } - if (force_no_swizzle) { - bool last_backend_enabled = false; - - force_no_swizzle = false; - for (i = 0; i < EVERGREEN_MAX_BACKENDS; ++i) { - if (((enabled_backends_mask >> i) & 1) == 1) { - if (last_backend_enabled) - force_no_swizzle = true; - last_backend_enabled = true; - } else - last_backend_enabled = false; - } - } - - switch (num_tile_pipes) { - case 1: - case 3: - case 5: - case 7: - DRM_ERROR("odd number of pipes!\n"); - break; - case 2: - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 1; - break; - case 4: - if (force_no_swizzle) { - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 1; - swizzle_pipe[2] = 2; - swizzle_pipe[3] = 3; - } else { - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 2; - swizzle_pipe[2] = 1; - swizzle_pipe[3] = 3; - } - break; - case 6: - if (force_no_swizzle) { - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 1; - swizzle_pipe[2] = 2; - swizzle_pipe[3] = 3; - swizzle_pipe[4] = 4; - swizzle_pipe[5] = 5; - } else { - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 2; -
Re: [PATCH 08/10] drm/radeon: replace pflip and sw_int counters with atomics
On Don, 2012-05-31 at 22:16 +0200, Christian König wrote: > > So we can skip the looking. 'locking' > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c > b/drivers/gpu/drm/radeon/radeon_irq_kms.c > index 73cd0fd..52f85ba 100644 > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > @@ -87,16 +87,7 @@ void radeon_driver_irq_preinstall_kms(struct drm_device > *dev) > > int radeon_driver_irq_postinstall_kms(struct drm_device *dev) > { > - struct radeon_device *rdev = dev->dev_private; > - unsigned long irqflags; > - unsigned i; > - > dev->max_vblank_count = 0x001f; > - spin_lock_irqsave(&rdev->irq.lock, irqflags); > - for (i = 0; i < RADEON_NUM_RINGS; i++) > - rdev->irq.sw_int[i] = true; > - radeon_irq_set(rdev); > - spin_unlock_irqrestore(&rdev->irq.lock, irqflags); > return 0; > } Why does this function no longer need to enable SW interrupts? If it really doesn't, that might be material for a separate patch. > @@ -225,25 +216,28 @@ void radeon_irq_kms_sw_irq_get(struct radeon_device > *rdev, int ring) > { > unsigned long irqflags; > > - spin_lock_irqsave(&rdev->irq.lock, irqflags); > - if (rdev->ddev->irq_enabled && (++rdev->irq.sw_refcount[ring] == 1)) { > - rdev->irq.sw_int[ring] = true; > + if (!rdev->ddev->irq_enabled) > + return; > + > + if (atomic_inc_return(&rdev->irq.ring_int[ring]) == 1) { > + spin_lock_irqsave(&rdev->irq.lock, irqflags); > radeon_irq_set(rdev); > + spin_unlock_irqrestore(&rdev->irq.lock, irqflags); > } > - spin_unlock_irqrestore(&rdev->irq.lock, irqflags); > } > > void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev, int ring) > { > unsigned long irqflags; > > - spin_lock_irqsave(&rdev->irq.lock, irqflags); > - BUG_ON(rdev->ddev->irq_enabled && rdev->irq.sw_refcount[ring] <= 0); > - if (rdev->ddev->irq_enabled && (--rdev->irq.sw_refcount[ring] == 0)) { > - rdev->irq.sw_int[ring] = false; > + if (!rdev->ddev->irq_enabled) > + return; > + > + if (atomic_dec_and_test(&rdev->irq.ring_int[ring])) { > + spin_lock_irqsave(&rdev->irq.lock, irqflags); > radeon_irq_set(rdev); > + spin_unlock_irqrestore(&rdev->irq.lock, irqflags); > } > - spin_unlock_irqrestore(&rdev->irq.lock, irqflags); > } > > void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int crtc) I think this might introduce a race condition: Thread 0 Thread 1 atomic_inc_return() returns 1 spin_lock_irqsave() atomic_dec_and_test() radeon_irq_set() => the interrupt won't be enabled. Maybe this explains why you couldn't remove the spinlock in patch 6? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/10] drm/radeon: replace pflip and sw_int counters with atomics
On Fre, 2012-06-01 at 08:19 +0200, Michel Dänzer wrote: > > I think this might introduce a race condition: > > Thread 0 Thread 1 > > atomic_inc_return() returns 1 > spin_lock_irqsave() > atomic_dec_and_test() > radeon_irq_set() > > => the interrupt won't be enabled. Hrmm, I messed up the formatting there, let me try one more time: Thread 0Thread 1 atomic_inc_return() returns 1 spin_lock_irqsave() atomic_dec_and_test() radeon_irq_set() -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 49981] On HD6850, Power Profile doesn't change if 2 screen is attached.
https://bugs.freedesktop.org/show_bug.cgi?id=49981 --- Comment #1 from Brad Campbell 2012-05-30 19:13:12 PDT --- Created attachment 62305 --> https://bugs.freedesktop.org/attachment.cgi?id=62305 The hack I use to enable 3 heads on my iMac without torching the Radeon -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 49981] On HD6850, Power Profile doesn't change if 2 screen is attached.
https://bugs.freedesktop.org/show_bug.cgi?id=49981 --- Comment #2 from Brad Campbell 2012-05-30 19:15:29 PDT --- I can replicate this on all radeon enabled kernels including current. It appears than when more than one head is enabled, a separate set of power profiles is used which attempts to set fire to the graphics card. I simply edited the second set of power profiles to re-enable low power mode. It's a nasty hack, but it at least makes my 2011 iMac usable with the radeon driver. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 43295] panic occurred switching back to text console ubuntu 11.10
https://bugzilla.kernel.org/show_bug.cgi?id=43295 --- Comment #4 from Thanigaivelan 2012-05-31 02:51:44 --- My System hangs when panic occurred switching back to text console. I have to restart it again. it happen many time. Please tell me what is the problem in kernel file. I am using kerner version 3.1.2-030102-generic. Thanks Thanigaivelan -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug.
Fw: [Intel-gfx] [PATCH] intel: add a timed wait function
On Thu, May 31, 2012 at 12:35 AM, Eric Anholt wrote: > > Did you want pointer for timeout in the userspace api? ?I don't feel > strongly about it, I just didn't see a use. ?The equivalent API I could > think of was select(), where apparently linux returns time unwaited, > while "everyone else" doesn't. ?I don't see a strong recommendation > either way from that. I wanted the kernel to return the unwaited time (or at least a upper bound to it, with coarse clocks that's the best we can do) so that ioctl restarting after a signal does the right thing. Exposing that any further than libdrm doesn't make much sense imo. -Daniel -- Daniel Vetter daniel.vetter at ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
[PATCH] drm: Add downclock quirk for Samsung LTN121AT10-301
On Wed, May 30, 2012 at 11:21:32AM -0400, Sean Paul wrote: > On Wed, May 30, 2012 at 10:16 AM, Adam Jackson wrote: > > On 5/30/12 8:05 AM, Sean Paul wrote: > > > >> Yes, definitely. The reason I can't set it via xrandr (easily) is > >> because we look for lvds downclock modes (in i915) on the driver init. > >> Since the driver initializes way before we have a chance to add a new > >> mode via xrandr, the driver won't have a downclock mode. > >> > >> I suppose the other option is to hack the i915 driver to allow a > >> downclocked mode to be added after it's been initialized. I haven't > >> looked into this solution, it might be worth investigating. > > > > > > Just so I'm clear, is what you're looking for "I want this pair of timings, > > with the driver magically switching between them"? ?If all you wanted was > > the lower clock speed all the time you could just, you know, do that, so I > > assume that's not what you're after. > > > > Yep, you got it. > > > If binding two timings together like that is what you want, then that seems > > like a pretty reasonable device-specific ioctl at first glance. ?I think the > > only thing to be careful of would be copying the slower timings into the > > driver private of the faster, rather than keeping a pointer or copy of the > > object id, since modes aren't refcounted. > > > > How big of a power savings do you see with this? ?Wondering if it's worth > > trying to make some common tooling for finding downclocked modes, if it's > > going to be worthwhile on multiple panels. > > > > I saw about 200mW savings with downclocking enabled. It's hard to say > how many panels this might apply to, some panels which report > downclocked states still flicker, and others that don't have it in the > EDID work just fine (like this Samsung panel). > > LVDS downclocking is enabled by default in Fedora and ChromiumOS, so > making this more accessible might be useful for users of these > distros. I think adding lvds connector properties for downclocking would make sense. I guess we need and enable knob and a frequency value. At boot time we'd fill these with the detected values, but then userspace would have full control (i.e. the enable knob should overwrite the i915 module options). That way downclocking would also be much easier to test (and maybe we could try to enable it by default eventually). -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
Lots of i915/drm spew on 3.4
On Wed, 30 May 2012 18:26:51 -0400, Dave Jones wrote: > On Wed, May 30, 2012 at 05:58:48PM -0400, Dave Jones wrote: > > On Wed, May 30, 2012 at 11:51:54PM +0200, Daniel Vetter wrote: > > > On Wed, May 30, 2012 at 11:31 PM, Dave Jones > wrote: > > > > On this hardware: > > > > > > > > 00:02.0 VGA compatible controller: Intel Corporation 82945G/GZ > Integrated Graphics Controller (rev 02) > > > > > > > > I get this every boot with Linus current tree (up to > af56e0aa35f3ae2a4c1a6d1000702df1dd78cb76) > > > > > > Just a quick question, is this a regression? > > > > seems so, I don't see it on 3.3 The WARN is new, the problem is old. commit c543188afb7a83e66161c026dc6fd5eb38dc0b63 Author: Paulo Zanoni Date: Tue May 15 18:09:02 2012 -0300 drm: add generic ioctls to get/set properties on any object Useless for connector properties (since they already have their own ioctls), but useful when we add properties to CRTCs, planes and other objects. Reviewed-by: Eugeni Dodonov Reviewed-by: Rob Clark Tested-by: Rob Clark Signed-off-by: Paulo Zanoni Signed-off-by: Dave Airlie Before that commit we had no idea that we had run out of property slots. I think the WARN is genuine, but maybe we should just bump the count set it to WARN_ONCE and hope the conversion to lists arrives sooner rather than latter. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v2] DRM: add drm gem cma helper
Many embedded drm devices do not have a IOMMU and no dedicated memory for graphics. These devices use cma (Contiguous Memory Allocator) backed graphics memory. This patch provides helper functions to be able to share the code. Signed-off-by: Sascha Hauer --- changes since v1: - map whole buffer at mmap time, not page by page at fault time - several cleanups as suggested by Lars-Peter Clausen and Laurent Pinchart I currently do a remap_pfn_range after drm_gem_mmap, so we release dev->struct_mutex in between. I don't know if this is correct or if we have to hold the lock over the whole process. drivers/gpu/drm/Kconfig |6 + drivers/gpu/drm/Makefile |1 + drivers/gpu/drm/drm_gem_cma_helper.c | 243 ++ include/drm/drm_gem_cma_helper.h | 52 4 files changed, 302 insertions(+) create mode 100644 drivers/gpu/drm/drm_gem_cma_helper.c create mode 100644 include/drm/drm_gem_cma_helper.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index e354bc0..f62717e 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -53,6 +53,12 @@ config DRM_TTM GPU memory types. Will be enabled automatically if a device driver uses it. +config DRM_GEM_CMA_HELPER + tristate + depends on DRM + help + Choose this if you need the GEM cma helper functions + config DRM_TDFX tristate "3dfx Banshee/Voodoo3+" depends on DRM && PCI diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index c20da5b..9a0d98a 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -15,6 +15,7 @@ drm-y :=drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \ drm_trace_points.o drm_global.o drm_prime.o drm-$(CONFIG_COMPAT) += drm_ioc32.o +drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm-usb-y := drm_usb.o diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c new file mode 100644 index 000..d8c0dc7 --- /dev/null +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -0,0 +1,243 @@ +/* + * drm gem cma (contiguous memory allocator) helper functions + * + * Copyright (C) 2012 Sascha Hauer, Pengutronix + * + * Based on Samsung Exynos code + * + * Copyright (c) 2011 Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#include +#include +#include + +static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) +{ + return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; +} + +static void drm_gem_cma_buf_destroy(struct drm_device *drm, + struct drm_gem_cma_object *cma_obj) +{ + dma_free_writecombine(drm->dev, cma_obj->base.size, cma_obj->vaddr, + cma_obj->paddr); +} + +/* + * drm_gem_cma_create - allocate an object with the given size + * + * returns a struct drm_gem_cma_object* on success or ERR_PTR values + * on failure. + */ +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, + unsigned int size) +{ + struct drm_gem_cma_object *cma_obj; + struct drm_gem_object *gem_obj; + int ret; + + size = round_up(size, PAGE_SIZE); + + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL); + if (!cma_obj) + return ERR_PTR(-ENOMEM); + + cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size, + &cma_obj->paddr, GFP_KERNEL | __GFP_NOWARN); + if (!cma_obj->vaddr) { + dev_err(drm->dev, "failed to allocate buffer with size %d\n", size); + ret = -ENOMEM; + goto err_dma_alloc; + } + + gem_obj = &cma_obj->base; + + ret = drm_gem_object_init(drm, gem_obj, size); + if (ret) + goto err_obj_init; + + ret = drm_gem_create_mmap_offset(gem_obj); + if (ret) + goto err_create_mmap_offset; + + return cma_obj; + +err_create_mmap_offset: + drm_gem_object_release(gem_obj); + +err_obj_init: + drm_gem_cma_buf_destroy(drm, cma_obj); + +err_dma_alloc: + kfree(cma_obj); + + return ERR_PTR(ret); +} +EXPORT_SYMBOL_GPL(drm_gem_cma_create); + +/* + * drm_gem_cma_create_with_handle - allocate an object with the given + * size and create a gem handle on it + * + * returns a struct drm_gem_cma_object* on success or ERR_PTR values + * on failure. + */ +struct drm_gem_cma_object *drm_gem_cma_create_with_handle( + struct drm_file *file_pri
[PATCH v2] DRM: Add DRM kms/fb cma helper
On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote: > This patch introduces a set of helper function for implementing the KMS > framebuffer layer for drivers which use the drm gem CMA helper function. > > Signed-off-by: Lars-Peter Clausen > > --- > Changes since v1: > * Some spelling fixes > * Add missing kfree in drm_fb_cma_alloc error path > * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second > parameter to specify the plane. > --- > drivers/gpu/drm/Kconfig | 11 + > drivers/gpu/drm/Makefile|1 + > drivers/gpu/drm/drm_fb_cma_helper.c | 384 > +++ > include/drm/drm_fb_cma_helper.h | 27 +++ > 4 files changed, 423 insertions(+) > create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c > create mode 100644 include/drm/drm_fb_cma_helper.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index a78dfbe..e7c0a3d 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER > help > Choose this if you need the GEM cma helper functions > > +config DRM_KMS_CMA_HELPER > + tristate > + select DRM_GEM_CMA_HELPER > + select DRM_KMS_HELPER > + select FB_SYS_FILLRECT > + select FB_SYS_COPYAREA > + select FB_SYS_IMAGEBLIT > + help > + Choose this if you need the KMS cma helper functions > + > + > config DRM_TDFX > tristate "3dfx Banshee/Voodoo3+" > depends on DRM && PCI > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 6e9e948..5dcb1a5 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o > drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o > > drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o > +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c > b/drivers/gpu/drm/drm_fb_cma_helper.c > new file mode 100644 > index 000..8821a98 > --- /dev/null > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -0,0 +1,384 @@ > +/* > + * drm kms/fb cma (contiguous memory allocator) helper functions > + * > + * Copyright (C) 2012 Analog Device Inc. > + * Author: Lars-Peter Clausen > + * > + * Based on udl_fbdev.c > + * Copyright (C) 2012 Red Hat > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +struct drm_fb_cma { > + struct drm_framebuffer fb; > + struct drm_gem_cma_obj *obj[4]; Can we have a define for this magic '4'? It is used several times in this patch. > +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, > + struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj, > + unsigned int num_planes) > +{ > + struct drm_fb_cma *fb_cma; > + int ret; > + int i; > + > + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); > + if (!fb_cma) > + return ERR_PTR(-ENOMEM); > + > + ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); > + if (ret) { > + dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret); > + kfree(fb_cma); > + return ERR_PTR(ret); > + } > + > + drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); > + > + for (i = 0; i < num_planes; i++) > + fb_cma->obj[i] = obj[i]; Check for valid num_planes before this loop? Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object in the patch I just sent. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function
On Thu, May 24, 2012 at 09:26:49PM +0200, Daniel Vetter wrote: > A 30 ms delay is simply way too big to waste cpu cycles on. > > Signed-off-by: Daniel Vetter I've queued this patch here for -next with Chris' irc ack added. -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[PATCH v2] DRM: add drm gem cma helper
Hi Sascha, Thanks for the patch. Here's a bit more extensive review. On Thursday 31 May 2012 10:08:54 Sascha Hauer wrote: > Many embedded drm devices do not have a IOMMU and no dedicated > memory for graphics. These devices use cma (Contiguous Memory > Allocator) backed graphics memory. This patch provides helper > functions to be able to share the code. > > Signed-off-by: Sascha Hauer > --- > > changes since v1: > > - map whole buffer at mmap time, not page by page at fault time > - several cleanups as suggested by Lars-Peter Clausen and Laurent Pinchart > > I currently do a remap_pfn_range after drm_gem_mmap, so we release > dev->struct_mutex in between. I don't know if this is correct or if we have > to hold the lock over the whole process. drm_gem_mmap() takes a reference on the GEM object so I don't think we'll have any issue there, but please don't take my word for it. dev->struct_mutex is the large mutex I still need to document, I don't know what it protects exactly. > drivers/gpu/drm/Kconfig |6 + > drivers/gpu/drm/Makefile |1 + > drivers/gpu/drm/drm_gem_cma_helper.c | 243 +++ > include/drm/drm_gem_cma_helper.h | 52 > 4 files changed, 302 insertions(+) > create mode 100644 drivers/gpu/drm/drm_gem_cma_helper.c > create mode 100644 include/drm/drm_gem_cma_helper.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index e354bc0..f62717e 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -53,6 +53,12 @@ config DRM_TTM > GPU memory types. Will be enabled automatically if a device driver > uses it. > > +config DRM_GEM_CMA_HELPER > + tristate > + depends on DRM > + help > + Choose this if you need the GEM cma helper functions I would put CMA in uppercase, but that's just nitpicking. BTW this helper is not strictly dedicated to CMA. It uses the DMA API to allocate memory, without caring about the underlying allocator. > + > config DRM_TDFX > tristate "3dfx Banshee/Voodoo3+" > depends on DRM && PCI [snip] > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c > b/drivers/gpu/drm/drm_gem_cma_helper.c new file mode 100644 > index 000..d8c0dc7 > --- /dev/null > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -0,0 +1,243 @@ > +/* > + * drm gem cma (contiguous memory allocator) helper functions > + * > + * Copyright (C) 2012 Sascha Hauer, Pengutronix > + * > + * Based on Samsung Exynos code > + * > + * Copyright (c) 2011 Samsung Electronics Co., Ltd. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#include > +#include > +#include > + > +static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) > +{ > + return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; > +} > + > +static void drm_gem_cma_buf_destroy(struct drm_device *drm, > + struct drm_gem_cma_object *cma_obj) > +{ > + dma_free_writecombine(drm->dev, cma_obj->base.size, cma_obj->vaddr, > + cma_obj->paddr); > +} > + > +/* > + * drm_gem_cma_create - allocate an object with the given size > + * > + * returns a struct drm_gem_cma_object* on success or ERR_PTR values > + * on failure. > + */ > +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm, > + unsigned int size) > +{ > + struct drm_gem_cma_object *cma_obj; > + struct drm_gem_object *gem_obj; > + int ret; > + > + size = round_up(size, PAGE_SIZE); > + > + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL); > + if (!cma_obj) > + return ERR_PTR(-ENOMEM); > + > + cma_obj->vaddr = dma_alloc_writecombine(drm->dev, size, > + &cma_obj->paddr, GFP_KERNEL | __GFP_NOWARN); > + if (!cma_obj->vaddr) { > + dev_err(drm->dev, "failed to allocate buffer with size %d\n", > size); > + ret = -ENOMEM; > + goto err_dma_alloc; > + } > + > + gem_obj = &cma_obj->base; > + > + ret = drm_gem_object_init(drm, gem_obj, size); > + if (ret) > + goto err_obj_init; > + > + ret = drm_gem_create_mmap_offset(gem_obj); > + if (ret) > + goto err_create_mmap_offset; > + > + return cma_obj; > + > +err_create_mmap_offset: > + drm_gem_object_release(gem_obj); > + > +err_obj_init: > + drm_gem_cma_buf_destroy(drm, cma_obj); > + > +err_dma_alloc: > + kfree(cma_obj); > + > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(drm_gem_c
[PATCH v2] DRM: Add DRM kms/fb cma helper
On 05/31/2012 10:13 AM, Sascha Hauer wrote: > On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote: >> This patch introduces a set of helper function for implementing the KMS >> framebuffer layer for drivers which use the drm gem CMA helper function. >> >> Signed-off-by: Lars-Peter Clausen >> >> --- >> Changes since v1: >> * Some spelling fixes >> * Add missing kfree in drm_fb_cma_alloc error path >> * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second >>parameter to specify the plane. >> --- >> drivers/gpu/drm/Kconfig | 11 + >> drivers/gpu/drm/Makefile|1 + >> drivers/gpu/drm/drm_fb_cma_helper.c | 384 >> +++ >> include/drm/drm_fb_cma_helper.h | 27 +++ >> 4 files changed, 423 insertions(+) >> create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c >> create mode 100644 include/drm/drm_fb_cma_helper.h >> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> index a78dfbe..e7c0a3d 100644 >> --- a/drivers/gpu/drm/Kconfig >> +++ b/drivers/gpu/drm/Kconfig >> @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER >> help >>Choose this if you need the GEM cma helper functions >> >> +config DRM_KMS_CMA_HELPER >> +tristate >> +select DRM_GEM_CMA_HELPER >> +select DRM_KMS_HELPER >> +select FB_SYS_FILLRECT >> +select FB_SYS_COPYAREA >> +select FB_SYS_IMAGEBLIT >> +help >> + Choose this if you need the KMS cma helper functions >> + >> + >> config DRM_TDFX >> tristate "3dfx Banshee/Voodoo3+" >> depends on DRM && PCI >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >> index 6e9e948..5dcb1a5 100644 >> --- a/drivers/gpu/drm/Makefile >> +++ b/drivers/gpu/drm/Makefile >> @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o >> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o >> >> drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o >> +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o >> >> obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o >> >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c >> b/drivers/gpu/drm/drm_fb_cma_helper.c >> new file mode 100644 >> index 000..8821a98 >> --- /dev/null >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -0,0 +1,384 @@ >> +/* >> + * drm kms/fb cma (contiguous memory allocator) helper functions >> + * >> + * Copyright (C) 2012 Analog Device Inc. >> + * Author: Lars-Peter Clausen >> + * >> + * Based on udl_fbdev.c >> + * Copyright (C) 2012 Red Hat >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * as published by the Free Software Foundation; either version 2 >> + * of the License, or (at your option) any later version. >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +struct drm_fb_cma { >> +struct drm_framebuffer fb; >> +struct drm_gem_cma_obj *obj[4]; > > Can we have a define for this magic '4'? It is used several times in > this patch. Yes, had the same though. The magic 4 is actually used all through all of DRM. Something like '#define DRM_MAX_PLANES 4' probably makes sense. > >> +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, >> +struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj, >> +unsigned int num_planes) >> +{ >> +struct drm_fb_cma *fb_cma; >> +int ret; >> +int i; >> + >> +fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); >> +if (!fb_cma) >> +return ERR_PTR(-ENOMEM); >> + >> +ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); >> +if (ret) { >> +dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret); >> +kfree(fb_cma); >> +return ERR_PTR(ret); >> +} >> + >> +drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); >> + >> +for (i = 0; i < num_planes; i++) >> +fb_cma->obj[i] = obj[i]; > > Check for valid num_planes before this loop? > Hm, I think the callers already take care of this. drm_format_num_planes will always return a valid number and the other caller passes 1 unconditionally. > > Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object > in the patch I just sent. > Do you think it makes sense for you to carry this patch as part of your iMX DRM series? Thanks, - Lars
[PATCH] DRM: add drm gem CMA helper
Hi Sascha, On Wednesday 30 May 2012 18:28:12 Sascha Hauer wrote: > On Wed, May 30, 2012 at 05:40:13PM +0200, Laurent Pinchart wrote: > > Hi Sascha, > > > > Thank you for the patch. I've successfully tested the helper with the new > > SH Mobile DRM driver. Just a couple of comments below in addition to > > Lars' comments (this is not a full review, just details that caught my > > attention when comparing the code with my implementation, and trying to > > use it). > > > +/* > > > + * drm_gem_cma_mmap - (struct file_operation)->mmap callback function > > > + */ > > > +int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma) > > > +{ > > > + int ret; > > > + > > > + ret = drm_gem_mmap(filp, vma); > > > + if (ret) > > > + return ret; > > > + > > > + vma->vm_flags &= ~VM_PFNMAP; > > > + vma->vm_flags |= VM_MIXEDMAP; > > > > Why is this a mixed map ? > > Honestly, I don't know. This is copied from the exynos driver. I don't > know much about these flags, so if you think something else is better > here than you're probably right ;) I wouldn't claim to be an expert on this matter :-) As far as I know, PFNMAP means that the mapping refers to physical memory with no struct page associated with it (that could be I/O memory, DRAM reserved at boot time, ... basically any memory outside of the system memory resources controlled by the kernel page allocator). MIXEDMAP means that the mapping refers to memory that contains both PFNMAP regions and non-PFNMAP regions. I would be surprised if dma_alloc_writecombine() returned such a mix. On the other hand the memory it returns might be PFNMAP or non-PFNMAP depending on the underneath allocator, maybe that's why VM_MIXEDMAP was used. I'd appreciate an expert's opinion on this :-) > > > + > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(drm_gem_cma_mmap); > > > > My implementation maps the whole buffer in one go at mmap time, not page > > by page at page fault time. Isn't that more efficient when mapping frame > > buffer memory ? > > I remember Alan has mentioned this in an earlier review. I'll update the > patch accordingly. > > I will fix this and the other things you mentioned and repost. -- Regards, Laurent Pinchart
[PATCH 4/4] drm: Renesas SH Mobile DRM driver
Hi Sascha, On Wednesday 30 May 2012 18:40:56 Sascha Hauer wrote: > On Wed, May 30, 2012 at 02:32:59PM +0200, Laurent Pinchart wrote: > > The SH Mobile LCD controller (LCDC) DRM driver supports the main > > graphics plane in RGB and YUV formats, as well as the overlay planes (in > > alpha-blending mode only). > > > > Only flat panel outputs using the parallel interface are supported. > > Support for SYS panels, HDMI and DSI is currently not implemented. > > > > Signed-off-by: Laurent Pinchart > > --- > > [...] > > > +int shmob_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) > > +{ > > + struct drm_file *priv = filp->private_data; > > + struct drm_device *dev = priv->minor->dev; > > + struct drm_gem_mm *mm = dev->mm_private; > > + struct shmob_drm_gem_object *sobj; > > + struct drm_local_map *map; > > + struct drm_gem_object *obj; > > + struct drm_hash_item *hash; > > + pgprot_t prot; > > + int ret; > > + > > + if (drm_device_is_unplugged(dev)) > > + return -ENODEV; > > + > > + mutex_lock(&dev->struct_mutex); > > + > > + if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) { > > + ret = -EINVAL; > > + goto out_unlock; > > + } > > + > > + map = drm_hash_entry(hash, struct drm_map_list, hash)->map; > > + if (!map || > > + ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) { > > + ret = -EPERM; > > + goto out_unlock; > > + } > > + > > + /* Check for valid size. */ > > + if (map->size < vma->vm_end - vma->vm_start) { > > + ret = -EINVAL; > > + goto out_unlock; > > + } > > + > > + obj = map->handle; > > + if (!obj->dev->driver->gem_vm_ops) { > > + ret = -EINVAL; > > + goto out_unlock; > > + } > > + > > + sobj = to_shmob_gem_object(obj); > > + prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); > > + ret = remap_pfn_range(vma, vma->vm_start, sobj->dma >> PAGE_SHIFT, > > + vma->vm_end - vma->vm_start, prot); > > + if (ret < 0) > > + goto out_unlock; > > + > > + vma->vm_flags |= VM_RESERVED | VM_IO | VM_PFNMAP | VM_DONTEXPAND; > > + vma->vm_ops = obj->dev->driver->gem_vm_ops; > > + vma->vm_private_data = obj; > > + vma->vm_page_prot = prot; > > + > > + /* Take a ref for this mapping of the object, so that the fault > > +* handler can dereference the mmap offset's pointer to the object. > > +* This reference is cleaned up by the corresponding vm_close > > +* (which should happen whether the vma was created by this call, or > > +* by a vm_open due to mremap or partial unmap or whatever). > > +*/ > > + drm_gem_object_reference(obj); > > + > > + drm_vm_open_locked(dev, vma); > > + > > +out_unlock: > > + mutex_unlock(&dev->struct_mutex); > > + > > + return ret; > > +} > > I just noticed this function is nearly a copy of drm_gem_mmap except for > the the additional remap_pfn_range here. Would it make sense to turn > drm_gem_mmap into a wrapper around a drm_gem_mmap_locked, call this one > from here and add the remap_pfn_range? > > If yes, I would do so in my cma gem helper patch. Seems you've already done so :-) I don't think locking is an issue, but as I mentioned in my review I don't know with 100% certainty what dev->struct_mutex covers. -- Regards, Laurent Pinchart
Lots of i915/drm spew on 3.4
2012/5/31 Chris Wilson : > Before that commit we had no idea that we had run out of property slots. > I think the WARN is genuine, but maybe we should just bump the count set > it to WARN_ONCE and hope the conversion to lists arrives sooner rather > than latter. > -Chris > Chris is right: this is not a regression. Before that patch, no one checked if property creation really worked. I chose not to use WARN_ONCE because we need to increase the variable once for each time you see the message. Assuming this message appears on your log less than 8 times, does this patch fix your problem? diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 73e4560..bac55c2 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -54,7 +54,7 @@ struct drm_mode_object { struct drm_object_properties *properties; }; -#define DRM_OBJECT_MAX_PROPERTY 16 +#define DRM_OBJECT_MAX_PROPERTY 24 struct drm_object_properties { int count; uint32_t ids[DRM_OBJECT_MAX_PROPERTY]; -- Paulo Zanoni
[PATCH] drm: Add downclock quirk for Samsung LTN121AT10-301
On Thu, May 31, 2012 at 3:09 AM, Daniel Vetter wrote: > On Wed, May 30, 2012 at 11:21:32AM -0400, Sean Paul wrote: >> On Wed, May 30, 2012 at 10:16 AM, Adam Jackson wrote: >> > On 5/30/12 8:05 AM, Sean Paul wrote: >> > >> >> Yes, definitely. The reason I can't set it via xrandr (easily) is >> >> because we look for lvds downclock modes (in i915) on the driver init. >> >> Since the driver initializes way before we have a chance to add a new >> >> mode via xrandr, the driver won't have a downclock mode. >> >> >> >> I suppose the other option is to hack the i915 driver to allow a >> >> downclocked mode to be added after it's been initialized. I haven't >> >> looked into this solution, it might be worth investigating. >> > >> > >> > Just so I'm clear, is what you're looking for "I want this pair of timings, >> > with the driver magically switching between them"? ?If all you wanted was >> > the lower clock speed all the time you could just, you know, do that, so I >> > assume that's not what you're after. >> > >> >> Yep, you got it. >> >> > If binding two timings together like that is what you want, then that seems >> > like a pretty reasonable device-specific ioctl at first glance. ?I think >> > the >> > only thing to be careful of would be copying the slower timings into the >> > driver private of the faster, rather than keeping a pointer or copy of the >> > object id, since modes aren't refcounted. >> > >> > How big of a power savings do you see with this? ?Wondering if it's worth >> > trying to make some common tooling for finding downclocked modes, if it's >> > going to be worthwhile on multiple panels. >> > >> >> I saw about 200mW savings with downclocking enabled. It's hard to say >> how many panels this might apply to, some panels which report >> downclocked states still flicker, and others that don't have it in the >> EDID work just fine (like this Samsung panel). >> >> LVDS downclocking is enabled by default in Fedora and ChromiumOS, so >> making this more accessible might be useful for users of these >> distros. > > I think adding lvds connector properties for downclocking would make > sense. I guess we need and enable knob and a frequency value. At boot time > we'd fill these with the detected values, but then userspace would have > full control (i.e. the enable knob should overwrite the i915 module > options). That way downclocking would also be much easier to test (and > maybe we could try to enable it by default eventually). Excellent, thanks to everyone for the feedback! I'll put together a patch when I get some free cycles. Sean > -Daniel > -- > Daniel Vetter > Mail: daniel at ffwll.ch > Mobile: +41 (0)79 365 57 48
[git pull] drm prime mmap/vmap
Hi Linus, as mentioned previously these are the extra bits of drm that relied on the dma-buf pull to work, the first three just stub out the mmap interface, and the next set provide vmap export to i915/radeon/nouveau and vmap import to udl. Dave. The following changes since commit af56e0aa35f3ae2a4c1a6d1000702df1dd78cb76: Merge git://git.kernel.org/pub/scm/linux/kernel/git/sage/ceph-client (2012-05-30 11:17:19 -0700) are available in the git repository at: git://people.freedesktop.org/~airlied/linux drm-prime-vmap for you to fetch changes up to 63bc620b45af8c743ac291c8725933278c712692: radeon: add radeon prime vmap support. (2012-05-31 14:14:01 +0100) Dave Airlie (7): i915: add stub dma-buf mmap callback. nouveau: add stub dma-buf mmap functionality. radeon: add stub dma-buf mmap functionality i915: add dma-buf vmap support for exporting vmapped buffer udl: support vmapping imported dma-bufs nouveau: add vmap support to nouveau prime support radeon: add radeon prime vmap support. drivers/gpu/drm/i915/i915_drv.h |3 ++ drivers/gpu/drm/i915/i915_gem_dmabuf.c | 61 +++ drivers/gpu/drm/nouveau/nouveau_drv.h |3 ++ drivers/gpu/drm/nouveau/nouveau_prime.c | 45 +++ drivers/gpu/drm/radeon/radeon.h |3 ++ drivers/gpu/drm/radeon/radeon_prime.c | 44 ++ drivers/gpu/drm/udl/udl_fb.c| 13 ++- drivers/gpu/drm/udl/udl_gem.c | 25 +++-- 8 files changed, 192 insertions(+), 5 deletions(-)
Lots of i915/drm spew on 3.4
2012/5/30 Dave Jones : > On Wed, May 30, 2012 at 05:58:48PM -0400, Dave Jones wrote: > ?> On Wed, May 30, 2012 at 11:51:54PM +0200, Daniel Vetter wrote: > ?> ?> On Wed, May 30, 2012 at 11:31 PM, Dave Jones > wrote: > ?> ?> > On this hardware: > ?> ?> > > ?> ?> > 00:02.0 VGA compatible controller: Intel Corporation 82945G/GZ > Integrated Graphics Controller (rev 02) > ?> ?> > > ?> ?> > I get this every boot with Linus current tree (up to > af56e0aa35f3ae2a4c1a6d1000702df1dd78cb76) > ?> ?> > ?> ?> Just a quick question, is this a regression? > ?> > ?> seems so, I don't see it on 3.3 > ?> > ?> ?> If so, can you please > ?> ?> attach the output of xrandr --verbose from a noisy and a quite kernel > ?> ?> (otherwise just please attach it from this noisy kernel). > ?> > ?> this machine runs headless, so has no X installed right now, I'll get it > in a while. > > Attached. > Just a little more information: you have a lot of connector properties because for some reason the driver thinks you have TV1, TV2 and TV3. Each TV connector has a lot of properties... With kernel 3.3 you have only TV1 and TV2. Maybe instead of increasing the maximum property count we should try to investigate why there's a new TV connector in the new kernel (and maybe this is also not a bug/regression...). -- Paulo Zanoni
warn_slowpath_common with linux-3.4
Hello all, here are some warning I got after resuming from suspend. It is not really clear to me how I could debug further. Please tell me if I can do something else. I also attached the whole dmesg. Best Regards, [170934.181984] [ cut here ] [170934.181991] WARNING: at drivers/gpu/drm/i915/intel_display.c:963 assert_plane+0x70/0x80() [170934.181992] Hardware name: Latitude E5420 [170934.181994] plane B assertion failure (expected off, current on) [170934.181995] Modules linked in: mac80211_hwsim vboxnetadp(O) vboxnetflt(O) vboxdrv(O) ppp_async crc_ccitt ppp_generic slhc hid_logitech_dj hid_cherry usbhid psmouse cpufreq_stats vfat fat usb_storage bluetooth fuse iwlwifi uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core videodev snd_hda_codec_hdmi mac80211 snd_hda_codec_idt snd_hda_intel snd_hda_codec snd_hwdep ehci_hcd snd_pcm cfg80211 usbcore crc32c_intel aesni_intel snd_page_alloc snd_timer snd aes_x86_64 cryptd usb_common soundcore [last unloaded: psmouse] [170934.182023] Pid: 29929, comm: kworker/u:5 Tainted: GW O 3.4.0ordex+ #3 [170934.182025] Call Trace: [170934.182030] [] warn_slowpath_common+0x7a/0xb0 [170934.182034] [] ? async_schedule+0x20/0x20 [170934.182036] [] warn_slowpath_fmt+0x41/0x50 [170934.182040] [] ? ironlake_crtc_dpms+0x86/0x90 [170934.182042] [] assert_plane+0x70/0x80 [170934.182044] [] intel_crtc_disable+0x6c/0xc0 [170934.182048] [] drm_helper_disable_unused_functions+0x115/0x190 [170934.182050] [] drm_helper_resume_force_mode+0x128/0x160 [170934.182053] [] ? async_schedule+0x20/0x20 [170934.182057] [] i915_drm_thaw+0x111/0x160 [170934.182059] [] i915_resume+0x49/0x80 [170934.182062] [] i915_pm_resume+0x11/0x20 [170934.182066] [] pci_pm_restore+0x79/0xe0 [170934.182068] [] ? pci_pm_suspend_noirq+0x170/0x170 [170934.182072] [] dpm_run_callback.isra.5+0x36/0x70 [170934.182074] [] device_resume+0xa0/0x160 [170934.182076] [] async_resume+0x1c/0x50 [170934.182079] [] async_run_entry_fn+0x79/0x170 [170934.182082] [] process_one_work+0x174/0x450 [170934.182085] [] ? process_one_work+0x116/0x450 [170934.182088] [] worker_thread+0x15f/0x350 [170934.182090] [] ? manage_workers.isra.28+0x220/0x220 [170934.182093] [] kthread+0x8e/0xa0 [170934.182098] [] kernel_thread_helper+0x4/0x10 [170934.182101] [] ? finish_task_switch+0x80/0x110 [170934.182103] [] ? finish_task_switch+0x80/0x110 [170934.182106] [] ? retint_restore_args+0xe/0xe [170934.182109] [] ? __init_kthread_worker+0x70/0x70 [170934.182111] [] ? gs_change+0xb/0xb [170934.182113] ---[ end trace e93713a9d40cd079 ]--- [170934.182114] [ cut here ] [170934.182116] WARNING: at drivers/gpu/drm/i915/intel_display.c:948 assert_pipe+0x86/0x90() [170934.182117] Hardware name: Latitude E5420 [170934.182119] pipe B assertion failure (expected off, current on) [170934.182120] Modules linked in: mac80211_hwsim vboxnetadp(O) vboxnetflt(O) vboxdrv(O) ppp_async crc_ccitt ppp_generic slhc hid_logitech_dj hid_cherry usbhid psmouse cpufreq_stats vfat fat usb_storage bluetooth fuse iwlwifi uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core videodev snd_hda_codec_hdmi mac80211 snd_hda_codec_idt snd_hda_intel snd_hda_codec snd_hwdep ehci_hcd snd_pcm cfg80211 usbcore crc32c_intel aesni_intel snd_page_alloc snd_timer snd aes_x86_64 cryptd usb_common soundcore [last unloaded: psmouse] [170934.182139] Pid: 29929, comm: kworker/u:5 Tainted: GW O 3.4.0ordex+ #3 [170934.182140] Call Trace: [170934.182142] [] warn_slowpath_common+0x7a/0xb0 [170934.182145] [] ? async_schedule+0x20/0x20 [170934.182147] [] warn_slowpath_fmt+0x41/0x50 [170934.182150] [] ? ironlake_crtc_dpms+0x86/0x90 [170934.182152] [] assert_pipe+0x86/0x90 [170934.182154] [] intel_crtc_disable+0x81/0xc0 [170934.182156] [] drm_helper_disable_unused_functions+0x115/0x190 [170934.182159] [] drm_helper_resume_force_mode+0x128/0x160 [170934.182162] [] ? async_schedule+0x20/0x20 [170934.182164] [] i915_drm_thaw+0x111/0x160 [170934.182167] [] i915_resume+0x49/0x80 [170934.182170] [] i915_pm_resume+0x11/0x20 [170934.182172] [] pci_pm_restore+0x79/0xe0 [170934.182174] [] ? pci_pm_suspend_noirq+0x170/0x170 [170934.182177] [] dpm_run_callback.isra.5+0x36/0x70 [170934.182179] [] device_resume+0xa0/0x160 [170934.182181] [] async_resume+0x1c/0x50 [170934.182184] [] async_run_entry_fn+0x79/0x170 [170934.182186] [] process_one_work+0x174/0x450 [170934.182193] [] ? process_one_work+0x116/0x450 [170934.182195] [] worker_thread+0x15f/0x350 [170934.182198] [] ? manage_workers.isra.28+0x220/0x220 [170934.182201] [] kthread+0x8e/0xa0 [170934.182204] [] kernel_thread_helper+0x4/0x10 [170934.182206] [] ? finish_task_switch+0x80/0x110 [170934.182209] [] ? finish_task_switch+0x80/0x110 [170934.182212] [] ? retint_restore_args+0xe/0xe [170934.182215] [] ? __init_kthread_worker+0x70/0x70 [170934.182218] [] ? gs_change+0xb/0xb [170934.182219] ---[ end trace e93713a9d40cd07a ]--- -- Anto
[PATCH 0/2] Miscellaneous mode set and page flip fixes
Hi everybody, Here are two small fixes that disallow format changes in page flip operations, and perform a full mode set instead of a mode set base in the CRTC helper set config handler if the pixel format changed. Laurent Pinchart (2): drm: Don't allow page flip to change pixel format drm: Perform a full mode set when the pixel format changed drivers/gpu/drm/drm_crtc.c|8 drivers/gpu/drm/drm_crtc_helper.c |3 +++ 2 files changed, 11 insertions(+), 0 deletions(-) -- Regards, Laurent Pinchart
[PATCH 1/2] drm: Don't allow page flip to change pixel format
A page flip is not a mode set, changing the frame buffer pixel format doesn't make sense and isn't handled by most drivers anyway. Disallow it. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/drm_crtc.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 92cea9d..0d15b56 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3530,6 +3530,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, goto out; } + if (crtc->fb->pixel_format != fb->pixel_format || + crtc->fb->bits_per_pixel != crtc->fb->bits_per_pixel || + crtc->fb->depth != fb->depth) { + DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n"); + ret = -EINVAL; + goto out; + } + if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) { ret = -ENOMEM; spin_lock_irqsave(&dev->event_lock, flags); -- 1.7.3.4
[PATCH 2/2] drm: Perform a full mode set when the pixel format changed
Test whether the pixel format changes in the mode set handler, and perform a full mode set instead of a mode set base if it does. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/drm_crtc_helper.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 3252e70..0e8cd89 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -609,6 +609,9 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) } else if (set->fb->bits_per_pixel != set->crtc->fb->bits_per_pixel) { mode_changed = true; + } else if (set->fb->pixel_format != + set->crtc->fb->pixel_format) { + mode_changed = true; } else fb_changed = true; } -- 1.7.3.4
[PATCH v2] DRM: Add DRM kms/fb cma helper
On Thu, May 31, 2012 at 11:34:37AM +0200, Lars-Peter Clausen wrote: > >> + drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); > >> + > >> + for (i = 0; i < num_planes; i++) > >> + fb_cma->obj[i] = obj[i]; > > > > Check for valid num_planes before this loop? > > > > Hm, I think the callers already take care of this. drm_format_num_planes will > always return a valid number and the other caller passes 1 unconditionally. As long as the array always is big enough to hold the maximum number of planes I think it's fine. However, if there is some format with 5 planes (don't know if there is, I only know yuv with multiple planes) it's not obvious that this code does not support this. > > > > > Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object > > in the patch I just sent. > > > > Do you think it makes sense for you to carry this patch as part of your iMX > DRM > series? I can carry this, but it could be that Laurent has his driver in an acceptable state earlier. Let's see. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code
? ? ? ?for (i = 0; i < RADEON_NUM_RINGS; i++) >> ? ? ? ? ? ? ? ?rdev->irq.sw_int[i] = true; >> ? ? ? ?radeon_irq_set(rdev); >> + ? ? ? spin_unlock_irqrestore(&rdev->irq.lock, irqflags); >> ? ? ? ?return 0; >> ?} >> >> ?void radeon_driver_irq_uninstall_kms(struct drm_device *dev) >> ?{ >> ? ? ? ?struct radeon_device *rdev = dev->dev_private; >> + ? ? ? unsigned long irqflags; >> ? ? ? ?unsigned i; >> >> ? ? ? ?if (rdev == NULL) { >> ? ? ? ? ? ? ? ?return; >> ? ? ? ?} >> + ? ? ? spin_lock_irqsave(&rdev->irq.lock, irqflags); >> ? ? ? ?/* Disable *all* interrupts */ >> ? ? ? ?for (i = 0; i < RADEON_NUM_RINGS; i++) >> ? ? ? ? ? ? ? ?rdev->irq.sw_int[i] = false; >> @@ -112,6 +122,7 @@ void radeon_driver_irq_uninstall_kms(struct drm_device >> *dev) >> ? ? ? ? ? ? ? ?rdev->irq.afmt[i] = false; >> ? ? ? ?} >> ? ? ? ?radeon_irq_set(rdev); >> + ? ? ? spin_unlock_irqrestore(&rdev->irq.lock, irqflags); >> ?} >> >> ?static bool radeon_msi_ok(struct radeon_device *rdev) >> @@ -168,15 +179,12 @@ static bool radeon_msi_ok(struct radeon_device *rdev) >> >> ?int radeon_irq_kms_init(struct radeon_device *rdev) >> ?{ >> - ? ? ? int i; >> ? ? ? ?int r = 0; >> >> ? ? ? ?INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); >> ? ? ? ?INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); >> >> - ? ? ? spin_lock_init(&rdev->irq.sw_lock); >> - ? ? ? for (i = 0; i < rdev->num_crtc; i++) >> - ? ? ? ? ? ? ? spin_lock_init(&rdev->irq.pflip_lock[i]); >> + ? ? ? spin_lock_init(&rdev->irq.lock); >> ? ? ? ?r = drm_vblank_init(rdev->ddev, rdev->num_crtc); >> ? ? ? ?if (r) { >> ? ? ? ? ? ? ? ?return r; >> @@ -217,25 +225,25 @@ void radeon_irq_kms_sw_irq_get(struct radeon_device >> *rdev, int ring) >> ?{ >> ? ? ? ?unsigned long irqflags; >> >> - ? ? ? spin_lock_irqsave(&rdev->irq.sw_lock, irqflags); >> + ? ? ? spin_lock_irqsave(&rdev->irq.lock, irqflags); >> ? ? ? ?if (rdev->ddev->irq_enabled && (++rdev->irq.sw_refcount[ring] == 1)) { >> ? ? ? ? ? ? ? ?rdev->irq.sw_int[ring] = true; >> ? ? ? ? ? ? ? ?radeon_irq_set(rdev); >> ? ? ? ?} >> - ? ? ? spin_unlock_irqrestore(&rdev->irq.sw_lock, irqflags); >> + ? ? ? spin_unlock_irqrestore(&rdev->irq.lock, irqflags); >> ?} >> >> ?void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev, int ring) >> ?{ >> ? ? ? ?unsigned long irqflags; >> >> - ? ? ? spin_lock_irqsave(&rdev->irq.sw_lock, irqflags); >> + ? ? ? spin_lock_irqsave(&rdev->irq.lock, irqflags); >> ? ? ? ?BUG_ON(rdev->ddev->irq_enabled && rdev->irq.sw_refcount[ring] <= 0); >> ? ? ? ?if (rdev->ddev->irq_enabled && (--rdev->irq.sw_refcount[ring] == 0)) { >> ? ? ? ? ? ? ? ?rdev->irq.sw_int[ring] = false; >> ? ? ? ? ? ? ? ?radeon_irq_set(rdev); >> ? ? ? ?} >> - ? ? ? spin_unlock_irqrestore(&rdev->irq.sw_lock, irqflags); >> + ? ? ? spin_unlock_irqrestore(&rdev->irq.lock, irqflags); >> ?} >> >> ?void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int crtc) >> @@ -245,12 +253,12 @@ void radeon_irq_kms_pflip_irq_get(struct radeon_device >> *rdev, int crtc) >> ? ? ? ?if (crtc < 0 || crtc >= rdev->num_crtc) >> ? ? ? ? ? ? ? ?return; >> >> - ? ? ? spin_lock_irqsave(&rdev->irq.pflip_lock[crtc], irqflags); >> + ? ? ? spin_lock_irqsave(&rdev->irq.lock, irqflags); >> ? ? ? ?if (rdev->ddev->irq_enabled && (++rdev->irq.pflip_refcount[crtc] == >> 1)) { >> ? ? ? ? ? ? ? ?rdev->irq.pflip[crtc] = true; >> ? ? ? ? ? ? ? ?radeon_irq_set(rdev); >> ? ? ? ?} >> - ? ? ? spin_unlock_irqrestore(&rdev->irq.pflip_lock[crtc], irqflags); >> + ? ? ? spin_unlock_irqrestore(&rdev->irq.lock, irqflags); >> ?} >> >> ?void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int crtc) >> @@ -260,12 +268,52 @@ void radeon_irq_kms_pflip_irq_put(struct radeon_device >> *rdev, int crtc) >> ? ? ? ?if (crtc < 0 || crtc >= rdev->num_crtc) >> ? ? ? ? ? ? ? ?return; >> >> - ? ? ? spin_lock_irqsave(&rdev->irq.pflip_lock[crtc], irqflags); >> + ? ? ? spin_lock_irqsave(&rdev->irq.lock, irqflags); >> ? ? ? ?BUG_ON(rdev->ddev->irq_enabled && rdev->irq.pflip_refcount[crtc] <= >> 0); >> ? ? ? ?if (rdev->ddev->irq_enabled && (--rdev->irq.pflip_refcount[crtc] == >> 0)) { >> ? ? ? ? ? ? ? ?rdev->irq.pflip[crtc] = false; >> ? ? ? ? ? ? ? ?radeon_irq_set(rdev); >> ? ? ? ?} >> - ? ? ? spin_unlock_irqrestore(&rdev->irq.pflip_lock[crtc], irqflags); >> + ? ? ? spin_unlock_irqrestore(&rdev->irq.lock, irqflags); >> +} >> + >> +void radeon_irq_kms_enable_afmt(struct radeon_device *rdev, int block) >> +{ >> + ? ? ? unsigned long irqflags; >> + >> + ? ? ? spin_lock_irqsave(&rdev->irq.lock, irqflags); >> + ? ? ? rdev->irq.afmt[block] = true; >> + ? ? ? radeon_irq_set(rdev); >> + ? ? ? spin_unlock_irqrestore(&rdev->irq.lock, irqflags); >> + >> +} >> + >> +void radeon_irq_kms_disable_afmt(struct radeon_device *rdev, int block) >> +{ >> + ? ? ? unsigned long irqflags; >> + >> + ? ? ? spin_lock_irqsave(&rdev->irq.lock, irqflags); >> + ? ? ? rdev->irq.afmt[block] = false; >> + ? ? ? radeon_irq_set(rdev); >> + ? ? ? spin_unlock_irqrestore(&rdev->irq.lock, irqflags); >> ?} >> > > Should probably also add radeon_irq_kms_[en|dis]able_hpd() function > and call replaced the calls to *_irq_set() in the *_hpd_init() and > *_hpd_fini() callbacks for display hotplug. See attached follow on patch. Alex -- next part -- A non-text attachment was scrubbed... Name: 0001-drm-radeon-apply-Murphy-s-law-to-the-kms-hpd-irq-cod.patch Type: text/x-patch Size: 13299 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120531/2386658a/attachment-0001.bin>
[PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code
On Thu, May 31, 2012 at 2:15 PM, Alex Deucher wrote: > On Thu, May 24, 2012 at 11:35 AM, Alex Deucher > wrote: >> On Thu, May 24, 2012 at 3:49 AM, Christian K?nig >> wrote: >>> From: Christian Koenig >>> >>> 1. It is really dangerous to have more than one >>> ? spinlock protecting the same information. >>> >>> 2. radeon_irq_set sometimes wasn't called with lock >>> ? protection, so it can happen that more than one >>> ? CPU would tamper with the irq regs at the same >>> ? time. >>> >>> 3. The pm.gui_idle variable was assuming that the 3D >>> ? engine wasn't becoming idle between testing the >>> ? register and setting the variable. So just remove >>> ? it and test the register directly. >>> >>> Signed-off-by: Christian Koenig >>> --- >>> ?drivers/gpu/drm/radeon/evergreen.c ? ? ?| ? ?1 - >>> ?drivers/gpu/drm/radeon/r100.c ? ? ? ? ? | ? ?1 - >>> ?drivers/gpu/drm/radeon/r600.c ? ? ? ? ? | ? ?1 - >>> ?drivers/gpu/drm/radeon/r600_hdmi.c ? ? ?| ? ?6 +-- >>> ?drivers/gpu/drm/radeon/radeon.h ? ? ? ? | ? 33 +++--- >>> ?drivers/gpu/drm/radeon/radeon_irq_kms.c | ? 72 >>> +-- >>> ?drivers/gpu/drm/radeon/radeon_kms.c ? ? | ? 12 -- >>> ?drivers/gpu/drm/radeon/radeon_pm.c ? ? ?| ? 12 +- >>> ?drivers/gpu/drm/radeon/rs600.c ? ? ? ? ?| ? ?1 - >>> ?drivers/gpu/drm/radeon/si.c ? ? ? ? ? ? | ? ?1 - >>> ?10 files changed, 90 insertions(+), 50 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/radeon/evergreen.c >>> b/drivers/gpu/drm/radeon/evergreen.c >>> index bfcb39e..9e9b3bb 100644 >>> --- a/drivers/gpu/drm/radeon/evergreen.c >>> +++ b/drivers/gpu/drm/radeon/evergreen.c >>> @@ -3254,7 +3254,6 @@ restart_ih: >>> ? ? ? ? ? ? ? ? ? ? ? ?break; >>> ? ? ? ? ? ? ? ?case 233: /* GUI IDLE */ >>> ? ? ? ? ? ? ? ? ? ? ? ?DRM_DEBUG("IH: GUI idle\n"); >>> - ? ? ? ? ? ? ? ? ? ? ? rdev->pm.gui_idle = true; >>> ? ? ? ? ? ? ? ? ? ? ? ?wake_up(&rdev->irq.idle_queue); >>> ? ? ? ? ? ? ? ? ? ? ? ?break; >>> ? ? ? ? ? ? ? ?default: >>> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c >>> index 415b7d8..2587426 100644 >>> --- a/drivers/gpu/drm/radeon/r100.c >>> +++ b/drivers/gpu/drm/radeon/r100.c >>> @@ -782,7 +782,6 @@ int r100_irq_process(struct radeon_device *rdev) >>> ? ? ? ? ? ? ? ?/* gui idle interrupt */ >>> ? ? ? ? ? ? ? ?if (status & RADEON_GUI_IDLE_STAT) { >>> ? ? ? ? ? ? ? ? ? ? ? ?rdev->irq.gui_idle_acked = true; >>> - ? ? ? ? ? ? ? ? ? ? ? rdev->pm.gui_idle = true; >>> ? ? ? ? ? ? ? ? ? ? ? ?wake_up(&rdev->irq.idle_queue); >>> ? ? ? ? ? ? ? ?} >>> ? ? ? ? ? ? ? ?/* Vertical blank interrupts */ >>> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c >>> index eadbb06..90c6639 100644 >>> --- a/drivers/gpu/drm/radeon/r600.c >>> +++ b/drivers/gpu/drm/radeon/r600.c >>> @@ -3542,7 +3542,6 @@ restart_ih: >>> ? ? ? ? ? ? ? ? ? ? ? ?break; >>> ? ? ? ? ? ? ? ?case 233: /* GUI IDLE */ >>> ? ? ? ? ? ? ? ? ? ? ? ?DRM_DEBUG("IH: GUI idle\n"); >>> - ? ? ? ? ? ? ? ? ? ? ? rdev->pm.gui_idle = true; >>> ? ? ? ? ? ? ? ? ? ? ? ?wake_up(&rdev->irq.idle_queue); >>> ? ? ? ? ? ? ? ? ? ? ? ?break; >>> ? ? ? ? ? ? ? ?default: >>> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c >>> b/drivers/gpu/drm/radeon/r600_hdmi.c >>> index 226379e..b76c0f2 100644 >>> --- a/drivers/gpu/drm/radeon/r600_hdmi.c >>> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c >>> @@ -523,8 +523,7 @@ void r600_hdmi_enable(struct drm_encoder *encoder) >>> >>> ? ? ? ?if (rdev->irq.installed) { >>> ? ? ? ? ? ? ? ?/* if irq is available use it */ >>> - ? ? ? ? ? ? ? rdev->irq.afmt[dig->afmt->id] = true; >>> - ? ? ? ? ? ? ? radeon_irq_set(rdev); >>> + ? ? ? ? ? ? ? radeon_irq_kms_enable_afmt(rdev, dig->afmt->id); >>> ? ? ? ?} >>> >>> ? ? ? ?dig->afmt->enabled = true; >>> @@ -560,8 +559,7 @@ void r600_hdmi_disable(struct drm_encoder *encoder) >>> ? ? ? ? ? ? ? ? ?offset, radeon_encoder->encoder_id); >>> >>> ? ? ? ?/* disable irq */ >>> - ? ? ? rdev->irq.afmt[dig->afmt->id] = false; >>> - ? ? ? radeon_irq_set(rdev); >>> + ? ? ? radeon_irq_kms_disable_afmt(rdev, dig->afmt->id); >>> >>> ? ? ? ?/* Older chipsets not handled by AtomBIOS */ >>> ? ? ? ?if (rdev->family >= CHIP_R600 && !ASIC_IS_DCE3(rdev)) { >>> diff --git a/drivers/gpu/drm/radeon/radeon.h >>> b/drivers/gpu/drm/radeon/radeon.h >>> index 8479096..23552b4 100644 >>> --- a/drivers/gpu/drm/radeon/radeon.h >>> +++ b/drivers/gpu/drm/radeon/radeon.h >>> @@ -610,21 +610,20 @@ union radeon_irq_stat_regs { >>> ?#define RADEON_MAX_AFMT_BLOCKS 6 >>> >>> ?struct radeon_irq { >>> - ? ? ? bool ? ? ? ? ? ?installed; >>> - ? ? ? bool ? ? ? ? ? ?sw_int[RADEON_NUM_RINGS]; >>> - ? ? ? bool ? ? ? ? ? ?crtc_vblank_int[RADEON_MAX_CRTCS]; >>> - ? ? ? bool ? ? ? ? ? ?pflip[RADEON_MAX_CRTCS]; >>> - ? ? ? wait_queue_head_t ? ? ? vblank_queue; >>> - ? ? ? bool ? ? ? ? ? ?hpd[RADEON_MAX_HPD_PINS]; >>> - ? ? ? bool ? ? ? ? ? ?gui_idle; >>> - ? ? ? bool ? ? ? ? ? ?gui_idle_acked; >>> - ? ? ? wait_queue_head_t ? ? ? idle_queue; >>> - ? ? ? bool ? ? ? ? ? ?afmt[RADEON_MAX_AFMT_B
[PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function
Is there a reason for the 30 ms delay in the first place? -Satyeshwar -Original Message- From: dri-devel-bounces+satyeshwar.singh=intel.com at lists.freedesktop.org [mailto:dri-devel-bounces+satyeshwar.singh=intel@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Thursday, May 31, 2012 1:29 AM To: Intel Graphics Development; DRI Development Cc: Daniel Vetter Subject: Re: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function On Thu, May 24, 2012 at 09:26:49PM +0200, Daniel Vetter wrote: > A 30 ms delay is simply way too big to waste cpu cycles on. > > Signed-off-by: Daniel Vetter I've queued this patch here for -next with Chris' irc ack added. -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel at lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function
On Thu, May 31, 2012 at 9:24 PM, Singh, Satyeshwar wrote: > Is there a reason for the 30 ms delay in the first place? git blame says there is, it supposedely fixes a bug with tv detection. -Daniel -- Daniel Vetter daniel.vetter at ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
[PATCH 1/2] drm: Don't allow page flip to change pixel format
Does this by extension mean that stride changes should also not be allowed while page flipping? Thanks, Satyeshwar -Original Message- From: dri-devel-bounces+satyeshwar.singh=intel.com at lists.freedesktop.org [mailto:dri-devel-bounces+satyeshwar.singh=intel@lists.freedesktop.org] On Behalf Of Laurent Pinchart Sent: Thursday, May 31, 2012 9:26 AM To: dri-devel at lists.freedesktop.org Subject: [PATCH 1/2] drm: Don't allow page flip to change pixel format A page flip is not a mode set, changing the frame buffer pixel format doesn't make sense and isn't handled by most drivers anyway. Disallow it. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/drm_crtc.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 92cea9d..0d15b56 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3530,6 +3530,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, goto out; } + if (crtc->fb->pixel_format != fb->pixel_format || + crtc->fb->bits_per_pixel != crtc->fb->bits_per_pixel || + crtc->fb->depth != fb->depth) { + DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n"); + ret = -EINVAL; + goto out; + } + if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) { ret = -ENOMEM; spin_lock_irqsave(&dev->event_lock, flags); -- 1.7.3.4 ___ dri-devel mailing list dri-devel at lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: Don't allow page flip to change pixel format
On Thu, May 31, 2012 at 3:44 PM, Singh, Satyeshwar wrote: > Does this by extension mean that stride changes should also not be allowed > while page flipping? Everything beyond a crtc base address change should require a full modeset. Alex > Thanks, > Satyeshwar > > -Original Message- > From: dri-devel-bounces+satyeshwar.singh=intel.com at lists.freedesktop.org > [mailto:dri-devel-bounces+satyeshwar.singh=intel.com at > lists.freedesktop.org] On Behalf Of Laurent Pinchart > Sent: Thursday, May 31, 2012 9:26 AM > To: dri-devel at lists.freedesktop.org > Subject: [PATCH 1/2] drm: Don't allow page flip to change pixel format > > A page flip is not a mode set, changing the frame buffer pixel format doesn't > make sense and isn't handled by most drivers anyway. Disallow it. > > Signed-off-by: Laurent Pinchart > --- > ?drivers/gpu/drm/drm_crtc.c | ? ?8 > ?1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index > 92cea9d..0d15b56 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -3530,6 +3530,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, > ? ? ? ? ? ? ? ?goto out; > ? ? ? ?} > > + ? ? ? if (crtc->fb->pixel_format != fb->pixel_format || > + ? ? ? ? ? crtc->fb->bits_per_pixel != crtc->fb->bits_per_pixel || > + ? ? ? ? ? crtc->fb->depth != fb->depth) { > + ? ? ? ? ? ? ? DRM_DEBUG_KMS("Page flip is not allowed to change frame > buffer format.\n"); > + ? ? ? ? ? ? ? ret = -EINVAL; > + ? ? ? ? ? ? ? goto out; > + ? ? ? } > + > ? ? ? ?if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) { > ? ? ? ? ? ? ? ?ret = -ENOMEM; > ? ? ? ? ? ? ? ?spin_lock_irqsave(&dev->event_lock, flags); > -- > 1.7.3.4 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code
On 31.05.2012 20:15, Alex Deucher wrote: > On Thu, May 24, 2012 at 11:35 AM, Alex Deucher > wrote: >> On Thu, May 24, 2012 at 3:49 AM, Christian K?nig >> wrote: >>> From: Christian Koenig >>> >>> 1. It is really dangerous to have more than one >>>spinlock protecting the same information. >>> >>> 2. radeon_irq_set sometimes wasn't called with lock >>>protection, so it can happen that more than one >>>CPU would tamper with the irq regs at the same >>>time. >>> >>> 3. The pm.gui_idle variable was assuming that the 3D >>>engine wasn't becoming idle between testing the >>>register and setting the variable. So just remove >>>it and test the register directly. >>> >>> Signed-off-by: Christian Koenig >>> --- >>> drivers/gpu/drm/radeon/evergreen.c |1 - >>> drivers/gpu/drm/radeon/r100.c |1 - >>> drivers/gpu/drm/radeon/r600.c |1 - >>> drivers/gpu/drm/radeon/r600_hdmi.c |6 +-- >>> drivers/gpu/drm/radeon/radeon.h | 33 +++--- >>> drivers/gpu/drm/radeon/radeon_irq_kms.c | 72 >>> +-- >>> drivers/gpu/drm/radeon/radeon_kms.c | 12 -- >>> drivers/gpu/drm/radeon/radeon_pm.c | 12 +- >>> drivers/gpu/drm/radeon/rs600.c |1 - >>> drivers/gpu/drm/radeon/si.c |1 - >>> 10 files changed, 90 insertions(+), 50 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/radeon/evergreen.c >>> b/drivers/gpu/drm/radeon/evergreen.c >>> index bfcb39e..9e9b3bb 100644 >>> --- a/drivers/gpu/drm/radeon/evergreen.c >>> +++ b/drivers/gpu/drm/radeon/evergreen.c >>> @@ -3254,7 +3254,6 @@ restart_ih: >>> break; >>> case 233: /* GUI IDLE */ >>> DRM_DEBUG("IH: GUI idle\n"); >>> - rdev->pm.gui_idle = true; >>> wake_up(&rdev->irq.idle_queue); >>> break; >>> default: >>> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100..c >>> index 415b7d8..2587426 100644 >>> --- a/drivers/gpu/drm/radeon/r100.c >>> +++ b/drivers/gpu/drm/radeon/r100.c >>> @@ -782,7 +782,6 @@ int r100_irq_process(struct radeon_device *rdev) >>> /* gui idle interrupt */ >>> if (status& RADEON_GUI_IDLE_STAT) { >>> rdev->irq.gui_idle_acked = true; >>> - rdev->pm.gui_idle = true; >>> wake_up(&rdev->irq.idle_queue); >>> } >>> /* Vertical blank interrupts */ >>> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600..c >>> index eadbb06..90c6639 100644 >>> --- a/drivers/gpu/drm/radeon/r600.c >>> +++ b/drivers/gpu/drm/radeon/r600.c >>> @@ -3542,7 +3542,6 @@ restart_ih: >>> break; >>> case 233: /* GUI IDLE */ >>> DRM_DEBUG("IH: GUI idle\n"); >>> - rdev->pm.gui_idle = true; >>> wake_up(&rdev->irq.idle_queue); >>> break; >>> default: >>> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c >>> b/drivers/gpu/drm/radeon/r600_hdmi.c >>> index 226379e..b76c0f2 100644 >>> --- a/drivers/gpu/drm/radeon/r600_hdmi.c >>> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c >>> @@ -523,8 +523,7 @@ void r600_hdmi_enable(struct drm_encoder *encoder) >>> >>> if (rdev->irq.installed) { >>> /* if irq is available use it */ >>> - rdev->irq.afmt[dig->afmt->id] = true; >>> - radeon_irq_set(rdev); >>> + radeon_irq_kms_enable_afmt(rdev, dig->afmt->id); >>> } >>> >>> dig->afmt->enabled = true; >>> @@ -560,8 +559,7 @@ void r600_hdmi_disable(struct drm_encoder *encoder) >>> offset, radeon_encoder->encoder_id); >>> >>> /* disable irq */ >>> - rdev->irq.afmt[dig->afmt->id] = false; >>> - radeon_irq_set(rdev); >>> + radeon_irq_kms_disable_afmt(rdev, dig->afmt->id); >>> >>> /* Older chipsets not handled by AtomBIOS */ >>> if (rdev->family>= CHIP_R600&& !ASIC_IS_DCE3(rdev)) { >>> diff --git a/drivers/gpu/drm/radeon/radeon.h >>> b/drivers/gpu/drm/radeon/radeon.h >>> index 8479096..23552b4 100644 >>> --- a/drivers/gpu/drm/radeon/radeon.h >>> +++ b/drivers/gpu/drm/radeon/radeon.h >>> @@ -610,21 +610,20 @@ union radeon_irq_stat_regs { >>> #define RADEON_MAX_AFMT_BLOCKS 6 >>> >>> struct radeon_irq { >>> - boolinstalled; >>> - boolsw_int[RADEON_NUM_RINGS]; >>> - boolcrtc_vblank_int[RADEON_MAX_CRTCS]; >>> - boolpflip[RADEON_MAX_CRTCS]; >>> - wait_queue_head_t vblank_queue; >>> - boolhpd[RADEON_MAX_HPD_PINS]; >>> - boolgui_idle; >>> - boolgui_idle_acked; >>> - wait_queue_head_t idle_queue; >>> - bool
[PATCH 02/10] drm/radeon: add infrastructure for advanced ring synchronization
Signed-off-by: Christian K?nig --- drivers/gpu/drm/radeon/radeon.h | 23 ++- drivers/gpu/drm/radeon/radeon_fence.c | 73 + 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 5e259b4..4e232c3 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -257,8 +257,8 @@ struct radeon_fence_driver { uint32_tscratch_reg; uint64_tgpu_addr; volatile uint32_t *cpu_addr; - /* seq is protected by ring emission lock */ - uint64_tseq; + /* sync_seq is protected by ring emission lock */ + uint64_tsync_seq[RADEON_NUM_RINGS]; atomic64_t last_seq; unsigned long last_activity; boolinitialized; @@ -288,6 +288,25 @@ int radeon_fence_wait_any(struct radeon_device *rdev, struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence); void radeon_fence_unref(struct radeon_fence **fence); unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring); +bool radeon_fence_need_sync(struct radeon_fence *fence, int ring); +void radeon_fence_note_sync(struct radeon_fence *fence, int ring); +static inline struct radeon_fence *radeon_fence_later(struct radeon_fence *a, + struct radeon_fence *b) +{ + if (!a) { + return b; + } + + if (!b) { + return a; + } + + if (a->seq > b->seq) { + return a; + } else { + return b; + } +} /* * Tiling registers diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index 401d346..7b55625 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -72,7 +72,7 @@ int radeon_fence_emit(struct radeon_device *rdev, } kref_init(&((*fence)->kref)); (*fence)->rdev = rdev; - (*fence)->seq = ++rdev->fence_drv[ring].seq; + (*fence)->seq = ++rdev->fence_drv[ring].sync_seq[ring]; (*fence)->ring = ring; radeon_fence_ring_emit(rdev, ring, *fence); trace_radeon_fence_emit(rdev->ddev, (*fence)->seq); @@ -449,7 +449,7 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring) * wait. */ seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL; - if (seq >= rdev->fence_drv[ring].seq) { + if (seq >= rdev->fence_drv[ring].sync_seq[ring]) { /* nothing to wait for, last_seq is already the last emited fence */ return -ENOENT; @@ -464,7 +464,7 @@ int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring) * activity can be scheduled so there won't be concurrent access * to seq value. */ - return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].seq, + return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].sync_seq[ring], ring, false, false); } @@ -492,7 +492,8 @@ unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring) * but it's ok to report slightly wrong fence count here. */ radeon_fence_process(rdev, ring); - emitted = rdev->fence_drv[ring].seq - atomic64_read(&rdev->fence_drv[ring].last_seq); + emitted = rdev->fence_drv[ring].sync_seq[ring] + - atomic64_read(&rdev->fence_drv[ring].last_seq); /* to avoid 32bits warp around */ if (emitted > 0x1000) { emitted = 0x1000; @@ -500,6 +501,51 @@ unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring) return (unsigned)emitted; } +bool radeon_fence_need_sync(struct radeon_fence *fence, int dst_ring) +{ + struct radeon_fence_driver *fdrv; + + if (!fence) { + return false; + } + + if (fence->ring == dst_ring) { + return false; + } + + /* we are protected by the ring mutex */ + fdrv = &fence->rdev->fence_drv[dst_ring]; + if (fence->seq <= fdrv->sync_seq[fence->ring]) { + return false; + } + + return true; +} + +void radeon_fence_note_sync(struct radeon_fence *fence, int dst_ring) +{ + struct radeon_fence_driver *dst, *src; + unsigned i; + + if (!fence) { + return; + } + + if (fence->ring == dst_ring) { + return; + } + + /* we are protected by the ring mutex */ + src = &fence->rdev->fence_drv[fence->ring]; + dst = &fence->rdev->fence_drv[dst_ring]; + for (i = 0; i < RADEON_NUM_RINGS; ++i) { + if (i == dst_ring) { + continue;
[PATCH 01/10] drm/radeon: remove radeon_fence_create
It is completely unnecessary to create fences before they are emitted, so remove it and a bunch of checks if fences are emitted or not. Signed-off-by: Christian K?nig --- drivers/gpu/drm/radeon/evergreen.c|2 +- drivers/gpu/drm/radeon/ni.c |2 +- drivers/gpu/drm/radeon/r100.c |4 +- drivers/gpu/drm/radeon/r200.c |4 +- drivers/gpu/drm/radeon/r600.c |4 +- drivers/gpu/drm/radeon/r600_blit_kms.c|6 +-- drivers/gpu/drm/radeon/radeon.h | 11 +++-- drivers/gpu/drm/radeon/radeon_asic.h |8 ++-- drivers/gpu/drm/radeon/radeon_benchmark.c | 10 + drivers/gpu/drm/radeon/radeon_fence.c | 42 ++ drivers/gpu/drm/radeon/radeon_ring.c | 19 + drivers/gpu/drm/radeon/radeon_sa.c|2 +- drivers/gpu/drm/radeon/radeon_test.c | 66 - drivers/gpu/drm/radeon/radeon_ttm.c | 30 + drivers/gpu/drm/radeon/si.c |6 +-- 15 files changed, 86 insertions(+), 130 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 58991af..dd3cea4 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -1366,7 +1366,7 @@ void evergreen_mc_program(struct radeon_device *rdev) */ void evergreen_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { - struct radeon_ring *ring = &rdev->ring[ib->fence->ring]; + struct radeon_ring *ring = &rdev->ring[ib->ring]; /* set to DX10/11 mode */ radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0)); diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index b01c2dd..9d9f5ac 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -1127,7 +1127,7 @@ void cayman_fence_ring_emit(struct radeon_device *rdev, void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { - struct radeon_ring *ring = &rdev->ring[ib->fence->ring]; + struct radeon_ring *ring = &rdev->ring[ib->ring]; /* set to DX10/11 mode */ radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0)); diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c index fb44e7e..415b7d8 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -883,7 +883,7 @@ int r100_copy_blit(struct radeon_device *rdev, uint64_t src_offset, uint64_t dst_offset, unsigned num_gpu_pages, - struct radeon_fence *fence) + struct radeon_fence **fence) { struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; uint32_t cur_pages; @@ -947,7 +947,7 @@ int r100_copy_blit(struct radeon_device *rdev, RADEON_WAIT_HOST_IDLECLEAN | RADEON_WAIT_DMA_GUI_IDLE); if (fence) { - r = radeon_fence_emit(rdev, fence); + r = radeon_fence_emit(rdev, fence, RADEON_RING_TYPE_GFX_INDEX); } radeon_ring_unlock_commit(rdev, ring); return r; diff --git a/drivers/gpu/drm/radeon/r200.c b/drivers/gpu/drm/radeon/r200.c index a26144d..f088925 100644 --- a/drivers/gpu/drm/radeon/r200.c +++ b/drivers/gpu/drm/radeon/r200.c @@ -85,7 +85,7 @@ int r200_copy_dma(struct radeon_device *rdev, uint64_t src_offset, uint64_t dst_offset, unsigned num_gpu_pages, - struct radeon_fence *fence) + struct radeon_fence **fence) { struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; uint32_t size; @@ -120,7 +120,7 @@ int r200_copy_dma(struct radeon_device *rdev, radeon_ring_write(ring, PACKET0(RADEON_WAIT_UNTIL, 0)); radeon_ring_write(ring, RADEON_WAIT_DMA_GUI_IDLE); if (fence) { - r = radeon_fence_emit(rdev, fence); + r = radeon_fence_emit(rdev, fence, RADEON_RING_TYPE_GFX_INDEX); } radeon_ring_unlock_commit(rdev, ring); return r; diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index f388a1d..e5279f9 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2369,7 +2369,7 @@ int r600_copy_blit(struct radeon_device *rdev, uint64_t src_offset, uint64_t dst_offset, unsigned num_gpu_pages, - struct radeon_fence *fence) + struct radeon_fence **fence) { struct radeon_sa_bo *vb = NULL; int r; @@ -2670,7 +2670,7 @@ void r600_fini(struct radeon_device *rdev) */ void r600_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { - struct radeon_ring *ring = &rdev->ring[ib->fence->ring]; + struct radeon_ring *ring = &rdev->ring[ib->ring]; /* FIXME: i
[PATCH 05/10] drm/radeon: remove some unneeded structure members
From: Christian Koenig Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/radeon.h |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 3e83480..618df9a 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -733,9 +733,7 @@ struct r600_ih { struct radeon_bo*ring_obj; volatile uint32_t *ring; unsignedrptr; - unsignedrptr_offs; unsignedwptr; - unsignedwptr_old; unsignedring_size; uint64_tgpu_addr; uint32_tptr_mask; -- 1.7.9.5
[PATCH 03/10] drm/radeon: rework ring syncing code
Move inter ring syncing with semaphores into the existing ring allocations, with that we need to lock the ring mutex only once. Signed-off-by: Christian K?nig --- drivers/gpu/drm/radeon/evergreen_blit_kms.c |3 +- drivers/gpu/drm/radeon/r600.c |5 +- drivers/gpu/drm/radeon/r600_blit_kms.c | 24 +++-- drivers/gpu/drm/radeon/radeon.h |6 +-- drivers/gpu/drm/radeon/radeon_asic.h|5 +- drivers/gpu/drm/radeon/radeon_cs.c | 38 +++--- drivers/gpu/drm/radeon/radeon_ring.c| 30 +-- drivers/gpu/drm/radeon/radeon_semaphore.c | 71 ++- drivers/gpu/drm/radeon/radeon_test.c|6 +-- drivers/gpu/drm/radeon/radeon_ttm.c | 20 10 files changed, 92 insertions(+), 116 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen_blit_kms.c b/drivers/gpu/drm/radeon/evergreen_blit_kms.c index 1e96bd4..e512560 100644 --- a/drivers/gpu/drm/radeon/evergreen_blit_kms.c +++ b/drivers/gpu/drm/radeon/evergreen_blit_kms.c @@ -622,7 +622,8 @@ int evergreen_blit_init(struct radeon_device *rdev) rdev->r600_blit.primitives.draw_auto = draw_auto; rdev->r600_blit.primitives.set_default_state = set_default_state; - rdev->r600_blit.ring_size_common = 55; /* shaders + def state */ + rdev->r600_blit.ring_size_common = 8; /* sync semaphore */ + rdev->r600_blit.ring_size_common += 55; /* shaders + def state */ rdev->r600_blit.ring_size_common += 16; /* fence emit for VB IB */ rdev->r600_blit.ring_size_common += 5; /* done copy */ rdev->r600_blit.ring_size_common += 16; /* fence emit for done copy */ diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index e5279f9..a8d8c44 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2371,15 +2371,16 @@ int r600_copy_blit(struct radeon_device *rdev, unsigned num_gpu_pages, struct radeon_fence **fence) { + struct radeon_semaphore *sem = NULL; struct radeon_sa_bo *vb = NULL; int r; - r = r600_blit_prepare_copy(rdev, num_gpu_pages, &vb); + r = r600_blit_prepare_copy(rdev, num_gpu_pages, fence, &vb, &sem); if (r) { return r; } r600_kms_blit_copy(rdev, src_offset, dst_offset, num_gpu_pages, vb); - r600_blit_done_copy(rdev, fence, vb); + r600_blit_done_copy(rdev, fence, vb, sem); return 0; } diff --git a/drivers/gpu/drm/radeon/r600_blit_kms.c b/drivers/gpu/drm/radeon/r600_blit_kms.c index 02f4eeb..2b8d641 100644 --- a/drivers/gpu/drm/radeon/r600_blit_kms.c +++ b/drivers/gpu/drm/radeon/r600_blit_kms.c @@ -512,7 +512,8 @@ int r600_blit_init(struct radeon_device *rdev) rdev->r600_blit.primitives.draw_auto = draw_auto; rdev->r600_blit.primitives.set_default_state = set_default_state; - rdev->r600_blit.ring_size_common = 40; /* shaders + def state */ + rdev->r600_blit.ring_size_common = 8; /* sync semaphore */ + rdev->r600_blit.ring_size_common += 40; /* shaders + def state */ rdev->r600_blit.ring_size_common += 5; /* done copy */ rdev->r600_blit.ring_size_common += 16; /* fence emit for done copy */ @@ -666,7 +667,8 @@ static unsigned r600_blit_create_rect(unsigned num_gpu_pages, int r600_blit_prepare_copy(struct radeon_device *rdev, unsigned num_gpu_pages, - struct radeon_sa_bo **vb) + struct radeon_fence **fence, struct radeon_sa_bo **vb, + struct radeon_semaphore **sem) { struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; int r; @@ -689,22 +691,37 @@ int r600_blit_prepare_copy(struct radeon_device *rdev, unsigned num_gpu_pages, return r; } + r = radeon_semaphore_create(rdev, sem); + if (r) { + radeon_sa_bo_free(rdev, vb, NULL); + return r; + } + /* calculate number of loops correctly */ ring_size = num_loops * dwords_per_loop; ring_size += rdev->r600_blit.ring_size_common; r = radeon_ring_lock(rdev, ring, ring_size); if (r) { radeon_sa_bo_free(rdev, vb, NULL); + radeon_semaphore_free(rdev, sem, NULL); return r; } + if (radeon_fence_need_sync(*fence, RADEON_RING_TYPE_GFX_INDEX)) { + radeon_semaphore_sync_rings(rdev, *sem, (*fence)->ring, + RADEON_RING_TYPE_GFX_INDEX); + radeon_fence_note_sync(*fence, RADEON_RING_TYPE_GFX_INDEX); + } else { + radeon_semaphore_free(rdev, sem, NULL); + } + rdev->r600_blit.primitives.set_default_state(rdev); rdev->r600_blit.primitives.set_shaders(rdev); return 0; } void r600_blit_done_copy(struct radeon_device *rdev, struct radeon_
[PATCH 06/10] drm/radeon: fix ih ring handling v2
From: Christian Koenig The spinlock was actually there to protect the rptr, but rptr was read outside of the locked area. v2: Keep the spinlock, looks like there is still something in the ih that doesn't like irqs enabled. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/evergreen.c |5 ++--- drivers/gpu/drm/radeon/r600.c |7 ++- drivers/gpu/drm/radeon/radeon.h|1 - drivers/gpu/drm/radeon/si.c|6 ++ 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index dd3cea4..e4c3321 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2950,11 +2950,11 @@ int evergreen_irq_process(struct radeon_device *rdev) if (!rdev->ih.enabled || rdev->shutdown) return IRQ_NONE; + spin_lock_irqsave(&rdev->ih.lock, flags); wptr = evergreen_get_ih_wptr(rdev); rptr = rdev->ih.rptr; DRM_DEBUG("r600_irq_process start: rptr %d, wptr %d\n", rptr, wptr); - spin_lock_irqsave(&rdev->ih.lock, flags); if (rptr == wptr) { spin_unlock_irqrestore(&rdev->ih.lock, flags); return IRQ_NONE; @@ -2966,7 +2966,6 @@ restart_ih: /* display interrupts */ evergreen_irq_ack(rdev); - rdev->ih.wptr = wptr; while (rptr != wptr) { /* wptr/rptr are in bytes! */ ring_index = rptr / 4; @@ -3267,7 +3266,7 @@ restart_ih: } /* make sure wptr hasn't changed while processing */ wptr = evergreen_get_ih_wptr(rdev); - if (wptr != rdev->ih.wptr) + if (wptr != rptr) goto restart_ih; if (queue_hotplug) schedule_work(&rdev->hotplug_work); diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index a8d8c44..e1861ac 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2921,7 +2921,6 @@ void r600_disable_interrupts(struct radeon_device *rdev) WREG32(IH_RB_RPTR, 0); WREG32(IH_RB_WPTR, 0); rdev->ih.enabled = false; - rdev->ih.wptr = 0; rdev->ih.rptr = 0; } @@ -3384,12 +3383,11 @@ int r600_irq_process(struct radeon_device *rdev) if (!rdev->msi_enabled) RREG32(IH_RB_WPTR); + spin_lock_irqsave(&rdev->ih.lock, flags); wptr = r600_get_ih_wptr(rdev); rptr = rdev->ih.rptr; DRM_DEBUG("r600_irq_process start: rptr %d, wptr %d\n", rptr, wptr); - spin_lock_irqsave(&rdev->ih.lock, flags); - if (rptr == wptr) { spin_unlock_irqrestore(&rdev->ih.lock, flags); return IRQ_NONE; @@ -3402,7 +3400,6 @@ restart_ih: /* display interrupts */ r600_irq_ack(rdev); - rdev->ih.wptr = wptr; while (rptr != wptr) { /* wptr/rptr are in bytes! */ ring_index = rptr / 4; @@ -3558,7 +3555,7 @@ restart_ih: } /* make sure wptr hasn't changed while processing */ wptr = r600_get_ih_wptr(rdev); - if (wptr != rdev->ih.wptr) + if (wptr != rptr) goto restart_ih; if (queue_hotplug) schedule_work(&rdev->hotplug_work); diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 618df9a..378d43b 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -733,7 +733,6 @@ struct r600_ih { struct radeon_bo*ring_obj; volatile uint32_t *ring; unsignedrptr; - unsignedwptr; unsignedring_size; uint64_tgpu_addr; uint32_tptr_mask; diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 5ca8ef5..93da01c 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -3095,7 +3095,6 @@ static void si_disable_interrupts(struct radeon_device *rdev) WREG32(IH_RB_RPTR, 0); WREG32(IH_RB_WPTR, 0); rdev->ih.enabled = false; - rdev->ih.wptr = 0; rdev->ih.rptr = 0; } @@ -3518,11 +3517,11 @@ int si_irq_process(struct radeon_device *rdev) if (!rdev->ih.enabled || rdev->shutdown) return IRQ_NONE; + spin_lock_irqsave(&rdev->ih.lock, flags); wptr = si_get_ih_wptr(rdev); rptr = rdev->ih.rptr; DRM_DEBUG("si_irq_process start: rptr %d, wptr %d\n", rptr, wptr); - spin_lock_irqsave(&rdev->ih.lock, flags); if (rptr == wptr) { spin_unlock_irqrestore(&rdev->ih.lock, flags); return IRQ_NONE; @@ -3534,7 +3533,6 @@ restart_ih: /* display interrupts */ si_irq_ack(rdev); - rdev->ih.wptr = wptr; while (rptr != wptr) { /* wptr/rptr are in bytes! */ ring_index = rptr / 4; @@ -3787,7 +3785,7 @
[PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code v2
From: Christian Koenig 1. It is really dangerous to have more than one spinlock protecting the same information. 2. radeon_irq_set sometimes wasn't called with lock protection, so it can happen that more than one CPU would tamper with the irq regs at the same time. 3. The pm.gui_idle variable was assuming that the 3D engine wasn't becoming idle between testing the register and setting the variable. So just remove it and test the register directly. v2: Also handle the hpd irq code the same way. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/evergreen.c | 21 ++- drivers/gpu/drm/radeon/r100.c | 29 ++ drivers/gpu/drm/radeon/r600.c | 38 drivers/gpu/drm/radeon/r600_hdmi.c |6 +- drivers/gpu/drm/radeon/radeon.h | 35 +-- drivers/gpu/drm/radeon/radeon_irq_kms.c | 96 +++ drivers/gpu/drm/radeon/radeon_kms.c | 12 +++- drivers/gpu/drm/radeon/radeon_pm.c | 12 +--- drivers/gpu/drm/radeon/rs600.c | 13 ++--- drivers/gpu/drm/radeon/si.c |1 - 10 files changed, 144 insertions(+), 119 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index e4c3321..48ec1a0 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -428,6 +428,7 @@ void evergreen_hpd_init(struct radeon_device *rdev) { struct drm_device *dev = rdev->ddev; struct drm_connector *connector; + unsigned enabled = 0; u32 tmp = DC_HPDx_CONNECTION_TIMER(0x9c4) | DC_HPDx_RX_INT_TIMER(0xfa) | DC_HPDx_EN; @@ -436,73 +437,64 @@ void evergreen_hpd_init(struct radeon_device *rdev) switch (radeon_connector->hpd.hpd) { case RADEON_HPD_1: WREG32(DC_HPD1_CONTROL, tmp); - rdev->irq.hpd[0] = true; break; case RADEON_HPD_2: WREG32(DC_HPD2_CONTROL, tmp); - rdev->irq.hpd[1] = true; break; case RADEON_HPD_3: WREG32(DC_HPD3_CONTROL, tmp); - rdev->irq.hpd[2] = true; break; case RADEON_HPD_4: WREG32(DC_HPD4_CONTROL, tmp); - rdev->irq.hpd[3] = true; break; case RADEON_HPD_5: WREG32(DC_HPD5_CONTROL, tmp); - rdev->irq.hpd[4] = true; break; case RADEON_HPD_6: WREG32(DC_HPD6_CONTROL, tmp); - rdev->irq.hpd[5] = true; break; default: break; } radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); + enabled |= 1 << radeon_connector->hpd.hpd; } - if (rdev->irq.installed) - evergreen_irq_set(rdev); + radeon_irq_kms_enable_hpd(rdev, enabled); } void evergreen_hpd_fini(struct radeon_device *rdev) { struct drm_device *dev = rdev->ddev; struct drm_connector *connector; + unsigned disabled = 0; list_for_each_entry(connector, &dev->mode_config.connector_list, head) { struct radeon_connector *radeon_connector = to_radeon_connector(connector); switch (radeon_connector->hpd.hpd) { case RADEON_HPD_1: WREG32(DC_HPD1_CONTROL, 0); - rdev->irq.hpd[0] = false; break; case RADEON_HPD_2: WREG32(DC_HPD2_CONTROL, 0); - rdev->irq.hpd[1] = false; break; case RADEON_HPD_3: WREG32(DC_HPD3_CONTROL, 0); - rdev->irq.hpd[2] = false; break; case RADEON_HPD_4: WREG32(DC_HPD4_CONTROL, 0); - rdev->irq.hpd[3] = false; break; case RADEON_HPD_5: WREG32(DC_HPD5_CONTROL, 0); - rdev->irq.hpd[4] = false; break; case RADEON_HPD_6: WREG32(DC_HPD6_CONTROL, 0); - rdev->irq.hpd[5] = false; break; default: break; } + disabled |= 1 << radeon_connector->hpd.hpd; } + radeon_irq_kms_disable_hpd(rdev, disabled); } /* watermark setup */ @@ -3252,7 +3244,6 @@ restart_ih: break; case 233: /* GUI IDLE */ DRM_DEBUG("IH: GUI idle\n"); - rde
[PATCH 08/10] drm/radeon: replace pflip and sw_int counters with atomics
From: Christian Koenig So we can skip the looking. Also renames sw_int to ring_int, cause that better matches its purpose. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/evergreen.c | 32 - drivers/gpu/drm/radeon/r100.c | 10 +++--- drivers/gpu/drm/radeon/r600.c | 10 +++--- drivers/gpu/drm/radeon/radeon.h |6 ++-- drivers/gpu/drm/radeon/radeon_irq_kms.c | 59 +++ drivers/gpu/drm/radeon/rs600.c | 10 +++--- drivers/gpu/drm/radeon/si.c | 30 7 files changed, 76 insertions(+), 81 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 48ec1a0..3d3520a 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2607,20 +2607,20 @@ int evergreen_irq_set(struct radeon_device *rdev) if (rdev->family >= CHIP_CAYMAN) { /* enable CP interrupts on all rings */ - if (rdev->irq.sw_int[RADEON_RING_TYPE_GFX_INDEX]) { + if (atomic_read(&rdev->irq.ring_int[RADEON_RING_TYPE_GFX_INDEX])) { DRM_DEBUG("evergreen_irq_set: sw int gfx\n"); cp_int_cntl |= TIME_STAMP_INT_ENABLE; } - if (rdev->irq.sw_int[CAYMAN_RING_TYPE_CP1_INDEX]) { + if (atomic_read(&rdev->irq.ring_int[CAYMAN_RING_TYPE_CP1_INDEX])) { DRM_DEBUG("evergreen_irq_set: sw int cp1\n"); cp_int_cntl1 |= TIME_STAMP_INT_ENABLE; } - if (rdev->irq.sw_int[CAYMAN_RING_TYPE_CP2_INDEX]) { + if (atomic_read(&rdev->irq.ring_int[CAYMAN_RING_TYPE_CP2_INDEX])) { DRM_DEBUG("evergreen_irq_set: sw int cp2\n"); cp_int_cntl2 |= TIME_STAMP_INT_ENABLE; } } else { - if (rdev->irq.sw_int[RADEON_RING_TYPE_GFX_INDEX]) { + if (atomic_read(&rdev->irq.ring_int[RADEON_RING_TYPE_GFX_INDEX])) { DRM_DEBUG("evergreen_irq_set: sw int gfx\n"); cp_int_cntl |= RB_INT_ENABLE; cp_int_cntl |= TIME_STAMP_INT_ENABLE; @@ -2628,32 +2628,32 @@ int evergreen_irq_set(struct radeon_device *rdev) } if (rdev->irq.crtc_vblank_int[0] || - rdev->irq.pflip[0]) { + atomic_read(&rdev->irq.pflip[0])) { DRM_DEBUG("evergreen_irq_set: vblank 0\n"); crtc1 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[1] || - rdev->irq.pflip[1]) { + atomic_read(&rdev->irq.pflip[1])) { DRM_DEBUG("evergreen_irq_set: vblank 1\n"); crtc2 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[2] || - rdev->irq.pflip[2]) { + atomic_read(&rdev->irq.pflip[2])) { DRM_DEBUG("evergreen_irq_set: vblank 2\n"); crtc3 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[3] || - rdev->irq.pflip[3]) { + atomic_read(&rdev->irq.pflip[3])) { DRM_DEBUG("evergreen_irq_set: vblank 3\n"); crtc4 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[4] || - rdev->irq.pflip[4]) { + atomic_read(&rdev->irq.pflip[4])) { DRM_DEBUG("evergreen_irq_set: vblank 4\n"); crtc5 |= VBLANK_INT_MASK; } if (rdev->irq.crtc_vblank_int[5] || - rdev->irq.pflip[5]) { + atomic_read(&rdev->irq.pflip[5])) { DRM_DEBUG("evergreen_irq_set: vblank 5\n"); crtc6 |= VBLANK_INT_MASK; } @@ -2974,7 +2974,7 @@ restart_ih: rdev->pm.vblank_sync = true; wake_up(&rdev->irq.vblank_queue); } - if (rdev->irq.pflip[0]) + if (atomic_read(&rdev->irq.pflip[0])) radeon_crtc_handle_flip(rdev, 0); rdev->irq.stat_regs.evergreen.disp_int &= ~LB_D1_VBLANK_INTERRUPT; DRM_DEBUG("IH: D1 vblank\n"); @@ -3000,7 +3000,7 @@ restart_ih: rdev->pm.vblank_sync = true; wake_up(&rdev->irq.vblank_queue); } - if (rdev->irq.pflip[1]) + if (atomic_read(&rdev->irq.pflip[1])) radeon_crtc_handle_flip(rdev, 1); rdev->irq.stat_regs.evergreen.disp_int_cont &= ~LB_D2_VBLANK_
[PATCH 09/10] drm/radeon: replace cs_mutex with vm_mutex
Try to remove or replace the cs_mutex with a vm_mutex where it is still needed. Signed-off-by: Christian K?nig --- drivers/gpu/drm/radeon/radeon.h| 44 +--- drivers/gpu/drm/radeon/radeon_cs.c |7 ++--- drivers/gpu/drm/radeon/radeon_device.c |2 +- drivers/gpu/drm/radeon/radeon_gart.c | 16 ++-- drivers/gpu/drm/radeon/radeon_gem.c|2 -- 5 files changed, 12 insertions(+), 59 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 19e7554..b24877f 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -159,48 +159,6 @@ static inline int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len) #endif bool radeon_get_bios(struct radeon_device *rdev); - -/* - * Mutex which allows recursive locking from the same process. - */ -struct radeon_mutex { - struct mutexmutex; - struct task_struct *owner; - int level; -}; - -static inline void radeon_mutex_init(struct radeon_mutex *mutex) -{ - mutex_init(&mutex->mutex); - mutex->owner = NULL; - mutex->level = 0; -} - -static inline void radeon_mutex_lock(struct radeon_mutex *mutex) -{ - if (mutex_trylock(&mutex->mutex)) { - /* The mutex was unlocked before, so it's ours now */ - mutex->owner = current; - } else if (mutex->owner != current) { - /* Another process locked the mutex, take it */ - mutex_lock(&mutex->mutex); - mutex->owner = current; - } - /* Otherwise the mutex was already locked by this process */ - - mutex->level++; -} - -static inline void radeon_mutex_unlock(struct radeon_mutex *mutex) -{ - if (--mutex->level > 0) - return; - - mutex->owner = NULL; - mutex_unlock(&mutex->mutex); -} - - /* * Dummy page */ @@ -1529,7 +1487,6 @@ struct radeon_device { struct radeon_gem gem; struct radeon_pmpm; uint32_tbios_scratch[RADEON_BIOS_NUM_SCRATCH]; - struct radeon_mutex cs_mutex; struct radeon_wbwb; struct radeon_dummy_pagedummy_page; boolshutdown; @@ -1563,6 +1520,7 @@ struct radeon_device { struct radeon_debugfs debugfs[RADEON_DEBUGFS_MAX_COMPONENTS]; unsigneddebugfs_count; /* virtual memory */ + struct mutexvm_mutex; struct radeon_vm_managervm_manager; }; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index d295821..d1ead9c 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -438,6 +438,7 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, } mutex_lock(&vm->mutex); + mutex_lock(&rdev->vm_mutex); r = radeon_vm_bind(rdev, vm); if (r) { goto out; @@ -474,6 +475,7 @@ out: } vm->fence = radeon_fence_ref(parser->ib.fence); } + mutex_unlock(&rdev->vm_mutex); mutex_unlock(&fpriv->vm.mutex); return r; } @@ -494,9 +496,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct radeon_cs_parser parser; int r; - radeon_mutex_lock(&rdev->cs_mutex); if (!rdev->accel_working) { - radeon_mutex_unlock(&rdev->cs_mutex); return -EBUSY; } /* initialize parser */ @@ -510,7 +510,6 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) DRM_ERROR("Failed to initialize parser !\n"); radeon_cs_parser_fini(&parser, r); r = radeon_cs_handle_lockup(rdev, r); - radeon_mutex_unlock(&rdev->cs_mutex); return r; } r = radeon_cs_parser_relocs(&parser); @@ -519,7 +518,6 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) DRM_ERROR("Failed to parse relocation %d!\n", r); radeon_cs_parser_fini(&parser, r); r = radeon_cs_handle_lockup(rdev, r); - radeon_mutex_unlock(&rdev->cs_mutex); return r; } r = radeon_cs_ib_chunk(rdev, &parser); @@ -533,7 +531,6 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) out: radeon_cs_parser_fini(&parser, r); r = radeon_cs_handle_lockup(rdev, r); - radeon_mutex_unlock(&rdev->cs_mutex); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 7667184..57143dd 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -728,7 +728,6 @@ int r
[PATCH 10/10] drm/radeon: work around bugs in caymans compute rings
From: Christian Koenig The shader preemption on cayman doesn't work correctly with multiple rings. So to be able to still make use of the compute rings we use a semaphore to make sure that only one IB can execute at the same time. This isn't as effective as shader preemption, but also isn't as bad as putting everything on the GFX ring. Signed-off-by: Christian Koenig --- drivers/gpu/drm/radeon/ni.c| 142 ++-- drivers/gpu/drm/radeon/radeon.h|2 + drivers/gpu/drm/radeon/radeon_cs.c |2 +- 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index 9d9f5ac..6a3e8a8 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -1125,13 +1125,75 @@ void cayman_fence_ring_emit(struct radeon_device *rdev, radeon_ring_write(ring, 0); } +/* The shader preemption on cayman doesn't work + * correctly with multiple rings. So to be able to + * still make use of the compute rings we use a + * semaphore to make sure that only one IB can execute + * at the same time + */ +static void cayman_cp_ring_create_workaround(struct radeon_device *rdev) +{ + struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; + int r; + + r = radeon_semaphore_create(rdev, &rdev->cayman_ring_lock); + if (r) { + dev_err(rdev->dev, "Can't allocate " + "cayman_ring_lock (%d)!\n", r); + return; + } + + r = radeon_ring_alloc(rdev, ring, 8); + if (r) { + dev_err(rdev->dev, "Can't initialize " + "cayman_ring_lock (%d)!\n", r); + radeon_semaphore_free(rdev, &rdev->cayman_ring_lock, NULL); + return; + } + + radeon_semaphore_emit_signal(rdev, RADEON_RING_TYPE_GFX_INDEX, +rdev->cayman_ring_lock); + + radeon_ring_commit(rdev, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]); +} + +static void cayman_cp_ring_cleanup_workaround(struct radeon_device *rdev) +{ + struct radeon_fence *fence; + int r; + + r = radeon_fence_emit(rdev, &fence, RADEON_RING_TYPE_GFX_INDEX); + if (r) { + dev_err(rdev->dev, "Can't cleanup " + "cayman_ring_lock (%d)!\n", r); + return; + } + + radeon_semaphore_free(rdev, &rdev->cayman_ring_lock, fence); + radeon_fence_unref(&fence); +} + void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) { struct radeon_ring *ring = &rdev->ring[ib->ring]; + if (ib->ring != RADEON_RING_TYPE_GFX_INDEX) { + if (rdev->cayman_ring_lock == NULL) { + cayman_cp_ring_create_workaround(rdev); + } + } else { + if (rdev->cayman_ring_lock != NULL && + !radeon_fence_count_emitted(rdev, CAYMAN_RING_TYPE_CP1_INDEX) && + !radeon_fence_count_emitted(rdev, CAYMAN_RING_TYPE_CP2_INDEX)) { + cayman_cp_ring_cleanup_workaround(rdev); + } + } + /* set to DX10/11 mode */ radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0)); radeon_ring_write(ring, 1); + if (rdev->cayman_ring_lock) + radeon_semaphore_emit_wait(rdev, ib->ring, rdev->cayman_ring_lock); radeon_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2)); radeon_ring_write(ring, #ifdef __BIG_ENDIAN @@ -1140,6 +1202,8 @@ void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib) (ib->gpu_addr & 0xFFFC)); radeon_ring_write(ring, upper_32_bits(ib->gpu_addr) & 0xFF); radeon_ring_write(ring, ib->length_dw | (ib->vm_id << 24)); + if (rdev->cayman_ring_lock) + radeon_semaphore_emit_signal(rdev, ib->ring, rdev->cayman_ring_lock); /* flush read cache over gart for this vmid */ radeon_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1)); @@ -1190,6 +1254,25 @@ static int cayman_cp_load_microcode(struct radeon_device *rdev) return 0; } +static int cayman_cp_start_compute(struct radeon_device *rdev, int ridx) +{ + struct radeon_ring *ring = &rdev->ring[ridx]; + int r; + + r = radeon_ring_lock(rdev, ring, 2); + if (r) { + DRM_ERROR("radeon: cp failed to lock ring (%d).\n", r); + return r; + } + + /* clear the compute context state */ + radeon_ring_write(ring, PACKET3(PACKET3_CLEAR_STATE, 0) | 2); + radeon_ring_write(ring, 0); + + radeon_ring_unlock_commit(rdev, ring); + return 0; +} + static int cayman_cp_start(struct radeon_device *rdev) { struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]; @@ -1251,7 +1334,17 @@ static int cayman_cp_start(struct radeon_device *rdev) radeon_ring_unlock_commit(rdev,
[PATCH 04/10] drm/radeon: replace vmram_mutex with mclk_lock v2
It is a rw_semaphore now and only write locked while changing the clock. Also the lock is renamed to better reflect what it is protecting. v2: Keep the ttm_vm_ops on IGPs Signed-off-by: Christian K?nig --- drivers/gpu/drm/radeon/radeon.h|3 ++- drivers/gpu/drm/radeon/radeon_device.c |2 +- drivers/gpu/drm/radeon/radeon_object.c |8 drivers/gpu/drm/radeon/radeon_pm.c |4 ++-- drivers/gpu/drm/radeon/radeon_ttm.c|4 ++-- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index aebaf28..3e83480 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1055,6 +1055,8 @@ struct radeon_power_state { struct radeon_pm { struct mutexmutex; + /* write locked while reprogramming mclk */ + struct rw_semaphore mclk_lock; u32 active_crtcs; int active_crtc_count; int req_vblank; @@ -1551,7 +1553,6 @@ struct radeon_device { struct work_struct audio_work; int num_crtc; /* number of crtcs */ struct mutex dc_hw_i2c_mutex; /* display controller hw i2c mutex */ - struct mutex vram_mutex; bool audio_enabled; struct r600_audio audio_status; /* audio stuff */ struct notifier_block acpi_nb; diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 066c98b..7667184 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -735,7 +735,7 @@ int radeon_device_init(struct radeon_device *rdev, spin_lock_init(&rdev->ih.lock); mutex_init(&rdev->gem.mutex); mutex_init(&rdev->pm.mutex); - mutex_init(&rdev->vram_mutex); + init_rwsem(&rdev->pm.mclk_lock); init_waitqueue_head(&rdev->irq.vblank_queue); init_waitqueue_head(&rdev->irq.idle_queue); r = radeon_gem_init(rdev); diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 830f1a7..6ecb200 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -154,11 +154,11 @@ retry: INIT_LIST_HEAD(&bo->va); radeon_ttm_placement_from_domain(bo, domain); /* Kernel allocation are uninterruptible */ - mutex_lock(&rdev->vram_mutex); + down_read(&rdev->pm.mclk_lock); r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type, &bo->placement, page_align, 0, !kernel, NULL, acc_size, sg, &radeon_ttm_bo_destroy); - mutex_unlock(&rdev->vram_mutex); + up_read(&rdev->pm.mclk_lock); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) { if (domain == RADEON_GEM_DOMAIN_VRAM) { @@ -219,9 +219,9 @@ void radeon_bo_unref(struct radeon_bo **bo) return; rdev = (*bo)->rdev; tbo = &((*bo)->tbo); - mutex_lock(&rdev->vram_mutex); + down_read(&rdev->pm.mclk_lock); ttm_bo_unref(&tbo); - mutex_unlock(&rdev->vram_mutex); + up_read(&rdev->pm.mclk_lock); if (tbo == NULL) *bo = NULL; } diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 0882554..d13b6ae 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -251,7 +251,7 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev) return; mutex_lock(&rdev->ddev->struct_mutex); - mutex_lock(&rdev->vram_mutex); + down_write(&rdev->pm.mclk_lock); mutex_lock(&rdev->ring_lock); /* gui idle int has issues on older chips it seems */ @@ -303,7 +303,7 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev) rdev->pm.dynpm_planned_action = DYNPM_ACTION_NONE; mutex_unlock(&rdev->ring_lock); - mutex_unlock(&rdev->vram_mutex); + up_write(&rdev->pm.mclk_lock); mutex_unlock(&rdev->ddev->struct_mutex); } diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index c43035c..0881131 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -797,9 +797,9 @@ static int radeon_ttm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) return VM_FAULT_NOPAGE; } rdev = radeon_get_rdev(bo->bdev); - mutex_lock(&rdev->vram_mutex); + down_read(&rdev->pm.mclk_lock); r = ttm_vm_ops->fault(vma, vmf); - mutex_unlock(&rdev->vram_mutex); + up_read(&rdev->pm.mclk_lock); return r; } -- 1.7.9.5
[PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code v2
On Thu, May 31, 2012 at 4:16 PM, Christian K?nig wrote: > From: Christian Koenig > > 1. It is really dangerous to have more than one > ? spinlock protecting the same information. > > 2. radeon_irq_set sometimes wasn't called with lock > ? protection, so it can happen that more than one > ? CPU would tamper with the irq regs at the same > ? time. > > 3. The pm.gui_idle variable was assuming that the 3D > ? engine wasn't becoming idle between testing the > ? register and setting the variable. So just remove > ? it and test the register directly. > > v2: Also handle the hpd irq code the same way. couple of comments below for clarity, other than that: Reviewed-by: Alex Deucher > > Signed-off-by: Christian Koenig > --- > ?drivers/gpu/drm/radeon/evergreen.c ? ? ?| ? 21 ++- > ?drivers/gpu/drm/radeon/r100.c ? ? ? ? ? | ? 29 ++ > ?drivers/gpu/drm/radeon/r600.c ? ? ? ? ? | ? 38 > ?drivers/gpu/drm/radeon/r600_hdmi.c ? ? ?| ? ?6 +- > ?drivers/gpu/drm/radeon/radeon.h ? ? ? ? | ? 35 +-- > ?drivers/gpu/drm/radeon/radeon_irq_kms.c | ? 96 > +++ > ?drivers/gpu/drm/radeon/radeon_kms.c ? ? | ? 12 +++- > ?drivers/gpu/drm/radeon/radeon_pm.c ? ? ?| ? 12 +--- > ?drivers/gpu/drm/radeon/rs600.c ? ? ? ? ?| ? 13 ++--- > ?drivers/gpu/drm/radeon/si.c ? ? ? ? ? ? | ? ?1 - > ?10 files changed, 144 insertions(+), 119 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/evergreen.c > b/drivers/gpu/drm/radeon/evergreen.c > index e4c3321..48ec1a0 100644 > --- a/drivers/gpu/drm/radeon/evergreen.c > +++ b/drivers/gpu/drm/radeon/evergreen.c > @@ -428,6 +428,7 @@ void evergreen_hpd_init(struct radeon_device *rdev) > ?{ > ? ? ? ?struct drm_device *dev = rdev->ddev; > ? ? ? ?struct drm_connector *connector; > + ? ? ? unsigned enabled = 0; > ? ? ? ?u32 tmp = DC_HPDx_CONNECTION_TIMER(0x9c4) | > ? ? ? ? ? ? ? ?DC_HPDx_RX_INT_TIMER(0xfa) | DC_HPDx_EN; > > @@ -436,73 +437,64 @@ void evergreen_hpd_init(struct radeon_device *rdev) > ? ? ? ? ? ? ? ?switch (radeon_connector->hpd.hpd) { > ? ? ? ? ? ? ? ?case RADEON_HPD_1: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD1_CONTROL, tmp); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[0] = true; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_2: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD2_CONTROL, tmp); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[1] = true; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_3: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD3_CONTROL, tmp); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[2] = true; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_4: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD4_CONTROL, tmp); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[3] = true; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_5: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD5_CONTROL, tmp); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[4] = true; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_6: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD6_CONTROL, tmp); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[5] = true; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?default: > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?} > ? ? ? ? ? ? ? ?radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); > + ? ? ? ? ? ? ? enabled |= 1 << radeon_connector->hpd.hpd; > ? ? ? ?} > - ? ? ? if (rdev->irq.installed) > - ? ? ? ? ? ? ? evergreen_irq_set(rdev); > + ? ? ? radeon_irq_kms_enable_hpd(rdev, enabled); > ?} > > ?void evergreen_hpd_fini(struct radeon_device *rdev) > ?{ > ? ? ? ?struct drm_device *dev = rdev->ddev; > ? ? ? ?struct drm_connector *connector; > + ? ? ? unsigned disabled = 0; > > ? ? ? ?list_for_each_entry(connector, &dev->mode_config.connector_list, head) > { > ? ? ? ? ? ? ? ?struct radeon_connector *radeon_connector = > to_radeon_connector(connector); > ? ? ? ? ? ? ? ?switch (radeon_connector->hpd.hpd) { > ? ? ? ? ? ? ? ?case RADEON_HPD_1: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD1_CONTROL, 0); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[0] = false; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_2: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD2_CONTROL, 0); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[1] = false; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_3: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD3_CONTROL, 0); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[2] = false; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_4: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD4_CONTROL, 0); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[3] = false; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_5: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD5_CONTROL, 0); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[4] = false; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?case RADEON_HPD_6: > ? ? ? ? ? ? ? ? ? ? ? ?WREG32(DC_HPD6_CONTROL, 0); > - ? ? ? ? ? ? ? ? ? ? ? rdev->irq.hpd[5] = false; > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?default: > ? ? ? ? ? ? ? ? ? ? ? ?break; > ? ? ? ? ? ? ? ?} > + ? ? ? ? ? ? ? dis
[PATCH 02/10] drm/radeon: add infrastructure for advanced ring synchronization
On Thu, May 31, 2012 at 4:15 PM, Christian K?nig wrote: > Signed-off-by: Christian K?nig > --- > ?drivers/gpu/drm/radeon/radeon.h ? ? ? | ? 23 ++- > ?drivers/gpu/drm/radeon/radeon_fence.c | ? 73 > + > ?2 files changed, 85 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index 5e259b4..4e232c3 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -257,8 +257,8 @@ struct radeon_fence_driver { > ? ? ? ?uint32_t ? ? ? ? ? ? ? ? ? ? ? ?scratch_reg; > ? ? ? ?uint64_t ? ? ? ? ? ? ? ? ? ? ? ?gpu_addr; > ? ? ? ?volatile uint32_t ? ? ? ? ? ? ? *cpu_addr; > - ? ? ? /* seq is protected by ring emission lock */ > - ? ? ? uint64_t ? ? ? ? ? ? ? ? ? ? ? ?seq; > + ? ? ? /* sync_seq is protected by ring emission lock */ > + ? ? ? uint64_t ? ? ? ? ? ? ? ? ? ? ? ?sync_seq[RADEON_NUM_RINGS]; > ? ? ? ?atomic64_t ? ? ? ? ? ? ? ? ? ? ?last_seq; > ? ? ? ?unsigned long ? ? ? ? ? ? ? ? ? last_activity; > ? ? ? ?bool ? ? ? ? ? ? ? ? ? ? ? ? ? ?initialized; > @@ -288,6 +288,25 @@ int radeon_fence_wait_any(struct radeon_device *rdev, > ?struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence); > ?void radeon_fence_unref(struct radeon_fence **fence); > ?unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring); > +bool radeon_fence_need_sync(struct radeon_fence *fence, int ring); > +void radeon_fence_note_sync(struct radeon_fence *fence, int ring); > +static inline struct radeon_fence *radeon_fence_later(struct radeon_fence *a, > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct radeon_fence *b) > +{ > + ? ? ? if (!a) { > + ? ? ? ? ? ? ? return b; > + ? ? ? } > + > + ? ? ? if (!b) { > + ? ? ? ? ? ? ? return a; > + ? ? ? } Please add : BUG_ON(a->ring != b->ring); So we can catch if someone badly use this function. > + > + ? ? ? if (a->seq > b->seq) { > + ? ? ? ? ? ? ? return a; > + ? ? ? } else { > + ? ? ? ? ? ? ? return b; > + ? ? ? } > +} > > ?/* > ?* Tiling registers > diff --git a/drivers/gpu/drm/radeon/radeon_fence.c > b/drivers/gpu/drm/radeon/radeon_fence.c > index 401d346..7b55625 100644 > --- a/drivers/gpu/drm/radeon/radeon_fence.c > +++ b/drivers/gpu/drm/radeon/radeon_fence.c > @@ -72,7 +72,7 @@ int radeon_fence_emit(struct radeon_device *rdev, > ? ? ? ?} > ? ? ? ?kref_init(&((*fence)->kref)); > ? ? ? ?(*fence)->rdev = rdev; > - ? ? ? (*fence)->seq = ++rdev->fence_drv[ring].seq; > + ? ? ? (*fence)->seq = ++rdev->fence_drv[ring].sync_seq[ring]; > ? ? ? ?(*fence)->ring = ring; > ? ? ? ?radeon_fence_ring_emit(rdev, ring, *fence); > ? ? ? ?trace_radeon_fence_emit(rdev->ddev, (*fence)->seq); > @@ -449,7 +449,7 @@ int radeon_fence_wait_next_locked(struct radeon_device > *rdev, int ring) > ? ? ? ? * wait. > ? ? ? ? */ > ? ? ? ?seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL; > - ? ? ? if (seq >= rdev->fence_drv[ring].seq) { > + ? ? ? if (seq >= rdev->fence_drv[ring].sync_seq[ring]) { > ? ? ? ? ? ? ? ?/* nothing to wait for, last_seq is > ? ? ? ? ? ? ? ? ? already the last emited fence */ > ? ? ? ? ? ? ? ?return -ENOENT; > @@ -464,7 +464,7 @@ int radeon_fence_wait_empty_locked(struct radeon_device > *rdev, int ring) > ? ? ? ? * activity can be scheduled so there won't be concurrent access > ? ? ? ? * to seq value. > ? ? ? ? */ > - ? ? ? return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].seq, > + ? ? ? return radeon_fence_wait_seq(rdev, > rdev->fence_drv[ring].sync_seq[ring], > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ring, false, false); > ?} > > @@ -492,7 +492,8 @@ unsigned radeon_fence_count_emitted(struct radeon_device > *rdev, int ring) > ? ? ? ? * but it's ok to report slightly wrong fence count here. > ? ? ? ? */ > ? ? ? ?radeon_fence_process(rdev, ring); > - ? ? ? emitted = rdev->fence_drv[ring].seq - > atomic64_read(&rdev->fence_drv[ring].last_seq); > + ? ? ? emitted = rdev->fence_drv[ring].sync_seq[ring] > + ? ? ? ? ? ? ? - atomic64_read(&rdev->fence_drv[ring].last_seq); > ? ? ? ?/* to avoid 32bits warp around */ > ? ? ? ?if (emitted > 0x1000) { > ? ? ? ? ? ? ? ?emitted = 0x1000; > @@ -500,6 +501,51 @@ unsigned radeon_fence_count_emitted(struct radeon_device > *rdev, int ring) > ? ? ? ?return (unsigned)emitted; > ?} > > +bool radeon_fence_need_sync(struct radeon_fence *fence, int dst_ring) > +{ > + ? ? ? struct radeon_fence_driver *fdrv; > + > + ? ? ? if (!fence) { > + ? ? ? ? ? ? ? return false; > + ? ? ? } > + > + ? ? ? if (fence->ring == dst_ring) { > + ? ? ? ? ? ? ? return false; > + ? ? ? } > + > + ? ? ? /* we are protected by the ring mutex */ > + ? ? ? fdrv = &fence->rdev->fence_drv[dst_ring]; > + ? ? ? if (fence->seq <= fdrv->sync_seq[fence->ring]) { > + ? ? ? ? ? ? ? return false; > + ? ? ? } > + > + ? ? ? return true; > +} > + > +void radeon_fence_note_sync(struct radeon_fence *fence, int dst_ring) > +{ > + ? ? ? struct radeon_fence_driver *dst, *src; > + ? ? ? unsigned i; > + > + ? ? ? if (!fence) { >
[PATCH] drm/radeon: fix bank information in tiling config
From: Alex Deucher While there are cards with more than 8 mem banks, the max number of banks from a tiling perspective is 8, so cap the tiling config at 8 banks. Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=43448 Signed-off-by: Alex Deucher Cc: stable at vger.kernel.org --- drivers/gpu/drm/radeon/evergreen.c |9 ++--- drivers/gpu/drm/radeon/ni.c|9 ++--- drivers/gpu/drm/radeon/rv770.c |8 ++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 58991af..5d9c2c6 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2136,9 +2136,12 @@ static void evergreen_gpu_init(struct radeon_device *rdev) /* num banks is 8 on all fusion asics. 0 = 4, 1 = 8, 2 = 16 */ if (rdev->flags & RADEON_IS_IGP) rdev->config.evergreen.tile_config |= 1 << 4; - else - rdev->config.evergreen.tile_config |= - ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) << 4; + else { + if ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) + rdev->config.evergreen.tile_config |= 1 << 4; + else + rdev->config.evergreen.tile_config |= 0 << 4; + } rdev->config.evergreen.tile_config |= ((mc_arb_ramcfg & BURSTLENGTH_MASK) >> BURSTLENGTH_SHIFT) << 8; rdev->config.evergreen.tile_config |= diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index ce4e7cc..15f950c 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -866,9 +866,12 @@ static void cayman_gpu_init(struct radeon_device *rdev) /* num banks is 8 on all fusion asics. 0 = 4, 1 = 8, 2 = 16 */ if (rdev->flags & RADEON_IS_IGP) rdev->config.cayman.tile_config |= 1 << 4; - else - rdev->config.cayman.tile_config |= - ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) << 4; + else { + if ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) + rdev->config.cayman.tile_config |= 1 << 4; + else + rdev->config.cayman.tile_config |= 0 << 4; + } rdev->config.cayman.tile_config |= ((gb_addr_config & PIPE_INTERLEAVE_SIZE_MASK) >> PIPE_INTERLEAVE_SIZE_SHIFT) << 8; rdev->config.cayman.tile_config |= diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index c2f473b..c824d49 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -689,8 +689,12 @@ static void rv770_gpu_init(struct radeon_device *rdev) if (rdev->family == CHIP_RV770) gb_tiling_config |= BANK_TILING(1); - else - gb_tiling_config |= BANK_TILING((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT); + else { + if ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) + gb_tiling_config |= BANK_TILING(1); + else + gb_tiling_config |= BANK_TILING(0); + } rdev->config.rv770.tiling_nbanks = 4 << ((gb_tiling_config >> 4) & 0x3); gb_tiling_config |= GROUP_SIZE((mc_arb_ramcfg & BURSTLENGTH_MASK) >> BURSTLENGTH_SHIFT); if ((mc_arb_ramcfg & BURSTLENGTH_MASK) >> BURSTLENGTH_SHIFT) -- 1.7.7.5
[PATCH] drm/radeon: properly program gart on rv740, juniper, cypress, barts
From: Alex Deucher Need to program an additional VM register. This doesn't not currently cause any problems, but allows us to program the proper backend map in a subsequent patch which should improve performance on these asics. Signed-off-by: Alex Deucher Cc: stable at vger.kernel.org --- drivers/gpu/drm/radeon/evergreen.c |5 + drivers/gpu/drm/radeon/evergreend.h |1 + drivers/gpu/drm/radeon/rv770.c |2 ++ drivers/gpu/drm/radeon/rv770d.h |1 + 4 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 5d9c2c6..0408ac2 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -1029,6 +1029,11 @@ int evergreen_pcie_gart_enable(struct radeon_device *rdev) WREG32(MC_VM_MD_L1_TLB0_CNTL, tmp); WREG32(MC_VM_MD_L1_TLB1_CNTL, tmp); WREG32(MC_VM_MD_L1_TLB2_CNTL, tmp); + if ((rdev->family == CHIP_JUNIPER) || + (rdev->family == CHIP_CYPRESS) || + (rdev->family == CHIP_HEMLOCK) || + (rdev->family == CHIP_BARTS)) + WREG32(MC_VM_MD_L1_TLB3_CNTL, tmp); } WREG32(MC_VM_MB_L1_TLB0_CNTL, tmp); WREG32(MC_VM_MB_L1_TLB1_CNTL, tmp); diff --git a/drivers/gpu/drm/radeon/evergreend.h b/drivers/gpu/drm/radeon/evergreend.h index 79130bf..3dd43e7 100644 --- a/drivers/gpu/drm/radeon/evergreend.h +++ b/drivers/gpu/drm/radeon/evergreend.h @@ -452,6 +452,7 @@ #defineMC_VM_MD_L1_TLB0_CNTL 0x2654 #defineMC_VM_MD_L1_TLB1_CNTL 0x2658 #defineMC_VM_MD_L1_TLB2_CNTL 0x265C +#defineMC_VM_MD_L1_TLB3_CNTL 0x2698 #defineFUS_MC_VM_MD_L1_TLB0_CNTL 0x265C #defineFUS_MC_VM_MD_L1_TLB1_CNTL 0x2660 diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index c824d49..c12349d 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -151,6 +151,8 @@ int rv770_pcie_gart_enable(struct radeon_device *rdev) WREG32(MC_VM_MD_L1_TLB0_CNTL, tmp); WREG32(MC_VM_MD_L1_TLB1_CNTL, tmp); WREG32(MC_VM_MD_L1_TLB2_CNTL, tmp); + if (rdev->family == CHIP_RV740) + WREG32(MC_VM_MD_L1_TLB3_CNTL, tmp); WREG32(MC_VM_MB_L1_TLB0_CNTL, tmp); WREG32(MC_VM_MB_L1_TLB1_CNTL, tmp); WREG32(MC_VM_MB_L1_TLB2_CNTL, tmp); diff --git a/drivers/gpu/drm/radeon/rv770d.h b/drivers/gpu/drm/radeon/rv770d.h index 9c549f7..7addbef 100644 --- a/drivers/gpu/drm/radeon/rv770d.h +++ b/drivers/gpu/drm/radeon/rv770d.h @@ -174,6 +174,7 @@ #defineMC_VM_MD_L1_TLB0_CNTL 0x2654 #defineMC_VM_MD_L1_TLB1_CNTL 0x2658 #defineMC_VM_MD_L1_TLB2_CNTL 0x265C +#defineMC_VM_MD_L1_TLB3_CNTL 0x2698 #defineMC_VM_SYSTEM_APERTURE_DEFAULT_ADDR 0x203C #defineMC_VM_SYSTEM_APERTURE_HIGH_ADDR 0x2038 #defineMC_VM_SYSTEM_APERTURE_LOW_ADDR 0x2034 -- 1.7.7.5
[PATCH 1/2] drm/radeon: fix HD6790, HD6570 backend programming
From: Jerome Glisse Bugs that should be fixed by this patch : https://bugs.freedesktop.org/show_bug.cgi?id=49792 https://bugzilla.kernel.org/show_bug.cgi?id=43207 https://bugs.freedesktop.org/show_bug.cgi?id=39282 Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/evergreen.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 0408ac2..6a57f0d 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2178,9 +2178,9 @@ static void evergreen_gpu_init(struct radeon_device *rdev) WREG32(CC_SYS_RB_BACKEND_DISABLE, rb); WREG32(GC_USER_RB_BACKEND_DISABLE, rb); WREG32(CC_GC_SHADER_PIPE_CONFIG, sp); -} + } - grbm_gfx_index |= SE_BROADCAST_WRITES; + grbm_gfx_index = INSTANCE_BROADCAST_WRITES | SE_BROADCAST_WRITES; WREG32(GRBM_GFX_INDEX, grbm_gfx_index); WREG32(RLC_GFX_INDEX, grbm_gfx_index); -- 1.7.7.5
[PATCH 2/2] drm/radeon: fixup tiling group size and backendmap on r6xx-r9xx (v4)
From: Alex Deucher Tiling group size is always 256bits on r6xx/r7xx/r8xx/9xx. Also fix and simplify render backend map. This now properly sets up the backend map on r6xx-9xx which should improve 3D performance. Signed-off-by: Alex Deucher Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/evergreen.c | 368 +-- drivers/gpu/drm/radeon/evergreend.h | 10 + drivers/gpu/drm/radeon/ni.c | 351 +++-- drivers/gpu/drm/radeon/nid.h| 11 + drivers/gpu/drm/radeon/r600.c | 199 +++ drivers/gpu/drm/radeon/r600d.h |2 + drivers/gpu/drm/radeon/radeon.h |5 + drivers/gpu/drm/radeon/rv770.c | 264 + drivers/gpu/drm/radeon/rv770d.h |3 + 9 files changed, 222 insertions(+), 991 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 6a57f0d..01550d0 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -1558,163 +1558,10 @@ int evergreen_cp_resume(struct radeon_device *rdev) /* * Core functions */ -static u32 evergreen_get_tile_pipe_to_backend_map(struct radeon_device *rdev, - u32 num_tile_pipes, - u32 num_backends, - u32 backend_disable_mask) -{ - u32 backend_map = 0; - u32 enabled_backends_mask = 0; - u32 enabled_backends_count = 0; - u32 cur_pipe; - u32 swizzle_pipe[EVERGREEN_MAX_PIPES]; - u32 cur_backend = 0; - u32 i; - bool force_no_swizzle; - - if (num_tile_pipes > EVERGREEN_MAX_PIPES) - num_tile_pipes = EVERGREEN_MAX_PIPES; - if (num_tile_pipes < 1) - num_tile_pipes = 1; - if (num_backends > EVERGREEN_MAX_BACKENDS) - num_backends = EVERGREEN_MAX_BACKENDS; - if (num_backends < 1) - num_backends = 1; - - for (i = 0; i < EVERGREEN_MAX_BACKENDS; ++i) { - if (((backend_disable_mask >> i) & 1) == 0) { - enabled_backends_mask |= (1 << i); - ++enabled_backends_count; - } - if (enabled_backends_count == num_backends) - break; - } - - if (enabled_backends_count == 0) { - enabled_backends_mask = 1; - enabled_backends_count = 1; - } - - if (enabled_backends_count != num_backends) - num_backends = enabled_backends_count; - - memset((uint8_t *)&swizzle_pipe[0], 0, sizeof(u32) * EVERGREEN_MAX_PIPES); - switch (rdev->family) { - case CHIP_CEDAR: - case CHIP_REDWOOD: - case CHIP_PALM: - case CHIP_SUMO: - case CHIP_SUMO2: - case CHIP_TURKS: - case CHIP_CAICOS: - force_no_swizzle = false; - break; - case CHIP_CYPRESS: - case CHIP_HEMLOCK: - case CHIP_JUNIPER: - case CHIP_BARTS: - default: - force_no_swizzle = true; - break; - } - if (force_no_swizzle) { - bool last_backend_enabled = false; - - force_no_swizzle = false; - for (i = 0; i < EVERGREEN_MAX_BACKENDS; ++i) { - if (((enabled_backends_mask >> i) & 1) == 1) { - if (last_backend_enabled) - force_no_swizzle = true; - last_backend_enabled = true; - } else - last_backend_enabled = false; - } - } - - switch (num_tile_pipes) { - case 1: - case 3: - case 5: - case 7: - DRM_ERROR("odd number of pipes!\n"); - break; - case 2: - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 1; - break; - case 4: - if (force_no_swizzle) { - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 1; - swizzle_pipe[2] = 2; - swizzle_pipe[3] = 3; - } else { - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 2; - swizzle_pipe[2] = 1; - swizzle_pipe[3] = 3; - } - break; - case 6: - if (force_no_swizzle) { - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 1; - swizzle_pipe[2] = 2; - swizzle_pipe[3] = 3; - swizzle_pipe[4] = 4; - swizzle_pipe[5] = 5; - } else { - swizzle_pipe[0] = 0; - swizzle_pipe[1] = 2; -