[pull] radeon drm-fixes-3.15
On 16 May 2014 23:54, Christian K?nig wrote: > Hi Dave, > > this is the next pull quested for stashed up radeon fixes for 3.15. > > Highlights are: > 1. Avoid sending SIGBUS on CPU access just because kernel can't handle > buffer placement. > 2. Some fixes for VM page table updates and buffer placements. > 3. Fixing two typos in the PLL and SI register spec. > 4. Checking VCE buffers ranges. > 5. Adding a mutex for I2C access. > 6. Mullins and non-VGA pci class fixes from Alex. This is starting to feel a bit large, we should be in regression fixing, oopses and black screens at this point, The main ones I'm concerned about are the VCE buffer ranges and i2c mutex, whether these fix known problems or issues, Dave.
[Bug 77738] [r600g, bisected] 4 piglit texwrap tests regressed
29 128 204
Fail with LINEAR and CLAMP at (6,125) @ 0,0
Expected: 159 82 235 216
Observed: 83 236 159 217
Fail with LINEAR and CLAMP_TO_BORDER at (95,125) @ 0,0
Expected: 127 25 229 204
Observed: 25 229 128 204
Fail with LINEAR and MIRROR_CLAMP_EXT at (184,125) @ 0,0
Expected: 159 82 235 216
Observed: 83 236 159 217
Fail with LINEAR and MIRROR_CLAMP_TO_BORDER_EXT at (273,125) @ 0,0
Expected: 127 25 229 204
Observed: 25 229 128 204
PIGLIT:subtest {'GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3, swizzled, border color
only' : 'fail'}
Testing GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5, swizzled, border color only
Fail with NEAREST and CLAMP_TO_BORDER at (95,36) @ 0,0
Expected: 127 25 229 204
Observed: 25 229 128 204
Fail with NEAREST and MIRROR_CLAMP_TO_BORDER_EXT at (273,36) @ 0,0
Expected: 127 25 229 204
Observed: 25 229 128 204
Fail with LINEAR and CLAMP at (6,125) @ 0,0
Expected: 159 82 235 216
Observed: 83 236 159 217
Fail with LINEAR and CLAMP_TO_BORDER at (95,125) @ 0,0
Expected: 127 25 229 204
Observed: 25 229 128 204
Fail with LINEAR and MIRROR_CLAMP_EXT at (184,125) @ 0,0
Expected: 159 82 235 216
Observed: 83 236 159 217
Fail with LINEAR and MIRROR_CLAMP_TO_BORDER_EXT at (273,125) @ 0,0
Expected: 127 25 229 204
Observed: 25 229 128 204
PIGLIT:subtest {'GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5, swizzled, border color
only' : 'fail'}
PIGLIT: {'result': 'fail' }
By quick look, except issue which described Pavel, I also see that all values
are mixed up.
it has wrong order:
Expected: 1 2 3 4
Observed: 2 3 1 4
--
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/084155d9/attachment.html>
[Bug 78870] New: [r600g] Driver fails to compile shader from the game tesseract - if/endif unbalanced in shader
https://bugs.freedesktop.org/show_bug.cgi?id=78870 Priority: medium Bug ID: 78870 Assignee: dri-devel at lists.freedesktop.org Summary: [r600g] Driver fails to compile shader from the game tesseract - if/endif unbalanced in shader Severity: normal Classification: Unclassified OS: All Reporter: tdroste at gmx.de Hardware: Other Status: NEW Version: git Component: Drivers/Gallium/r600 Product: Mesa Created attachment 99288 --> https://bugs.freedesktop.org/attachment.cgi?id=99288&action=edit TGSI shader dump While trying different settings in tesseract r600g gives me this for the maximum setting of MLAA: EE r600_shader.c:6180 tgsi_endif - if/endif unbalanced in shader EE r600_shader.c:157 r600_pipe_shader_create - translation from TGSI failed ! EE r600_state_common.c:750 r600_shader_select - Failed to build shader variant (type=1) -1 The offending TGSI shader is attachend. From a simple count of UIF end ENDIF it seems that they are not unbalanced and the problem is somewhere else. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/da5aa229/attachment.html>
[Bug 75401] vgaswitcheroo doesn't work for AMD Radeon 8870m (possibly due to "wrong" PCI class)
https://bugzilla.kernel.org/show_bug.cgi?id=75401 --- Comment #13 from Alex Deucher --- (In reply to Pali Roh?r from comment #12) > @Alex Deucher: Why is this bug while loop needed? IIRC, sometimes the method is hung off the other GPU. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 75917] backlight switches off when starting X - since kernel-3.13
https://bugs.freedesktop.org/show_bug.cgi?id=75917 --- Comment #3 from Alex Deucher --- Does disabling runpm help? Boot with radeon.runpm=0 on the kernel command line in grub. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/9682ceea/attachment.html>
[Bug 78453] [HAWAII] Get acceleration working
https://bugs.freedesktop.org/show_bug.cgi?id=78453 --- Comment #33 from Michel D?nzer --- (In reply to comment #28) > What does "sa_manager is not empty, clearing anyway" mean ? Can it create > gpu lock up ? No, I think that's just an artifact of the attempt to reset the GPU. (In reply to comment #29) > Sometimes when I run ./egltri_screen I have a shaded triangle, that doesnt > move, it looks like the first frame is correctly processed. egltri only renders one frame. :) (In reply to comment #32) > I have the firmware from fedora 20, I think they are newer than what was > used for initial hawaii bring up [...] Trying older firmware is worth a shot I guess, though not very likely to make a difference I'm afraid. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/0ac734cd/attachment.html>
[Bug 72685] [radeonsi hyperz] Artifacts in Unigine Sanctuary
https://bugs.freedesktop.org/show_bug.cgi?id=72685 --- Comment #16 from Michel D?nzer --- (In reply to comment #15) > GPU model: Gallium 0.4 on AMD SUMO 3.0 Mesa 10.3.0-devel (git-3a1da0a) 256Mb This bug report is about the radeonsi driver, which doesn't support that card. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/6b5d7d21/attachment.html>
[Bug 77785] (radeonsi) Some lighting issues in games, textures goes black
https://bugs.freedesktop.org/show_bug.cgi?id=77785 --- Comment #19 from Michel D?nzer --- (In reply to comment #14) > It is XRGB, MESA_FORMAT_B8G8R8X8_UNORM but need to checked it out in real > situations... a think intel disable support for this "memory saver" format > in screen, but radeon in classic and gallium leave it enabled :). MESA_FORMAT_B8G8R8X8_UNORM has nothing to do with memory saving (it's the same as MESA_FORMAT_B8G8R8A8_UNORM as far as memory layout is concerned), it just means that the contents of the 'alpha' channel in memory are ignored. > Does that format need to be supported anymore? Yes. We need to find out where and how this format ends up being used incorrectly. Note that this might be in Wine / in the apps themselves. Anyway, your comments #17 and #18 mean that the PTTM/ATH and AirStrike3D issues are probably not one and the same bug and need to be tracked separately. The AirStrike3D and SuperTuxKart issues might be related, but it's better to assume they're separate as well for now, to avoid cluttering up a single report with unrelated issues. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/6681743a/attachment-0001.html>
[PATCH 0/2] drm/gk20a: FB fixes
Fix a very shameful memory leak and a compilation error due to the use of non-exported CMA functions. The workaround for the latter is not really elegant (replace the CMA functions by a runtime failure if we are compiled as a module), but is temporary and still an improvement over the current situation (compile error). Alexandre Courbot (2): drm/gk20a/fb: fix huge memory leak drm/gk20a/fb: fix compile error whith CMA and module drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c | 95 --- 1 file changed, 67 insertions(+), 28 deletions(-) -- 1.9.2
[PATCH 1/2] drm/gk20a/fb: fix huge memory leak
CMA-allocated memory must be freed by an exact mirror call to
dma_release_from_contiguous(). It cannot be freed page-by-page as was
previously believed without severe memory leakage.
This page records the address and size of every allocated memory chunk
so they can be properly freed when needed.
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c | 74 ++-
1 file changed, 46 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
b/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
index 7effd1a63458..5904af52e6d6 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
@@ -28,28 +28,34 @@
#include
#include
+struct gk20a_mem_chunk {
+ struct list_head list;
+ struct page *pages;
+ u32 npages;
+};
+
+struct gk20a_mem {
+ struct nouveau_mem base;
+ struct list_head head;
+};
+
static void
gk20a_ram_put(struct nouveau_fb *pfb, struct nouveau_mem **pmem)
{
struct device *dev = nv_device_base(nv_device(pfb));
- struct nouveau_mem *mem = *pmem;
- int i;
+ struct gk20a_mem *mem = container_of(*pmem, struct gk20a_mem, base);
+ struct gk20a_mem_chunk *chunk, *n;
*pmem = NULL;
if (unlikely(mem == NULL))
return;
- for (i = 0; i < mem->size; i++) {
- struct page *page;
-
- if (mem->pages[i] == 0)
- break;
-
- page = pfn_to_page(mem->pages[i] >> PAGE_SHIFT);
- dma_release_from_contiguous(dev, page, 1);
+ list_for_each_entry_safe(chunk, n, &mem->head, list) {
+ dma_release_from_contiguous(dev, chunk->pages, chunk->npages);
+ kfree(chunk);
}
- kfree(mem->pages);
+ kfree(mem->base.pages);
kfree(mem);
}
@@ -58,9 +64,8 @@ gk20a_ram_get(struct nouveau_fb *pfb, u64 size, u32 align,
u32 ncmin,
u32 memtype, struct nouveau_mem **pmem)
{
struct device *dev = nv_device_base(nv_device(pfb));
- struct nouveau_mem *mem;
+ struct gk20a_mem *mem;
int type = memtype & 0xff;
- dma_addr_t dma_addr;
int npages;
int order;
int i;
@@ -95,44 +100,57 @@ gk20a_ram_get(struct nouveau_fb *pfb, u64 size, u32 align,
u32 ncmin,
if (!mem)
return -ENOMEM;
- mem->size = npages;
- mem->memtype = type;
+ mem->base.size = npages;
+ mem->base.memtype = type;
- mem->pages = kzalloc(sizeof(dma_addr_t) * npages, GFP_KERNEL);
- if (!mem) {
+ mem->base.pages = kzalloc(sizeof(dma_addr_t) * npages, GFP_KERNEL);
+ if (!mem->base.pages) {
kfree(mem);
return -ENOMEM;
}
+ INIT_LIST_HEAD(&mem->head);
+
+ *pmem = &mem->base;
+
while (npages) {
- struct page *pages;
+ struct gk20a_mem_chunk *chunk;
+ dma_addr_t addr;
int pos = 0;
/* don't overflow in case size is not a multiple of ncmin */
if (ncmin > npages)
ncmin = npages;
- pages = dma_alloc_from_contiguous(dev, ncmin, order);
- if (!pages) {
- gk20a_ram_put(pfb, &mem);
+ chunk = kzalloc(sizeof(*chunk), GFP_KERNEL);
+ if (!chunk) {
+ gk20a_ram_put(pfb, pmem);
return -ENOMEM;
}
- dma_addr = (dma_addr_t)(page_to_pfn(pages) << PAGE_SHIFT);
+ chunk->pages = dma_alloc_from_contiguous(dev, ncmin, order);
+ if (!chunk->pages) {
+ kfree(chunk);
+ gk20a_ram_put(pfb, pmem);
+ return -ENOMEM;
+ }
- nv_debug(pfb, " alloc count: %x, order: %x, addr: %pad\n",
ncmin,
-order, &dma_addr);
+ chunk->npages = ncmin;
+ list_add_tail(&chunk->list, &mem->head);
+
+ addr = (dma_addr_t)(page_to_pfn(chunk->pages) << PAGE_SHIFT);
+
+ nv_debug(pfb, " alloc count: %x, order: %x, addr: %pad\n",
+ncmin, order, &addr);
for (i = 0; i < ncmin; i++)
- mem->pages[pos + i] = dma_addr + (PAGE_SIZE * i);
+ mem->base.pages[pos + i] = addr + (PAGE_SIZE * i);
pos += ncmin;
npages -= ncmin;
}
- mem->offset = (u64)mem->pages[0];
-
- *pmem = mem;
+ mem->base.offset = (u64)mem->base.pages[0];
return 0;
}
--
1.9.2
[PATCH 2/2] drm/gk20a/fb: fix compile error whith CMA and module
CMA functions are not available to kernel modules, but the GK20A FB
driver currently (and temporarily) relies on them.
This patch replaces the calls to CMA functions in problematic cases (CMA
enabled and Nouveau compiled as a module) with dummy stubs that will
make this particular driver fail, but at least won't produce a compile
error.
This is a temporary fix until a better memory allocation scheme is
devised.
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c | 25 +--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
b/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
index 5904af52e6d6..fa867ce5449e 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/fb/ramgk20a.c
@@ -39,6 +39,27 @@ struct gk20a_mem {
struct list_head head;
};
+/*
+ * CMA is not available to modules. Until we find a better solution, make
+ * memory allocations fail in that case.
+ */
+#if IS_ENABLED(CONFIG_CMA) && IS_MODULE(CONFIG_DRM_NOUVEAU)
+static inline struct page *
+alloc_contiguous_memory(struct device *dev, int count, unsigned int order)
+{
+ dev_err(dev, "cannot use CMA from a module - allocation failed\n");
+ return NULL;
+}
+
+static inline void
+release_contiguous_memory(struct device *dev, struct page *page, int count)
+{
+}
+#else
+#define alloc_contiguous_memory(d, c, o) dma_alloc_from_contiguous(d, c, o)
+#define release_contiguous_memory(d, p, c) dma_release_from_contiguous(d, p, c)
+#endif
+
static void
gk20a_ram_put(struct nouveau_fb *pfb, struct nouveau_mem **pmem)
{
@@ -51,7 +72,7 @@ gk20a_ram_put(struct nouveau_fb *pfb, struct nouveau_mem
**pmem)
return;
list_for_each_entry_safe(chunk, n, &mem->head, list) {
- dma_release_from_contiguous(dev, chunk->pages, chunk->npages);
+ release_contiguous_memory(dev, chunk->pages, chunk->npages);
kfree(chunk);
}
@@ -128,7 +149,7 @@ gk20a_ram_get(struct nouveau_fb *pfb, u64 size, u32 align,
u32 ncmin,
return -ENOMEM;
}
- chunk->pages = dma_alloc_from_contiguous(dev, ncmin, order);
+ chunk->pages = alloc_contiguous_memory(dev, ncmin, order);
if (!chunk->pages) {
kfree(chunk);
gk20a_ram_put(pfb, pmem);
--
1.9.2
[PATCH] drm/mm: Adjust start/end for coloring first
The current user of the coloring will adjust the end points of the node
to leave a hole between disjoint memory types. This adjustment must be
performed first or else the derived size will conflict with the
adjustment and trigger the BUG_ON sanity checks that the node is within
bounds.
Fixes regression from
commit 62347f9e0f81d50e9b0923ec8a192f60ab7a1801
Author: Lauri Kasanen
Date: Wed Apr 2 20:03:57 2014 +0300
drm: Add support for two-ended allocation, v3
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/drm_mm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 04a209e2b66d..911863bed9f3 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -264,12 +264,12 @@ static void drm_mm_insert_helper_range(struct drm_mm_node
*hole_node,
if (adj_end > end)
adj_end = end;
- if (flags & DRM_MM_CREATE_TOP)
- adj_start = adj_end - size;
-
if (mm->color_adjust)
mm->color_adjust(hole_node, color, &adj_start, &adj_end);
+ if (flags & DRM_MM_CREATE_TOP)
+ adj_start = adj_end - size;
+
if (alignment) {
unsigned tmp = adj_start % alignment;
if (tmp) {
--
2.0.0.rc2
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 16 May 2014 20:19, Tomasz Figa wrote:
> On 16.05.2014 16:30, Rahul Sharma wrote:
>> On 16 May 2014 16:20, Tomasz Figa wrote:
>>> On 16.05.2014 12:35, Rahul Sharma wrote:
On 16 May 2014 15:12, Rahul Sharma wrote:
> On 16 May 2014 03:14, Tomasz Figa wrote:
>> On 15.05.2014 06:01, Rahul Sharma wrote:
[snip]
the PHY provider.
>>>
>>> Please correct me if I got you wrong. You want somthing like this:
>>>
>>> pmu_system_controller: system-controller at 1004 {
>>> ...
>>> simple_phys: simple-phys {
>>> compatible = "samsung,exynos5420-simple-phy";
>>> ...
>>> };
>>> };
>>
>> Not exactly.
>>
>> What I meant is that the PMU node itself should be the PHY provider, e.g.
>>
>> pmu_system_controller: system-controller at 1004 {
>> /* ... */
>> #phy-cells = <1>;
>> };
>>
>> and then the PMU node should instantiate the Exynos simple PHY driver,
>> as this is a driver for a facility existing entirely inside of the PMU.
>> Moreover, the driver should be rather called Exynos PMU PHY.
>>
>> I know this isn't really possible at the moment, but with device tree we
>> must design things carefully, so it's better to take a bit more time and
>> do things properly.
>>
>> So my opinion on this is that there should be a central Exynos PMU
>> driver that claims the IO region and instantiates necessary subdrivers,
>> such as Exynos PMU PHY driver, Exynos CLKOUT driver, Exynos cpuidle
>> driver and more, similar to what is being done in drivers/mfd.
>
Hi Tomasz,
These PHYs are not part of PMU as such. I am not sure if it is correct to
probe them as phy provider for all these phys. Only relation of these phys
with
the PMU is 'enable/disable control'.
>>>
>>> Well, in reality what is implemented by this driver is not even a PHY,
>>> just some kind of power controllers, which are contained entirely in the
>>> PMU.
>>>
>>
>> I agree. Actually the role of generic phy framework for these 'simple' phys
>> is
>> only that much.
>>
Controlling this bit using regmap interface
still looks better to me.
>>>
>>> Well, when there is a choice between using regmap and not using regmap,
>>> I'd rather choose the latter. Why would you want to introduce additional
>>> abstraction layer if there is no need for such?
>>>
IMHO Ideal method would be probing these PHYs independently and resolving
the necessary dependencies like syscon handle, clocks etc. This way we will
not be having any common phy provider for all these independent PHYs and it
would be clean to add each of these phy nodes in DT. Please see my original
comment below.
http://lkml.iu.edu/hypermail/linux/kernel/1404.1/00701.html
>>>
>>> With the solution I proposed, you don't need any kind of dependencies
>>> for those simple power controllers. They are just single bits that don't
>>> need anything special to operate, except PMU clock running.
>>
>> In that case we can further trim it down and let the drivers use the regmap
>> interface to control this bit. Many drivers including HDMI, DP just need that
>> much functionality from the phy provider.
>
> Well, this is what several drivers already do, like USB PHY (dedicated
> IP block), watchdog (for watchdog mask), SATA PHY (dedicated IP block
> too) or will do, like I2C (for configuration of I2C mux on Exynos5).
>
> At least this would be consistent with them and wouldn't be an API
> abuse, so I'd be inclined to go this way more than introducing
> abstractions like this patch does.
Ok. I had already posted a patch for this at
http://www.spinics.net/lists/linux-samsung-soc/msg28049.html
I will revive that thread.
@Tomasz Stanislawski, Do you have different opinion here?
Regards,
Rahul Sharma.
>
> Best regards,
> Tomasz
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] drm/ttm: nouveau: memory coherency fixes for ARM
This small series introduces TTM helper functions as well as Nouveau hooks that are needed to ensure buffer coherency on ARM. Most of this series is a forward-port of some patches Lucas Stach sent last year and that are also needed for Nouveau GK20A support: http://lists.freedesktop.org/archives/nouveau/2013-August/014026.html Another patch takes care of flushing the CPU write-buffer when writing BOs through a non-BAR path. Alexandre Courbot (1): drm/nouveau: introduce CPU cache flushing macro Lucas Stach (3): drm/ttm: recognize ARM arch in ioprot handler drm/ttm: introduce dma cache sync helpers drm/nouveau: hook up cache sync functions drivers/gpu/drm/nouveau/core/os.h | 17 +++ drivers/gpu/drm/nouveau/nouveau_bo.c | 40 +-- drivers/gpu/drm/nouveau/nouveau_bo.h | 20 ++ drivers/gpu/drm/nouveau/nouveau_gem.c | 8 ++- drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +- drivers/gpu/drm/ttm/ttm_tt.c | 25 ++ include/drm/ttm/ttm_bo_driver.h | 28 7 files changed, 136 insertions(+), 4 deletions(-) -- 1.9.2
[PATCH 1/4] drm/ttm: recognize ARM arch in ioprot handler
From: Lucas Stach Signed-off-by: Lucas Stach Signed-off-by: Alexandre Courbot --- drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 1df856f78568..30e5d90cb7bc 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -500,7 +500,7 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp) pgprot_val(tmp) |= _PAGE_GUARDED; } #endif -#if defined(__ia64__) +#if defined(__ia64__) || defined(__arm__) if (caching_flags & TTM_PL_FLAG_WC) tmp = pgprot_writecombine(tmp); else -- 1.9.2
[PATCH 2/4] drm/ttm: introduce dma cache sync helpers
From: Lucas Stach
On arches with non-coherent PCI, we need to flush caches ourselfes at
the appropriate places. Introduce two small helpers to make things easy
for TTM based drivers.
Signed-off-by: Lucas Stach
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/ttm/ttm_tt.c| 25 +
include/drm/ttm/ttm_bo_driver.h | 28
2 files changed, 53 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 75f319090043..05a316b71ad1 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -248,6 +249,30 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma)
}
EXPORT_SYMBOL(ttm_dma_tt_fini);
+void ttm_dma_tt_cache_sync_for_device(struct ttm_dma_tt *ttm_dma,
+ struct device *dev)
+{
+ int i;
+
+ for (i = 0; i < ttm_dma->ttm.num_pages; i++) {
+ dma_sync_single_for_device(dev, ttm_dma->dma_address[i],
+ PAGE_SIZE, DMA_TO_DEVICE);
+ }
+}
+EXPORT_SYMBOL(ttm_dma_tt_cache_sync_for_device);
+
+void ttm_dma_tt_cache_sync_for_cpu(struct ttm_dma_tt *ttm_dma,
+ struct device *dev)
+{
+ int i;
+
+ for (i = 0; i < ttm_dma->ttm.num_pages; i++) {
+ dma_sync_single_for_cpu(dev, ttm_dma->dma_address[i],
+ PAGE_SIZE, DMA_FROM_DEVICE);
+ }
+}
+EXPORT_SYMBOL(ttm_dma_tt_cache_sync_for_cpu);
+
void ttm_tt_unbind(struct ttm_tt *ttm)
{
int ret;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index a5183da3ef92..52fb709568fc 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
struct ttm_backend_func {
/**
@@ -690,6 +691,33 @@ extern int ttm_tt_swapout(struct ttm_tt *ttm,
*/
extern void ttm_tt_unpopulate(struct ttm_tt *ttm);
+/**
+ * ttm_dma_tt_cache_sync_for_device:
+ *
+ * @ttm A struct ttm_tt of the type returned by ttm_dma_tt_init.
+ * @dev A struct device representing the device to which to sync.
+ *
+ * This function will flush the CPU caches on arches where snooping in the
+ * TT is not available. On fully coherent arches this will turn into an
(almost)
+ * noop. This makes sure that data written by the CPU is visible to the device.
+ */
+extern void ttm_dma_tt_cache_sync_for_device(struct ttm_dma_tt *ttm_dma,
+struct device *dev);
+
+/**
+ * ttm_dma_tt_cache_sync_for_cpu:
+ *
+ * @ttm A struct ttm_tt of the type returned by ttm_dma_tt_init.
+ * @dev A struct device representing the device from which to sync.
+ *
+ * This function will invalidate the CPU caches on arches where snooping in the
+ * TT is not available. On fully coherent arches this will turn into an
(almost)
+ * noop. This makes sure that the CPU does not read any stale cached or
+ * prefetched data.
+ */
+extern void ttm_dma_tt_cache_sync_for_cpu(struct ttm_dma_tt *ttm_dma,
+ struct device *dev);
+
/*
* ttm_bo.c
*/
--
1.9.2
[PATCH 3/4] drm/nouveau: hook up cache sync functions
From: Lucas Stach
Signed-off-by: Lucas Stach
[acourbot at nvidia.com: make conditional and platform-friendly]
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 32
drivers/gpu/drm/nouveau/nouveau_bo.h | 20
drivers/gpu/drm/nouveau/nouveau_gem.c | 8 +++-
3 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index b6dc85c614be..0886f47e5244 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -407,6 +407,8 @@ nouveau_bo_validate(struct nouveau_bo *nvbo, bool
interruptible,
{
int ret;
+ nouveau_bo_sync_for_device(nvbo);
+
ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement,
interruptible, no_wait_gpu);
if (ret)
@@ -487,6 +489,36 @@ nouveau_bo_invalidate_caches(struct ttm_bo_device *bdev,
uint32_t flags)
return 0;
}
+#ifdef NOUVEAU_NEED_CACHE_SYNC
+void
+nouveau_bo_sync_for_cpu(struct nouveau_bo *nvbo)
+{
+ struct nouveau_device *device;
+ struct ttm_tt *ttm = nvbo->bo.ttm;
+
+ device = nouveau_dev(nouveau_bdev(ttm->bdev)->dev);
+
+ if (nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
+ ttm_dma_tt_cache_sync_for_cpu((struct ttm_dma_tt *)nvbo->bo.ttm,
+ nv_device_base(device));
+}
+
+void
+nouveau_bo_sync_for_device(struct nouveau_bo *nvbo)
+{
+ struct ttm_tt *ttm = nvbo->bo.ttm;
+
+ if (ttm && ttm->caching_state == tt_cached) {
+ struct nouveau_device *device;
+
+ device = nouveau_dev(nouveau_bdev(ttm->bdev)->dev);
+
+ ttm_dma_tt_cache_sync_for_device((struct ttm_dma_tt *)ttm,
+nv_device_base(device));
+ }
+}
+#endif
+
static int
nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
struct ttm_mem_type_manager *man)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h
b/drivers/gpu/drm/nouveau/nouveau_bo.h
index ff17c1f432fc..ead214931223 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
@@ -89,6 +89,26 @@ int nouveau_bo_vma_add(struct nouveau_bo *, struct
nouveau_vm *,
struct nouveau_vma *);
void nouveau_bo_vma_del(struct nouveau_bo *, struct nouveau_vma *);
+#if IS_ENABLED(CONFIG_ARCH_TEGRA)
+#define NOUVEAU_NEED_CACHE_SYNC
+#endif
+
+#ifdef NOUVEAU_NEED_CACHE_SYNC
+void nouveau_bo_sync_for_cpu(struct nouveau_bo *);
+void nouveau_bo_sync_for_device(struct nouveau_bo *);
+#else
+static inline void
+nouveau_bo_sync_for_cpu(struct nouveau_bo *)
+{
+}
+
+static inline void
+nouveau_bo_sync_for_device(struct nouveau_bo *)
+{
+}
+#endif
+
+
/* TODO: submit equivalent to TTM generic API upstream? */
static inline void __iomem *
nvbo_kmap_obj_iovirtual(struct nouveau_bo *nvbo)
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index c90c0dc0afe8..b7e42fdc9634 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -897,7 +897,13 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void
*data,
ret = ttm_bo_wait(&nvbo->bo, true, true, no_wait);
spin_unlock(&nvbo->bo.bdev->fence_lock);
drm_gem_object_unreference_unlocked(gem);
- return ret;
+
+ if (ret)
+ return ret;
+
+ nouveau_bo_sync_for_cpu(nvbo);
+
+ return 0;
}
int
--
1.9.2
[PATCH 4/4] drm/nouveau: introduce CPU cache flushing macro
Some architectures (e.g. ARM) need the CPU buffers to be explicitely
flushed for a memory write to take effect. Not doing so results in
synchronization issues, especially after writing to BOs.
This patch introduces a macro that flushes the caches on ARM and
translates to a no-op on other architectures, and uses it when
writing to in-memory BOs. It will also be useful for implementations of
instmem that access shared memory directly instead of going through
PRAMIN.
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/nouveau/core/os.h| 17 +
drivers/gpu/drm/nouveau/nouveau_bo.c | 8 ++--
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/core/os.h
b/drivers/gpu/drm/nouveau/core/os.h
index d0ced94ca54c..274b4460bb03 100644
--- a/drivers/gpu/drm/nouveau/core/os.h
+++ b/drivers/gpu/drm/nouveau/core/os.h
@@ -38,4 +38,21 @@
#endif /* def __BIG_ENDIAN else */
#endif /* !ioread32_native */
+#if defined(__arm__)
+
+#define nv_cpu_cache_flush_area(va, size) \
+do { \
+ phys_addr_t pa = virt_to_phys(va); \
+ __cpuc_flush_dcache_area(va, size); \
+ outer_flush_range(pa, pa + size); \
+} while (0)
+
+#else
+
+#define nv_cpu_cache_flush_area(va, size) \
+do { \
+} while (0)
+
+#endif /* defined(__arm__) */
+
#endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 0886f47e5244..b9c9729c5733 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -437,8 +437,10 @@ nouveau_bo_wr16(struct nouveau_bo *nvbo, unsigned index,
u16 val)
mem = &mem[index];
if (is_iomem)
iowrite16_native(val, (void __force __iomem *)mem);
- else
+ else {
*mem = val;
+ nv_cpu_cache_flush_area(mem, 2);
+ }
}
u32
@@ -461,8 +463,10 @@ nouveau_bo_wr32(struct nouveau_bo *nvbo, unsigned index,
u32 val)
mem = &mem[index];
if (is_iomem)
iowrite32_native(val, (void __force __iomem *)mem);
- else
+ else {
*mem = val;
+ nv_cpu_cache_flush_area(mem, 4);
+ }
}
static struct ttm_tt *
--
1.9.2
[Intel-gfx] [PATCH 1/2] drm: Refactor framebuffer creation to allow internal use
On Fri, May 16, 2014 at 03:36:47PM -0700, Matt Roper wrote:
> Refactor DRM framebuffer creation into a new function that returns a
> struct drm_framebuffer directly. The upcoming universal cursor support
> will want to create framebuffers internally to wrap cursor buffers, so
> we want to be able to share that framebuffer creation with the
> drm_mode_addfb2 ioctl handler.
>
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/drm_crtc.c | 64
> +++---
> 1 file changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b6d6c04..1a1a5f4 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2796,56 +2796,39 @@ static int framebuffer_check(const struct
> drm_mode_fb_cmd2 *r)
> return 0;
> }
>
> -/**
> - * drm_mode_addfb2 - add an FB to the graphics configuration
> - * @dev: drm device for the ioctl
> - * @data: data pointer for the ioctl
> - * @file_priv: drm file for the ioctl call
> - *
> - * Add a new FB to the specified CRTC, given a user request with format.
> This is
> - * the 2nd version of the addfb ioctl, which supports multi-planar
> framebuffers
> - * and uses fourcc codes as pixel format specifiers.
> - *
> - * Called by the user via ioctl.
> - *
> - * Returns:
> - * Zero on success, errno on failure.
> - */
> -int drm_mode_addfb2(struct drm_device *dev,
> - void *data, struct drm_file *file_priv)
> +static struct drm_framebuffer *add_framebuffer_internal(struct drm_device
> *dev,
> + void *data,
s/void/struct drm_mode_fb_cmd2/
With that fixed this is Reviewed-by: Daniel Vetter
> + struct drm_file
> *file_priv)
> {
> struct drm_mode_fb_cmd2 *r = data;
> struct drm_mode_config *config = &dev->mode_config;
> struct drm_framebuffer *fb;
> int ret;
>
> - if (!drm_core_check_feature(dev, DRIVER_MODESET))
> - return -EINVAL;
> -
> if (r->flags & ~DRM_MODE_FB_INTERLACED) {
> DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
> - return -EINVAL;
> + return ERR_PTR(-EINVAL);
> }
>
> if ((config->min_width > r->width) || (r->width > config->max_width)) {
> DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <=
> %d\n",
> r->width, config->min_width, config->max_width);
> - return -EINVAL;
> + return ERR_PTR(-EINVAL);
> }
> if ((config->min_height > r->height) || (r->height >
> config->max_height)) {
> DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <=
> %d\n",
> r->height, config->min_height, config->max_height);
> - return -EINVAL;
> + return ERR_PTR(-EINVAL);
> }
>
> ret = framebuffer_check(r);
> if (ret)
> - return ret;
> + return ERR_PTR(ret);
>
> fb = dev->mode_config.funcs->fb_create(dev, file_priv, r);
> if (IS_ERR(fb)) {
> DRM_DEBUG_KMS("could not create framebuffer\n");
> - return PTR_ERR(fb);
> + return fb;
> }
>
> mutex_lock(&file_priv->fbs_lock);
> @@ -2854,8 +2837,37 @@ int drm_mode_addfb2(struct drm_device *dev,
> DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
> mutex_unlock(&file_priv->fbs_lock);
>
> + return fb;
> +}
> +
> +/**
> + * drm_mode_addfb2 - add an FB to the graphics configuration
> + * @dev: drm device for the ioctl
> + * @data: data pointer for the ioctl
> + * @file_priv: drm file for the ioctl call
> + *
> + * Add a new FB to the specified CRTC, given a user request with format.
> This is
> + * the 2nd version of the addfb ioctl, which supports multi-planar
> framebuffers
> + * and uses fourcc codes as pixel format specifiers.
> + *
> + * Called by the user via ioctl.
> + *
> + * Returns:
> + * Zero on success, errno on failure.
> + */
> +int drm_mode_addfb2(struct drm_device *dev,
> + void *data, struct drm_file *file_priv)
> +{
> + struct drm_framebuffer *fb;
>
> - return ret;
> + if (!drm_core_check_feature(dev, DRIVER_MODESET))
> + return -EINVAL;
> +
> + fb = add_framebuffer_internal(dev, data, file_priv);
> + if (IS_ERR(fb))
> + return PTR_ERR(fb);
> +
> + return 0;
> }
>
> /**
> --
> 1.8.5.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 2/2] drm: Refactor setplane to allow internal use
On Fri, May 16, 2014 at 03:36:48PM -0700, Matt Roper wrote:
> Refactor DRM setplane code into a new setplane_internal() function that
> takes DRM objects directly as parameters rather than looking them up by
> ID. We'll use this in a future patch when we implement legacy cursor
> ioctls on top of the universal plane interface.
>
> Signed-off-by: Matt Roper
A few small things below, with those addressed this is
Reviewed-by: Daniel Vetter
Aside: Disabling a plane doesn't require a valid crtc, I think we should
have an igt testcase for this.
> ---
> drivers/gpu/drm/drm_crtc.c | 178
> +
> 1 file changed, 100 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 1a1a5f4..201c317 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2075,48 +2075,39 @@ out:
> return ret;
> }
>
> -/**
> - * drm_mode_setplane - configure a plane's configuration
> - * @dev: DRM device
> - * @data: ioctl data*
> - * @file_priv: DRM file info
> +/*
> + * setplane_internal - setplane handler for internal callers
> *
> - * Set plane configuration, including placement, fb, scaling, and other
> factors.
> - * Or pass a NULL fb to disable.
> + * Note that we assume an extra reference has already been taken on fb. If
> the
> + * update fails, this reference will be dropped before return; if it
> succeeds,
> + * the previous framebuffer (if any) will be unreferenced instead.
> *
> - * Returns:
> - * Zero on success, errno on failure.
> + * src_{x,y,w,h} are provided in 16.16 fixed point format
> */
> -int drm_mode_setplane(struct drm_device *dev, void *data,
> - struct drm_file *file_priv)
> +static int setplane_internal(struct drm_crtc *crtc,
> + struct drm_plane *plane,
> + struct drm_framebuffer *fb,
> + int32_t crtc_x, int32_t crtc_y,
> + uint32_t crtc_w, uint32_t crtc_h,
> + /* src_{x,y,w,h} values are 16.16 fixed point */
> + uint32_t src_x, uint32_t src_y,
> + uint32_t src_w, uint32_t src_h)
> {
> - struct drm_mode_set_plane *plane_req = data;
> - struct drm_mode_object *obj;
> - struct drm_plane *plane;
> - struct drm_crtc *crtc;
> - struct drm_framebuffer *fb = NULL, *old_fb = NULL;
> + struct drm_device *dev = crtc->dev;
> + struct drm_framebuffer *old_fb = NULL;
> int ret = 0;
> unsigned int fb_width, fb_height;
> int i;
>
> - if (!drm_core_check_feature(dev, DRIVER_MODESET))
> - return -EINVAL;
> -
> - /*
> - * First, find the plane, crtc, and fb objects. If not available,
> - * we don't bother to call the driver.
> - */
> - obj = drm_mode_object_find(dev, plane_req->plane_id,
> -DRM_MODE_OBJECT_PLANE);
> - if (!obj) {
> - DRM_DEBUG_KMS("Unknown plane ID %d\n",
> - plane_req->plane_id);
> - return -ENOENT;
> + /* Check whether this plane is usable on this CRTC */
> + if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
> + DRM_DEBUG_KMS("Invalid crtc for plane\n");
> + ret = -EINVAL;
> + goto out;
> }
Should we keep this check below the no fb case? For shutting off a plane
you don't need to spec a valid crtc afaik.
> - plane = obj_to_plane(obj);
>
> /* No fb means shut it down */
> - if (!plane_req->fb_id) {
> + if (!fb) {
> drm_modeset_lock_all(dev);
> old_fb = plane->fb;
> ret = plane->funcs->disable_plane(plane);
> @@ -2130,31 +2121,6 @@ int drm_mode_setplane(struct drm_device *dev, void
> *data,
> goto out;
> }
>
> - obj = drm_mode_object_find(dev, plane_req->crtc_id,
> -DRM_MODE_OBJECT_CRTC);
> - if (!obj) {
> - DRM_DEBUG_KMS("Unknown crtc ID %d\n",
> - plane_req->crtc_id);
> - ret = -ENOENT;
> - goto out;
> - }
> - crtc = obj_to_crtc(obj);
> -
> - /* Check whether this plane is usable on this CRTC */
> - if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
> - DRM_DEBUG_KMS("Invalid crtc for plane\n");
> - ret = -EINVAL;
> - goto out;
> - }
> -
> - fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
> - if (!fb) {
> - DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
> - plane_req->fb_id);
> - ret = -ENOENT;
> - goto out;
> - }
> -
> /* Check whether this plane supports the fb pixel format. */
> for (i = 0; i < plane->format_count; i++)
> if (fb->pixel_format == plane->format_types[i])
> @@ -2170,32 +2136,27 @@ int drm
[PATCH] drivers/gpu/drm/i915/intel_display: coding style fixes
On Sun, May 18, 2014 at 02:24:50AM +0200, Robin Schroer wrote:
> Fixed several switch statements, curly braces, dereference operators
> and keywords.
>
> Signed-off-by: Robin Schroer
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b9256aa..ff21924 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -962,7 +962,7 @@ bool ibx_digital_port_connected(struct drm_i915_private
> *dev_priv,
> u32 bit;
>
> if (HAS_PCH_IBX(dev_priv->dev)) {
> - switch(port->port) {
> + switch (port->port) {
> case PORT_B:
> bit = SDE_PORTB_HOTPLUG;
> break;
> @@ -976,7 +976,7 @@ bool ibx_digital_port_connected(struct drm_i915_private
> *dev_priv,
> return true;
> }
> } else {
> - switch(port->port) {
> + switch (port->port) {
> case PORT_B:
> bit = SDE_PORTB_HOTPLUG_CPT;
> break;
> @@ -3221,9 +3221,8 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
> udelay(100);
>
> /* Ironlake workaround, disable clock pointer after downing FDI */
> - if (HAS_PCH_IBX(dev)) {
> + if (HAS_PCH_IBX(dev))
> I915_WRITE(FDI_RX_CHICKEN(pipe), FDI_RX_PHASE_SYNC_POINTER_OVR);
> - }
>
> /* still set train pattern 1 */
> reg = FDI_TX_CTL(pipe);
> @@ -5844,7 +5843,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> } else {
> i9xx_update_pll(intel_crtc,
> has_reduced_clock ? &reduced_clock : NULL,
> -num_connectors);
> + num_connectors);
> }
>
> skip_dpll:
> @@ -6220,8 +6219,7 @@ static void ironlake_init_pch_refclk(struct drm_device
> *dev)
> if (intel_panel_use_ssc(dev_priv) && can_ssc) {
> DRM_DEBUG_KMS("Using SSC on eDP\n");
> val |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> - }
> - else
> + } else
> val |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> } else
> val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> @@ -8844,7 +8842,7 @@ void intel_prepare_page_flip(struct drm_device *dev,
> int plane)
> spin_unlock_irqrestore(&dev->event_lock, flags);
> }
>
> -inline static void intel_mark_page_flip_active(struct intel_crtc *intel_crtc)
> +static inline void intel_mark_page_flip_active(struct intel_crtc *intel_crtc)
> {
> /* Ensure that the work item is consistent when activating it ... */
> smp_wmb();
> @@ -9054,7 +9052,7 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
> if (ret)
> goto err;
>
> - switch(intel_crtc->plane) {
> + switch (intel_crtc->plane) {
> case PLANE_A:
> plane_bit = MI_DISPLAY_FLIP_IVB_PLANE_A;
> break;
> @@ -9333,7 +9331,7 @@ static void intel_modeset_commit_output_state(struct
> drm_device *dev)
> }
>
> static void
> -connected_sink_compute_bpp(struct intel_connector * connector,
> +connected_sink_compute_bpp(struct intel_connector *connector,
> struct intel_crtc_config *pipe_config)
> {
> int bpp = pipe_config->pipe_bpp;
> --
> 1.9.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Intel-gfx] [PATCH 1/4] drm: Support legacy cursor ioctls via universal planes when possible (v2)
I didn't spot anything else, so with the below issue addressed this patch
is Reviewed-by: Daniel Vetter
On Sat, May 17, 2014 at 12:43:04AM +0200, Daniel Vetter wrote:
> On Sat, May 17, 2014 at 12:38 AM, Matt Roper
> wrote:
> > + if (ret) {
> > + if (req->flags & DRM_MODE_CURSOR_BO)
> > + drm_mode_rmfb(dev, &fb->base.id, file_priv);
> > + return ret;
> > + }
>
> With the new refcount logic an unconditional
> drm_framebuffer_unreference should be here instead of this. I think,
> but please double-check since it's late here ;-)
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 2/5] drm/exynos: use regmap interface to set hdmiphy control bit in pmu
On 11 April 2014 07:22, Rahul Sharma wrote:
> Thanks Tomasz,
>
> This patch is not longer required after rebasing to Tomasz Stanislawski's
> Simple Phy patches.
>
Hi All,
We had further discussion on the "Simple Phy Driver" at
http://www.spinics.net/lists/linux-samsung-soc/msg31100.html. The
discussion went in favor of the use of regmap interface.
As many other drivers are already using regmap interfaces for the same
purpose, it would also be consistent to use PMU syscon handle to control
phy enable bit in the hdmi driver. I will address the pending comments from
Tomasz Figa and post the next version of this patch.
Please let me know if any concern.
Regards,
Rahul Sharma
> Regards,
> Rahul Sharma.
>
> On 10 April 2014 22:30, Tomasz Figa wrote:
>> Hi Rahul,
>>
>> On 02.04.2014 19:13, Rahul Sharma wrote:
>>>
>>> From: Rahul Sharma
>>>
>>> Hdmiphy control bit needs to be set before setting the resolution
>>> to hdmi hardware. This was handled using dummy hdmiphy clock which
>>> is removed now.
>>>
>>> PMU is already defined as system controller for exynos SoC. Registers
>>> of PMU are accessed using regmap interfaces.
>>>
>>> Devicetree binding document for hdmi is also updated.
>>>
>>> Signed-off-by: Rahul Sharma
>>> ---
>>> .../devicetree/bindings/video/exynos_hdmi.txt |2 ++
>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 17
>>> +
>>> drivers/gpu/drm/exynos/regs-hdmi.h |4
>>> 3 files changed, 23 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> index f9187a2..243a499 100644
>>> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> @@ -27,6 +27,7 @@ Required properties:
>>> "hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy" and "mout_hdmi".
>>> - ddc: phandle to the hdmi ddc node
>>> - phy: phandle to the hdmi phy node
>>> +- samsung,syscon-phandle: phandle for system controller node for PMU.
>>>
>>> Example:
>>>
>>> @@ -37,4 +38,5 @@ Example:
>>> hpd-gpio = <&gpx3 7 1>;
>>> ddc = <&hdmi_ddc_node>;
>>> phy = <&hdmi_phy_node>;
>>> + samsung,syscon-phandle = <&pmu_system_controller>;
>>> };
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index 23abfa0..47b8c06 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> @@ -36,6 +36,8 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> +#include
>>>
>>> #include
>>>
>>> @@ -194,6 +196,7 @@ struct hdmi_context {
>>> struct hdmi_resources res;
>>>
>>> int hpd_gpio;
>>> + struct regmap *pmureg;
>>>
>>> enum hdmi_type type;
>>> };
>>> @@ -1853,6 +1856,9 @@ static void hdmi_poweron(struct exynos_drm_display
>>> *display)
>>> if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
>>> DRM_DEBUG_KMS("failed to enable regulator bulk\n");
>>>
>>> + /* set pmu hdmiphy control bit to enable hdmiphy */
>>> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>>> + PMU_HDMI_PHY_ENABLE_BIT, 1);
>>> clk_prepare_enable(res->hdmi);
>>> clk_prepare_enable(res->sclk_hdmi);
>>>
>>> @@ -1879,6 +1885,10 @@ static void hdmi_poweroff(struct exynos_drm_display
>>> *display)
>>>
>>> clk_disable_unprepare(res->sclk_hdmi);
>>> clk_disable_unprepare(res->hdmi);
>>
>>
>> nit: Blank line would beautify the code a bit.
>>
>>> + /* reset pmu hdmiphy control bit to disable hdmiphy */
>>> + regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
>>> + PMU_HDMI_PHY_ENABLE_BIT, 0);
>>> +
>>> regulator_bulk_disable(res->regul_count, res->regul_bulk);
>>>
>>> pm_runtime_put_sync(hdata->dev);
>>> @@ -2128,6 +2138,13 @@ static int hdmi_probe(struct platform_device *pdev)
>>> goto err_hdmiphy;
>>> }
>>>
>>> + hdata->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
>>> + "samsung,syscon-phandle");
>>> + if (IS_ERR_OR_NULL(hdata->pmureg)) {
>>
>>
>> IS_ERR() is the correct macro to check return value of this function.
>>
>>> + DRM_ERROR("syscon regmap lookup failed.\n");
>>> + goto err_hdmiphy;
>>> + }
>>> +
>>> pm_runtime_enable(dev);
>>>
>>> hdmi_display.ctx = hdata;
>>> diff --git a/drivers/gpu/drm/exynos/regs-hdmi.h
>>> b/drivers/gpu/drm/exynos/regs-hdmi.h
>>> index ef1b3eb..9811d6f 100644
>>> --- a/drivers/gpu/drm/exynos/regs-hdmi.h
>>> +++ b/drivers/gpu/drm/exynos/regs-hdmi.h
>>> @@ -578,4 +578,8 @@
>>> #define HDMI_TG_VACT_ST4_HHDMI_TG_BASE(0x0074)
>>> #define
[RFC PATCH v1 08/16] drm/radeon: use common fence implementation for fences
op 15-05-14 18:13, Christian K?nig schreef:
> Am 15.05.2014 17:58, schrieb Maarten Lankhorst:
>> op 15-05-14 17:48, Christian K?nig schreef:
>>> Am 15.05.2014 16:18, schrieb Maarten Lankhorst:
op 15-05-14 15:19, Christian K?nig schreef:
> Am 15.05.2014 15:04, schrieb Maarten Lankhorst:
>> op 15-05-14 11:42, Christian K?nig schreef:
>>> Am 15.05.2014 11:38, schrieb Maarten Lankhorst:
op 15-05-14 11:21, Christian K?nig schreef:
> Am 15.05.2014 03:06, schrieb Maarten Lankhorst:
>> op 14-05-14 17:29, Christian K?nig schreef:
+/* did fence get signaled after we enabled the sw irq? */
+if
(atomic64_read(&fence->rdev->fence_drv[fence->ring].last_seq) >=
fence->seq) {
+ radeon_irq_kms_sw_irq_put(fence->rdev, fence->ring);
+return false;
+}
+
+fence->fence_wake.flags = 0;
+fence->fence_wake.private = NULL;
+fence->fence_wake.func = radeon_fence_check_signaled;
+ __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake);
+fence_get(f);
>>> That looks like a race condition to me. The fence needs to be added
>>> to the wait queue before the check, not after.
>>>
>>> Apart from that the whole approach looks like a really bad idea to
>>> me. How for example is lockup detection supposed to happen with
>>> this?
>> It's not a race condition because fence_queue.lock is held when this
>> function is called.
> Ah, I see. That's also the reason why you moved the wake_up_all out
> of the processing function.
Correct. :-)
>> Lockup's a bit of a weird problem, the changes wouldn't allow core
>> ttm code to handle the lockup any more,
>> but any driver specific wait code would still handle this. I did
>> this by design, because in future patches the wait
>> function may be called from outside of the radeon driver. The
>> official wait function takes a timeout parameter,
>> so lockups wouldn't be fatal if the timeout is set to something like
>> 30*HZ for example, it would still return
>> and report that the function timed out.
> Timeouts help with the detection of the lockup, but not at all with
> the handling of them.
>
> What we essentially need is a wait callback into the driver that is
> called in non atomic context without any locks held.
>
> This way we can block for the fence to become signaled with a timeout
> and can then also initiate the reset handling if necessary.
>
> The way you designed the interface now means that the driver never
> gets a chance to wait for the hardware to become idle and so never
> has the opportunity to the reset the whole thing.
You could set up a hangcheck timer like intel does, and end up with a
reliable hangcheck detection that doesn't depend on cpu waits. :-) Or
override the default wait function and restore the old behavior.
>>>
>>> Overriding the default wait function sounds better, please implement it
>>> this way.
>>>
>>> Thanks,
>>> Christian.
>>
>> Does this modification look sane?
> Adding the timeout is on my todo list for quite some time as well, so
> this part makes sense.
>
>> +static long __radeon_fence_wait(struct fence *f, bool intr, long
>> timeout)
>> +{
>> +struct radeon_fence *fence = to_radeon_fence(f);
>> +u64 target_seq[RADEON_NUM_RINGS] = {};
>> +
>> +target_seq[fence->ring] = fence->seq;
>> +return radeon_fence_wait_seq_timeout(fence->rdev, target_seq, intr,
>> timeout);
>> +}
> When this call is comming from outside the radeon driver you need to lock
> rdev->exclusive_lock here to make sure not to interfere with a possible
> reset.
Ah thanks, I'll add that.
>> .get_timeline_name = radeon_fence_get_timeline_name,
>> .enable_signaling = radeon_fence_enable_signaling,
>> .signaled = __radeon_fence_signaled,
> Do we still need those callback when we implemented the wait callback?
.get_timeline_name is used for debugging (trace events).
.signaled is the non-blocking call to check if the fence is signaled or
not.
.enable_signaling is used for adding callbacks upon fence completion, the
default 'fence_default_wait' uses it, so
when it works no separate implementation is needed unless you want to do
more than just waiting.
It's also used when fence_add_callback is called. i915 can be patched to
use it. ;-)
>>>
>>> I just meant enable_signaling, the other ones are fine with me. The problem
>>> with enable_signaling is that i
[PATCH] drm/mm: Adjust start/end for coloring first
On Mon, May 19, 2014 at 07:52:37AM +0100, Chris Wilson wrote:
> The current user of the coloring will adjust the end points of the node
> to leave a hole between disjoint memory types. This adjustment must be
> performed first or else the derived size will conflict with the
> adjustment and trigger the BUG_ON sanity checks that the node is within
> bounds.
>
> Fixes regression from
> commit 62347f9e0f81d50e9b0923ec8a192f60ab7a1801
> Author: Lauri Kasanen
> Date: Wed Apr 2 20:03:57 2014 +0300
>
> drm: Add support for two-ended allocation, v3
>
> Signed-off-by: Chris Wilson
Do we have a bugzilla for this, or why did igt not scream about this
failure?
-Daniel
> ---
> drivers/gpu/drm/drm_mm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 04a209e2b66d..911863bed9f3 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -264,12 +264,12 @@ static void drm_mm_insert_helper_range(struct
> drm_mm_node *hole_node,
> if (adj_end > end)
> adj_end = end;
>
> - if (flags & DRM_MM_CREATE_TOP)
> - adj_start = adj_end - size;
> -
> if (mm->color_adjust)
> mm->color_adjust(hole_node, color, &adj_start, &adj_end);
>
> + if (flags & DRM_MM_CREATE_TOP)
> + adj_start = adj_end - size;
> +
> if (alignment) {
> unsigned tmp = adj_start % alignment;
> if (tmp) {
> --
> 2.0.0.rc2
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/mm: Adjust start/end for coloring first
On Mon, May 19, 2014 at 10:14:27AM +0200, Daniel Vetter wrote: > On Mon, May 19, 2014 at 07:52:37AM +0100, Chris Wilson wrote: > > The current user of the coloring will adjust the end points of the node > > to leave a hole between disjoint memory types. This adjustment must be > > performed first or else the derived size will conflict with the > > adjustment and trigger the BUG_ON sanity checks that the node is within > > bounds. > > > > Fixes regression from > > commit 62347f9e0f81d50e9b0923ec8a192f60ab7a1801 > > Author: Lauri Kasanen > > Date: Wed Apr 2 20:03:57 2014 +0300 > > > > drm: Add support for two-ended allocation, v3 > > > > Signed-off-by: Chris Wilson > > Do we have a bugzilla for this, or why did igt not scream about this > failure? How would igt scream? Look at the patch and think of how many possible ways the current kernel would explode. Then think about how they are exposed to userspace. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[RFC PATCH v1 08/16] drm/radeon: use common fence implementation for fences
Am 19.05.2014 10:00, schrieb Maarten Lankhorst:
> op 15-05-14 18:13, Christian K?nig schreef:
>> Am 15.05.2014 17:58, schrieb Maarten Lankhorst:
>>> op 15-05-14 17:48, Christian K?nig schreef:
Am 15.05.2014 16:18, schrieb Maarten Lankhorst:
> op 15-05-14 15:19, Christian K?nig schreef:
>> Am 15.05.2014 15:04, schrieb Maarten Lankhorst:
>>> op 15-05-14 11:42, Christian K?nig schreef:
Am 15.05.2014 11:38, schrieb Maarten Lankhorst:
> op 15-05-14 11:21, Christian K?nig schreef:
>> Am 15.05.2014 03:06, schrieb Maarten Lankhorst:
>>> op 14-05-14 17:29, Christian K?nig schreef:
> +/* did fence get signaled after we enabled the sw
> irq? */
> +if
> (atomic64_read(&fence->rdev->fence_drv[fence->ring].last_seq)
> >= fence->seq) {
> + radeon_irq_kms_sw_irq_put(fence->rdev, fence->ring);
> +return false;
> +}
> +
> +fence->fence_wake.flags = 0;
> +fence->fence_wake.private = NULL;
> +fence->fence_wake.func = radeon_fence_check_signaled;
> + __add_wait_queue(&fence->rdev->fence_queue,
> &fence->fence_wake);
> +fence_get(f);
That looks like a race condition to me. The fence needs to
be added to the wait queue before the check, not after.
Apart from that the whole approach looks like a really bad
idea to me. How for example is lockup detection supposed to
happen with this?
>>> It's not a race condition because fence_queue.lock is held
>>> when this function is called.
>> Ah, I see. That's also the reason why you moved the
>> wake_up_all out of the processing function.
> Correct. :-)
>>> Lockup's a bit of a weird problem, the changes wouldn't
>>> allow core ttm code to handle the lockup any more,
>>> but any driver specific wait code would still handle this. I
>>> did this by design, because in future patches the wait
>>> function may be called from outside of the radeon driver.
>>> The official wait function takes a timeout parameter,
>>> so lockups wouldn't be fatal if the timeout is set to
>>> something like 30*HZ for example, it would still return
>>> and report that the function timed out.
>> Timeouts help with the detection of the lockup, but not at
>> all with the handling of them.
>>
>> What we essentially need is a wait callback into the driver
>> that is called in non atomic context without any locks held.
>>
>> This way we can block for the fence to become signaled with a
>> timeout and can then also initiate the reset handling if
>> necessary.
>>
>> The way you designed the interface now means that the driver
>> never gets a chance to wait for the hardware to become idle
>> and so never has the opportunity to the reset the whole thing.
> You could set up a hangcheck timer like intel does, and end up
> with a reliable hangcheck detection that doesn't depend on cpu
> waits. :-) Or override the default wait function and restore
> the old behavior.
Overriding the default wait function sounds better, please
implement it this way.
Thanks,
Christian.
>>>
>>> Does this modification look sane?
>> Adding the timeout is on my todo list for quite some time as
>> well, so this part makes sense.
>>
>>> +static long __radeon_fence_wait(struct fence *f, bool intr,
>>> long timeout)
>>> +{
>>> +struct radeon_fence *fence = to_radeon_fence(f);
>>> +u64 target_seq[RADEON_NUM_RINGS] = {};
>>> +
>>> +target_seq[fence->ring] = fence->seq;
>>> +return radeon_fence_wait_seq_timeout(fence->rdev,
>>> target_seq, intr, timeout);
>>> +}
>> When this call is comming from outside the radeon driver you need
>> to lock rdev->exclusive_lock here to make sure not to interfere
>> with a possible reset.
> Ah thanks, I'll add that.
>
>>> .get_timeline_name = radeon_fence_get_timeline_name,
>>> .enable_signaling = radeon_fence_enable_signaling,
>>> .signaled = __radeon_fence_signaled,
>> Do we still need those callback when we implemented the wait
>> callback?
> .get_timeline_name is used for debugging (trace events).
> .signaled is the non-blocking call to check if the fence is
> signaled or not.
> .enable_signaling is used for adding callbacks upon fence
> completion, the default 'fence_default_wait' uses it, so
> when it works no separate implementation is needed unless you want
> to do more than just waiting.
> It's
[PATCH 2/4] drm/ttm: introduce dma cache sync helpers
On Mon, May 19, 2014 at 04:10:56PM +0900, Alexandre Courbot wrote:
> From: Lucas Stach
>
> On arches with non-coherent PCI,
I guess since this applies to gk20a
> we need to flush caches ourselfes at
"ourselves". Or perhaps even reword to something like: "..., caches need
to be flushed and invalidated explicitly", since dma_sync_for_cpu() does
invalidate rather than flush.
> the appropriate places. Introduce two small helpers to make things easy
> for TTM based drivers.
>
> Signed-off-by: Lucas Stach
> Signed-off-by: Alexandre Courbot
> ---
> drivers/gpu/drm/ttm/ttm_tt.c| 25 +
> include/drm/ttm/ttm_bo_driver.h | 28
> 2 files changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
[...]
> +void ttm_dma_tt_cache_sync_for_device(struct ttm_dma_tt *ttm_dma,
> + struct device *dev)
> +{
> + int i;
This should probably be unsigned long to match the type of
ttm_dma->ttm.num_pages.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/ff67b2bc/attachment.sig>
[PATCH 3/4] drm/nouveau: hook up cache sync functions
On Mon, May 19, 2014 at 04:10:57PM +0900, Alexandre Courbot wrote:
> From: Lucas Stach
>
> Signed-off-by: Lucas Stach
> [acourbot at nvidia.com: make conditional and platform-friendly]
> Signed-off-by: Alexandre Courbot
Perhaps having a propery commit message here would be good.
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
[...]
> +#ifdef NOUVEAU_NEED_CACHE_SYNC
> +void
> +nouveau_bo_sync_for_cpu(struct nouveau_bo *nvbo)
> +{
> + struct nouveau_device *device;
> + struct ttm_tt *ttm = nvbo->bo.ttm;
> +
> + device = nouveau_dev(nouveau_bdev(ttm->bdev)->dev);
> +
> + if (nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
> + ttm_dma_tt_cache_sync_for_cpu((struct ttm_dma_tt *)nvbo->bo.ttm,
> + nv_device_base(device));
Can we be certain at this point that the struct ttm_tt is in fact a
struct ttm_dma_tt?
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h
> b/drivers/gpu/drm/nouveau/nouveau_bo.h
[...]
> +#if IS_ENABLED(CONFIG_ARCH_TEGRA)
> +#define NOUVEAU_NEED_CACHE_SYNC
> +#endif
I know I gave this as an example myself when we discussed this offline,
but I'm now thinking that this might actually be better off in Kconfig.
> +#ifdef NOUVEAU_NEED_CACHE_SYNC
> +void nouveau_bo_sync_for_cpu(struct nouveau_bo *);
> +void nouveau_bo_sync_for_device(struct nouveau_bo *);
> +#else
> +static inline void
> +nouveau_bo_sync_for_cpu(struct nouveau_bo *)
> +{
> +}
> +
> +static inline void
> +nouveau_bo_sync_for_device(struct nouveau_bo *)
> +{
> +}
> +#endif
> +
> +
There's a gratuituous blank line here.
> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c
> b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index c90c0dc0afe8..b7e42fdc9634 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -897,7 +897,13 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void
> *data,
> ret = ttm_bo_wait(&nvbo->bo, true, true, no_wait);
> spin_unlock(&nvbo->bo.bdev->fence_lock);
> drm_gem_object_unreference_unlocked(gem);
> - return ret;
> +
> + if (ret)
> + return ret;
> +
> + nouveau_bo_sync_for_cpu(nvbo);
> +
> + return 0;
> }
This could be rewritten as:
if (!ret)
nouveau_bo_sync_for_cpu(nvbo);
return ret;
Which would be slightly shorter.
On second thought, perhaps part of nouveau_gem_ioctl_cpu_prep() could be
refactored into a separate function to make this more symmetric. If we
put that in nouveau_bo.c and name it nouveau_bo_wait() for example, the
dummies can go away and both nouveau_bo_sync_for_{cpu,device}() can be
made static. I also think that's cleaner because it has both variants of
the nouveau_bo_sync_for_*() calls in the same file.
Thierry
------ next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/a9b3c9d4/attachment.sig>
[PATCH v2 5/7] drm/tegra: Convert to master/component framework
2014-05-13 17:30 GMT+02:00 Thierry Reding : > From: Thierry Reding > > Instead of the current implementation, reuse the recently introduced > master/component framework, which is equivalent in most regards. One > issue is that there is no device to bind the DRM driver to. In order > to still allow the driver to be probed, expose an interface from the > host1x device and provide an interface driver to bind to that. > > The interface driver then registers (or removes) the master that will be > used to instantiate and bind the DRM driver. Since the drm_host1x bus > implementation is now gone the dummy device instantiated by it can no > longer be used as the parent for the DRM device. However since the > parent device doesn't need to be modified, the host1x parent device that > exposes the interface can be used instead. > > Signed-off-by: Thierry Reding Is there a git tree somewhere to have a look at? I am interested in the usage of the master/component framework. thanks -- Christian Gmeiner, MSc https://soundcloud.com/christian-gmeiner
[PATCH 4/4] drm/nouveau: introduce CPU cache flushing macro
On Mon, May 19, 2014 at 04:10:58PM +0900, Alexandre Courbot wrote:
> Some architectures (e.g. ARM) need the CPU buffers to be explicitely
> flushed for a memory write to take effect. Not doing so results in
> synchronization issues, especially after writing to BOs.
It seems to me that the above is generally true for all architectures,
not just ARM.
Also: s/explicitely/explicitly/
> This patch introduces a macro that flushes the caches on ARM and
> translates to a no-op on other architectures, and uses it when
> writing to in-memory BOs. It will also be useful for implementations of
> instmem that access shared memory directly instead of going through
> PRAMIN.
Presumably instmem can access shared memory on all architectures, so
this doesn't seem like a property of the architecture but rather of the
memory pool backing the instmem.
In that case I wonder if this shouldn't be moved into an operation that
is implemented by the backing memory pool and be a noop where the cache
doesn't need explicit flushing.
> diff --git a/drivers/gpu/drm/nouveau/core/os.h
> b/drivers/gpu/drm/nouveau/core/os.h
> index d0ced94ca54c..274b4460bb03 100644
> --- a/drivers/gpu/drm/nouveau/core/os.h
> +++ b/drivers/gpu/drm/nouveau/core/os.h
> @@ -38,4 +38,21 @@
> #endif /* def __BIG_ENDIAN else */
> #endif /* !ioread32_native */
>
> +#if defined(__arm__)
> +
> +#define nv_cpu_cache_flush_area(va, size)\
> +do { \
> + phys_addr_t pa = virt_to_phys(va); \
> + __cpuc_flush_dcache_area(va, size); \
> + outer_flush_range(pa, pa + size); \
> +} while (0)
Couldn't this be a static inline function?
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
[...]
> index 0886f47e5244..b9c9729c5733 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -437,8 +437,10 @@ nouveau_bo_wr16(struct nouveau_bo *nvbo, unsigned index,
> u16 val)
> mem = &mem[index];
> if (is_iomem)
> iowrite16_native(val, (void __force __iomem *)mem);
> - else
> + else {
> *mem = val;
> + nv_cpu_cache_flush_area(mem, 2);
> + }
> }
>
> u32
> @@ -461,8 +463,10 @@ nouveau_bo_wr32(struct nouveau_bo *nvbo, unsigned index,
> u32 val)
> mem = &mem[index];
> if (is_iomem)
> iowrite32_native(val, (void __force __iomem *)mem);
> - else
> + else {
> *mem = val;
> + nv_cpu_cache_flush_area(mem, 4);
> + }
This looks rather like a sledgehammer to me. Effectively this turns nvbo
into an uncached buffer. With additional overhead of constantly flushing
caches. Wouldn't it make more sense to locate the places where these are
called and flush the cache after all the writes have completed?
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/a1b385f2/attachment.sig>
[PATCH 4/4] drm/nouveau: introduce CPU cache flushing macro
Am Montag, den 19.05.2014, 11:02 +0200 schrieb Thierry Reding:
> On Mon, May 19, 2014 at 04:10:58PM +0900, Alexandre Courbot wrote:
> > Some architectures (e.g. ARM) need the CPU buffers to be explicitely
> > flushed for a memory write to take effect. Not doing so results in
> > synchronization issues, especially after writing to BOs.
>
> It seems to me that the above is generally true for all architectures,
> not just ARM.
>
No, on PCI coherent arches, like x86 and some PowerPCs, the GPU will
snoop the CPU caches and therefore an explicit cache flush is not
required.
> Also: s/explicitely/explicitly/
>
> > This patch introduces a macro that flushes the caches on ARM and
> > translates to a no-op on other architectures, and uses it when
> > writing to in-memory BOs. It will also be useful for implementations of
> > instmem that access shared memory directly instead of going through
> > PRAMIN.
>
> Presumably instmem can access shared memory on all architectures, so
> this doesn't seem like a property of the architecture but rather of the
> memory pool backing the instmem.
>
> In that case I wonder if this shouldn't be moved into an operation that
> is implemented by the backing memory pool and be a noop where the cache
> doesn't need explicit flushing.
>
> > diff --git a/drivers/gpu/drm/nouveau/core/os.h
> > b/drivers/gpu/drm/nouveau/core/os.h
> > index d0ced94ca54c..274b4460bb03 100644
> > --- a/drivers/gpu/drm/nouveau/core/os.h
> > +++ b/drivers/gpu/drm/nouveau/core/os.h
> > @@ -38,4 +38,21 @@
> > #endif /* def __BIG_ENDIAN else */
> > #endif /* !ioread32_native */
> >
> > +#if defined(__arm__)
> > +
> > +#define nv_cpu_cache_flush_area(va, size) \
> > +do { \
> > + phys_addr_t pa = virt_to_phys(va); \
> > + __cpuc_flush_dcache_area(va, size); \
> > + outer_flush_range(pa, pa + size); \
> > +} while (0)
>
> Couldn't this be a static inline function?
>
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> [...]
> > index 0886f47e5244..b9c9729c5733 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > @@ -437,8 +437,10 @@ nouveau_bo_wr16(struct nouveau_bo *nvbo, unsigned
> > index, u16 val)
> > mem = &mem[index];
> > if (is_iomem)
> > iowrite16_native(val, (void __force __iomem *)mem);
> > - else
> > + else {
> > *mem = val;
> > + nv_cpu_cache_flush_area(mem, 2);
> > + }
> > }
> >
> > u32
> > @@ -461,8 +463,10 @@ nouveau_bo_wr32(struct nouveau_bo *nvbo, unsigned
> > index, u32 val)
> > mem = &mem[index];
> > if (is_iomem)
> > iowrite32_native(val, (void __force __iomem *)mem);
> > - else
> > + else {
> > *mem = val;
> > + nv_cpu_cache_flush_area(mem, 4);
> > + }
>
> This looks rather like a sledgehammer to me. Effectively this turns nvbo
> into an uncached buffer. With additional overhead of constantly flushing
> caches. Wouldn't it make more sense to locate the places where these are
> called and flush the cache after all the writes have completed?
>
I don't think the explicit flushing for those things makes sense. I
think it is a lot more effective to just map the BOs write-combined on
PCI non-coherent arches. This way any writes will be buffered. Reads
will be slow, but I don't think nouveau is reading back a lot from those
buffers.
Using the write-combining buffer doesn't need any additional
synchronization as it will get flushed on pushbuf kickoff anyways.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[PATCH 0/5] drm/nouveau: platform devices and GK20A probing
This patch series is the final (?) step towards the initial support of GK20A, allowing it to be probed and used (currently at a very slow speed, and for offscreen rendering only) on the Jetson TK1 and Venice 2 boards. The main piece if the first patch which adds platform devices probing support to Nouveau. There are probably lots of things that need to be discussed about it, e.g.: * The way the DRM device is created, especially with respect to the ongoing changes to the DRM framework, * The fact that the same drm_driver instance is used for the PCI and platform drivers, * Whether we should have only one platform driver capable of probing all platform devices, or one driver per GPU (in this case, where should all these drivers reside?) So there are still some rough edges, but we are getting there. :) The first patch should go through the Nouveau tree, while the 4 others are rather intended for Tegra. Alexandre Courbot (3): drm/nouveau: support for probing platform devices ARM: tegra: of: add GK20A device tree binding ARM: tegra: jetson-tk1: enable GK20A GPU Thierry Reding (2): ARM: tegra: add GK20A GPU to Tegra124 DT ARM: tegra: venice2: enable GK20A GPU .../devicetree/bindings/gpu/nvidia,gk20a.txt | 45 + arch/arm/boot/dts/tegra124-jetson-tk1.dts | 8 +- arch/arm/boot/dts/tegra124-venice2.dts | 8 +- arch/arm/boot/dts/tegra124.dtsi| 15 ++ drivers/gpu/drm/nouveau/Kconfig| 8 + drivers/gpu/drm/nouveau/Makefile | 3 + drivers/gpu/drm/nouveau/nouveau_drm.c | 33 ++-- drivers/gpu/drm/nouveau/nouveau_drm.h | 21 +++ drivers/gpu/drm/nouveau/nouveau_platform.c | 191 + 9 files changed, 315 insertions(+), 17 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt create mode 100644 drivers/gpu/drm/nouveau/nouveau_platform.c -- 1.9.2
[PATCH 1/5] drm/nouveau: support for probing platform devices
Add a platform driver for Nouveau devices declared using the device tree
or platform data. This driver currently supports GK20A on Tegra
platforms and is only compiled for these platforms if Nouveau is
enabled.
Nouveau will probe the chip type itself using the BOOT0 register, so all
this driver really needs to do is to make sure the module is powered and
its clocks active before calling nouveau_drm_platform_probe().
Heavily based on work done by Thierry Reding.
Signed-off-by: Thierry Reding
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/nouveau/Kconfig| 8 ++
drivers/gpu/drm/nouveau/Makefile | 3 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 33 ++---
drivers/gpu/drm/nouveau/nouveau_drm.h | 21
drivers/gpu/drm/nouveau/nouveau_platform.c | 191 +
5 files changed, 241 insertions(+), 15 deletions(-)
create mode 100644 drivers/gpu/drm/nouveau/nouveau_platform.c
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 637c29a33127..b8834ad55eb8 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -25,6 +25,14 @@ config DRM_NOUVEAU
help
Choose this option for open-source nVidia support.
+config NOUVEAU_PLATFORM_DRIVER
+ bool
+ depends on DRM_NOUVEAU
+ default y if ARCH_TEGRA
+ help
+Support for Nouveau platform driver, used for integrated GPUs as found
+on NVIDIA Tegra K1.
+
config NOUVEAU_DEBUG
int "Maximum debug level"
depends on DRM_NOUVEAU
diff --git a/drivers/gpu/drm/nouveau/Makefile b/drivers/gpu/drm/nouveau/Makefile
index 1aaa2ef577d9..45b17b6b1b59 100644
--- a/drivers/gpu/drm/nouveau/Makefile
+++ b/drivers/gpu/drm/nouveau/Makefile
@@ -331,6 +331,9 @@ nouveau-y += nv50_display.o
# drm/pm
nouveau-y += nouveau_hwmon.o nouveau_sysfs.o
+# platform driver
+nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
+
# other random bits
nouveau-$(CONFIG_COMPAT) += nouveau_ioc32.o
ifdef CONFIG_X86
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index ddd83756b9a2..1538d0004e6e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -359,6 +359,9 @@ nouveau_drm_load(struct drm_device *dev, unsigned long
flags)
dev->dev_private = drm;
drm->dev = dev;
+ if (dev->platformdev)
+ platform_set_drvdata(dev->platformdev, dev);
+
nouveau_client(drm)->debug = nouveau_dbgopt(nouveau_debug, "DRM");
INIT_LIST_HEAD(&drm->clients);
@@ -1003,23 +1006,19 @@ nouveau_drm_pci_driver = {
.driver.pm = &nouveau_pm_ops,
};
-int nouveau_drm_platform_probe(struct platform_device *pdev)
+int nouveau_drm_platform_device_create(struct platform_device *pdev, int
length,
+ void **pobject)
{
- struct nouveau_device *device;
- int ret;
-
- ret = nouveau_device_create(pdev, NOUVEAU_BUS_PLATFORM,
- nouveau_platform_name(pdev),
- dev_name(&pdev->dev), nouveau_config,
- nouveau_debug, &device);
-
- ret = drm_platform_init(&driver, pdev);
- if (ret) {
- nouveau_object_ref(NULL, (struct nouveau_object **)&device);
- return ret;
- }
+ return nouveau_device_create_(pdev, NOUVEAU_BUS_PLATFORM,
+ nouveau_platform_name(pdev),
+ dev_name(&pdev->dev),
+ nouveau_config, nouveau_debug, length,
+ pobject);
+}
- return ret;
+int nouveau_drm_platform_device_init(struct platform_device *pdev)
+{
+ return drm_platform_init(&driver, pdev);
}
static int __init
@@ -1035,6 +1034,8 @@ nouveau_drm_init(void)
if (!nouveau_modeset)
return 0;
+ nouveau_platform_driver_init();
+
nouveau_register_dsm_handler();
return drm_pci_init(&driver, &nouveau_drm_pci_driver);
}
@@ -1047,6 +1048,8 @@ nouveau_drm_exit(void)
drm_pci_exit(&driver, &nouveau_drm_pci_driver);
nouveau_unregister_dsm_handler();
+
+ nouveau_platform_driver_exit();
}
module_init(nouveau_drm_init);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.h
b/drivers/gpu/drm/nouveau/nouveau_drm.h
index 7efbafaf7c1d..178f8cfb148c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.h
@@ -157,6 +157,27 @@ nouveau_dev(struct drm_device *dev)
int nouveau_pmops_suspend(struct device *);
int nouveau_pmops_resume(struct device *);
+int nouveau_drm_platform_device_create(struct platform_device *, int, void **);
+int nouveau_drm_platform_device_init(struct platform_device *);
+
+#if IS_ENABLED(CONFIG_NOUVEAU_PLATFORM_DRIVER)
+
+int nouveau_platform_driver_init(void);
+void nouveau
[PATCH 2/5] ARM: tegra: of: add GK20A device tree binding
Add the device tree binding documentation for the GK20A GPU used in
Tegra K1 SoCs.
Signed-off-by: Alexandre Courbot
---
.../devicetree/bindings/gpu/nvidia,gk20a.txt | 45 ++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
new file mode 100644
index ..eda04c963af0
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -0,0 +1,45 @@
+NVIDIA GK20A Graphics Processing Unit
+
+Required properties:
+- compatible: "nvidia,-"
+ Currently recognized values:
+ - nvidia,tegra124-gk20a
+- reg: Physical base address and length of the controller's registers.
+ Must contain two entries:
+ - first entry for bar0
+ - second entry for bar1
+- interrupts: The interrupt outputs from the controller.
+- interrupt-names: Must include the following entries:
+ - stall
+ - nonstall
+- vdd-supply: regulator for supply voltage.
+- clocks: Must contain an entry for each entry in clock-names.
+ See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+ - gpu
+ - pll
+- resets: Must contain an entry for each entry in reset-names.
+ See ../reset/reset.txt for details.
+- reset-names: Must include the following entries:
+ - gpu
+
+Example:
+
+/ {
+ gpu at 0,5700 {
+ compatible = "nvidia,gk20a";
+ reg = <0x0 0x5700 0x0 0x0100>,
+ <0x0 0x5800 0x0 0x0100>;
+ interrupts = ,
+;
+ interrupt-names = "stall", "nonstall";
+ vdd-supply = <&vdd_gpu>;
+ clocks = <&tegra_car TEGRA124_CLK_GPU>,
+<&tegra_car TEGRA124_CLK_PLL_P_OUT5>;
+ clock-names = "gpu", "pll";
+ resets = <&tegra_car 184>;
+ reset-names = "gpu";
+ status = "disabled";
+ };
+
+};
--
1.9.2
[PATCH 3/5] ARM: tegra: add GK20A GPU to Tegra124 DT
From: Thierry Reding
Add the GK20A device node to Tegra124's device tree.
Signed-off-by: Thierry Reding
Signed-off-by: Alexandre Courbot
---
arch/arm/boot/dts/tegra124.dtsi | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 6e6bc4e8185c..0bccd6af81d6 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -102,6 +102,21 @@
(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
};
+ gpu at 0,5700 {
+ compatible = "nvidia,gk20a";
+ reg = <0x0 0x5700 0x0 0x0100>,
+ <0x0 0x5800 0x0 0x0100>;
+ interrupts = ,
+;
+ interrupt-names = "stall", "nonstall";
+ clocks = <&tegra_car TEGRA124_CLK_GPU>,
+<&tegra_car TEGRA124_CLK_PLL_P_OUT5>;
+ clock-names = "gpu", "pll";
+ resets = <&tegra_car 184>;
+ reset-names = "gpu";
+ status = "disabled";
+ };
+
timer at 0,60005000 {
compatible = "nvidia,tegra124-timer", "nvidia,tegra20-timer";
reg = <0x0 0x60005000 0x0 0x400>;
--
1.9.2
[PATCH 4/5] ARM: tegra: venice2: enable GK20A GPU
From: Thierry Reding
Signed-off-by: Thierry Reding
Signed-off-by: Alexandre Courbot
---
arch/arm/boot/dts/tegra124-venice2.dts | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/tegra124-venice2.dts
b/arch/arm/boot/dts/tegra124-venice2.dts
index f0bb84244025..86970ee48707 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -42,6 +42,12 @@
};
};
+ gpu at 0,5700 {
+ status = "okay";
+
+ vdd-supply = <&vdd_gpu>;
+ };
+
pinmux: pinmux at 0,7868 {
pinctrl-names = "default";
pinctrl-0 = <&pinmux_default>;
@@ -726,7 +732,7 @@
regulator-always-on;
};
- sd6 {
+ vdd_gpu: sd6 {
regulator-name = "+VDD_GPU_AP";
regulator-min-microvolt = <65>;
regulator-max-microvolt = <120>;
--
1.9.2
[PATCH 5/5] ARM: tegra: jetson-tk1: enable GK20A GPU
Signed-off-by: Alexandre Courbot
---
arch/arm/boot/dts/tegra124-jetson-tk1.dts | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index e31fb61a81d3..15a194d1277f 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -30,6 +30,12 @@
};
};
+ gpu at 0,5700 {
+ status = "okay";
+
+ vdd-supply = <&vdd_gpu>;
+ };
+
pinmux: pinmux at 0,7868 {
pinctrl-names = "default";
pinctrl-0 = <&state_default>;
@@ -1505,7 +1511,7 @@
regulator-always-on;
};
- sd6 {
+ vdd_gpu: sd6 {
regulator-name = "+VDD_GPU_AP";
regulator-min-microvolt = <65>;
regulator-max-microvolt = <120>;
--
1.9.2
[PATCH 3/4] drm/nouveau: hook up cache sync functions
Am Montag, den 19.05.2014, 16:10 +0900 schrieb Alexandre Courbot:
> From: Lucas Stach
>
> Signed-off-by: Lucas Stach
> [acourbot at nvidia.com: make conditional and platform-friendly]
> Signed-off-by: Alexandre Courbot
> ---
> drivers/gpu/drm/nouveau/nouveau_bo.c | 32
> drivers/gpu/drm/nouveau/nouveau_bo.h | 20
> drivers/gpu/drm/nouveau/nouveau_gem.c | 8 +++-
> 3 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index b6dc85c614be..0886f47e5244 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -407,6 +407,8 @@ nouveau_bo_validate(struct nouveau_bo *nvbo, bool
> interruptible,
> {
> int ret;
>
> + nouveau_bo_sync_for_device(nvbo);
> +
> ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement,
> interruptible, no_wait_gpu);
> if (ret)
> @@ -487,6 +489,36 @@ nouveau_bo_invalidate_caches(struct ttm_bo_device *bdev,
> uint32_t flags)
> return 0;
> }
>
> +#ifdef NOUVEAU_NEED_CACHE_SYNC
I don't like this ifdef at all. I know calling this functions will add a
little overhead to x86 where it isn't strictly required, but I think
it's negligible.
When I looked at them the dma_sync_single_for_[device|cpu] functions
which are called here map out to just a drain of the PCI store buffer on
x86, which should be fast enough to be done unconditionally. They won't
so any time-consuming cache synchronization on PCI coherent arches.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[PATCH 3/4] drm/nouveau: hook up cache sync functions
Am Montag, den 19.05.2014, 10:46 +0200 schrieb Thierry Reding:
> On Mon, May 19, 2014 at 04:10:57PM +0900, Alexandre Courbot wrote:
> > From: Lucas Stach
> >
> > Signed-off-by: Lucas Stach
> > [acourbot at nvidia.com: make conditional and platform-friendly]
> > Signed-off-by: Alexandre Courbot
>
> Perhaps having a propery commit message here would be good.
>
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> [...]
> > +#ifdef NOUVEAU_NEED_CACHE_SYNC
> > +void
> > +nouveau_bo_sync_for_cpu(struct nouveau_bo *nvbo)
> > +{
> > + struct nouveau_device *device;
> > + struct ttm_tt *ttm = nvbo->bo.ttm;
> > +
> > + device = nouveau_dev(nouveau_bdev(ttm->bdev)->dev);
> > +
> > + if (nvbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached)
> > + ttm_dma_tt_cache_sync_for_cpu((struct ttm_dma_tt *)nvbo->bo.ttm,
> > + nv_device_base(device));
>
> Can we be certain at this point that the struct ttm_tt is in fact a
> struct ttm_dma_tt?
Yes, for all cases except AGP, where things are mapped WC anyways (so
caching_state is always != cached) nouveau_bo_driver uses
nouveau_ttm_tt_create() as its ttm_tt_create callback. This in turn
calls nouveau_sgdma_create_ttm() which uses ttm_dma_tt_init()
unconditionally.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[RFC] drm/nouveau: disable caching for VRAM BOs on ARM
This patch is not meant to be merged, but rather to try and understand why this is needed and what a more suitable solution could be. Allowing BOs to be write-cached results in the following happening when trying to run any program on Tegra/GK20A: Unhandled fault: external abort on non-linefetch (0x1008) at 0xf0036010 ... (nouveau_bo_rd32) from [] (nouveau_fence_update+0x5c/0x80) (nouveau_fence_update) from [] (nouveau_fence_done+0x1c/0x38) (nouveau_fence_done) from [] (ttm_bo_wait+0xec/0x168) (ttm_bo_wait) from [] (nouveau_gem_ioctl_cpu_prep+0x44/0x100) (nouveau_gem_ioctl_cpu_prep) from [] (drm_ioctl+0x1d8/0x4f4) (drm_ioctl) from [] (nouveau_drm_ioctl+0x54/0x80) (nouveau_drm_ioctl) from [] (do_vfs_ioctl+0x3dc/0x5a0) (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x5c) (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x30 The offending nouveau_bo_rd32 is done over an IO-mapped BO, e.g. a BO mapped through the BAR. Any idea about the origin of this behavior? Does ARM forbid cached mappings over IO regions? Signed-off-by: Alexandre Courbot --- drivers/gpu/drm/nouveau/nouveau_bo.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 8db54a217232..9cfb8e61f5c4 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -552,7 +552,11 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type, TTM_MEMTYPE_FLAG_MAPPABLE; man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC; +#if defined(__arm__) + man->default_caching = TTM_PL_FLAG_UNCACHED; +#else man->default_caching = TTM_PL_FLAG_WC; +#endif break; case TTM_PL_TT: if (nv_device(drm->device)->card_type >= NV_50) -- 1.9.2
[Bug 66963] Rv6xx dpm problems
https://bugs.freedesktop.org/show_bug.cgi?id=66963 --- Comment #233 from Francisco Pina Martins --- @lockheed: The frosting screen forced me to do a reboot, since after it happened, X became unusably slow. I am using the stable version of mesa: 10.1.3-1 I have used mesa-git in the past (well before DPM), to squeeze a bit of extra performance from my card. But since this is no longer my main machine I have reverted to stable packages in order to lighten up the maintenance burden. Let's hope DPM starts working fine on your card too. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/71e2fc30/attachment.html>
[RFC] drm/nouveau: disable caching for VRAM BOs on ARM
Am Montag, den 19.05.2014, 18:46 +0900 schrieb Alexandre Courbot: > This patch is not meant to be merged, but rather to try and understand > why this is needed and what a more suitable solution could be. > > Allowing BOs to be write-cached results in the following happening when > trying to run any program on Tegra/GK20A: > > Unhandled fault: external abort on non-linefetch (0x1008) at 0xf0036010 > ... > (nouveau_bo_rd32) from [] (nouveau_fence_update+0x5c/0x80) > (nouveau_fence_update) from [] (nouveau_fence_done+0x1c/0x38) > (nouveau_fence_done) from [] (ttm_bo_wait+0xec/0x168) > (ttm_bo_wait) from [] (nouveau_gem_ioctl_cpu_prep+0x44/0x100) > (nouveau_gem_ioctl_cpu_prep) from [] (drm_ioctl+0x1d8/0x4f4) > (drm_ioctl) from [] (nouveau_drm_ioctl+0x54/0x80) > (nouveau_drm_ioctl) from [] (do_vfs_ioctl+0x3dc/0x5a0) > (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x5c) > (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x30 > > The offending nouveau_bo_rd32 is done over an IO-mapped BO, e.g. a BO > mapped through the BAR. > Um wait, this memory is behind an already mapped bar? I think ioremap on ARM defaults to uncached mappings, so if you want to access the memory behind this bar as WC you need to map the BAR as a whole as WC by using ioremap_wc. Regards, Lucas > Any idea about the origin of this behavior? Does ARM forbid cached > mappings over IO regions? > > Signed-off-by: Alexandre Courbot > --- > drivers/gpu/drm/nouveau/nouveau_bo.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c > b/drivers/gpu/drm/nouveau/nouveau_bo.c > index 8db54a217232..9cfb8e61f5c4 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > @@ -552,7 +552,11 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, > uint32_t type, >TTM_MEMTYPE_FLAG_MAPPABLE; > man->available_caching = TTM_PL_FLAG_UNCACHED | >TTM_PL_FLAG_WC; > +#if defined(__arm__) > + man->default_caching = TTM_PL_FLAG_UNCACHED; > +#else > man->default_caching = TTM_PL_FLAG_WC; > +#endif > break; > case TTM_PL_TT: > if (nv_device(drm->device)->card_type >= NV_50) -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ |
[PATCH 4/4] drm/nouveau: introduce CPU cache flushing macro
On Mon, May 19, 2014 at 11:22:11AM +0200, Lucas Stach wrote:
> Am Montag, den 19.05.2014, 11:02 +0200 schrieb Thierry Reding:
> > On Mon, May 19, 2014 at 04:10:58PM +0900, Alexandre Courbot wrote:
> > > Some architectures (e.g. ARM) need the CPU buffers to be explicitely
> > > flushed for a memory write to take effect. Not doing so results in
> > > synchronization issues, especially after writing to BOs.
> >
> > It seems to me that the above is generally true for all architectures,
> > not just ARM.
> >
> No, on PCI coherent arches, like x86 and some PowerPCs, the GPU will
> snoop the CPU caches and therefore an explicit cache flush is not
> required.
I was criticizing the wording in the commit message. Perhaps it could be
enhanced with what you just said.
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > [...]
> > > index 0886f47e5244..b9c9729c5733 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > @@ -437,8 +437,10 @@ nouveau_bo_wr16(struct nouveau_bo *nvbo, unsigned
> > > index, u16 val)
> > > mem = &mem[index];
> > > if (is_iomem)
> > > iowrite16_native(val, (void __force __iomem *)mem);
> > > - else
> > > + else {
> > > *mem = val;
> > > + nv_cpu_cache_flush_area(mem, 2);
> > > + }
> > > }
> > >
> > > u32
> > > @@ -461,8 +463,10 @@ nouveau_bo_wr32(struct nouveau_bo *nvbo, unsigned
> > > index, u32 val)
> > > mem = &mem[index];
> > > if (is_iomem)
> > > iowrite32_native(val, (void __force __iomem *)mem);
> > > - else
> > > + else {
> > > *mem = val;
> > > + nv_cpu_cache_flush_area(mem, 4);
> > > + }
> >
> > This looks rather like a sledgehammer to me. Effectively this turns nvbo
> > into an uncached buffer. With additional overhead of constantly flushing
> > caches. Wouldn't it make more sense to locate the places where these are
> > called and flush the cache after all the writes have completed?
> >
> I don't think the explicit flushing for those things makes sense. I
> think it is a lot more effective to just map the BOs write-combined on
> PCI non-coherent arches. This way any writes will be buffered. Reads
> will be slow, but I don't think nouveau is reading back a lot from those
> buffers.
> Using the write-combining buffer doesn't need any additional
> synchronization as it will get flushed on pushbuf kickoff anyways.
Sounds good to me.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/83d18f32/attachment.sig>
[PATCH 0/5] drm/nouveau: platform devices and GK20A probing
Am Montag, den 19.05.2014, 18:24 +0900 schrieb Alexandre Courbot: > This patch series is the final (?) step towards the initial support of GK20A, > allowing it to be probed and used (currently at a very slow speed, and for > offscreen rendering only) on the Jetson TK1 and Venice 2 boards. > This seem to still need external firmware. Any chance to get this into linux-firmware? Regards, Lucas > The main piece if the first patch which adds platform devices probing support > to Nouveau. There are probably lots of things that need to be discussed about > it, e.g.: > > * The way the DRM device is created, especially with respect to the ongoing > changes to the DRM framework, > * The fact that the same drm_driver instance is used for the PCI and platform > drivers, > * Whether we should have only one platform driver capable of probing all > platform devices, or one driver per GPU (in this case, where should all > these > drivers reside?) > > So there are still some rough edges, but we are getting there. :) > > The first patch should go through the Nouveau tree, while the 4 others are > rather intended for Tegra. > > Alexandre Courbot (3): > drm/nouveau: support for probing platform devices > ARM: tegra: of: add GK20A device tree binding > ARM: tegra: jetson-tk1: enable GK20A GPU > > Thierry Reding (2): > ARM: tegra: add GK20A GPU to Tegra124 DT > ARM: tegra: venice2: enable GK20A GPU > > .../devicetree/bindings/gpu/nvidia,gk20a.txt | 45 + > arch/arm/boot/dts/tegra124-jetson-tk1.dts | 8 +- > arch/arm/boot/dts/tegra124-venice2.dts | 8 +- > arch/arm/boot/dts/tegra124.dtsi| 15 ++ > drivers/gpu/drm/nouveau/Kconfig| 8 + > drivers/gpu/drm/nouveau/Makefile | 3 + > drivers/gpu/drm/nouveau/nouveau_drm.c | 33 ++-- > drivers/gpu/drm/nouveau/nouveau_drm.h | 21 +++ > drivers/gpu/drm/nouveau/nouveau_platform.c | 191 > + > 9 files changed, 315 insertions(+), 17 deletions(-) > create mode 100644 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt > create mode 100644 drivers/gpu/drm/nouveau/nouveau_platform.c > -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ |
[RFC] drm/nouveau: disable caching for VRAM BOs on ARM
On 05/19/2014 06:57 PM, Lucas Stach wrote: > Am Montag, den 19.05.2014, 18:46 +0900 schrieb Alexandre Courbot: >> This patch is not meant to be merged, but rather to try and understand >> why this is needed and what a more suitable solution could be. >> >> Allowing BOs to be write-cached results in the following happening when >> trying to run any program on Tegra/GK20A: >> >> Unhandled fault: external abort on non-linefetch (0x1008) at 0xf0036010 >> ... >> (nouveau_bo_rd32) from [] (nouveau_fence_update+0x5c/0x80) >> (nouveau_fence_update) from [] (nouveau_fence_done+0x1c/0x38) >> (nouveau_fence_done) from [] (ttm_bo_wait+0xec/0x168) >> (ttm_bo_wait) from [] (nouveau_gem_ioctl_cpu_prep+0x44/0x100) >> (nouveau_gem_ioctl_cpu_prep) from [] (drm_ioctl+0x1d8/0x4f4) >> (drm_ioctl) from [] (nouveau_drm_ioctl+0x54/0x80) >> (nouveau_drm_ioctl) from [] (do_vfs_ioctl+0x3dc/0x5a0) >> (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x5c) >> (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x30 >> >> The offending nouveau_bo_rd32 is done over an IO-mapped BO, e.g. a BO >> mapped through the BAR. >> > Um wait, this memory is behind an already mapped bar? I think ioremap on > ARM defaults to uncached mappings, so if you want to access the memory > behind this bar as WC you need to map the BAR as a whole as WC by using > ioremap_wc. Tried mapping the BAR using ioremap_wc(), but to no avail. On the other hand, could it be that VRAM BOs end up creating a mapping over an already-mapped region? I seem to remember that ARM might not like it...
[PATCH 0/5] drm/nouveau: platform devices and GK20A probing
On Mon, May 19, 2014 at 12:04:28PM +0200, Lucas Stach wrote: > Am Montag, den 19.05.2014, 18:24 +0900 schrieb Alexandre Courbot: > > This patch series is the final (?) step towards the initial support of > > GK20A, > > allowing it to be probed and used (currently at a very slow speed, and for > > offscreen rendering only) on the Jetson TK1 and Venice 2 boards. > > > This seem to still need external firmware. Any chance to get this into > linux-firmware? I think that should be possible. There didn't seem to be any objections last time I asked but let me clarify again and get back to you. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/3baba6dc/attachment-0001.sig>
[RFC PATCH v1 08/16] drm/radeon: use common fence implementation for fences
op 19-05-14 10:27, Christian K?nig schreef:
> Am 19.05.2014 10:00, schrieb Maarten Lankhorst:
>> op 15-05-14 18:13, Christian K?nig schreef:
>>> Am 15.05.2014 17:58, schrieb Maarten Lankhorst:
op 15-05-14 17:48, Christian K?nig schreef:
> Am 15.05.2014 16:18, schrieb Maarten Lankhorst:
>> op 15-05-14 15:19, Christian K?nig schreef:
>>> Am 15.05.2014 15:04, schrieb Maarten Lankhorst:
op 15-05-14 11:42, Christian K?nig schreef:
> Am 15.05.2014 11:38, schrieb Maarten Lankhorst:
>> op 15-05-14 11:21, Christian K?nig schreef:
>>> Am 15.05.2014 03:06, schrieb Maarten Lankhorst:
op 14-05-14 17:29, Christian K?nig schreef:
>> +/* did fence get signaled after we enabled the sw irq? */
>> +if
>> (atomic64_read(&fence->rdev->fence_drv[fence->ring].last_seq) >=
>> fence->seq) {
>> + radeon_irq_kms_sw_irq_put(fence->rdev, fence->ring);
>> +return false;
>> +}
>> +
>> +fence->fence_wake.flags = 0;
>> +fence->fence_wake.private = NULL;
>> +fence->fence_wake.func = radeon_fence_check_signaled;
>> + __add_wait_queue(&fence->rdev->fence_queue,
>> &fence->fence_wake);
>> +fence_get(f);
> That looks like a race condition to me. The fence needs to be
> added to the wait queue before the check, not after.
>
> Apart from that the whole approach looks like a really bad idea
> to me. How for example is lockup detection supposed to happen
> with this?
It's not a race condition because fence_queue.lock is held when
this function is called.
>>> Ah, I see. That's also the reason why you moved the wake_up_all out
>>> of the processing function.
>> Correct. :-)
Lockup's a bit of a weird problem, the changes wouldn't allow core
ttm code to handle the lockup any more,
but any driver specific wait code would still handle this. I did
this by design, because in future patches the wait
function may be called from outside of the radeon driver. The
official wait function takes a timeout parameter,
so lockups wouldn't be fatal if the timeout is set to something
like 30*HZ for example, it would still return
and report that the function timed out.
>>> Timeouts help with the detection of the lockup, but not at all with
>>> the handling of them.
>>>
>>> What we essentially need is a wait callback into the driver that is
>>> called in non atomic context without any locks held.
>>>
>>> This way we can block for the fence to become signaled with a
>>> timeout and can then also initiate the reset handling if necessary.
>>>
>>> The way you designed the interface now means that the driver never
>>> gets a chance to wait for the hardware to become idle and so never
>>> has the opportunity to the reset the whole thing.
>> You could set up a hangcheck timer like intel does, and end up with
>> a reliable hangcheck detection that doesn't depend on cpu waits. :-)
>> Or override the default wait function and restore the old behavior.
>
> Overriding the default wait function sounds better, please implement
> it this way.
>
> Thanks,
> Christian.
Does this modification look sane?
>>> Adding the timeout is on my todo list for quite some time as well, so
>>> this part makes sense.
>>>
+static long __radeon_fence_wait(struct fence *f, bool intr, long
timeout)
+{
+struct radeon_fence *fence = to_radeon_fence(f);
+u64 target_seq[RADEON_NUM_RINGS] = {};
+
+target_seq[fence->ring] = fence->seq;
+return radeon_fence_wait_seq_timeout(fence->rdev, target_seq,
intr, timeout);
+}
>>> When this call is comming from outside the radeon driver you need to
>>> lock rdev->exclusive_lock here to make sure not to interfere with a
>>> possible reset.
>> Ah thanks, I'll add that.
>>
.get_timeline_name = radeon_fence_get_timeline_name,
.enable_signaling = radeon_fence_enable_signaling,
.signaled = __radeon_fence_signaled,
>>> Do we still need those callback when we implemented the wait callback?
>> .get_timeline_name is used for debugging (trace events).
>> .signaled is the non-blocking call to check if the fence is signaled or
>> not.
>> .enable_signaling is used for adding callbacks upon fence completion,
>> the default 'fence_default_wait' uses it, so
>> when it works no s
[RFC] drm/nouveau: disable caching for VRAM BOs on ARM
Am Montag, den 19.05.2014, 19:06 +0900 schrieb Alexandre Courbot: > On 05/19/2014 06:57 PM, Lucas Stach wrote: > > Am Montag, den 19.05.2014, 18:46 +0900 schrieb Alexandre Courbot: > >> This patch is not meant to be merged, but rather to try and understand > >> why this is needed and what a more suitable solution could be. > >> > >> Allowing BOs to be write-cached results in the following happening when > >> trying to run any program on Tegra/GK20A: > >> > >> Unhandled fault: external abort on non-linefetch (0x1008) at 0xf0036010 > >> ... > >> (nouveau_bo_rd32) from [] (nouveau_fence_update+0x5c/0x80) > >> (nouveau_fence_update) from [] (nouveau_fence_done+0x1c/0x38) > >> (nouveau_fence_done) from [] (ttm_bo_wait+0xec/0x168) > >> (ttm_bo_wait) from [] (nouveau_gem_ioctl_cpu_prep+0x44/0x100) > >> (nouveau_gem_ioctl_cpu_prep) from [] (drm_ioctl+0x1d8/0x4f4) > >> (drm_ioctl) from [] (nouveau_drm_ioctl+0x54/0x80) > >> (nouveau_drm_ioctl) from [] (do_vfs_ioctl+0x3dc/0x5a0) > >> (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x5c) > >> (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x30 > >> > >> The offending nouveau_bo_rd32 is done over an IO-mapped BO, e.g. a BO > >> mapped through the BAR. > >> > > Um wait, this memory is behind an already mapped bar? I think ioremap on > > ARM defaults to uncached mappings, so if you want to access the memory > > behind this bar as WC you need to map the BAR as a whole as WC by using > > ioremap_wc. > > Tried mapping the BAR using ioremap_wc(), but to no avail. On the other > hand, could it be that VRAM BOs end up creating a mapping over an > already-mapped region? I seem to remember that ARM might not like it... Multiple mapping are generally allowed, as long as they have the same caching state. It's conflicting mappings (uncached vs cached, or cached vs wc), that are documented to yield undefined results. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ |
[PATCH] drm/mm: Adjust start/end for coloring first
On Mon, May 19, 2014 at 09:21:23AM +0100, Chris Wilson wrote: > On Mon, May 19, 2014 at 10:14:27AM +0200, Daniel Vetter wrote: > > On Mon, May 19, 2014 at 07:52:37AM +0100, Chris Wilson wrote: > > > The current user of the coloring will adjust the end points of the node > > > to leave a hole between disjoint memory types. This adjustment must be > > > performed first or else the derived size will conflict with the > > > adjustment and trigger the BUG_ON sanity checks that the node is within > > > bounds. > > > > > > Fixes regression from > > > commit 62347f9e0f81d50e9b0923ec8a192f60ab7a1801 > > > Author: Lauri Kasanen > > > Date: Wed Apr 2 20:03:57 2014 +0300 > > > > > > drm: Add support for two-ended allocation, v3 > > > > > > Signed-off-by: Chris Wilson > > > > Do we have a bugzilla for this, or why did igt not scream about this > > failure? > > How would igt scream? Look at the patch and think of how many possible > ways the current kernel would explode. Then think about how they are > exposed to userspace. Yeah, reading the patch helps ;-) So this only collides with the top-down vs. bottum-up separation patch for cache levels you've posted somewhere. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/mm: Adjust start/end for coloring first
On Mon, May 19, 2014 at 07:52:37AM +0100, Chris Wilson wrote:
> The current user of the coloring will adjust the end points of the node
> to leave a hole between disjoint memory types. This adjustment must be
> performed first or else the derived size will conflict with the
> adjustment and trigger the BUG_ON sanity checks that the node is within
> bounds.
>
> Fixes regression from
> commit 62347f9e0f81d50e9b0923ec8a192f60ab7a1801
> Author: Lauri Kasanen
> Date: Wed Apr 2 20:03:57 2014 +0300
>
> drm: Add support for two-ended allocation, v3
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/drm_mm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 04a209e2b66d..911863bed9f3 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -264,12 +264,12 @@ static void drm_mm_insert_helper_range(struct
> drm_mm_node *hole_node,
> if (adj_end > end)
> adj_end = end;
>
> - if (flags & DRM_MM_CREATE_TOP)
> - adj_start = adj_end - size;
> -
> if (mm->color_adjust)
> mm->color_adjust(hole_node, color, &adj_start, &adj_end);
>
> + if (flags & DRM_MM_CREATE_TOP)
> + adj_start = adj_end - size;
With this we still don't handle interactions with alignment correctly. I
guess while we're at this we might as well fix that, too.
-Daniel
> +
> if (alignment) {
> unsigned tmp = adj_start % alignment;
> if (tmp) {
> --
> 2.0.0.rc2
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Nouveau] [PATCH 4/4] drm/nouveau: introduce CPU cache flushing macro
On Mon, May 19, 2014 at 12:03:17PM +0200, Thierry Reding wrote: > On Mon, May 19, 2014 at 11:22:11AM +0200, Lucas Stach wrote: > > Am Montag, den 19.05.2014, 11:02 +0200 schrieb Thierry Reding: > > > On Mon, May 19, 2014 at 04:10:58PM +0900, Alexandre Courbot wrote: > > > > Some architectures (e.g. ARM) need the CPU buffers to be explicitely > > > > flushed for a memory write to take effect. Not doing so results in > > > > synchronization issues, especially after writing to BOs. > > > > > > It seems to me that the above is generally true for all architectures, > > > not just ARM. > > > > > No, on PCI coherent arches, like x86 and some PowerPCs, the GPU will > > snoop the CPU caches and therefore an explicit cache flush is not > > required. > > I was criticizing the wording in the commit message. Perhaps it could be > enhanced with what you just said. Shouldn't this be done in the dma mapping layer? I know that i915 does all the cpu cache flushing itself, but that's because the x86 dma layer refuses to believe that there are non-coherent platforms on x86. But on arm it can cope. This is somewhat important for dma-buf buffer sharing since if the cpu cache control is done in drivers you must do double-flushing on shared buffers. Atm you have to do that anyway, but at least this would make it easier. The other problem is that ttm reinvents half of the dma mapping functions. Just my 2 cents. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 75931] 3.13.11 kernel breaks Xorg on AMD APU graphics
https://bugzilla.kernel.org/show_bug.cgi?id=75931 Alan changed: What|Removed |Added CC||alan at lxorguk.ukuu.org.uk Component|Video(Other)|Video(DRI - non Intel) Assignee|drivers_video-other at kernel- |drivers_video-dri at kernel-bu |bugs.osdl.org |gs.osdl.org -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 75921] Horizontal lines dancing all around the screen in Xorg
https://bugzilla.kernel.org/show_bug.cgi?id=75921 Alan changed: What|Removed |Added CC||alan at lxorguk.ukuu.org.uk Component|Video(Other)|Video(DRI - non Intel) Assignee|drivers_video-other at kernel- |drivers_video-dri at kernel-bu |bugs.osdl.org |gs.osdl.org -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 0/8] drm/exynos/ipp: image post processing improvements
This set of independent patches contains various improvement and fixes for exynos_drm ipp framework and drivers. The patchset is based on drm-exynos/exynos-drm-next branch. Regards Andrzej Andrzej Hajda (8): drm/exynos/ipp: fix get_property IOCTL drm/exynos/ipp: correct ipp_id field initialization drm/exynos/ipp: simplify property list allocation drm/exynos/fimc: simplify pre-scaler ratio calculation drm/exynos/fimc: simplify irq masking function drm/exynos/fimc: replace hw access macros with functions drm/exynos/fimc: replace mutex by spinlock drm/exynos/fimc: simplify and rename fimc_dst_get_buf_seq drivers/gpu/drm/exynos/exynos_drm_fimc.c| 427 drivers/gpu/drm/exynos/exynos_drm_gsc.c | 10 +- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 16 +- drivers/gpu/drm/exynos/exynos_drm_ipp.h | 3 +- drivers/gpu/drm/exynos/exynos_drm_rotator.c | 8 +- 5 files changed, 196 insertions(+), 268 deletions(-) -- 1.9.1
[PATCH 1/8] drm/exynos/ipp: fix get_property IOCTL
Due to incorrect assignment in EXYNOS_IPP_GET_PROPERTY IOCTL handler this IOCTL did not work at all. The patch fixes it. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index 09312b8..5aaf21f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -346,7 +346,7 @@ int exynos_drm_ipp_get_property(struct drm_device *drm_dev, void *data, return PTR_ERR(ippdrv); } - prop_list = ippdrv->prop_list; + *prop_list = *ippdrv->prop_list; } return 0; -- 1.9.1
[PATCH 2/8] drm/exynos/ipp: correct ipp_id field initialization
prop_list.ipp_id field is not initialized properly.
The patch fixes it, additionally it removes redundant field from ippdrv.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 14 ++
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 1 -
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 5aaf21f..e6ef415 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1699,23 +1699,21 @@ static int ipp_subdrv_probe(struct drm_device *drm_dev,
struct device *dev)
/* get ipp driver entry */
list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) {
+ u32 ipp_id;
+
ippdrv->drm_dev = drm_dev;
ret = ipp_create_id(&ctx->ipp_idr, &ctx->ipp_lock, ippdrv,
- &ippdrv->ipp_id);
- if (ret) {
+ &ipp_id);
+ if (ret || ipp_id == 0) {
DRM_ERROR("failed to create id.\n");
goto err_idr;
}
DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]ipp_id[%d]\n",
- count++, (int)ippdrv, ippdrv->ipp_id);
+ count++, (int)ippdrv, ipp_id);
- if (ippdrv->ipp_id == 0) {
- DRM_ERROR("failed to get ipp_id[%d]\n",
- ippdrv->ipp_id);
- goto err_idr;
- }
+ ippdrv->prop_list->ipp_id = ipp_id;
/* store parent device for node */
ippdrv->parent_dev = dev;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
index ab1634b..4aa71b2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
@@ -160,7 +160,6 @@ struct exynos_drm_ippdrv {
struct device *parent_dev;
struct device *dev;
struct drm_device *drm_dev;
- u32 ipp_id;
booldedicated;
struct exynos_drm_ipp_ops *ops[EXYNOS_DRM_OPS_MAX];
struct workqueue_struct *event_workq;
--
1.9.1
[PATCH 3/8] drm/exynos/ipp: simplify property list allocation
prop_list is always allocated, so instead of allocating it dynamically
the pointer can be replaced by the structure itself.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c| 10 ++
drivers/gpu/drm/exynos/exynos_drm_gsc.c | 10 ++
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 4 ++--
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 +-
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 8 +---
5 files changed, 8 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 30d76b2..5060f8a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1342,11 +1342,7 @@ static irqreturn_t fimc_irq_handler(int irq, void
*dev_id)
static int fimc_init_prop_list(struct exynos_drm_ippdrv *ippdrv)
{
- struct drm_exynos_ipp_prop_list *prop_list;
-
- prop_list = devm_kzalloc(ippdrv->dev, sizeof(*prop_list), GFP_KERNEL);
- if (!prop_list)
- return -ENOMEM;
+ struct drm_exynos_ipp_prop_list *prop_list = &ippdrv->prop_list;
prop_list->version = 1;
prop_list->writeback = 1;
@@ -1371,8 +1367,6 @@ static int fimc_init_prop_list(struct exynos_drm_ippdrv
*ippdrv)
prop_list->scale_min.hsize = FIMC_SCALE_MIN;
prop_list->scale_min.vsize = FIMC_SCALE_MIN;
- ippdrv->prop_list = prop_list;
-
return 0;
}
@@ -1395,7 +1389,7 @@ static int fimc_ippdrv_check_property(struct device *dev,
{
struct fimc_context *ctx = get_fimc_context(dev);
struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
- struct drm_exynos_ipp_prop_list *pp = ippdrv->prop_list;
+ struct drm_exynos_ipp_prop_list *pp = &ippdrv->prop_list;
struct drm_exynos_ipp_config *config;
struct drm_exynos_pos *pos;
struct drm_exynos_sz *sz;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index fa75059..9e3ff16 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -1335,11 +1335,7 @@ static irqreturn_t gsc_irq_handler(int irq, void *dev_id)
static int gsc_init_prop_list(struct exynos_drm_ippdrv *ippdrv)
{
- struct drm_exynos_ipp_prop_list *prop_list;
-
- prop_list = devm_kzalloc(ippdrv->dev, sizeof(*prop_list), GFP_KERNEL);
- if (!prop_list)
- return -ENOMEM;
+ struct drm_exynos_ipp_prop_list *prop_list = &ippdrv->prop_list;
prop_list->version = 1;
prop_list->writeback = 1;
@@ -1363,8 +1359,6 @@ static int gsc_init_prop_list(struct exynos_drm_ippdrv
*ippdrv)
prop_list->scale_min.hsize = GSC_SCALE_MIN;
prop_list->scale_min.vsize = GSC_SCALE_MIN;
- ippdrv->prop_list = prop_list;
-
return 0;
}
@@ -1387,7 +1381,7 @@ static int gsc_ippdrv_check_property(struct device *dev,
{
struct gsc_context *ctx = get_gsc_context(dev);
struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
- struct drm_exynos_ipp_prop_list *pp = ippdrv->prop_list;
+ struct drm_exynos_ipp_prop_list *pp = &ippdrv->prop_list;
struct drm_exynos_ipp_config *config;
struct drm_exynos_pos *pos;
struct drm_exynos_sz *sz;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index e6ef415..bf71d97 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -346,7 +346,7 @@ int exynos_drm_ipp_get_property(struct drm_device *drm_dev,
void *data,
return PTR_ERR(ippdrv);
}
- *prop_list = *ippdrv->prop_list;
+ *prop_list = ippdrv->prop_list;
}
return 0;
@@ -1713,7 +1713,7 @@ static int ipp_subdrv_probe(struct drm_device *drm_dev,
struct device *dev)
DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]ipp_id[%d]\n",
count++, (int)ippdrv, ipp_id);
- ippdrv->prop_list->ipp_id = ipp_id;
+ ippdrv->prop_list.ipp_id = ipp_id;
/* store parent device for node */
ippdrv->parent_dev = dev;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
index 4aa71b2..eea4db3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
@@ -165,7 +165,7 @@ struct exynos_drm_ippdrv {
struct workqueue_struct *event_workq;
struct drm_exynos_ipp_cmd_node *c_node;
struct list_headcmd_list;
- struct drm_exynos_ipp_prop_list *prop_list;
+ struct drm_exynos_ipp_prop_list prop_list;
int (*check_property)(struct device *dev,
struct drm_exynos_ipp_property *property);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
index c675427..f01fbb6 100644
--- a/drivers/gpu/drm/exyno
[PATCH 4/8] drm/exynos/fimc: simplify pre-scaler ratio calculation
The patch replaces dedicated function for scaling ratio
calculation by fls calls.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 56
1 file changed, 13 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 5060f8a..d40b7fb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -957,43 +957,13 @@ static int fimc_dst_set_transf(struct device *dev,
return 0;
}
-static int fimc_get_ratio_shift(u32 src, u32 dst, u32 *ratio, u32 *shift)
-{
- DRM_DEBUG_KMS("src[%d]dst[%d]\n", src, dst);
-
- if (src >= dst * 64) {
- DRM_ERROR("failed to make ratio and shift.\n");
- return -EINVAL;
- } else if (src >= dst * 32) {
- *ratio = 32;
- *shift = 5;
- } else if (src >= dst * 16) {
- *ratio = 16;
- *shift = 4;
- } else if (src >= dst * 8) {
- *ratio = 8;
- *shift = 3;
- } else if (src >= dst * 4) {
- *ratio = 4;
- *shift = 2;
- } else if (src >= dst * 2) {
- *ratio = 2;
- *shift = 1;
- } else {
- *ratio = 1;
- *shift = 0;
- }
-
- return 0;
-}
-
static int fimc_set_prescaler(struct fimc_context *ctx, struct fimc_scaler *sc,
struct drm_exynos_pos *src, struct drm_exynos_pos *dst)
{
struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
u32 cfg, cfg_ext, shfactor;
u32 pre_dst_width, pre_dst_height;
- u32 pre_hratio, hfactor, pre_vratio, vfactor;
+ u32 hfactor, vfactor;
int ret = 0;
u32 src_w, src_h, dst_w, dst_h;
@@ -1014,24 +984,24 @@ static int fimc_set_prescaler(struct fimc_context *ctx,
struct fimc_scaler *sc,
dst_h = dst->h;
}
- ret = fimc_get_ratio_shift(src_w, dst_w, &pre_hratio, &hfactor);
- if (ret) {
+ /* fimc_ippdrv_check_property assures that dividers are not null */
+ hfactor = fls(src_w / dst_w / 2);
+ if (hfactor > FIMC_SHFACTOR / 2) {
dev_err(ippdrv->dev, "failed to get ratio horizontal.\n");
- return ret;
+ return -EINVAL;
}
- ret = fimc_get_ratio_shift(src_h, dst_h, &pre_vratio, &vfactor);
- if (ret) {
+ vfactor = fls(src_h / dst_h / 2);
+ if (vfactor > FIMC_SHFACTOR / 2) {
dev_err(ippdrv->dev, "failed to get ratio vertical.\n");
- return ret;
+ return -EINVAL;
}
- pre_dst_width = src_w / pre_hratio;
- pre_dst_height = src_h / pre_vratio;
+ pre_dst_width = src_w >> hfactor;
+ pre_dst_height = src_h >> vfactor;
DRM_DEBUG_KMS("pre_dst_width[%d]pre_dst_height[%d]\n",
pre_dst_width, pre_dst_height);
- DRM_DEBUG_KMS("pre_hratio[%d]hfactor[%d]pre_vratio[%d]vfactor[%d]\n",
- pre_hratio, hfactor, pre_vratio, vfactor);
+ DRM_DEBUG_KMS("hfactor[%d]vfactor[%d]\n", hfactor, vfactor);
sc->hratio = (src_w << 14) / (dst_w << hfactor);
sc->vratio = (src_h << 14) / (dst_h << vfactor);
@@ -1044,8 +1014,8 @@ static int fimc_set_prescaler(struct fimc_context *ctx,
struct fimc_scaler *sc,
DRM_DEBUG_KMS("shfactor[%d]\n", shfactor);
cfg = (EXYNOS_CISCPRERATIO_SHFACTOR(shfactor) |
- EXYNOS_CISCPRERATIO_PREHORRATIO(pre_hratio) |
- EXYNOS_CISCPRERATIO_PREVERRATIO(pre_vratio));
+ EXYNOS_CISCPRERATIO_PREHORRATIO(1 << hfactor) |
+ EXYNOS_CISCPRERATIO_PREVERRATIO(1 << vfactor));
fimc_write(cfg, EXYNOS_CISCPRERATIO);
cfg = (EXYNOS_CISCPREDST_PREDSTWIDTH(pre_dst_width) |
--
1.9.1
[PATCH 7/8] drm/exynos/fimc: replace mutex by spinlock
Function fimc_dst_set_buf_seq is called by irq handler
so it should not use mutexes. This patch fixes it.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 7a66f18..9860d38 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -158,7 +159,7 @@ struct fimc_context {
struct exynos_drm_ippdrvippdrv;
struct resource *regs_res;
void __iomem*regs;
- struct mutexlock;
+ spinlock_t lock;
struct clk *clocks[FIMC_CLKS_MAX];
u32 clk_frequency;
struct regmap *sysreg;
@@ -1146,10 +1147,11 @@ static int fimc_dst_set_buf_seq(struct fimc_context
*ctx, u32 buf_id,
u32 cfg;
u32 mask = 0x0001 << buf_id;
int ret = 0;
+ unsigned long flags;
DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
- mutex_lock(&ctx->lock);
+ spin_lock_irqsave(&ctx->lock, flags);
/* mask register set */
cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
@@ -1183,7 +1185,7 @@ static int fimc_dst_set_buf_seq(struct fimc_context *ctx,
u32 buf_id,
fimc_mask_irq(ctx, false);
err_unlock:
- mutex_unlock(&ctx->lock);
+ spin_unlock_irqrestore(&ctx->lock, flags);
return ret;
}
@@ -1794,7 +1796,7 @@ static int fimc_probe(struct platform_device *pdev)
DRM_DEBUG_KMS("id[%d]ippdrv[0x%x]\n", ctx->id, (int)ippdrv);
- mutex_init(&ctx->lock);
+ spin_lock_init(&ctx->lock);
platform_set_drvdata(pdev, ctx);
pm_runtime_set_active(dev);
@@ -1825,7 +1827,6 @@ static int fimc_remove(struct platform_device *pdev)
struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
exynos_drm_ippdrv_unregister(ippdrv);
- mutex_destroy(&ctx->lock);
fimc_put_clocks(ctx);
pm_runtime_set_suspended(dev);
--
1.9.1
[PATCH 5/8] drm/exynos/fimc: simplify irq masking function
The name fimc_handle_irq suggests it is irq handler, but the function
is for irq mask configuration. The patch renames the function to
fimc_mask_irq and removes unused arguments.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index d40b7fb..409775f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -290,25 +290,18 @@ static void fimc_handle_jpeg(struct fimc_context *ctx,
bool enable)
fimc_write(cfg, EXYNOS_CIGCTRL);
}
-static void fimc_handle_irq(struct fimc_context *ctx, bool enable,
- bool overflow, bool level)
+static void fimc_mask_irq(struct fimc_context *ctx, bool enable)
{
u32 cfg;
- DRM_DEBUG_KMS("enable[%d]overflow[%d]level[%d]\n",
- enable, overflow, level);
+ DRM_DEBUG_KMS("enable[%d]\n", enable);
cfg = fimc_read(EXYNOS_CIGCTRL);
if (enable) {
- cfg &= ~(EXYNOS_CIGCTRL_IRQ_OVFEN | EXYNOS_CIGCTRL_IRQ_LEVEL);
- cfg |= EXYNOS_CIGCTRL_IRQ_ENABLE;
- if (overflow)
- cfg |= EXYNOS_CIGCTRL_IRQ_OVFEN;
- if (level)
- cfg |= EXYNOS_CIGCTRL_IRQ_LEVEL;
+ cfg &= ~EXYNOS_CIGCTRL_IRQ_OVFEN;
+ cfg |= EXYNOS_CIGCTRL_IRQ_ENABLE | EXYNOS_CIGCTRL_IRQ_LEVEL;
} else
- cfg &= ~(EXYNOS_CIGCTRL_IRQ_OVFEN | EXYNOS_CIGCTRL_IRQ_ENABLE);
-
+ cfg &= ~EXYNOS_CIGCTRL_IRQ_ENABLE;
fimc_write(cfg, EXYNOS_CIGCTRL);
}
@@ -1180,12 +1173,12 @@ static int fimc_dst_set_buf_seq(struct fimc_context
*ctx, u32 buf_id,
/* interrupt enable */
if (buf_type == IPP_BUF_ENQUEUE &&
fimc_dst_get_buf_seq(ctx) >= FIMC_BUF_START)
- fimc_handle_irq(ctx, true, false, true);
+ fimc_mask_irq(ctx, true);
/* interrupt disable */
if (buf_type == IPP_BUF_DEQUEUE &&
fimc_dst_get_buf_seq(ctx) <= FIMC_BUF_STOP)
- fimc_handle_irq(ctx, false, false, true);
+ fimc_mask_irq(ctx, false);
err_unlock:
mutex_unlock(&ctx->lock);
@@ -1520,7 +1513,7 @@ static int fimc_ippdrv_start(struct device *dev, enum
drm_exynos_ipp_cmd cmd)
property = &c_node->property;
- fimc_handle_irq(ctx, true, false, true);
+ fimc_mask_irq(ctx, true);
for_each_ipp_ops(i) {
config = &property->config[i];
@@ -1639,7 +1632,7 @@ static void fimc_ippdrv_stop(struct device *dev, enum
drm_exynos_ipp_cmd cmd)
break;
}
- fimc_handle_irq(ctx, false, false, true);
+ fimc_mask_irq(ctx, false);
/* reset sequence */
fimc_write(0x0, EXYNOS_CIFCNTSEQ);
--
1.9.1
[PATCH 6/8] drm/exynos/fimc: replace hw access macros with functions
HW access macros implicitly depended on presence of ctx local variable.
This patch replaces them with C functions.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 311 +++
1 file changed, 150 insertions(+), 161 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 409775f..7a66f18 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -69,9 +69,6 @@
#define get_fimc_context(dev) platform_get_drvdata(to_platform_device(dev))
#define get_ctx_from_ippdrv(ippdrv)container_of(ippdrv,\
struct fimc_context, ippdrv);
-#define fimc_read(offset) readl(ctx->regs + (offset))
-#define fimc_write(cfg, offset)writel(cfg, ctx->regs + (offset))
-
enum fimc_wb {
FIMC_WB_NONE,
FIMC_WB_A,
@@ -172,39 +169,53 @@ struct fimc_context {
boolsuspended;
};
+static u32 fimc_read(struct fimc_context *ctx, u32 reg)
+{
+ return readl(ctx->regs + reg);
+}
+
+static void fimc_write(struct fimc_context *ctx, u32 val, u32 reg)
+{
+ writel(val, ctx->regs + reg);
+}
+
+static void fimc_set_bits(struct fimc_context *ctx, u32 reg, u32 bits)
+{
+ void __iomem *r = ctx->regs + reg;
+
+ writel(readl(r) | bits, r);
+}
+
+static void fimc_clear_bits(struct fimc_context *ctx, u32 reg, u32 bits)
+{
+ void __iomem *r = ctx->regs + reg;
+
+ writel(readl(r) & ~bits, r);
+}
+
static void fimc_sw_reset(struct fimc_context *ctx)
{
u32 cfg;
/* stop dma operation */
- cfg = fimc_read(EXYNOS_CISTATUS);
- if (EXYNOS_CISTATUS_GET_ENVID_STATUS(cfg)) {
- cfg = fimc_read(EXYNOS_MSCTRL);
- cfg &= ~EXYNOS_MSCTRL_ENVID;
- fimc_write(cfg, EXYNOS_MSCTRL);
- }
+ cfg = fimc_read(ctx, EXYNOS_CISTATUS);
+ if (EXYNOS_CISTATUS_GET_ENVID_STATUS(cfg))
+ fimc_clear_bits(ctx, EXYNOS_MSCTRL, EXYNOS_MSCTRL_ENVID);
- cfg = fimc_read(EXYNOS_CISRCFMT);
- cfg |= EXYNOS_CISRCFMT_ITU601_8BIT;
- fimc_write(cfg, EXYNOS_CISRCFMT);
+ fimc_set_bits(ctx, EXYNOS_CISRCFMT, EXYNOS_CISRCFMT_ITU601_8BIT);
/* disable image capture */
- cfg = fimc_read(EXYNOS_CIIMGCPT);
- cfg &= ~(EXYNOS_CIIMGCPT_IMGCPTEN_SC | EXYNOS_CIIMGCPT_IMGCPTEN);
- fimc_write(cfg, EXYNOS_CIIMGCPT);
+ fimc_clear_bits(ctx, EXYNOS_CIIMGCPT,
+ EXYNOS_CIIMGCPT_IMGCPTEN_SC | EXYNOS_CIIMGCPT_IMGCPTEN);
/* s/w reset */
- cfg = fimc_read(EXYNOS_CIGCTRL);
- cfg |= (EXYNOS_CIGCTRL_SWRST);
- fimc_write(cfg, EXYNOS_CIGCTRL);
+ fimc_set_bits(ctx, EXYNOS_CIGCTRL, EXYNOS_CIGCTRL_SWRST);
/* s/w reset complete */
- cfg = fimc_read(EXYNOS_CIGCTRL);
- cfg &= ~EXYNOS_CIGCTRL_SWRST;
- fimc_write(cfg, EXYNOS_CIGCTRL);
+ fimc_clear_bits(ctx, EXYNOS_CIGCTRL, EXYNOS_CIGCTRL_SWRST);
/* reset sequence */
- fimc_write(0x0, EXYNOS_CIFCNTSEQ);
+ fimc_write(ctx, 0x0, EXYNOS_CIFCNTSEQ);
}
static int fimc_set_camblk_fimd0_wb(struct fimc_context *ctx)
@@ -220,7 +231,7 @@ static void fimc_set_type_ctrl(struct fimc_context *ctx,
enum fimc_wb wb)
DRM_DEBUG_KMS("wb[%d]\n", wb);
- cfg = fimc_read(EXYNOS_CIGCTRL);
+ cfg = fimc_read(ctx, EXYNOS_CIGCTRL);
cfg &= ~(EXYNOS_CIGCTRL_TESTPATTERN_MASK |
EXYNOS_CIGCTRL_SELCAM_ITU_MASK |
EXYNOS_CIGCTRL_SELCAM_MIPI_MASK |
@@ -246,7 +257,7 @@ static void fimc_set_type_ctrl(struct fimc_context *ctx,
enum fimc_wb wb)
break;
}
- fimc_write(cfg, EXYNOS_CIGCTRL);
+ fimc_write(ctx, cfg, EXYNOS_CIGCTRL);
}
static void fimc_set_polarity(struct fimc_context *ctx,
@@ -259,7 +270,7 @@ static void fimc_set_polarity(struct fimc_context *ctx,
DRM_DEBUG_KMS("inv_href[%d]inv_hsync[%d]\n",
pol->inv_href, pol->inv_hsync);
- cfg = fimc_read(EXYNOS_CIGCTRL);
+ cfg = fimc_read(ctx, EXYNOS_CIGCTRL);
cfg &= ~(EXYNOS_CIGCTRL_INVPOLPCLK | EXYNOS_CIGCTRL_INVPOLVSYNC |
EXYNOS_CIGCTRL_INVPOLHREF | EXYNOS_CIGCTRL_INVPOLHSYNC);
@@ -272,7 +283,7 @@ static void fimc_set_polarity(struct fimc_context *ctx,
if (pol->inv_hsync)
cfg |= EXYNOS_CIGCTRL_INVPOLHSYNC;
- fimc_write(cfg, EXYNOS_CIGCTRL);
+ fimc_write(ctx, cfg, EXYNOS_CIGCTRL);
}
static void fimc_handle_jpeg(struct fimc_context *ctx, bool enable)
@@ -281,13 +292,13 @@ static void fimc_handle_jpeg(struct fimc_context *ctx,
bool enable)
DRM_DEBUG_KMS("enable[%d]\n", enable);
- cfg = fimc_read(EXYNOS_CIGCTRL);
+ cfg = fimc_read(ctx, EXYNOS_CIGCTRL);
if (enable)
cfg |= EXYNOS_CIGCTRL_CAM_JPEG;
else
cfg &= ~EXYNOS_CIGCTRL_CAM_JPEG;
- fimc_write(cfg, EXYNOS_CIGCTRL);
+
[PATCH 8/8] drm/exynos/fimc: simplify and rename fimc_dst_get_buf_seq
fimc_dst_get_buf_seq returns number of buffers
so the name should be fimc_dst_get_buf_count.
Function body has been simplified.
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 9860d38..831dde9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -58,7 +58,6 @@
#define FIMC_SHFACTOR 10
#define FIMC_BUF_STOP 1
#define FIMC_BUF_START 2
-#define FIMC_REG_SZ32
#define FIMC_WIDTH_ITU_709 1280
#define FIMC_REFRESH_MAX 60
#define FIMC_REFRESH_MIN 12
@@ -1123,16 +1122,13 @@ static int fimc_dst_set_size(struct device *dev, int
swap,
return 0;
}
-static int fimc_dst_get_buf_seq(struct fimc_context *ctx)
+static int fimc_dst_get_buf_count(struct fimc_context *ctx)
{
- u32 cfg, i, buf_num = 0;
- u32 mask = 0x0001;
+ u32 cfg, buf_num;
cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
- for (i = 0; i < FIMC_REG_SZ; i++)
- if (cfg & (mask << i))
- buf_num++;
+ buf_num = hweight32(cfg);
DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
@@ -1176,12 +1172,12 @@ static int fimc_dst_set_buf_seq(struct fimc_context
*ctx, u32 buf_id,
/* interrupt enable */
if (buf_type == IPP_BUF_ENQUEUE &&
- fimc_dst_get_buf_seq(ctx) >= FIMC_BUF_START)
+ fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
fimc_mask_irq(ctx, true);
/* interrupt disable */
if (buf_type == IPP_BUF_DEQUEUE &&
- fimc_dst_get_buf_seq(ctx) <= FIMC_BUF_STOP)
+ fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
fimc_mask_irq(ctx, false);
err_unlock:
--
1.9.1
[PATCH v3 1/3] phy: Add exynos-simple-phy driver
On 19.05.2014 09:10, Rahul Sharma wrote:
> On 16 May 2014 20:19, Tomasz Figa wrote:
>> On 16.05.2014 16:30, Rahul Sharma wrote:
>>> On 16 May 2014 16:20, Tomasz Figa wrote:
On 16.05.2014 12:35, Rahul Sharma wrote:
> On 16 May 2014 15:12, Rahul Sharma wrote:
>> On 16 May 2014 03:14, Tomasz Figa wrote:
>>> On 15.05.2014 06:01, Rahul Sharma wrote:
> [snip]
> the PHY provider.
>
Please correct me if I got you wrong. You want somthing like this:
pmu_system_controller: system-controller at 1004 {
...
simple_phys: simple-phys {
compatible = "samsung,exynos5420-simple-phy";
...
};
};
>>>
>>> Not exactly.
>>>
>>> What I meant is that the PMU node itself should be the PHY provider,
>>> e.g.
>>>
>>> pmu_system_controller: system-controller at 1004 {
>>> /* ... */
>>> #phy-cells = <1>;
>>> };
>>>
>>> and then the PMU node should instantiate the Exynos simple PHY driver,
>>> as this is a driver for a facility existing entirely inside of the PMU.
>>> Moreover, the driver should be rather called Exynos PMU PHY.
>>>
>>> I know this isn't really possible at the moment, but with device tree we
>>> must design things carefully, so it's better to take a bit more time and
>>> do things properly.
>>>
>>> So my opinion on this is that there should be a central Exynos PMU
>>> driver that claims the IO region and instantiates necessary subdrivers,
>>> such as Exynos PMU PHY driver, Exynos CLKOUT driver, Exynos cpuidle
>>> driver and more, similar to what is being done in drivers/mfd.
>>
>
> Hi Tomasz,
>
> These PHYs are not part of PMU as such. I am not sure if it is correct to
> probe them as phy provider for all these phys. Only relation of these
> phys with
> the PMU is 'enable/disable control'.
Well, in reality what is implemented by this driver is not even a PHY,
just some kind of power controllers, which are contained entirely in the
PMU.
>>>
>>> I agree. Actually the role of generic phy framework for these 'simple' phys
>>> is
>>> only that much.
>>>
> Controlling this bit using regmap interface
> still looks better to me.
Well, when there is a choice between using regmap and not using regmap,
I'd rather choose the latter. Why would you want to introduce additional
abstraction layer if there is no need for such?
>
> IMHO Ideal method would be probing these PHYs independently and resolving
> the necessary dependencies like syscon handle, clocks etc. This way we
> will
> not be having any common phy provider for all these independent PHYs and
> it
> would be clean to add each of these phy nodes in DT. Please see my
> original
> comment below.
>
> http://lkml.iu.edu/hypermail/linux/kernel/1404.1/00701.html
With the solution I proposed, you don't need any kind of dependencies
for those simple power controllers. They are just single bits that don't
need anything special to operate, except PMU clock running.
>>>
>>> In that case we can further trim it down and let the drivers use the regmap
>>> interface to control this bit. Many drivers including HDMI, DP just need
>>> that
>>> much functionality from the phy provider.
>>
>> Well, this is what several drivers already do, like USB PHY (dedicated
>> IP block), watchdog (for watchdog mask), SATA PHY (dedicated IP block
>> too) or will do, like I2C (for configuration of I2C mux on Exynos5).
>>
>> At least this would be consistent with them and wouldn't be an API
>> abuse, so I'd be inclined to go this way more than introducing
>> abstractions like this patch does.
>
> Ok. I had already posted a patch for this at
> http://www.spinics.net/lists/linux-samsung-soc/msg28049.html
> I will revive that thread.
Looks good to me.
>
> @Tomasz Stanislawski, Do you have different opinion here?
I'm afraid Tomasz might not be very responsive during next few days, as
he is on a business trip. You might be able to reach him on our internal
communicator, though.
Best regards,
Tomasz
[RFC PATCH 1/2] drm: store connector name in connector struct
Hi
On Wed, May 14, 2014 at 3:58 PM, Jani Nikula wrote:
> This makes drm_get_connector_name() thread safe.
>
> Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3 at
> DC1-MBX-01.ptsecurity.ru
> Signed-off-by: Jani Nikula
I like that approach, but we should also kill drm_get_connector_name()
in a follow-up. I don't see why that wrapper is needed.
Reviewed-by: David Herrmann
Thanks
David
> ---
> drivers/gpu/drm/drm_crtc.c | 36
> include/drm/drm_crtc.h | 2 ++
> 2 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 461d19bd14ee..5781130b4126 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -277,21 +277,11 @@ EXPORT_SYMBOL(drm_get_encoder_name);
>
> /**
> * drm_get_connector_name - return a string for connector
> - * @connector: connector to compute name of
> - *
> - * Note that the buffer used by this function is globally shared and owned by
> - * the function itself.
> - *
> - * FIXME: This isn't really multithreading safe.
> + * @connector: the connector to get name for
> */
> const char *drm_get_connector_name(const struct drm_connector *connector)
> {
> - static char buf[32];
> -
> - snprintf(buf, 32, "%s-%d",
> -drm_connector_enum_list[connector->connector_type].name,
> -connector->connector_type_id);
> - return buf;
> + return connector->name;
> }
> EXPORT_SYMBOL(drm_get_connector_name);
>
> @@ -824,7 +814,7 @@ int drm_connector_init(struct drm_device *dev,
>
> ret = drm_mode_object_get(dev, &connector->base,
> DRM_MODE_OBJECT_CONNECTOR);
> if (ret)
> - goto out;
> + goto out_unlock;
>
> connector->base.properties = &connector->properties;
> connector->dev = dev;
> @@ -834,9 +824,17 @@ int drm_connector_init(struct drm_device *dev,
> ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
> if (connector->connector_type_id < 0) {
> ret = connector->connector_type_id;
> - drm_mode_object_put(dev, &connector->base);
> - goto out;
> + goto out_out;
> }
> + connector->name =
> + kasprintf(GFP_KERNEL, "%s-%d",
> + drm_connector_enum_list[connector_type].name,
> + connector->connector_type_id);
> + if (!connector->name) {
> + ret = -ENOMEM;
> + goto out_put;
> + }
> +
> INIT_LIST_HEAD(&connector->probed_modes);
> INIT_LIST_HEAD(&connector->modes);
> connector->edid_blob_ptr = NULL;
> @@ -853,7 +851,11 @@ int drm_connector_init(struct drm_device *dev,
> drm_object_attach_property(&connector->base,
> dev->mode_config.dpms_property, 0);
>
> - out:
> +out_put:
> + if (ret)
> + drm_mode_object_put(dev, &connector->base);
> +
> +out_unlock:
> drm_modeset_unlock_all(dev);
>
> return ret;
> @@ -881,6 +883,8 @@ void drm_connector_cleanup(struct drm_connector
> *connector)
>connector->connector_type_id);
>
> drm_mode_object_put(dev, &connector->base);
> + kfree(connector->name);
> + connector->name = NULL;
> list_del(&connector->head);
> dev->mode_config.num_connector--;
> }
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c061bb372199..d4cd7e513280 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -444,6 +444,7 @@ struct drm_encoder {
> * @attr: sysfs attributes
> * @head: list management
> * @base: base KMS object
> + * @name: connector name
> * @connector_type: one of the %DRM_MODE_CONNECTOR_ types from
> drm_mode.h
> * @connector_type_id: index into connector type enum
> * @interlace_allowed: can this connector handle interlaced modes?
> @@ -482,6 +483,7 @@ struct drm_connector {
>
> struct drm_mode_object base;
>
> + char *name;
> int connector_type;
> int connector_type_id;
> bool interlace_allowed;
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 0/2] drm: make drm_get_{connector, encoder}_name thread safe
Hi CC: Dave & Daniel On Wed, May 14, 2014 at 3:58 PM, Jani Nikula wrote: > Hi all - > > This series stores connector/encoder names in the relevant structs to > make the name getters thread safe. > > What say you, is the wasted memory too high a price to pay for the > thread safety and implementation simplicity of this approach? I think > making drm_get_connector_name and drm_get_encoder_name return allocated > buffers makes a lot of code really ugly and error prone. > > I am assuming connector_type, connector_type_id, encoder_type, and > encoder.base.id remain unchanged for the lifetime of the > connector/encoder - is that a correct assumption? I like the approach. Using dev-name does not work for encoders, so I'd keep them separate. I mean, we're talking about ~32 bytes per connector here.. Acked-by: David Herrmann Thanks David
[Bug 61533] [r600g][lockup] kernel 3.8-3.12 caused by Opera browser hardware accelerated rendering
https://bugs.freedesktop.org/show_bug.cgi?id=61533 --- Comment #15 from Eugene --- (In reply to comment #14) > Have you upgraded your software packaged ? Mesa 9.2 is not supported anymore. Yes, now I'm using: OpenGL version string: 3.0 Mesa 10.3.0-devel (git-5646319 trusty-oibaf-ppa) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/34d68798/attachment.html>
[Bug 75301] DGP isn't switched off by radeon driver
https://bugzilla.kernel.org/show_bug.cgi?id=75301 Alan changed: What|Removed |Added CC||alan at lxorguk.ukuu.org.uk Component|Video(Other)|Video(DRI - non Intel) Assignee|drivers_video-other at kernel- |drivers_video-dri at kernel-bu |bugs.osdl.org |gs.osdl.org -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 76919] Random junk at the bottom of non-multiple-of-4 compressed textures (original or mipmapped)
https://bugs.freedesktop.org/show_bug.cgi?id=76919 Tapani P?lli changed: What|Removed |Added CC||lemody at gmail.com --- Comment #4 from Tapani P?lli --- Are you able to reproduce this bug with simple program using a standard dds file? If this is possible, please attach such example. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/a1b29995/attachment.html>
[RFC PATCH v1 08/16] drm/radeon: use common fence implementation for fences
Am 19.05.2014 12:10, schrieb Maarten Lankhorst:
> op 19-05-14 10:27, Christian K?nig schreef:
>> Am 19.05.2014 10:00, schrieb Maarten Lankhorst:
>> [SNIP]
>> The problem here is that the whole approach collides with the way we
>> do reset handling from a conceptual point of view. Every IOCTL or
>> other call chain into the driver is protected by the read side of the
>> exclusive_lock semaphore. So in the case of a GPU lockup we can take
>> the write side of the semaphore and so make sure that we have nobody
>> else accessing the hardware or internal driver structures only
>> changed at init time.
>>
>> Leaking a drivers IRQ context into another driver as well as calling
>> into a driver in atomic context is just a quite uncommon approach and
>> should be considered very carefully.
>>
>> I would rather vote for a completely synchronous interface only
>> allowing blocking waits and checks if a fence is signaled from not
>> atomic context.
>>
>> If a driver needs to avoid blocking it should just use a workqueue
>> and checking a fence outside your own driver is probably be better
>> done in a bottom halve handler anyway.
>
> Except that you might want to do something like
> fence_is_signaled() in another driver to check whether you need to
> defer, or can submit the batch buffer immediately, saving a bunch of
> context switches. Running the is_signaled atomic is really useful here
> because it means you can't do too many scary things in your is_signaled
> handler.
This is indeed a nice optimization, but nothing more. If you want to
provide a is_signalled interface for atomic context then this should be
optional, not mandatory.
>
> In case of enable_signaling it was the only sane solution, because
> fence_signal can be called from irq context, and any calls after that to
> fence_add_callback and fence_wait aren't allowed to do anything, so
> fence_enable_sw_signaling and the default wait implementation must be
> atomic. fence_wait itself doesn't have to be, so it's easy to grab
> exclusive_lock there.
I don't think you understood my point here: Completely drop
enable_signaling, it's unnecessary and only complicates the interface.
We purposely avoided exactly this paradigm in the past and I haven't
seen any good argument to start with it now.
Christian.
>
> Simple fence drivers may drop some state after calling fence_signal, so
> calling .enable_signaling after fence_signal is bad, and fence_signal
> must also wait for any previous call to enable_signaling to be
> completed. This means those functions have to be implemented with the
> atomic spinlock held and irqs disabled. :-) The .signaled callback could
> strictly speaking still be called after fence_signal is called, but
> this function is optional.
>
> I tried other locking approaches, but when I used a separate spinlock
> for the fence things got even messier and I ended up with impossible to
> eliminate locking inversions, or I removed the guarantee that calling
> fence_signal meant that enable_signaling had either been not called or
> completed, or other bugs and much harder to read code.
>
> ~Maarten
>
> Revised diff below.
> ---
> diff --git a/drivers/gpu/drm/radeon/radeon.h
> b/drivers/gpu/drm/radeon/radeon.h
> index 68528619834a..a7d839a158ae 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -64,6 +64,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -113,9 +114,6 @@ extern int radeon_hard_reset;
> #define RADEONFB_CONN_LIMIT4
> #define RADEON_BIOS_NUM_SCRATCH8
>
> -/* fence seq are set to this number when signaled */
> -#define RADEON_FENCE_SIGNALED_SEQ0LL
> -
> /* internal ring indices */
> /* r1xx+ has gfx CP ring */
> #define RADEON_RING_TYPE_GFX_INDEX0
> @@ -347,12 +345,15 @@ struct radeon_fence_driver {
> };
>
> struct radeon_fence {
> +struct fence base;
> +
> struct radeon_device*rdev;
> -struct krefkref;
> /* protected by radeon_fence.lock */
> uint64_tseq;
> /* RB, DMA, etc. */
> unsignedring;
> +
> +wait_queue_t fence_wake;
> };
>
> int radeon_fence_driver_start_ring(struct radeon_device *rdev, int
> ring);
> @@ -2256,6 +2257,7 @@ struct radeon_device {
> struct radeon_mmanmman;
> struct radeon_fence_driverfence_drv[RADEON_NUM_RINGS];
> wait_queue_head_tfence_queue;
> +unsignedfence_context;
> struct mutexring_lock;
> struct radeon_ringring[RADEON_NUM_RINGS];
> boolib_pool_ready;
> @@ -2346,11 +2348,6 @@ u32 cik_mm_rdoorbell(struct radeon_device
> *rdev, u32 index);
> void cik_mm_wdoorbell(struct radeon_device *rdev, u32 index, u32 v);
>
> /*
> - * Cast helper
> - */
> -#define to_radeon_fence(p) ((struct radeon_fence *)(p))
> -
> -/*
> * Registers read & write functions.
> */
> #define RREG8(reg) r
[Bug 74551] Unable to enable ACPI
https://bugzilla.kernel.org/show_bug.cgi?id=74551 Alan changed: What|Removed |Added Status|NEW |NEEDINFO -- You are receiving this mail because: You are watching the assignee of the bug.
[RFC PATCH 2/2] drm: store encoder name in encoder struct
Hi
On Wed, May 14, 2014 at 3:58 PM, Jani Nikula wrote:
> This makes drm_get_encoder_name() thread safe.
>
> Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3 at
> DC1-MBX-01\
> .ptsecurity.ru
> Signed-off-by: Jani Nikula
Same as for 1/2, a followup would be nice.
Reviewed-by: David Herrmann
Thanks
David
> ---
> drivers/gpu/drm/drm_crtc.c | 31 +--
> include/drm/drm_crtc.h | 2 ++
> 2 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5781130b4126..1c4cb74ede77 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -257,21 +257,11 @@ void drm_connector_ida_destroy(void)
>
> /**
> * drm_get_encoder_name - return a string for encoder
> - * @encoder: encoder to compute name of
> - *
> - * Note that the buffer used by this function is globally shared and owned by
> - * the function itself.
> - *
> - * FIXME: This isn't really multithreading safe.
> + * @encoder: the encoder to get name for
> */
> const char *drm_get_encoder_name(const struct drm_encoder *encoder)
> {
> - static char buf[32];
> -
> - snprintf(buf, 32, "%s-%d",
> -drm_encoder_enum_list[encoder->encoder_type].name,
> -encoder->base.id);
> - return buf;
> + return encoder->name;
> }
> EXPORT_SYMBOL(drm_get_encoder_name);
>
> @@ -986,16 +976,27 @@ int drm_encoder_init(struct drm_device *dev,
>
> ret = drm_mode_object_get(dev, &encoder->base,
> DRM_MODE_OBJECT_ENCODER);
> if (ret)
> - goto out;
> + goto out_unlock;
>
> encoder->dev = dev;
> encoder->encoder_type = encoder_type;
> encoder->funcs = funcs;
> + encoder->name = kasprintf(GFP_KERNEL, "%s-%d",
> + drm_encoder_enum_list[encoder_type].name,
> + encoder->base.id);
> + if (!encoder->name) {
> + ret = -ENOMEM;
> + goto out_put;
> + }
>
> list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
> dev->mode_config.num_encoder++;
>
> - out:
> +out_put:
> + if (ret)
> + drm_mode_object_put(dev, &encoder->base);
> +
> +out_unlock:
> drm_modeset_unlock_all(dev);
>
> return ret;
> @@ -1013,6 +1014,8 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> struct drm_device *dev = encoder->dev;
> drm_modeset_lock_all(dev);
> drm_mode_object_put(dev, &encoder->base);
> + kfree(encoder->name);
> + encoder->name = NULL;
> list_del(&encoder->head);
> dev->mode_config.num_encoder--;
> drm_modeset_unlock_all(dev);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d4cd7e513280..219b533a2c15 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -400,6 +400,7 @@ struct drm_encoder_funcs {
> * @dev: parent DRM device
> * @head: list management
> * @base: base KMS object
> + * @name: encoder name
> * @encoder_type: one of the %DRM_MODE_ENCODER_ types in drm_mode.h
> * @possible_crtcs: bitmask of potential CRTC bindings
> * @possible_clones: bitmask of potential sibling encoders for cloning
> @@ -416,6 +417,7 @@ struct drm_encoder {
> struct list_head head;
>
> struct drm_mode_object base;
> + char *name;
> int encoder_type;
> uint32_t possible_crtcs;
> uint32_t possible_clones;
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v1 08/16] drm/radeon: use common fence implementation for fences
op 19-05-14 14:30, Christian K?nig schreef: > Am 19.05.2014 12:10, schrieb Maarten Lankhorst: >> op 19-05-14 10:27, Christian K?nig schreef: >>> Am 19.05.2014 10:00, schrieb Maarten Lankhorst: >>> [SNIP] >>> The problem here is that the whole approach collides with the way we do >>> reset handling from a conceptual point of view. Every IOCTL or other call >>> chain into the driver is protected by the read side of the exclusive_lock >>> semaphore. So in the case of a GPU lockup we can take the write side of the >>> semaphore and so make sure that we have nobody else accessing the hardware >>> or internal driver structures only changed at init time. >>> >>> Leaking a drivers IRQ context into another driver as well as calling into a >>> driver in atomic context is just a quite uncommon approach and should be >>> considered very carefully. >>> >>> I would rather vote for a completely synchronous interface only allowing >>> blocking waits and checks if a fence is signaled from not atomic context. >>> >>> If a driver needs to avoid blocking it should just use a workqueue and >>> checking a fence outside your own driver is probably be better done in a >>> bottom halve handler anyway. >> >> Except that you might want to do something like >> fence_is_signaled() in another driver to check whether you need to >> defer, or can submit the batch buffer immediately, saving a bunch of >> context switches. Running the is_signaled atomic is really useful here >> because it means you can't do too many scary things in your is_signaled >> handler. > > This is indeed a nice optimization, but nothing more. If you want to provide > a is_signalled interface for atomic context then this should be optional, not > mandatory. See below. >> In case of enable_signaling it was the only sane solution, because >> fence_signal can be called from irq context, and any calls after that to >> fence_add_callback and fence_wait aren't allowed to do anything, so >> fence_enable_sw_signaling and the default wait implementation must be >> atomic. fence_wait itself doesn't have to be, so it's easy to grab >> exclusive_lock there. > > I don't think you understood my point here: Completely drop enable_signaling, > it's unnecessary and only complicates the interface. > > We purposely avoided exactly this paradigm in the past and I haven't seen any > good argument to start with it now. In the common case a lot more fences will be emitted than will be waited on. This means it makes sense to delay signaling a fence with fence_signal for as long as possible. But when a fence user wants to work with a fence some way is needed to ensure that the fence will complete. This is the idea behind .enable_signaling, it tells the fence driver to call fence_signal on the fence 'soon' because there are now waiters for it. The atomic .signaled is optional, and can be set to NULL, but there is no guarantee that fence_is_signaled will ever return true in that case, unless fence_enable_sw_signaling is called (which calls .enable_signaling). Providing a custom wait function is optional in the interface, if the default wait function is used all waiters are signaled when fence_signal is called. Removing enable_signaling would only make sense if fence_signal was removed too, but that would mean that fence_is_signaled could no longer exist in the core fence code, and would mean completely rewriting the interface. ~Maarten
[PATCH] drm/sysfs: expose the "force" connector attribute
Signed-off-by: Thomas Wood
---
drivers/gpu/drm/drm_sysfs.c | 49 +
1 file changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index c22c309..257816e 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -338,11 +338,60 @@ static ssize_t select_subconnector_show(struct device
*device,
drm_get_dvi_i_select_name((int)subconnector));
}
+static ssize_t force_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_connector *connector = to_drm_connector(device);
+ char *status;
+
+ switch (connector->force) {
+ case DRM_FORCE_ON:
+ status = "on";
+ break;
+
+ case DRM_FORCE_ON_DIGITAL:
+ status = "digital";
+ break;
+
+ case DRM_FORCE_OFF:
+ status = "off";
+ break;
+
+ case DRM_FORCE_UNSPECIFIED:
+ status = "unspecified";
+ break;
+
+ default:
+ return 0;
+ }
+
+ return snprintf(buf, PAGE_SIZE, "%s\n", status);
+}
+
+ssize_t force_store(struct device *device, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct drm_connector *connector = to_drm_connector(device);
+
+ if (strcmp(buf, "on") == 0)
+ connector->force = DRM_FORCE_ON;
+ else if (strcmp(buf, "digital") == 0)
+ connector->force = DRM_FORCE_ON_DIGITAL;
+ else if (strcmp(buf, "off") == 0)
+ connector->force = DRM_FORCE_OFF;
+ else if (strcmp(buf, "unspecified") == 0)
+ connector->force = DRM_FORCE_UNSPECIFIED;
+
+ return count;
+}
+
static struct device_attribute connector_attrs[] = {
__ATTR_RO(status),
__ATTR_RO(enabled),
__ATTR_RO(dpms),
__ATTR_RO(modes),
+ __ATTR_RW(force),
};
/* These attributes are for both DVI-I connectors and all types of tv-out. */
--
1.9.0
[RFC PATCH 2/2 with seqcount v3] reservation: add suppport for read-only access using rcu
Hi, Maarten!
Some nitpicks, and that krealloc within rcu lock still worries me.
Otherwise looks good.
/Thomas
On 04/23/2014 12:15 PM, Maarten Lankhorst wrote:
> This adds 4 more functions to deal with rcu.
>
> reservation_object_get_fences_rcu() will obtain the list of shared
> and exclusive fences without obtaining the ww_mutex.
>
> reservation_object_wait_timeout_rcu() will wait on all fences of the
> reservation_object, without obtaining the ww_mutex.
>
> reservation_object_test_signaled_rcu() will test if all fences of the
> reservation_object are signaled without using the ww_mutex.
>
> reservation_object_get_excl() is added because touching the fence_excl
> member directly will trigger a sparse warning.
>
> Signed-off-by: Maarten Lankhorst
> ---
> Using seqcount and fixing some lockdep bugs.
> Changes since v2:
> - Fix some crashes, remove some unneeded barriers when provided by
> seqcount writes
> - Fix code to work correctly with sparse's RCU annotations.
> - Create a global string for the seqcount lock to make lockdep happy.
>
> Can I get this version reviewed? If it looks correct I'll mail the
> full series
> because it's intertwined with the TTM conversion to use this code.
>
> See
> https://urldefense.proofpoint.com/v1/url?u=http://cgit.freedesktop.org/~mlankhorst/linux/log/?h%3Dvmwgfx_wip&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=SvhtRcVv7pxLHaMBAL4xy2Rr2XDJ%2B95q18FDS13r8FQ%3D%0A&s=07fbe960788dca202856a797e8c915c44f05746a04d899c76459653042ea0112
> ---
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index d89a98d2c37b..0df673f812eb 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -137,7 +137,7 @@ static unsigned int dma_buf_poll(struct file
> *file, poll_table *poll)
> struct reservation_object_list *fobj;
> struct fence *fence_excl;
> unsigned long events;
> -unsigned shared_count;
> +unsigned shared_count, seq;
>
> dmabuf = file->private_data;
> if (!dmabuf || !dmabuf->resv)
> @@ -151,14 +151,20 @@ static unsigned int dma_buf_poll(struct file
> *file, poll_table *poll)
> if (!events)
> return 0;
>
> -ww_mutex_lock(&resv->lock, NULL);
> +retry:
> +seq = read_seqcount_begin(&resv->seq);
> +rcu_read_lock();
>
> -fobj = resv->fence;
> -if (!fobj)
> -goto out;
> -
> -shared_count = fobj->shared_count;
> -fence_excl = resv->fence_excl;
> +fobj = rcu_dereference(resv->fence);
> +if (fobj)
> +shared_count = fobj->shared_count;
> +else
> +shared_count = 0;
> +fence_excl = rcu_dereference(resv->fence_excl);
> +if (read_seqcount_retry(&resv->seq, seq)) {
> +rcu_read_unlock();
> +goto retry;
> +}
>
> if (fence_excl && (!(events & POLLOUT) || shared_count == 0)) {
> struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_excl;
> @@ -176,14 +182,20 @@ static unsigned int dma_buf_poll(struct file
> *file, poll_table *poll)
> spin_unlock_irq(&dmabuf->poll.lock);
>
> if (events & pevents) {
> -if (!fence_add_callback(fence_excl, &dcb->cb,
> +if (!fence_get_rcu(fence_excl)) {
> +/* force a recheck */
> +events &= ~pevents;
> +dma_buf_poll_cb(NULL, &dcb->cb);
> +} else if (!fence_add_callback(fence_excl, &dcb->cb,
> dma_buf_poll_cb)) {
> events &= ~pevents;
> +fence_put(fence_excl);
> } else {
> /*
> * No callback queued, wake up any additional
> * waiters.
> */
> +fence_put(fence_excl);
> dma_buf_poll_cb(NULL, &dcb->cb);
> }
> }
> @@ -205,13 +217,26 @@ static unsigned int dma_buf_poll(struct file
> *file, poll_table *poll)
> goto out;
>
> for (i = 0; i < shared_count; ++i) {
> -struct fence *fence = fobj->shared[i];
> +struct fence *fence = rcu_dereference(fobj->shared[i]);
>
> +if (!fence_get_rcu(fence)) {
> +/*
> + * fence refcount dropped to zero, this means
> + * that fobj has been freed
> + *
> + * call dma_buf_poll_cb and force a recheck!
> + */
> +events &= ~POLLOUT;
> +dma_buf_poll_cb(NULL, &dcb->cb);
> +break;
> +}
> if (!fence_add_callback(fence, &dcb->cb,
> dma_buf_poll_cb)) {
> +fence_put(fence);
> events &= ~POLLOUT;
> break;
> }
> +fence_put(fence);
> }
>
> /* No callback queued, wake up any additional waiters. */
> @@ -220,7 +245,7 @@ static unsigned int dma_buf_poll(struct file
> *file, poll_table *poll)
[Bug 75276] Implement VGPR Register Spilling
https://bugs.freedesktop.org/show_bug.cgi?id=75276 --- Comment #23 from darkbasic --- Here is tesseract with R600_DEBUG=ps,vs,gs: http://bpaste.net/show/287559/ -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/e7b2c590/attachment.html>
[Bug 75276] Implement VGPR Register Spilling
https://bugs.freedesktop.org/show_bug.cgi?id=75276 --- Comment #24 from darkbasic --- I forgot to say I'm using attachment 99254. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/4ad3ceb5/attachment.html>
[Bug 75301] DGP isn't switched off by radeon driver
https://bugzilla.kernel.org/show_bug.cgi?id=75301 Alex Deucher changed: What|Removed |Added CC||alexdeucher at gmail.com --- Comment #1 from Alex Deucher --- Please attach your dmesg output. Also, do you have a debugfs switcheroo entry? If not, can you try the latest 3.15 kernel? -- You are receiving this mail because: You are watching the assignee of the bug.
[pull] radeon drm-fixes-3.15
> -Original Message- > From: Koenig, Christian > Sent: Monday, May 19, 2014 4:50 AM > To: Dave Airlie > Cc: Deucher, Alexander; dri-devel at lists.freedesktop.org > Subject: Re: [pull] radeon drm-fixes-3.15 > > Am 19.05.2014 01:04, schrieb Dave Airlie: > > On 16 May 2014 23:54, Christian K?nig wrote: > >> Hi Dave, > >> > >> this is the next pull quested for stashed up radeon fixes for 3.15. > >> > >> Highlights are: > >> 1. Avoid sending SIGBUS on CPU access just because kernel can't handle > >> buffer placement. > >> 2. Some fixes for VM page table updates and buffer placements. > >> 3. Fixing two typos in the PLL and SI register spec. > >> 4. Checking VCE buffers ranges. > >> 5. Adding a mutex for I2C access. > >> 6. Mullins and non-VGA pci class fixes from Alex. > > This is starting to feel a bit large, we should be in regression > > fixing, oopses and black screens at this point, > > > > The main ones I'm concerned about are the VCE buffer ranges and i2c > > mutex, whether these fix known problems or issues, > > For the VCE buffer range checks that fixed a lockup I experienced while > working on the userspace driver (VCE writing over the end of a buffer > into an shader, GPU then trying to execute the messed up shader > code...). Apart from that VCE is a new feature for 3.15 so we can't > really break it compared to 3.14. > > I can't judge on the I2C mutex since that was Alex suggestion and I'm > not sure how invasive and/or necessary this is. The i2c mutex can wait for 3.16 if need be. I don't know of any actual bugs it fixes. Alex > > Regards, > Christian. > > > > > Dave.
[Bug 75301] DGP isn't switched off by radeon driver
https://bugzilla.kernel.org/show_bug.cgi?id=75301 --- Comment #2 from Alex Deucher --- This is a duplicate bug 75401. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 75931] 3.13.11 kernel breaks Xorg on AMD APU graphics
https://bugzilla.kernel.org/show_bug.cgi?id=75931 Alex Deucher changed: What|Removed |Added CC||alexdeucher at gmail.com --- Comment #1 from Alex Deucher --- Can you bisect? -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 75931] 3.13.11 kernel breaks Xorg on AMD APU graphics
https://bugzilla.kernel.org/show_bug.cgi?id=75931 --- Comment #2 from Alex Deucher --- duplicate of bug 75921. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 75921] Horizontal lines dancing all around the screen in Xorg
https://bugzilla.kernel.org/show_bug.cgi?id=75921 Alex Deucher changed: What|Removed |Added CC||alexdeucher at gmail.com --- Comment #1 from Alex Deucher --- Can you bisect? -- You are receiving this mail because: You are watching the assignee of the bug.
[RFC PATCH 2/2 with seqcount v3] reservation: add suppport for read-only access using rcu
op 19-05-14 15:42, Thomas Hellstrom schreef:
> Hi, Maarten!
>
> Some nitpicks, and that krealloc within rcu lock still worries me.
> Otherwise looks good.
>
> /Thomas
>
>
>
> On 04/23/2014 12:15 PM, Maarten Lankhorst wrote:
>> @@ -55,8 +60,8 @@ int reservation_object_reserve_shared(struct
>> reservation_object *obj)
>> kfree(obj->staged);
>> obj->staged = NULL;
>> return 0;
>> -}
>> -max = old->shared_max * 2;
>> +} else
>> +max = old->shared_max * 2;
> Perhaps as a separate reformatting patch?
I'll fold it in to the patch that added reservation_object_reserve_shared.
>> +
>> +int reservation_object_get_fences_rcu(struct reservation_object *obj,
>> + struct fence **pfence_excl,
>> + unsigned *pshared_count,
>> + struct fence ***pshared)
>> +{
>> +unsigned shared_count = 0;
>> +unsigned retry = 1;
>> +struct fence **shared = NULL, *fence_excl = NULL;
>> +int ret = 0;
>> +
>> +while (retry) {
>> +struct reservation_object_list *fobj;
>> +unsigned seq;
>> +
>> +seq = read_seqcount_begin(&obj->seq);
>> +
>> +rcu_read_lock();
>> +
>> +fobj = rcu_dereference(obj->fence);
>> +if (fobj) {
>> +struct fence **nshared;
>> +
>> +shared_count = ACCESS_ONCE(fobj->shared_count);
> ACCESS_ONCE() shouldn't be needed inside the seqlock?
Yes it is, shared_count may be increased, leading to potential different sizes
for krealloc and memcpy
if the ACCESS_ONCE is removed. I could use shared_max here instead, which stays
the same,
but it would waste more memory.
>> +nshared = krealloc(shared, sizeof(*shared) *
>> shared_count, GFP_KERNEL);
> Again, krealloc should be a sleeping function, and not suitable within a
> RCU read lock? I still think this krealloc should be moved to the start
> of the retry loop, and we should start with a suitable guess of
> shared_count (perhaps 0?) It's not like we're going to waste a lot of
> memory
But shared_count is only known when holding the rcu lock.
What about this change?
@@ -254,16 +254,27 @@ int reservation_object_get_fences_rcu(struct
reservation_object *obj,
fobj = rcu_dereference(obj->fence);
if (fobj) {
struct fence **nshared;
+ size_t sz;
shared_count = ACCESS_ONCE(fobj->shared_count);
- nshared = krealloc(shared, sizeof(*shared) *
shared_count, GFP_KERNEL);
+ sz = sizeof(*shared) * shared_count;
+
+ nshared = krealloc(shared, sz,
+ GFP_NOWAIT | __GFP_NOWARN);
if (!nshared) {
+ rcu_read_unlock();
+ nshared = krealloc(shared, sz, GFP_KERNEL)
+ if (nshared) {
+ shared = nshared;
+ continue;
+ }
+
ret = -ENOMEM;
- shared_count = retry = 0;
- goto unlock;
+ shared_count = 0;
+ break;
}
shared = nshared;
- memcpy(shared, fobj->shared, sizeof(*shared) *
shared_count);
+ memcpy(shared, fobj->shared, sz);
} else
shared_count = 0;
fence_excl = rcu_dereference(obj->fence_excl);
>> +
>> +/*
>> + * There could be a read_seqcount_retry here, but nothing cares
>> + * about whether it's the old or newer fence pointers that are
>> + * signale. That race could still have happened after checking
> Typo.
Oops.
[PATCH] drm/sysfs: expose the "force" connector attribute
Hi
On Mon, May 19, 2014 at 3:37 PM, Thomas Wood wrote:
> Signed-off-by: Thomas Wood
The commit-msg lacks any discussion why this change is done. What is
the reason to do that? Isn't the kernel-command-line enough? Why is
this a regular feature instead of a debugfs attribute?
> ---
> drivers/gpu/drm/drm_sysfs.c | 49
> +
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index c22c309..257816e 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -338,11 +338,60 @@ static ssize_t select_subconnector_show(struct device
> *device,
> drm_get_dvi_i_select_name((int)subconnector));
> }
>
> +static ssize_t force_show(struct device *device,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct drm_connector *connector = to_drm_connector(device);
> + char *status;
"const char *" or gcc might warn on string assignments.
> +
> + switch (connector->force) {
> + case DRM_FORCE_ON:
> + status = "on";
> + break;
> +
> + case DRM_FORCE_ON_DIGITAL:
> + status = "digital";
> + break;
> +
> + case DRM_FORCE_OFF:
> + status = "off";
> + break;
> +
> + case DRM_FORCE_UNSPECIFIED:
> + status = "unspecified";
> + break;
> +
> + default:
> + return 0;
> + }
> +
> + return snprintf(buf, PAGE_SIZE, "%s\n", status);
> +}
> +
> +ssize_t force_store(struct device *device, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct drm_connector *connector = to_drm_connector(device);
> +
> + if (strcmp(buf, "on") == 0)
> + connector->force = DRM_FORCE_ON;
> + else if (strcmp(buf, "digital") == 0)
> + connector->force = DRM_FORCE_ON_DIGITAL;
> + else if (strcmp(buf, "off") == 0)
> + connector->force = DRM_FORCE_OFF;
> + else if (strcmp(buf, "unspecified") == 0)
> + connector->force = DRM_FORCE_UNSPECIFIED;
else
return -EINVAL;
This really looks like a debug-feature to me. If it's a real feature,
we _must_ rescan connectors here, otherwise it seems odd writing "on"
into that file but nothing happens until the next modeset.
Thanks
David
> +
> + return count;
> +}
> +
> static struct device_attribute connector_attrs[] = {
> __ATTR_RO(status),
> __ATTR_RO(enabled),
> __ATTR_RO(dpms),
> __ATTR_RO(modes),
> + __ATTR_RW(force),
> };
>
> /* These attributes are for both DVI-I connectors and all types of tv-out. */
> --
> 1.9.0
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v1 08/16] drm/radeon: use common fence implementation for fences
Am 19.05.2014 15:35, schrieb Maarten Lankhorst: > op 19-05-14 14:30, Christian K?nig schreef: >> Am 19.05.2014 12:10, schrieb Maarten Lankhorst: >>> op 19-05-14 10:27, Christian K?nig schreef: Am 19.05.2014 10:00, schrieb Maarten Lankhorst: [SNIP] The problem here is that the whole approach collides with the way we do reset handling from a conceptual point of view. Every IOCTL or other call chain into the driver is protected by the read side of the exclusive_lock semaphore. So in the case of a GPU lockup we can take the write side of the semaphore and so make sure that we have nobody else accessing the hardware or internal driver structures only changed at init time. Leaking a drivers IRQ context into another driver as well as calling into a driver in atomic context is just a quite uncommon approach and should be considered very carefully. I would rather vote for a completely synchronous interface only allowing blocking waits and checks if a fence is signaled from not atomic context. If a driver needs to avoid blocking it should just use a workqueue and checking a fence outside your own driver is probably be better done in a bottom halve handler anyway. >>> >>> Except that you might want to do something like >>> fence_is_signaled() in another driver to check whether you need to >>> defer, or can submit the batch buffer immediately, saving a bunch of >>> context switches. Running the is_signaled atomic is really useful here >>> because it means you can't do too many scary things in your is_signaled >>> handler. >> >> This is indeed a nice optimization, but nothing more. If you want to >> provide a is_signalled interface for atomic context then this should >> be optional, not mandatory. > See below. >>> In case of enable_signaling it was the only sane solution, because >>> fence_signal can be called from irq context, and any calls after >>> that to >>> fence_add_callback and fence_wait aren't allowed to do anything, so >>> fence_enable_sw_signaling and the default wait implementation must be >>> atomic. fence_wait itself doesn't have to be, so it's easy to grab >>> exclusive_lock there. >> >> I don't think you understood my point here: Completely drop >> enable_signaling, it's unnecessary and only complicates the interface. >> >> We purposely avoided exactly this paradigm in the past and I haven't >> seen any good argument to start with it now. > > In the common case a lot more fences will be emitted than will be > waited on. > This means it makes sense to delay signaling a fence with fence_signal > for > as long as possible. But when a fence user wants to work with a fence > some way is needed to ensure that the fence will complete. This is the > idea > behind .enable_signaling, it tells the fence driver to call > fence_signal on > the fence 'soon' because there are now waiters for it. > > The atomic .signaled is optional, and can be set to NULL, but there is > no guarantee that fence_is_signaled will ever return true in that case, > unless fence_enable_sw_signaling is called (which calls > .enable_signaling). > > Providing a custom wait function is optional in the interface, if the > default wait > function is used all waiters are signaled when fence_signal is called. > > Removing enable_signaling would only make sense if fence_signal was > removed too, > but that would mean that fence_is_signaled could no longer exist in > the core fence > code, and would mean completely rewriting the interface. > And this is what I'm suggesting here. We have avoided quite hard adding any form of those callbacks in the past and I don't really see a reason why that would have changed. For example see the discussion here: http://lists.freedesktop.org/archives/dri-devel/2012-May/022388.html Jerome and Dave rejected my approach for handling the sub allocator through a callback for exactly the same reason. And that was even for call chains inside the same driver, you're suggesting this for cross driver synchronization. Christian.
[Bug 76321] Incorrect hwmon temperature when radeon card is turned off
https://bugzilla.kernel.org/show_bug.cgi?id=76321 --- Comment #5 from Alex Deucher --- (In reply to Pali Roh?r from comment #4) > > And for power_dpm_state & power_dpm_force_performance_level: I understand > that it cannot be changed when card is turned off (I see that it also > disappear from PCI bus space), but I would like to see option to set > "initial" dpm state/level which will be used when card is turned om again. > This can be usefull for scripts which setting powersave options depending on > hostplugging AC adapter. What do you think about using same sysfs entries > for it? That's fine if someone wants to implement it. It's not a high priority for me at the moment. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 1/2] drm/radeon: add large PTE support for NI, SI and CIK v5
On Sat, May 10, 2014 at 6:17 AM, Christian K?nig
wrote:
> From: Christian K?nig
>
> This patch implements support for VRAM page table entry compression.
> PTE construction is enhanced to identify physically contiguous page
> ranges and mark them in the PTE fragment field. L1/L2 TLB support is
> enabled for 64KB (SI/CIK) and 256KB (NI) PTE fragments, significantly
> improving TLB utilization for VRAM allocations.
>
> Linear store bandwidth is improved from 60GB/s to 125GB/s on Pitcairn.
> Unigine Heaven 3.0 sees an average improvement from 24.7 to 27.7 FPS
> on default settings at 1920x1200 resolution with vsync disabled.
>
> See main comment in radeon_vm.c for a technical description.
>
> v2 (chk): rebased and simplified.
> v3 (chk): add missing hw setup
> v4 (chk): rebased on current drm-fixes-3.15
> v5 (chk): fix comments and commit text
>
> Signed-off-by: Jay Cornwall
> Signed-off-by: Christian K?nig
Applied both to my 3.16 tree.
Thanks!
Alex
> ---
> drivers/gpu/drm/radeon/cik.c | 4 +-
> drivers/gpu/drm/radeon/ni.c| 2 +
> drivers/gpu/drm/radeon/radeon.h| 5 +++
> drivers/gpu/drm/radeon/radeon_vm.c | 91
> +++---
> drivers/gpu/drm/radeon/si.c| 5 ++-
> 5 files changed, 98 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 5143e0b..ce26b2a 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -5329,6 +5329,7 @@ static int cik_pcie_gart_enable(struct radeon_device
> *rdev)
> WREG32(MC_VM_MX_L1_TLB_CNTL,
>(0xA << 7) |
>ENABLE_L1_TLB |
> + ENABLE_L1_FRAGMENT_PROCESSING |
>SYSTEM_ACCESS_MODE_NOT_IN_SYS |
>ENABLE_ADVANCED_DRIVER_MODEL |
>SYSTEM_APERTURE_UNMAPPED_ACCESS_PASS_THRU);
> @@ -5341,7 +5342,8 @@ static int cik_pcie_gart_enable(struct radeon_device
> *rdev)
>CONTEXT1_IDENTITY_ACCESS_MODE(1));
> WREG32(VM_L2_CNTL2, INVALIDATE_ALL_L1_TLBS | INVALIDATE_L2_CACHE);
> WREG32(VM_L2_CNTL3, L2_CACHE_BIGK_ASSOCIATIVITY |
> - L2_CACHE_BIGK_FRAGMENT_SIZE(6));
> + BANK_SELECT(4) |
> + L2_CACHE_BIGK_FRAGMENT_SIZE(4));
> /* setup context0 */
> WREG32(VM_CONTEXT0_PAGE_TABLE_START_ADDR, rdev->mc.gtt_start >> 12);
> WREG32(VM_CONTEXT0_PAGE_TABLE_END_ADDR, rdev->mc.gtt_end >> 12);
> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> index d246e04..5e8db9b 100644
> --- a/drivers/gpu/drm/radeon/ni.c
> +++ b/drivers/gpu/drm/radeon/ni.c
> @@ -1228,12 +1228,14 @@ static int cayman_pcie_gart_enable(struct
> radeon_device *rdev)
>SYSTEM_APERTURE_UNMAPPED_ACCESS_PASS_THRU);
> /* Setup L2 cache */
> WREG32(VM_L2_CNTL, ENABLE_L2_CACHE |
> + ENABLE_L2_FRAGMENT_PROCESSING |
>ENABLE_L2_PTE_CACHE_LRU_UPDATE_BY_WRITE |
>ENABLE_L2_PDE0_CACHE_LRU_UPDATE_BY_WRITE |
>EFFECTIVE_L2_QUEUE_SIZE(7) |
>CONTEXT1_IDENTITY_ACCESS_MODE(1));
> WREG32(VM_L2_CNTL2, INVALIDATE_ALL_L1_TLBS | INVALIDATE_L2_CACHE);
> WREG32(VM_L2_CNTL3, L2_CACHE_BIGK_ASSOCIATIVITY |
> + BANK_SELECT(6) |
>L2_CACHE_BIGK_FRAGMENT_SIZE(6));
> /* setup context0 */
> WREG32(VM_CONTEXT0_PAGE_TABLE_START_ADDR, rdev->mc.gtt_start >> 12);
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 91be3d6..5c4742c 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -854,6 +854,11 @@ struct radeon_mec {
> #define R600_PTE_READABLE (1 << 5)
> #define R600_PTE_WRITEABLE (1 << 6)
>
> +/* PTE (Page Table Entry) fragment field for different page sizes */
> +#define R600_PTE_FRAG_4KB (0 << 7)
> +#define R600_PTE_FRAG_64KB (4 << 7)
> +#define R600_PTE_FRAG_256KB(6 << 7)
> +
> struct radeon_vm_pt {
> struct radeon_bo*bo;
> uint64_taddr;
> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
> b/drivers/gpu/drm/radeon/radeon_vm.c
> index 2aae6ce..f8d5b65 100644
> --- a/drivers/gpu/drm/radeon/radeon_vm.c
> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
> @@ -658,6 +658,84 @@ int radeon_vm_update_page_directory(struct radeon_device
> *rdev,
> }
>
> /**
> + * radeon_vm_frag_ptes - add fragment information to PTEs
> + *
> + * @rdev: radeon_device pointer
> + * @ib: IB for the update
> + * @pe_start: first PTE to handle
> + * @pe_end: last PTE to handle
> + * @addr: addr those PTEs should point to
> + * @flags: hw mapping flags
> + *
> + * Global and local mutex must be locked!
> + */
> +static void radeon_vm_frag_ptes(struct radeon_device *rdev,
> + struct radeon_ib *ib,
> + uint64_t pe_start, uint64_t pe_end,
> + uin
[PATCH] drm/radeon: avoid segfault on device open when accel is not working.
On Sat, May 17, 2014 at 10:16 PM, Dieter N?tzel wrote: > Hello Christian, > > no one has picked this. > http://lists.freedesktop.org/archives/dri-devel/2014-May/059189.html > > Even Alex ACKed it. > http://lists.freedesktop.org/archives/dri-devel/2014-May/059191.html Christian, can you grab this for 3.15? Alex
[PATCH] drm/radeon: avoid segfault on device open when accel is not working.
Am 19.05.2014 16:36, schrieb Alex Deucher: > On Sat, May 17, 2014 at 10:16 PM, Dieter N?tzel > wrote: >> Hello Christian, >> >> no one has picked this. >> http://lists.freedesktop.org/archives/dri-devel/2014-May/059189.html >> >> Even Alex ACKed it. >> http://lists.freedesktop.org/archives/dri-devel/2014-May/059191.html > Christian, can you grab this for 3.15? Wasn't sure if it's urgent enough for 3.15, just added it to my queue. Christian. > > Alex
[PATCH] drm/sysfs: expose the "force" connector attribute
On 19 May 2014 15:13, David Herrmann wrote:
> Hi
>
> On Mon, May 19, 2014 at 3:37 PM, Thomas Wood wrote:
>> Signed-off-by: Thomas Wood
>
> The commit-msg lacks any discussion why this change is done. What is
> the reason to do that? Isn't the kernel-command-line enough? Why is
> this a regular feature instead of a debugfs attribute?
It was intended as a debug/testing feature to allow tests in
intel-gpu-tools to enable or disable connectors:
http://lists.freedesktop.org/archives/intel-gfx/2014-May/045556.html
I'll update the commit message for the next version of the patch.
>
>> ---
>> drivers/gpu/drm/drm_sysfs.c | 49
>> +
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>> index c22c309..257816e 100644
>> --- a/drivers/gpu/drm/drm_sysfs.c
>> +++ b/drivers/gpu/drm/drm_sysfs.c
>> @@ -338,11 +338,60 @@ static ssize_t select_subconnector_show(struct device
>> *device,
>> drm_get_dvi_i_select_name((int)subconnector));
>> }
>>
>> +static ssize_t force_show(struct device *device,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct drm_connector *connector = to_drm_connector(device);
>> + char *status;
>
> "const char *" or gcc might warn on string assignments.
>
>> +
>> + switch (connector->force) {
>> + case DRM_FORCE_ON:
>> + status = "on";
>> + break;
>> +
>> + case DRM_FORCE_ON_DIGITAL:
>> + status = "digital";
>> + break;
>> +
>> + case DRM_FORCE_OFF:
>> + status = "off";
>> + break;
>> +
>> + case DRM_FORCE_UNSPECIFIED:
>> + status = "unspecified";
>> + break;
>> +
>> + default:
>> + return 0;
>> + }
>> +
>> + return snprintf(buf, PAGE_SIZE, "%s\n", status);
>> +}
>> +
>> +ssize_t force_store(struct device *device, struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct drm_connector *connector = to_drm_connector(device);
>> +
>> + if (strcmp(buf, "on") == 0)
>> + connector->force = DRM_FORCE_ON;
>> + else if (strcmp(buf, "digital") == 0)
>> + connector->force = DRM_FORCE_ON_DIGITAL;
>> + else if (strcmp(buf, "off") == 0)
>> + connector->force = DRM_FORCE_OFF;
>> + else if (strcmp(buf, "unspecified") == 0)
>> + connector->force = DRM_FORCE_UNSPECIFIED;
>
> else
> return -EINVAL;
>
>
> This really looks like a debug-feature to me. If it's a real feature,
> we _must_ rescan connectors here, otherwise it seems odd writing "on"
> into that file but nothing happens until the next modeset.
>
> Thanks
> David
>
>> +
>> + return count;
>> +}
>> +
>> static struct device_attribute connector_attrs[] = {
>> __ATTR_RO(status),
>> __ATTR_RO(enabled),
>> __ATTR_RO(dpms),
>> __ATTR_RO(modes),
>> + __ATTR_RW(force),
>> };
>>
>> /* These attributes are for both DVI-I connectors and all types of tv-out.
>> */
>> --
>> 1.9.0
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sysfs: expose the "force" connector attribute
Hi On Mon, May 19, 2014 at 4:41 PM, Thomas Wood wrote: > On 19 May 2014 15:13, David Herrmann wrote: >> Hi >> >> On Mon, May 19, 2014 at 3:37 PM, Thomas Wood >> wrote: >>> Signed-off-by: Thomas Wood >> >> The commit-msg lacks any discussion why this change is done. What is >> the reason to do that? Isn't the kernel-command-line enough? Why is >> this a regular feature instead of a debugfs attribute? > > > It was intended as a debug/testing feature to allow tests in > intel-gpu-tools to enable or disable connectors: > > http://lists.freedesktop.org/archives/intel-gfx/2014-May/045556.html > > > I'll update the commit message for the next version of the patch. Thanks! But please make it a debugfs feature, if possible. We shouldn't expose interfaces in sysfs that aren't part of the core API. Note that this might require you to encode the connector-name in the debugfs-attribute-name. Thanks David
[RFC PATCH 2/2 with seqcount v3] reservation: add suppport for read-only access using rcu
On 05/19/2014 03:13 PM, Maarten Lankhorst wrote:
> op 19-05-14 15:42, Thomas Hellstrom schreef:
>> Hi, Maarten!
>>
>> Some nitpicks, and that krealloc within rcu lock still worries me.
>> Otherwise looks good.
>>
>> /Thomas
>>
>>
>>
>> On 04/23/2014 12:15 PM, Maarten Lankhorst wrote:
>>> @@ -55,8 +60,8 @@ int reservation_object_reserve_shared(struct
>>> reservation_object *obj)
>>> kfree(obj->staged);
>>> obj->staged = NULL;
>>> return 0;
>>> -}
>>> -max = old->shared_max * 2;
>>> +} else
>>> +max = old->shared_max * 2;
>> Perhaps as a separate reformatting patch?
> I'll fold it in to the patch that added
> reservation_object_reserve_shared.
>>> +
>>> +int reservation_object_get_fences_rcu(struct reservation_object *obj,
>>> + struct fence **pfence_excl,
>>> + unsigned *pshared_count,
>>> + struct fence ***pshared)
>>> +{
>>> +unsigned shared_count = 0;
>>> +unsigned retry = 1;
>>> +struct fence **shared = NULL, *fence_excl = NULL;
>>> +int ret = 0;
>>> +
>>> +while (retry) {
>>> +struct reservation_object_list *fobj;
>>> +unsigned seq;
>>> +
>>> +seq = read_seqcount_begin(&obj->seq);
>>> +
>>> +rcu_read_lock();
>>> +
>>> +fobj = rcu_dereference(obj->fence);
>>> +if (fobj) {
>>> +struct fence **nshared;
>>> +
>>> +shared_count = ACCESS_ONCE(fobj->shared_count);
>> ACCESS_ONCE() shouldn't be needed inside the seqlock?
> Yes it is, shared_count may be increased, leading to potential
> different sizes for krealloc and memcpy
> if the ACCESS_ONCE is removed. I could use shared_max here instead,
> which stays the same,
> but it would waste more memory.
OK.
>
>>> +nshared = krealloc(shared, sizeof(*shared) *
>>> shared_count, GFP_KERNEL);
>> Again, krealloc should be a sleeping function, and not suitable within a
>> RCU read lock? I still think this krealloc should be moved to the start
>> of the retry loop, and we should start with a suitable guess of
>> shared_count (perhaps 0?) It's not like we're going to waste a lot of
>> memory
> But shared_count is only known when holding the rcu lock.
>
> What about this change?
Sure. That should work.
/Thomas
>
> @@ -254,16 +254,27 @@ int reservation_object_get_fences_rcu(struct
> reservation_object *obj,
> fobj = rcu_dereference(obj->fence);
> if (fobj) {
> struct fence **nshared;
> +size_t sz;
>
> shared_count = ACCESS_ONCE(fobj->shared_count);
> -nshared = krealloc(shared, sizeof(*shared) *
> shared_count, GFP_KERNEL);
> +sz = sizeof(*shared) * shared_count;
> +
> +nshared = krealloc(shared, sz,
> + GFP_NOWAIT | __GFP_NOWARN);
> if (!nshared) {
> +rcu_read_unlock();
> +nshared = krealloc(shared, sz, GFP_KERNEL)
> +if (nshared) {
> +shared = nshared;
> +continue;
> +}
> +
> ret = -ENOMEM;
> -shared_count = retry = 0;
> -goto unlock;
> +shared_count = 0;
> +break;
> }
> shared = nshared;
> -memcpy(shared, fobj->shared, sizeof(*shared) *
> shared_count);
> +memcpy(shared, fobj->shared, sz);
> } else
> shared_count = 0;
> fence_excl = rcu_dereference(obj->fence_excl);
>
>
>>> +
>>> +/*
>>> + * There could be a read_seqcount_retry here, but nothing
>>> cares
>>> + * about whether it's the old or newer fence pointers that are
>>> + * signale. That race could still have happened after checking
>> Typo.
> Oops
[PATCH] drm/sysfs: expose the "force" connector attribute
On Mon, May 19, 2014 at 04:44:15PM +0200, David Herrmann wrote: > Hi > > On Mon, May 19, 2014 at 4:41 PM, Thomas Wood wrote: > > On 19 May 2014 15:13, David Herrmann wrote: > >> Hi > >> > >> On Mon, May 19, 2014 at 3:37 PM, Thomas Wood > >> wrote: > >>> Signed-off-by: Thomas Wood > >> > >> The commit-msg lacks any discussion why this change is done. What is > >> the reason to do that? Isn't the kernel-command-line enough? Why is > >> this a regular feature instead of a debugfs attribute? > > > > > > It was intended as a debug/testing feature to allow tests in > > intel-gpu-tools to enable or disable connectors: > > > > http://lists.freedesktop.org/archives/intel-gfx/2014-May/045556.html > > > > > > I'll update the commit message for the next version of the patch. > > Thanks! But please make it a debugfs feature, if possible. We > shouldn't expose interfaces in sysfs that aren't part of the core API. > Note that this might require you to encode the connector-name in the > debugfs-attribute-name. Imo having the read and write side in completely different parts doesn't make a lot of sense. Hence I think doing this in sysfs is ok. Also users might want to frob this for testing, and usually debugfs is a bit further away on most systems. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 77244] UVD microcode fails to load on HD 8970M (Neptune XT -- PITCAIRN)
https://bugs.freedesktop.org/show_bug.cgi?id=77244 Nathan Schulte changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WORKSFORME --- Comment #11 from Nathan Schulte --- This bug seems resolved for me in 3.14.4. UVD initializes fine. PRIME is still having issues, though. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/4e3affc6/attachment-0001.html>
[Bug 78453] [HAWAII] Get acceleration working
https://bugs.freedesktop.org/show_bug.cgi?id=78453 --- Comment #34 from vincent --- FWIW I also tested with R600_DEBUG=nohyperz and R600_DEBUG=nodma but it doesnt solve the issue (although I'm not sure these env var are useful on radeonsi). Unfortunatly egltri_screen display a triangle but sometimes and in a very broken way : always a message about gpu stall, sometimes it display garbage, sometimes a triangle but on the right or on the left. The gpu is definitively doing some rendering works correctly but it fails before swapping buffer. BTW why eglinfo also make the gpu stall ? As far as I can tell there is no draw operation involved, just egl initialisation and unintialisation. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/cef35873/attachment.html>
[deathsimple/drm-next-3.16][PATCH V4 1/4] drm/radeon/hdmi: use separated file for DCE 3.1/3.2 code
On Fri, May 16, 2014 at 5:36 AM, Rafa? Mi?ecki wrote:
> DCE 3.1 and 3.2 should be programmed in a different way than DCE 2 and
> DCE 3. The order of setting registers and sets of registers are
> different.
> It's still unsure how we will handle DCE 3.1 vs. DCE 3.2, since they
> have few differences as well.
> For now separate DCE 2 and DCE 3 path, so we can work on it without a
> risk of breaking DCE 3.1+.
>
> This has been tested for possible regressions on DCE32 HD4550 (RV710).
>
> Signed-off-by: Rafa? Mi?ecki
Added the series to my 3.16 queue.
Alex
> ---
> V4: Add Copyright header. Alex is an author of
> dce3_2_afmt_write_speaker_allocation
> dce3_2_afmt_write_sad_regs
> and I'm pretty much an author of
> dce3_1_hdmi_setmode
> ---
> drivers/gpu/drm/radeon/Makefile | 2 +-
> drivers/gpu/drm/radeon/dce3_1_afmt.c | 244
> +++
> drivers/gpu/drm/radeon/r600_hdmi.c | 149 ++---
> drivers/gpu/drm/radeon/radeon_asic.c | 2 +-
> drivers/gpu/drm/radeon/radeon_asic.h | 7 +
> 5 files changed, 263 insertions(+), 141 deletions(-)
> create mode 100644 drivers/gpu/drm/radeon/dce3_1_afmt.c
>
> diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile
> index 0943353..dbcbfe8 100644
> --- a/drivers/gpu/drm/radeon/Makefile
> +++ b/drivers/gpu/drm/radeon/Makefile
> @@ -72,7 +72,7 @@ radeon-y += radeon_device.o radeon_asic.o radeon_kms.o \
> radeon_cs.o radeon_bios.o radeon_benchmark.o r100.o r300.o r420.o \
> rs400.o rs600.o rs690.o rv515.o r520.o r600.o rv770.o radeon_test.o \
> r200.o radeon_legacy_tv.o r600_cs.o r600_blit_shaders.o \
> - radeon_pm.o atombios_dp.o r600_audio.o r600_hdmi.o \
> + radeon_pm.o atombios_dp.o r600_audio.o r600_hdmi.o dce3_1_afmt.o \
> evergreen.o evergreen_cs.o evergreen_blit_shaders.o \
> evergreen_hdmi.o radeon_trace_points.o ni.o cayman_blit_shaders.o \
> atombios_encoders.o radeon_semaphore.o radeon_sa.o atombios_i2c.o
> si.o \
> diff --git a/drivers/gpu/drm/radeon/dce3_1_afmt.c
> b/drivers/gpu/drm/radeon/dce3_1_afmt.c
> new file mode 100644
> index 000..51800e3
> --- /dev/null
> +++ b/drivers/gpu/drm/radeon/dce3_1_afmt.c
> @@ -0,0 +1,244 @@
> +/*
> + * Copyright 2013 Advanced Micro Devices, Inc.
> + * Copyright 2014 Rafa? Mi?ecki
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +#include
> +#include
> +#include "radeon.h"
> +#include "radeon_asic.h"
> +#include "r600d.h"
> +
> +static void dce3_2_afmt_write_speaker_allocation(struct drm_encoder *encoder)
> +{
> + struct radeon_device *rdev = encoder->dev->dev_private;
> + struct drm_connector *connector;
> + struct radeon_connector *radeon_connector = NULL;
> + u32 tmp;
> + u8 *sadb;
> + int sad_count;
> +
> + list_for_each_entry(connector,
> &encoder->dev->mode_config.connector_list, head) {
> + if (connector->encoder == encoder) {
> + radeon_connector = to_radeon_connector(connector);
> + break;
> + }
> + }
> +
> + if (!radeon_connector) {
> + DRM_ERROR("Couldn't find encoder's connector\n");
> + return;
> + }
> +
> + sad_count = drm_edid_to_speaker_allocation(radeon_connector->edid,
> &sadb);
> + if (sad_count < 0) {
> + DRM_ERROR("Couldn't read Speaker Allocation Data Block:
> %d\n", sad_count);
> + return;
> + }
> +
> + /* program the speaker allocation */
> + tmp = RREG32(AZ_F0_CODEC_PIN0_CONTROL_CHANNEL_SPEAKER);
> + tmp &= ~(DP_CONNECTION | SPEAKER_ALLOCATION_MASK);
> + /* set HDMI mode */
> + tmp |= HDMI_CONNECTION;
> + if (sad_count)
> + tmp |= SPEAKER_ALLOCATION(sadb[0]);
> + else
> + tmp |= SPEAKER_ALLOCATION
[PATCH v2 5/7] drm/tegra: Convert to master/component framework
On Mon, May 19, 2014 at 10:56:08AM +0200, Christian Gmeiner wrote: > 2014-05-13 17:30 GMT+02:00 Thierry Reding : > > From: Thierry Reding > > > > Instead of the current implementation, reuse the recently introduced > > master/component framework, which is equivalent in most regards. One > > issue is that there is no device to bind the DRM driver to. In order > > to still allow the driver to be probed, expose an interface from the > > host1x device and provide an interface driver to bind to that. > > > > The interface driver then registers (or removes) the master that will be > > used to instantiate and bind the DRM driver. Since the drm_host1x bus > > implementation is now gone the dummy device instantiated by it can no > > longer be used as the parent for the DRM device. However since the > > parent device doesn't need to be modified, the host1x parent device that > > exposes the interface can be used instead. > > > > Signed-off-by: Thierry Reding > > Is there a git tree somewhere to have a look at? I am interested in the usage > of the master/component framework. I've pushed my latest working branches to gitorious[0]. staging/work is the branch that has everything. Thierry [0]: https://gitorious.org/thierryreding/linux -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/7571aa61/attachment.sig>
[RFC V3 2/3] drm/bridge: add a dummy panel driver to support lvds bridges
On Sun, May 18, 2014 at 01:50:36PM +0530, Ajay kumar wrote: > On Sun, May 18, 2014 at 2:44 AM, Thierry Reding > wrote: > > On Thu, May 15, 2014 at 05:10:16PM +0530, Ajay kumar wrote: > >> On Thu, May 15, 2014 at 1:43 PM, Thierry Reding >> gmail.com> wrote: > > [...] > >> > I still don't see how controlling the enable GPIO from the panel will be > >> > in any way better, or different for that matter, from simply enabling > >> > the backlight and let the backlight driver handle the enable GPIO. Have > >> > you tried to do that and found that it doesn't work? > >> It works, but it gives me glitch when I try to configure video. > >> This is because backlight_on(pwm-backlight) happens much before > >> I configure video(inside drm), and while configuring video there would > >> be a glitch, > >> and that glitch would be visible to the user since backlight is already > >> enabled. > > > > Okay, so this means that your backlight is turned on too early. Instead > > of working around that problem by moving control of the backlight enable > > GPIO from the backlight driver into the panel driver, the correct way to > > fix it is to make sure the backlight stays off until video is ready. > Ok. Please suggest an idea how to do the same! I did post a patch[0] a long time ago that added a way to fix this for pwm-backlight at least. > I have already suggested a simple idea which conforms to a valid spec. > Just because enable_gpio is already a part of pwm_bl.c, I somewhat feel > like we are forcing people to handle enable_gpio inside pwm_bl.c. And that's a good thing. That's what people will expect. Backlights are exposed via sysfs, which is currently also the only way to control the backlight from userspace. If the driver for that doesn't have everything required to control the backlight how can userspace control it? > Note that, pwm_bl can still work properly without enabling the backlight GPIO. > And, I did point out to a valid datasheet from AUO, which clearly indicates > why > backlight enable GPIO should be a part of panel driver and not pwm_bl driver. Just because some spec mentions the backlight enable pin in some panel power up sequence diagram that doesn't mean we have to implement it as part of the panel driver. > I am not trying to say we should remove enable_gpio from pwm_bl. > Provided that its already an optional property, and if someone wants > to control it in a panel driver, what is wrong in doing so? See above. Thierry [0]: https://lkml.org/lkml/2013/10/7/188 -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140519/ae2f37b2/attachment.sig>
[PATCH] drm/sysfs: expose the "force" connector attribute
Hi On Mon, May 19, 2014 at 4:53 PM, Daniel Vetter wrote: > On Mon, May 19, 2014 at 04:44:15PM +0200, David Herrmann wrote: >> On Mon, May 19, 2014 at 4:41 PM, Thomas Wood >> wrote: >> > It was intended as a debug/testing feature to allow tests in >> > intel-gpu-tools to enable or disable connectors: >> > >> > http://lists.freedesktop.org/archives/intel-gfx/2014-May/045556.html >> > >> > >> > I'll update the commit message for the next version of the patch. >> >> Thanks! But please make it a debugfs feature, if possible. We >> shouldn't expose interfaces in sysfs that aren't part of the core API. >> Note that this might require you to encode the connector-name in the >> debugfs-attribute-name. > > Imo having the read and write side in completely different parts doesn't > make a lot of sense. Hence I think doing this in sysfs is ok. Also users > might want to frob this for testing, and usually debugfs is a bit further > away on most systems. So the read-side is not debug-only? That wasn't clear to me. In that case, I'm fine with keeping it in sysfs, although I'm not entirely sure why anyone is interested in "force" information. Thanks David
