[Bug 51787] performance regression with llvm shader compiler in ut2004

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=51787

--- Comment #3 from Andy Furniss  2012-08-02 
22:32:43 UTC ---
(In reply to comment #2)
> It might not help in fixing this, but I found that the framerate is much more
> consistent if I load all cpu cores with something while playing ut2004.
> Normally, the framerate counter is green, but flashes into yellow and purple,
> which makes movements very choppy. When something is running in the 
> background,
> the framerate counter stays green, and all movements are smooth.

Maybe cpufreq is causing this here's what I get on the demo on an HD4890 +
4x3.4GHz phenom II.

R600_LLVM=0 ut2004
as-convoy?spectatoronly=1?numbots=8?quickstart=1?attractcam=1 -benchmark
-seconds=120 -nosound

cpufreq ondemand - 

30.027578 / 76.497787 / 162.457611 fps

cpufreq set to performance

37.627441 / 93.523949 / 197.051376 fps

> 
> With the llvm compiler the game is unplayable either way.

I see the stalling in the demo benchmark.

2.030273 / 70.589706 / 196.047073 fps

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

[Bug 51787] performance regression with llvm shader compiler in ut2004

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=51787

--- Comment #2 from almos  2012-08-02 22:04:09 UTC ---
It might not help in fixing this, but I found that the framerate is much more
consistent if I load all cpu cores with something while playing ut2004.
Normally, the framerate counter is green, but flashes into yellow and purple,
which makes movements very choppy. When something is running in the background,
the framerate counter stays green, and all movements are smooth.

With the llvm compiler the game is unplayable either way.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

[Alex Deucher]

On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui  wrote:
> On ?, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote:
>> On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote:
>> > On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote:
>> > > AMD ACPI interface may overload the standard event
>> > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such
>> > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the
>> > > userspace because the user did not press the mode switch key (the
>> > > spurious keypress confuses the DE which usually changes the
>> > > display configuration and messes up a dual-screen setup).
>> > > This patch gives the radeon driver the chance to examine the event and
>> > > block the keypress if the event is an "AMD event".
>> > >
>> > > Signed-off-by: Luca Tettamanti 
>> > > ---
>> > > Any comment from ACPI front?
>> > >
>> > it looks good to me.
>> > But I'm wondering if we can use the following code for ACPI part, which
>> > looks cleaner.
>> > I know this may change the behavior of other events, but in theory, we
>> > should not send any input event if we know something wrong in kernel.
>> >
>> > what do you think?
>>
>> I like it, it's cleaner.
>> I've split the patch in two pieces (one for video, the other for
>> radeon) and adopted your suggestion.
>>
> Great.
> Acked-by: Zhang Rui 
>
> hmm, who should take these two patches?

I'm happy to take the patches.

> and which tree the second patch is based on?

I've got a tree with all the radeon ACPI patches on the acpi_patches
branches of my git tree:
git://people.freedesktop.org/~agd5f/linux

Alex

>
> thanks,
> rui
>

drm/nouveau: crash regression in 3.5

[Marcin Slusarz]

On Thu, Aug 02, 2012 at 01:26:55PM +0200, Ortwin Gl?ck wrote:
> I have managed to turn the crash into a WARN_ON, by adding this to the 
> begin of nouveau_software_vblank():
> 
>  if (!psw) {
>  WARN_ON(1);
>  return;
>  }

Yes, I know about it, but this change fixes only a symptom. We should not get
into this code at all without enabling vblank.

> And I have also managed to load the module manually instead by udev. So 
> I am happy to attach a full log of what's going on here. See also my 
> added printk's starting with XXX that mark some interesting points in 
> the initialization.
> 
> This should give you enough information to track down the cause of the 
> problem. To my non-expert eyes it looks like "noaccel" prevents 
> registration of NVOBJ_ENGINE_SW or at least delays it too much.

Yes, that's what I wrote in my last patch - with noaccel it's not registered,
which leads to NULL pointer derefence.

I'm currently out of ideas why my patch does not work. But I have some ideas
how to debug it. Can you come to nouveau IRC channel at freenode and do some
additional tests? My nick is "joi" and I'm available on IRC between 6pm and
11pm CEST.

Marcin

[PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Luca Tettamanti]

On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher  wrote:
> I admit I'm not really an ACPI expert, but thinking about this more,
> I'm wondering if maybe we should just send the appropriate brightness
> change, switch display, etc. event to userspace rather than handling
> it directly in the radeon driver, then let userspace callback down via
> the bl interface, etc.  With backlight for example, does handling it
> in the kernel driver as per your patch prevent userspace from seeing
> the brightness up/down event?  Wouldn't that break things like OSD
> brightness displays and such?

No, the event is sent to userspace by the standard ACPI video driver,
it works as before.
Changing brightness usually goes like this:
1) user presses a hotkey
2) a notification is generated (0x86 or 0x87)
3) video.ko handles the notification and calls into ACPI to change the
level (_BCM) and firmware does its magic
4) a key press (brightness up/down) is sent to userspace

With ATIF step 3 does not actually change the brightness, it just send
out another event (VIDEO_PROBE, or one of the device specific ones) so
we need to take care of that too. The rest of the process, including
the delivery of the key presses, stays the same.

Luca

[PATCHv7 03/15] v4l: vb2: add support for shared buffer (dma_buf)

[Tomasz Stanislawski]

Hi Laurent, Hi Dima,

On 06/27/2012 10:40 PM, Laurent Pinchart wrote:
> Hi Dima,
> 
> On Tuesday 26 June 2012 13:53:34 Dima Zavin wrote:
>> On Tue, Jun 26, 2012 at 2:40 AM, Hans Verkuil  wrote:
>>> On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote:
 On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote:
> Hi Dima Zavin,
> Thank you for the patch and for a ping remainder :).
>
> You are right. The unmap is missing in __vb2_queue_cancel.
> I will apply your fix into next version of V4L2 support for dmabuf.
>
> Please refer to some comments below.
>
> On 06/20/2012 08:12 AM, Dima Zavin wrote:
>> Tomasz,
>>
>> I've encountered an issue with this patch when userspace does several
>> stream_on/stream_off cycles. When the user tries to qbuf a buffer
>> after doing stream_off, we trigger the "dmabuf already pinned"
>> warning since we didn't unmap the buffer as dqbuf was never called.
>>
>> The below patch adds calls to unmap in queue_cancel, but my feeling
>> is that we probably should be calling detach too (i.e. put_dmabuf).

 According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart
 of aborting or finishing any DMA in progress, unlocks any user pointer
 buffers locked in physical memory, and it removes all buffers from the
 incoming and outgoing queues".
>>>
>>> Correct. And what that means in practice is that after a streamoff all
>>> buffers are returned to the state they had just before STREAMON was
>>> called.
>>
>> That can't be right. The buffers had to have been returned to the
>> state just *after REQBUFS*, not just *before STREAMON*. You need to
>> re-enqueue buffers before calling STREAMON. I assume that's what you
>> meant?
> 
> Your interpretation is correct.
> 

So now we should decide what should be changed: the spec or vb2 ?
Bringing the queue state back to *after REQBUFS* may make the
next (STREAMON + QBUFs) very costly operations.

Reattaching and mapping a DMABUF might trigger DMA allocation and
*will* trigger creation of IOMMU mappings. In case of a user pointer,
calling next get_user_pages may cause numerous fault events and
will *create* new IOMMU mapping.

Is there any need to do such a cleanup if the destruction of buffers
and all caches can be explicitly executed by REQBUFS(count = 0) ?

Maybe it would be easier to change the spec by removing
"apart of ... in physical memory" part?

STREAMOFF should mean stopping streaming, and that resources are no
longer used. DMABUFs are unmapped but unmapping does not mean releasing.
The exporter may keep the resource in its caches.

Currently, vb2 does not follow the policy from the spec.
The put_userptr ops is called on:
- VIDIOC_REBUFS
- VIDIOC_CREATE_BUFS
- vb2_queue_release() which is usually called on close() syscall

The put_userptr is not called and streamoff therefore the user pages
are locked after STREAMOFF.

BTW. What does 'buffer locked' mean?

Does it mean that a buffer is pinned or referenced by a driver/HW?

Regards,
Tomasz Stanislawski


>>> So after STREAMOFF you can immediately queue all buffers again with QBUF
>>> and call STREAMON to restart streaming. No mmap or other operations
>>> should be required. This behavior must be kept.
>>>
>>> VIDIOC_REQBUFS() or a close() are the only two operations that will
>>> actually free the buffers completely.
>>>
>>> In practice, a STREAMOFF is either followed by a STREAMON at a later time,
>>> or almost immediately followed by REQBUFS or close() to tear down the
>>> buffers. So I don't think the buffers should be detached at streamoff.
>>
>> I agree. I was leaning this way which is why I left it out of my patch
>> and wanted to hear your guys' opinion as you are much more familiar
>> with the intended behavior than I am.
>>
>> Thanks!
> 
> You're welcome. Thank you for reporting the problem and providing a patch.
> 

[PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Alex Deucher]

On Thu, Aug 2, 2012 at 12:33 PM, Alex Deucher  wrote:
> On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti  
> wrote:
>> On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher  
>> wrote:
>>> I admit I'm not really an ACPI expert, but thinking about this more,
>>> I'm wondering if maybe we should just send the appropriate brightness
>>> change, switch display, etc. event to userspace rather than handling
>>> it directly in the radeon driver, then let userspace callback down via
>>> the bl interface, etc.  With backlight for example, does handling it
>>> in the kernel driver as per your patch prevent userspace from seeing
>>> the brightness up/down event?  Wouldn't that break things like OSD
>>> brightness displays and such?
>>
>> No, the event is sent to userspace by the standard ACPI video driver,
>> it works as before.
>> Changing brightness usually goes like this:
>> 1) user presses a hotkey
>> 2) a notification is generated (0x86 or 0x87)
>> 3) video.ko handles the notification and calls into ACPI to change the
>> level (_BCM) and firmware does its magic
>> 4) a key press (brightness up/down) is sent to userspace
>>
>> With ATIF step 3 does not actually change the brightness, it just send
>> out another event (VIDEO_PROBE, or one of the device specific ones) so
>> we need to take care of that too. The rest of the process, including
>> the delivery of the key presses, stays the same.

Updated tree with fixes to a few existing patches and improved support
for ATPX and initial support for ATCS:
http://cgit.freedesktop.org/~agd5f/linux/?h=acpi_patches

Alex

[RFC 0/3] solving omapdrm/omapdss layering issues

[Tomi Valkeinen]

On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote:
> On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen  
> wrote:
> > On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote:
> >> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen  
> >> wrote:
> >
> >> > I guess the fact is that DRM concepts do not really match the OMAP DSS
> >> > hardware, and we'll have to use whatever gives us least problems.
> >>
> >> Actually, I think it does map fairly well to the hardware.. at least
> >> more so than to omapdss ;-)
> >
> > Hm, I'm not sure I understand, omapdss concepts map directly to the
> > hardware.
> 
> I think it is mainly exposing the encoder and panel as two separate
> entities.. which seems to be what Archit is working on

I still don't follow =) They are separate entities. Omapdss models the
HW quite directly, I think. It doesn't expose everything, though, as the
output drivers (dsi.c, dpi.c etc) are used via the panel drivers.

> in case of something like DVI bridge from DPI, this seems pretty
> straightforward.. only the connector needs to know about DDC stuff,
> which i2c to use and that sort of thing.  So at kms level we would
> have (for example) an omap_dpi_encoder which would be the same for DPI
> panel (connector) or DPI->DVI bridge.  For HDMI I'm still looking
> through the code to see how this would work.  Honestly I've looked
> less at this part of code and encoder related registers in the TRM,
> compared to the ovl/mgr parts, but at least from the 'DSS overview'
> picture in the TRM it seems to make sense ;-)
> 
> KMS even exposes the idea that certain crtcs can connect to only
> certain encoders.  Or that you could you could have certain connectors
> switched between encoders.  For example if you had a hw w/ DPI out,
> and some mux to switch that back and forth between a DPI lcd panel and
> a DPI->DVI bridge.  (Ok, I'm not aware of any board that actually does
> this, but it is in theory possible.)  So we could expose possible
> video chain topologies to userspace in this way.

OMAP3 SDP board has such a setup, with manual switch to select between
LCD and DVI.

> The other thing is that we don't need to propagate timings from the
> panel up to the mgr at the dss level.. kms is already handling this
> for us.  In my latest version, which I haven't pushed, I removed the
> 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'.

You're thinking only about simple DPI cases. Consider this example, with
a DSI-to-DP bridge chip. What we have is the following flow of data:

DISPC -> DSI -> DSI-2-DP -> DP monitor

The timings you are thinking about are in the DISPC, but here they are
only one part of the link. And here the DISPC timings are not actually
the timings what the user is interested about. The user wants his
timings to be between DSI-2-DP chip and the DP monitor.

Timings programmed to DISPC are not the same. The timings for DISPC come
from the DSI driver, and they may be very different than the user's
timings. With DSI video mode, the DISPC timings would have some
resemblance to the user's timings, mainly the time to send one line
would be the same. With DSI cmd mode, the DISPC timings would be
something totally different, most likely with 0 blank times and as fast
pixel clock as possible.

What omapdss does currently is that you set the user's timings to the
right side of the chain, which propagate back to DSS. This allows the
DSI-2-DP bridge use DSI timings that work optimally for the bridge, and
DSI driver will use DISPC timings that work optimally for it.

And it's not only about timings above, but also other settings related
to the busses between the components. Clock dividers, polarities, stuff
like that.

> I think the problem was there were some cases, like ovl updates before
> setting the mgr, where the user_info_dirty flag would be cleared but
> the registers not updated.  This is what I meant by sequence of

Hmm, I'd appreciate more info about this if you can give. Sounds like a
bug, that shouldn't happen.

So you mean that you have an ovl, with no manager set. And you change
the settings of the ovl before assigning it to a mgr? That's something I
haven't really tested, so it could bug, true.

> operations at KMS level confusing omapdss.  This should be fixable
> with some debugging.  Although getting rid of the state tracking at
> omapdss level altogether was a much simpler solution, and is the one I
> prefer :-)

Yes, I don't prefer the state tracking either (we didn't have it in
earlier versions of omapdss), but I still don't see an option to it if
we want to support all the stuff we have.

 Tomi

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120802/ca1d188c/attachment.pgp>

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

[Luca Tettamanti]

On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote:
> On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote:
> > AMD ACPI interface may overload the standard event
> > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such
> > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the
> > userspace because the user did not press the mode switch key (the
> > spurious keypress confuses the DE which usually changes the
> > display configuration and messes up a dual-screen setup).
> > This patch gives the radeon driver the chance to examine the event and
> > block the keypress if the event is an "AMD event".
> > 
> > Signed-off-by: Luca Tettamanti 
> > ---
> > Any comment from ACPI front?
> > 
> it looks good to me.
> But I'm wondering if we can use the following code for ACPI part, which
> looks cleaner.
> I know this may change the behavior of other events, but in theory, we
> should not send any input event if we know something wrong in kernel.
> 
> what do you think?

I like it, it's cleaner.
I've split the patch in two pieces (one for video, the other for
radeon) and adopted your suggestion.

BTW, I'm leaving for vacation in a few hours, I'll be offline till mid
August.

Luca
-- next part --
>From acce30c96b90d1bc550e82a9e7f19226fa194d5e Mon Sep 17 00:00:00 2001
From: Luca Tettamanti 
Date: Thu, 2 Aug 2012 15:30:27 +0200
Subject: [PATCH 1/2] ACPI video: allow events handlers to veto the keypress

The standard video events may be overloaded for device specific
purposes. For example AMD ACPI interface overloads
ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such
cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the
userspace because the user did not press the mode switch key (the
spurious keypress confuses the DE which usually changes the
display configuration and messes up a dual-screen setup).
This patch gives the handlers the chance to examine the event and
block the keypress if the event is device specific.
v2: refactor as suggested by Zhang Rui 

Signed-off-by: Luca Tettamanti 
---
 drivers/acpi/video.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 1e0a9e1..d75642a 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
 * most likely via hotkey. */
acpi_bus_generate_proc_event(device, event, 0);
-   if (!acpi_notifier_call_chain(device, event, 0))
-   keycode = KEY_SWITCHVIDEOMODE;
+   keycode = KEY_SWITCHVIDEOMODE;
break;

case ACPI_VIDEO_NOTIFY_PROBE:   /* User plugged in or removed a video
@@ -1479,8 +1478,9 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
break;
}

-   if (event != ACPI_VIDEO_NOTIFY_SWITCH)
-   acpi_notifier_call_chain(device, event, 0);
+   if (acpi_notifier_call_chain(device, event, 0))
+   /* Something vetoed the keypress. */
+   keycode = 0;

if (keycode) {
input_report_key(input, keycode, 1);
-- 
1.7.10.4

-- next part --
>From def5119d8f617eef0fac2cd35d7bb18c176ff8f6 Mon Sep 17 00:00:00 2001
From: Luca Tettamanti 
Date: Thu, 2 Aug 2012 15:33:03 +0200
Subject: [PATCH 2/2] drm/radeon: block the keypress on ATIF events

The AMD ACPI interface may use ACPI_VIDEO_NOTIFY_PROBE to signal SBIOS
requests; block the keypress in this case since the user did not
actually press the mode switch key.

Signed-off-by: Luca Tettamanti 
---
 drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
b/drivers/gpu/drm/radeon/radeon_acpi.c
index 96de08d..ee0d29e 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
}
/* TODO: check other events */

-   return NOTIFY_OK;
+   /* We've handled the event, stop the notifier chain. The ACPI interface
+* overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
+* userspace if the event was generated only to signal a SBIOS
+* request.
+*/
+   return NOTIFY_BAD;
 }

 /* Call all ACPI methods here */
-- 
1.7.10.4

[PATCH V3 11/16] drm/radeon: Make radeon card usable for Loongson.

[Huacai Chen]

Hi, Lucas, sorry for so long a delay because I have a holiday for one month.

I found Loongson-3 must turn on SWIOTLB if the system memory has
addresses above 4G. As I know, there are two ways to get a DMA addr,
the first way is use dma_alloc_coherent(), and the other one is use
map_page()/map_sg() on an exsisting address. The first way can make
sure DMA addr below 4G, but the second way cannot (unless the memory
is so little that all the address space is below 4G).

Take SATA driver as an example, during initialization an 'ata_device'
struct is allocated and its address is probably above 4G (because this
is first used by CPU, not for DMA). 'ata_device' has a member 'id' and
'id' will be use for DMA in such a call path:

ata_host_register() --> ata_scsi_add_hosts() --> async_port_probe()
--> ata_port_probe() --> ata_bus_probe() --> ata_dev_read_id() -->
ata_exec_internal() --> ata_exec_internal_sg() --> ata_qc_issue() -->
ata_sg_setup() --> dma_map_sg()

Here,  dma_map_sg() will get a DMA addr above 4G, then SATA init failed.

In OHCI driver, there are similar situations.

P.S.:  With recently drm changes, I found radeon with SWIOTLB can
already work after suspend/resume, so my next version of Loongson
patches will not modify radeon_ttm.c.

On Fri, Jun 22, 2012 at 1:59 PM, Huacai Chen  wrote:
> On Fri, Jun 22, 2012 at 1:25 PM, Lucas Stach  wrote:
>> Hello Huacai,
>>
>> Am Freitag, den 22.06.2012, 11:01 +0800 schrieb Huacai Chen:
>>> 1, Handle io prot correctly for MIPS.
>>> 2, Define SAREA_MAX as the size of one page.
>>> 3, Don't use swiotlb on Loongson machines (Loonson need swioitlb, but
>>>when use swiotlb, GPU reset occurs at resume from suspend).
>>>
>> I still think this is wrong. You say Loongson needs SWIOTLB, but when
>> it's actually used you ignore it in the radeon driver code.
>>
>> I looked up why you are using SWIOTLB and I don't agree with you that it
>> is needed. SWIOTLB just gives you bounce pages for DMA memory above
>> DMA32 and therefore papers over your >4GB DMA platform bug in some
>> cases, while hurting performance.
>>
>> Please fix your DMA platform code so that region DMA is an alias for
>> region DMA32. It should allow you to drop all those ugly workarounds.
>>
> Hmm, you are probably right, I think I should have a discuss with the
> original author of this part of code.
>
>>> Signed-off-by: Huacai Chen 
>>> Signed-off-by: Hongliang Tao 
>>> Signed-off-by: Hua Yan 
>>> Reviewed-by: Michel D?nzer 
>>> Reviewed-by: Alex Deucher 
>>> Reviewed-by: Lucas Stach 
>>> Reviewed-by: j.glisse 
>>
>> You should probably only stick this tag on your patches after the people
>> you are naming explicitly gave their r-b for a specific version of a
>> patch.
>>
>> Thanks,
>> Lucas
>>> Cc: dri-devel at lists.freedesktop.org
>>> ---
>>>  drivers/gpu/drm/drm_vm.c|2 +-
>>>  drivers/gpu/drm/radeon/radeon_ttm.c |6 +++---
>>>  drivers/gpu/drm/ttm/ttm_bo_util.c   |2 +-
>>>  include/drm/drm_sarea.h |2 ++
>>>  4 files changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
>>> index 961ee08..3f06166 100644
>>> --- a/drivers/gpu/drm/drm_vm.c
>>> +++ b/drivers/gpu/drm/drm_vm.c
>>> @@ -62,7 +62,7 @@ static pgprot_t drm_io_prot(uint32_t map_type, struct 
>>> vm_area_struct *vma)
>>>   tmp = pgprot_writecombine(tmp);
>>>   else
>>>   tmp = pgprot_noncached(tmp);
>>> -#elif defined(__sparc__) || defined(__arm__)
>>> +#elif defined(__sparc__) || defined(__arm__) || defined(__mips__)
>>>   tmp = pgprot_noncached(tmp);
>>>  #endif
>>>   return tmp;
>>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
>>> b/drivers/gpu/drm/radeon/radeon_ttm.c
>>> index c94a225..f49bdd1 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>>> @@ -630,7 +630,7 @@ static int radeon_ttm_tt_populate(struct ttm_tt *ttm)
>>>   }
>>>  #endif
>>>
>>> -#ifdef CONFIG_SWIOTLB
>>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3)
>>>   if (swiotlb_nr_tbl()) {
>>>   return ttm_dma_populate(>ttm, rdev->dev);
>>>   }
>>> @@ -676,7 +676,7 @@ static void radeon_ttm_tt_unpopulate(struct ttm_tt *ttm)
>>>   }
>>>  #endif
>>>
>>> -#ifdef CONFIG_SWIOTLB
>>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3)
>>>   if (swiotlb_nr_tbl()) {
>>>   ttm_dma_unpopulate(>ttm, rdev->dev);
>>>   return;
>>> @@ -906,7 +906,7 @@ static int radeon_ttm_debugfs_init(struct radeon_device 
>>> *rdev)
>>>   radeon_mem_types_list[i].show = _page_alloc_debugfs;
>>>   radeon_mem_types_list[i].driver_features = 0;
>>>   radeon_mem_types_list[i++].data = NULL;
>>> -#ifdef CONFIG_SWIOTLB
>>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3)
>>>   if (swiotlb_nr_tbl()) {
>>>   sprintf(radeon_mem_types_names[i], "ttm_dma_page_pool");
>>>   

[PATCH v2] of: Add videomode helper

[Stephen Warren]

On 07/05/2012 08:51 AM, Rob Herring wrote:
> On 07/04/2012 02:56 AM, Sascha Hauer wrote:
>> This patch adds a helper function for parsing videomodes from the devicetree.
>> The videomode can be either converted to a struct drm_display_mode or a
>> struct fb_videomode.

>> diff --git a/Documentation/devicetree/bindings/video/displaymode 
>> b/Documentation/devicetree/bindings/video/displaymode

>> +Example:
>> +
>> + display at 0 {
>> + /* 1920x1080p24 */
>> + clock = <5200>;
> 
> Should this use the clock binding? You probably need both constraints
> and clock binding though.

I don't think the clock binding is appropriate here. This binding
specifies a desired video mode, and should specify just the expected
clock frequency for that mode. Perhaps any tolerance imposed by the
specification used to calculate the mode timing could be specified too,
but the allowed tolerance (for a mode to be recognized by the dispaly)
is more driven by the receiving device than the transmitting device.

The clock bindings should come into play in the display controller that
sends a signal in that display mode. Something like:

mode: hd1080p {
clock = <5200>;
xres = <1920>;
yres = <1080>;

};

display-controller {
pixel-clock-source = < ...>; // common clock bindings here
default-mode = <>;

> Often you don't know the frequency up front and/or have limited control
> of the frequency (i.e. integer dividers). Then you have to adjust the
> margins to get the desired refresh rate. To do that, you need to know
> the ranges of values a panel can support. Perhaps you just assume you
> can increase the right-margin and lower-margins as I think you will hit
> pixel clock frequency max before any limit on margins.

I think this is more usually dealt with in HW design and matching
components.

The PLL/... feeding the display controller is going to have some known
specifications that imply which pixel clocks it can generate. HW
designers will pick a panel that accepts a clock the display controller
can generate. The driver for the display controller will just generate
as near of a pixel clock as it can to the rate specified in the mode
definition. I believe it'd be unusual for a display driver to start
fiddling with front-back porch (or margin) widths to munge the timing;
that kind of thing probably worked OK with analog displays, but with
digital displays where each pixel is clocked even in the margins, I
think that would cause more problems than it solved.

Similarly for external displays, the display controller will just pick
the nearest clock it can. If it can't generate a close enough clock, it
will just refuse to set the mode. In fact, a display controller driver
would typically validate this when the set of legal modes was enumerated
when initially detecting the display, so user-space typically wouldn't
even request invalid modes.

[PATCH v2] of: Add videomode helper

[Stephen Warren]

On 07/04/2012 01:56 AM, Sascha Hauer wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.

> diff --git a/Documentation/devicetree/bindings/video/displaymode 
> b/Documentation/devicetree/bindings/video/displaymode

> +Required properties:
> + - xres, yres: Display resolution
> + - left-margin, right-margin, hsync-len: Horizontal Display timing parameters
> +   in pixels
> +   upper-margin, lower-margin, vsync-len: Vertical display timing parameters 
> in
> +   lines

Perhaps bike-shedding, but...

For the margin property names, wouldn't it be better to use terminology
more commonly used for video timings rather than Linux FB naming. In
other words naming like:

hactive, hsync-len, hfront-porch, hback-porch?

> + - clock: displayclock in Hz
> +
> +Optional properties:
> + - width-mm, height-mm: Display dimensions in mm
> + - hsync-active-high (bool): Hsync pulse is active high
> + - vsync-active-high (bool): Vsync pulse is active high
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree 
> representation
> +corresponds to the one used by the Linux Framebuffer framework described 
> here in
> +Documentation/fb/framebuffer.txt. This representation has been chosen 
> because it's
> +the only format which does not allow for inconsistent parameters.Unlike the 
> Framebuffer
> +framework the devicetree has the clock in Hz instead of ps.

As Rob mentioned, I think there shouldn't be any mention of Linux FB here.

> +
> +Example:
> +
> + display at 0 {

This node appears to describe a video mode, not a display, hence the
node name seems wrong.

Many displays can support multiple different video modes. As mentioned
elsewhere, properties like display width/height are properties of the
display not the mode.

So, I might expect something more like the following (various overhead
properties like reg/#address-cells etc. elided for simplicity):

disp: display {
width-mm = <...>;
height-mm = <...>;
modes {
mode at 0 {
/* 1920x1080p24 */
clock = <5200>;
xres = <1920>;
yres = <1080>;
left-margin = <25>;
right-margin = <25>;
hsync-len = <25>;
lower-margin = <2>;
upper-margin = <2>;
vsync-len = <2>;
hsync-active-high;
};
mode at 1 {
};
};
};

display-connector {
display = <>;
// interface-specific properties such as pixel format here
};

Finally, have you considered just using an EDID instead of creating
something custom? I know that creating an EDID is harder than writing a
few simple properties into a DT, but EDIDs have the following advantages:

a) They're already standardized and very common.

b) They allow other information such as a display's HDMI audio
capabilities to be represented, which can then feed into an ALSA driver.

c) The few LCD panel datasheets I've seen actually quote their
specification as an EDID already, so deriving the EDID may actually be easy.

d) People familiar with displays are almost certainly familiar with
EDID's mode representation. There are many ways of representing display
modes (sync position vs. porch widths, htotal specified rather than
specifying all the components and hence htotal being calculated etc.).
Not everyone will be familiar with all representations. Conversion
errors are less likely if the target is EDID's familiar format.

e) You'll end up with exactly the same data as if you have a DDC-based
external monitor rather than an internal panel, so you end up getting to
a common path in display handling code much more quickly.

drm/nouveau: crash regression in 3.5

[Ortwin Glück]

I have managed to turn the crash into a WARN_ON, by adding this to the 
begin of nouveau_software_vblank():

 if (!psw) {
 WARN_ON(1);
 return;
 }

And I have also managed to load the module manually instead by udev. So 
I am happy to attach a full log of what's going on here. See also my 
added printk's starting with XXX that mark some interesting points in 
the initialization.

This should give you enough information to track down the cause of the 
problem. To my non-expert eyes it looks like "noaccel" prevents 
registration of NVOBJ_ENGINE_SW or at least delays it too much.

Thanks,

Ortwin
-- next part --
Aug  2 13:03:22 localhost kernel: Linux version 3.5.0 (root at ortwin-hp) (gcc 
version 4.5.3 (Gentoo 4.5.3-r2 p1.1, pie-0.4.7) ) #5 SMP PREEMPT Thu Aug 2 
13:01:46 CEST 2012
Aug  2 13:03:22 localhost kernel: Command line: root=/dev/sda5 rootfstype=ext4 
pciehp_force=1 nouveau.modeset=1 nouveau.noaccel=1 netconsole= at 
10.11.1.234/eth0, at 10.11.1.19/00:17:f2:c7:5f:06 drm.debug=15 debug nox
Aug  2 13:03:22 localhost kernel: e820: BIOS-provided physical RAM map:
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0x-0x0009fbff] usable
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0x0009fc00-0x0009] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0x000e-0x000f] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0x0010-0xbefc1fff] usable
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xbefc2000-0xbf6c1fff] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xbf6c2000-0xbf7c1fff] ACPI NVS
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xbf7c2000-0xbf7fefff] ACPI data
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xbf7ff000-0xbf7f] usable
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xbf80-0xbfff] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xe000-0xefff] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xfec0-0xfec00fff] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xfed1-0xfed13fff] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xfed19000-0xfed19fff] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xfed1b000-0xfed1] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xfee0-0xfee00fff] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0xffd0-0x] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0x0001-0x0001fbff] usable
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0x0001fc00-0x0001] reserved
Aug  2 13:03:22 localhost kernel: BIOS-e820: [mem 
0x0002-0x00023bff] usable
Aug  2 13:03:22 localhost kernel: NX (Execute Disable) protection: active
Aug  2 13:03:22 localhost kernel: DMI 2.6 present.
Aug  2 13:03:22 localhost kernel: DMI: Hewlett-Packard HP EliteBook 8540w/1521, 
BIOS 68CVD Ver. F.0E 11/25/2010
Aug  2 13:03:22 localhost kernel: e820: update [mem 0x-0x] 
usable ==> reserved
Aug  2 13:03:22 localhost kernel: e820: remove [mem 0x000a-0x000f] 
usable
Aug  2 13:03:22 localhost kernel: No AGP bridge found
Aug  2 13:03:22 localhost kernel: e820: last_pfn = 0x23c000 max_arch_pfn = 
0x4
Aug  2 13:03:22 localhost start-stop-daemon: 
pam_unix(start-stop-daemon:session): session opened for user nobody by (uid=0)
Aug  2 13:03:22 localhost kernel: MTRR default type: uncachable
Aug  2 13:03:22 localhost kernel: MTRR fixed ranges enabled:
Aug  2 13:03:22 localhost kernel: 0-9 write-back
Aug  2 13:03:22 localhost kernel: A-B uncachable
Aug  2 13:03:22 localhost kernel: C-F write-protect
Aug  2 13:03:22 localhost kernel: MTRR variable ranges enabled:
Aug  2 13:03:22 localhost kernel: 0 base 0FFC0 mask FFFC0 write-protect
Aug  2 13:03:22 localhost kernel: 1 base 0 mask F8000 write-back
Aug  2 13:03:22 localhost kernel: 2 base 08000 mask FC000 write-back
Aug  2 13:03:22 localhost kernel: 3 base 1 mask F write-back
Aug  2 13:03:22 localhost kernel: 4 base 2 mask FC000 write-back
Aug  2 13:03:22 localhost kernel: 5 base 23C00 mask FFC00 uncachable
Aug  2 13:03:22 localhost kernel: 6 disabled
Aug  2 13:03:22 localhost kernel: 7 disabled
Aug  2 13:03:22 localhost kernel: x86 PAT enabled: cpu 0, old 0x7040600070406, 
new 0x7010600070106
Aug  2 13:03:22 localhost kernel: e820: last_pfn = 0xbf800 max_arch_pfn = 
0x4
Aug  2 13:03:22 localhost kernel: initial memory mapped: [mem 
0x-0x1fff]
Aug  2 13:03:22 localhost kernel: Base memory trampoline at [88099000] 
99000 

[PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Alex Deucher]

On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti  wrote:
> On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher  wrote:
>> I admit I'm not really an ACPI expert, but thinking about this more,
>> I'm wondering if maybe we should just send the appropriate brightness
>> change, switch display, etc. event to userspace rather than handling
>> it directly in the radeon driver, then let userspace callback down via
>> the bl interface, etc.  With backlight for example, does handling it
>> in the kernel driver as per your patch prevent userspace from seeing
>> the brightness up/down event?  Wouldn't that break things like OSD
>> brightness displays and such?
>
> No, the event is sent to userspace by the standard ACPI video driver,
> it works as before.
> Changing brightness usually goes like this:
> 1) user presses a hotkey
> 2) a notification is generated (0x86 or 0x87)
> 3) video.ko handles the notification and calls into ACPI to change the
> level (_BCM) and firmware does its magic
> 4) a key press (brightness up/down) is sent to userspace
>
> With ATIF step 3 does not actually change the brightness, it just send
> out another event (VIDEO_PROBE, or one of the device specific ones) so
> we need to take care of that too. The rest of the process, including
> the delivery of the key presses, stays the same.

Excellent!  thanks for clarifying.

Alex

[PATCH] drm/radeon: fix virtual memory locking in case of reset

[Alex Deucher]

On Thu, Aug 2, 2012 at 12:05 PM,   wrote:
> From: Jerome Glisse 
>
> Lock/unlock mutex in proper order to avoid deadlock in case
> of GPU reset triggered from VM code path.
>
> Cc: stable at vger.kernel.org [3.5]
> Signed-off-by: Jerome Glisse 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/radeon/radeon_gart.c |   11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gart.c 
> b/drivers/gpu/drm/radeon/radeon_gart.c
> index b372005..7eabb59 100644
> --- a/drivers/gpu/drm/radeon/radeon_gart.c
> +++ b/drivers/gpu/drm/radeon/radeon_gart.c
> @@ -508,14 +508,19 @@ static void radeon_vm_unbind_locked(struct 
> radeon_device *rdev,
> while (vm->fence) {
> int r;
> r = radeon_fence_wait(vm->fence, false);
> -   if (r)
> +   if (r) {
> DRM_ERROR("error while waiting for fence: %d\n", r);
> +   }
> if (r == -EDEADLK) {
> +   /* release mutex and lock in right order */
> mutex_unlock(>vm_manager.lock);
> +   mutex_unlock(>mutex);
> r = radeon_gpu_reset(rdev);
> mutex_lock(>vm_manager.lock);
> -   if (!r)
> +   mutex_lock(>mutex);
> +   if (!r) {
> continue;
> +   }
> }
> break;
> }
> @@ -551,7 +556,9 @@ void radeon_vm_manager_fini(struct radeon_device *rdev)
> mutex_lock(>vm_manager.lock);
> /* unbind all active vm */
> list_for_each_entry_safe(vm, tmp, >vm_manager.lru_vm, list) {
> +   mutex_lock(>mutex);
> radeon_vm_unbind_locked(rdev, vm);
> +   mutex_unlock(>mutex);
> }
> rdev->vm_manager.funcs->fini(rdev);
> mutex_unlock(>vm_manager.lock);
> --
> 1.7.10.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/radeon: fix virtual memory locking in case of reset

[j.gli...@gmail.com]

From: Jerome Glisse 

Lock/unlock mutex in proper order to avoid deadlock in case
of GPU reset triggered from VM code path.

Cc: stable at vger.kernel.org [3.5]
Signed-off-by: Jerome Glisse 
---
 drivers/gpu/drm/radeon/radeon_gart.c |   11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gart.c 
b/drivers/gpu/drm/radeon/radeon_gart.c
index b372005..7eabb59 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -508,14 +508,19 @@ static void radeon_vm_unbind_locked(struct radeon_device 
*rdev,
while (vm->fence) {
int r;
r = radeon_fence_wait(vm->fence, false);
-   if (r)
+   if (r) {
DRM_ERROR("error while waiting for fence: %d\n", r);
+   }
if (r == -EDEADLK) {
+   /* release mutex and lock in right order */
mutex_unlock(>vm_manager.lock);
+   mutex_unlock(>mutex);
r = radeon_gpu_reset(rdev);
mutex_lock(>vm_manager.lock);
-   if (!r)
+   mutex_lock(>mutex);
+   if (!r) {
continue;
+   }
}
break;
}
@@ -551,7 +556,9 @@ void radeon_vm_manager_fini(struct radeon_device *rdev)
mutex_lock(>vm_manager.lock);
/* unbind all active vm */
list_for_each_entry_safe(vm, tmp, >vm_manager.lru_vm, list) {
+   mutex_lock(>mutex);
radeon_vm_unbind_locked(rdev, vm);
+   mutex_unlock(>mutex);
}
rdev->vm_manager.funcs->fini(rdev);
mutex_unlock(>vm_manager.lock);
-- 
1.7.10.4

[PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Alex Deucher]

I admit I'm not really an ACPI expert, but thinking about this more,
I'm wondering if maybe we should just send the appropriate brightness
change, switch display, etc. event to userspace rather than handling
it directly in the radeon driver, then let userspace callback down via
the bl interface, etc.  With backlight for example, does handling it
in the kernel driver as per your patch prevent userspace from seeing
the brightness up/down event?  Wouldn't that break things like OSD
brightness displays and such?

Alex

On Wed, Aug 1, 2012 at 9:56 AM, Alex Deucher  wrote:
> On Wed, Aug 1, 2012 at 4:57 AM, Luca Tettamanti  
> wrote:
>> On Tue, Jul 31, 2012 at 05:33:16PM -0400, Alex Deucher wrote:
>>> Patches look good.  I picked them up and combined them with may
>>> patches plus a few other small fixes.  They are available here:
>>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches
>>> Let me know what you think.
>>
>> Looks ok, I lost one fix along the road though, I'm attaching the patch.
>
> Thanks, I rolled it into your original patch.  New tree:
> http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches
>
> Alex
>
>>
>> Luca

[RFC 0/3] solving omapdrm/omapdss layering issues

[Tomi Valkeinen]

On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote:
> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen  
> wrote:

> > I guess the fact is that DRM concepts do not really match the OMAP DSS
> > hardware, and we'll have to use whatever gives us least problems.
> 
> Actually, I think it does map fairly well to the hardware.. at least
> more so than to omapdss ;-)

Hm, I'm not sure I understand, omapdss concepts map directly to the
hardware.

> The one area that kms mismatches a bit is decoupling of ovl from mgr
> that we have in our hw..  I've partially solved that a while back w/

What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't
the drm_plane stuff separate these?

> For enabling/disabling an output (manager+encoder), this is relatively
> infrequent, so it can afford to block to avoid races.  (Like userspace
> enabling and then rapidly disabling an output part way through the
> enable.)  But enabling/disabling an overlay, or adjusting position or
> scanout address must not block.  And ideally, if possible, switching
> an overlay between two managers should not block.

Adjusting the position or address of the buffer are simple, they can be
easily done without any blocking.

But ovl enable/disable and switching an ovl to another mgr do (possibly)
take multiple vsyncs (and in the switch case, vsyncs of two separate
outputs). So if those do not block, we'll need to handle them as a state
machine and try to avoid races etc. It'll be "interesting".

However, we can sometimes do those operations immediately. So I think we
should have these conditional fast-paths in the code, and do them in
non-blocking manner when possible.

> I'll look again, but as far as I remember that at least wasn't
> addressing the performance issues from making overlay enable/disable

Right, it wasn't addressing those issues. But your branch doesn't really
address those issues either, as it doesn't handle the problems related
to ovl enable/disable.

> synchronous.  And fixing that would, I expect, trigger the same
> problems that I already spent a few days debugging before switching
> over to handle apply in omapdrm.

I was under the impression that the apply mechanism, the caching and
setting of the configs, was the major issue you had. But you're hinting
that the actual problem is related to ovl enable/disable? I haven't
tried fixing the ovl enable/disable, as I didn't know it's an issue for
omapdrm. Or are they both as big issues?

 Tomi

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120802/7f7e4611/attachment-0001.pgp>

[PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

Le jeudi 2 ao?t 2012 09:35:58 Hans Verkuil, vous avez ?crit :
> On Wed August 1 2012 22:49:57 R?mi Denis-Courmont wrote:
> > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at
> > > runtime ?
> > 
> > Does CREATE_BUFS always work while already streaming has already started?
> > If it depends on the driver, it's kinda helpless.
> 
> Yes, it does. It's one of the reasons it exists in the first place. But
> there are currently only a handful of drivers that implement it. I hope
> that as more and more drivers are converted to vb2 that the availability
> of create_bufs will increase.

That's contradictory. If most drivers do not support it, then it won't work 
during streaming.

> > What's the guaranteed minimum buffer count? It seems in any case, MMAP
> > has a hard limit of 32 buffers (at least videobuf2 has), though one
> > might argue this should be more than enough.
> 
> Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
> Although drivers may force a lower maximum if they want. I have no idea
> whether there are drivers that do that. There probably are.

The smallest of the maxima of all drivers.

> The minimum is usually between 1 and 3, depending on hardware limitations.

And that's clearly insufficient without memory copy to userspace buffers.

It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for 
REQBUFS+USERBUF then.

-- 
R?mi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

[PATCH] drm: ignore disconnected <-> unknown status changes

[Knut Petersen]

On an AOpen i915GMm-hfs the hotplug events generated
by transitions between connector_status_unknown and
connector_status_disconnected cause screen distortions.

The attached patch cures the problem by disabling the
generation of hotplug events in those cases. That should
be safe for everybody as the only relevant changes are
those from / to connector_status_connected.

cu,
  Knut
-- next part --
A non-text attachment was scrubbed...
Name: 0001-drm-ignore-disconnected-unknown-status-changes.patch
Type: text/x-patch
Size: 1539 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120802/eb35539b/attachment-0001.bin>

[RFC 0/3] solving omapdrm/omapdss layering issues

[Rob Clark]

On Thu, Aug 2, 2012 at 8:15 AM, Tomi Valkeinen  wrote:
> On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote:
>> On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen  
>> wrote:
>> > On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote:
>> >> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen  
>> >> wrote:
>> >
>> >> > I guess the fact is that DRM concepts do not really match the OMAP DSS
>> >> > hardware, and we'll have to use whatever gives us least problems.
>> >>
>> >> Actually, I think it does map fairly well to the hardware.. at least
>> >> more so than to omapdss ;-)
>> >
>> > Hm, I'm not sure I understand, omapdss concepts map directly to the
>> > hardware.
>>
>> I think it is mainly exposing the encoder and panel as two separate
>> entities.. which seems to be what Archit is working on
>
> I still don't follow =) They are separate entities. Omapdss models the
> HW quite directly, I think. It doesn't expose everything, though, as the
> output drivers (dsi.c, dpi.c etc) are used via the panel drivers.

right.. so we just need to expose the output drivers as separate
entities, and let omapdrm propagate information such as timings
between them

>> in case of something like DVI bridge from DPI, this seems pretty
>> straightforward.. only the connector needs to know about DDC stuff,
>> which i2c to use and that sort of thing.  So at kms level we would
>> have (for example) an omap_dpi_encoder which would be the same for DPI
>> panel (connector) or DPI->DVI bridge.  For HDMI I'm still looking
>> through the code to see how this would work.  Honestly I've looked
>> less at this part of code and encoder related registers in the TRM,
>> compared to the ovl/mgr parts, but at least from the 'DSS overview'
>> picture in the TRM it seems to make sense ;-)
>>
>> KMS even exposes the idea that certain crtcs can connect to only
>> certain encoders.  Or that you could you could have certain connectors
>> switched between encoders.  For example if you had a hw w/ DPI out,
>> and some mux to switch that back and forth between a DPI lcd panel and
>> a DPI->DVI bridge.  (Ok, I'm not aware of any board that actually does
>> this, but it is in theory possible.)  So we could expose possible
>> video chain topologies to userspace in this way.
>
> OMAP3 SDP board has such a setup, with manual switch to select between
> LCD and DVI.

ahh, good to know.. so I'm not crazy for wanting to expose this
possibility to userspace

>> The other thing is that we don't need to propagate timings from the
>> panel up to the mgr at the dss level.. kms is already handling this
>> for us.  In my latest version, which I haven't pushed, I removed the
>> 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'.
>
> You're thinking only about simple DPI cases. Consider this example, with
> a DSI-to-DP bridge chip. What we have is the following flow of data:
>
> DISPC -> DSI -> DSI-2-DP -> DP monitor
>
> The timings you are thinking about are in the DISPC, but here they are
> only one part of the link. And here the DISPC timings are not actually
> the timings what the user is interested about. The user wants his
> timings to be between DSI-2-DP chip and the DP monitor.
>
> Timings programmed to DISPC are not the same. The timings for DISPC come
> from the DSI driver, and they may be very different than the user's
> timings. With DSI video mode, the DISPC timings would have some
> resemblance to the user's timings, mainly the time to send one line
> would be the same. With DSI cmd mode, the DISPC timings would be
> something totally different, most likely with 0 blank times and as fast
> pixel clock as possible.

hmm, well kms already has a concept of adjusted_timings, which could
perhaps be used here to propagate the timings between crtc->encoder..
although the order is probably backwards from what we want (it comes
from the crtc to the encoder.. and if I understand properly we want it
the other way and actually possibly from the connector).  But that
isn't to say that internally in omapdrm the crtc couldn't get the
adjusted timings from the connector.  So I still think the parameter
flow doesn't need to be 'under the hood' in omapdss.

And fwiw, the adjusted_timings stuff is handled by drm_crtc_helper
fxns, so if the way the core kms handles it isn't what we want, we can
just plug in our own fxn instead of using drm_crtc_helper_set_mode(),
so that isn't really a big problem.

> What omapdss does currently is that you set the user's timings to the
> right side of the chain, which propagate back to DSS. This allows the
> DSI-2-DP bridge use DSI timings that work optimally for the bridge, and
> DSI driver will use DISPC timings that work optimally for it.
>
> And it's not only about timings above, but also other settings related
> to the busses between the components. Clock dividers, polarities, stuff
> like that.

I expect we could handle other settings in the same way as the timings.

>> I think the problem was there were some cases, like ovl updates 

[PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Thu August 2 2012 08:56:43 R?mi Denis-Courmont wrote:
> Le jeudi 2 ao?t 2012 09:35:58 Hans Verkuil, vous avez ?crit :
> > On Wed August 1 2012 22:49:57 R?mi Denis-Courmont wrote:
> > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at
> > > > runtime ?
> > > 
> > > Does CREATE_BUFS always work while already streaming has already started?
> > > If it depends on the driver, it's kinda helpless.
> > 
> > Yes, it does. It's one of the reasons it exists in the first place. But
> > there are currently only a handful of drivers that implement it. I hope
> > that as more and more drivers are converted to vb2 that the availability
> > of create_bufs will increase.
> 
> That's contradictory. If most drivers do not support it, then it won't work 
> during streaming.

IF create_bufs is implemented in the driver, THEN you can use it during 
streaming.
I.e., it will never return EBUSY as an error due to the fact that streaming is 
in
progress.

Obviously it won't work if the driver didn't implement it in the first place.

> 
> > > What's the guaranteed minimum buffer count? It seems in any case, MMAP
> > > has a hard limit of 32 buffers (at least videobuf2 has), though one
> > > might argue this should be more than enough.
> > 
> > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
> > Although drivers may force a lower maximum if they want. I have no idea
> > whether there are drivers that do that. There probably are.
> 
> The smallest of the maxima of all drivers.

I've no idea. Most will probably abide by the 32 maximum, but without analyzing
all drivers I can't guarantee it.

> > The minimum is usually between 1 and 3, depending on hardware limitations.
> 
> And that's clearly insufficient without memory copy to userspace buffers.
> 
> It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for 
> REQBUFS+USERBUF then.

Just to put your mind at rest: USERPTR mode will *not* disappear or be 
deprecated
in any way. It's been there for a long time, it's in heavy use, it's easy to use
and it will not be turned into a second class citizen, because it isn't. Just
because there is a new dmabuf mode available doesn't mean that everything should
be done as a mmap+dmabuf thing.

Regards,

Hans

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

[Zhang Rui]

On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote:
> AMD ACPI interface may overload the standard event
> ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such
> cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the
> userspace because the user did not press the mode switch key (the
> spurious keypress confuses the DE which usually changes the
> display configuration and messes up a dual-screen setup).
> This patch gives the radeon driver the chance to examine the event and
> block the keypress if the event is an "AMD event".
> 
> Signed-off-by: Luca Tettamanti 
> ---
> Any comment from ACPI front?
> 
it looks good to me.
But I'm wondering if we can use the following code for ACPI part, which
looks cleaner.
I know this may change the behavior of other events, but in theory, we
should not send any input event if we know something wrong in kernel.

what do you think?

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 1e0a9e1..8ad1f53 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
 * most likely via hotkey. */
acpi_bus_generate_proc_event(device, event, 0);
-   if (!acpi_notifier_call_chain(device, event, 0))
-   keycode = KEY_SWITCHVIDEOMODE;
+   keycode = KEY_SWITCHVIDEOMODE;
break;

case ACPI_VIDEO_NOTIFY_PROBE:   /* User plugged in or removed a video
@@ -1479,8 +1478,8 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
break;
}

-   if (event != ACPI_VIDEO_NOTIFY_SWITCH)
-   acpi_notifier_call_chain(device, event, 0);
+   if (acpi_notifier_call_chain(device, event, 0))
+   keycode = 0;

if (keycode) {
input_report_key(input, keycode, 1);


>  drivers/acpi/video.c |   10 --
>  drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 1e0a9e1..a8592c4 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device 
> *device, u32 event)
>   acpi_video_device_enumerate(video);
>   acpi_video_device_rebind(video);
>   acpi_bus_generate_proc_event(device, event, 0);
> - keycode = KEY_SWITCHVIDEOMODE;
> + /* This event is also overloaded by AMD ACPI interface, don't
> +  * send the key press if the event has been handled elsewhere
> +  * (e.g. radeon DRM driver).
> +  */
> + if (!acpi_notifier_call_chain(device, event, 0))
> + keycode = KEY_SWITCHVIDEOMODE;
>   break;
>  
>   case ACPI_VIDEO_NOTIFY_CYCLE:   /* Cycle Display output hotkey pressed. 
> */
> @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device 
> *device, u32 event)
>   break;
>   }
>  
> - if (event != ACPI_VIDEO_NOTIFY_SWITCH)
> + if (event != ACPI_VIDEO_NOTIFY_SWITCH &&
> + event != ACPI_VIDEO_NOTIFY_PROBE)
>   acpi_notifier_call_chain(device, event, 0);
>  
>   if (keycode) {
> diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
> b/drivers/gpu/drm/radeon/radeon_acpi.c
> index 96de08d..ee0d29e 100644
> --- a/drivers/gpu/drm/radeon/radeon_acpi.c
> +++ b/drivers/gpu/drm/radeon/radeon_acpi.c
> @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
>   }
>   /* TODO: check other events */
>  
> - return NOTIFY_OK;
> + /* We've handled the event, stop the notifier chain. The ACPI interface
> +  * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
> +  * userspace if the event was generated only to signal a SBIOS
> +  * request.
> +  */
> + return NOTIFY_BAD;
>  }
>  
>  /* Call all ACPI methods here */

[PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Wed August 1 2012 22:49:57 R?mi Denis-Courmont wrote:
> Le mercredi 1 ao?t 2012 14:35:03 Laurent Pinchart, vous avez ?crit :
> > > But in general, the V4L element in the pipeline does not know how fast
> > > the downstream element(s) will consume the buffers. Thus it has to copy
> > > from the MMAP buffers into anonymous user memory pending processing.
> > > Then any dequeued buffer can be requeued as soon as possible. In theory,
> > > it might also be that, even though the latency is known, the number of
> > > required buffers exceeds the maximum MMAP buffers count of the V4L
> > > device. Either way, user space ends up doing memory copy from MMAP to
> > > custom buffers.
> > > 
> > > This problem does not exist with USERBUF - the V4L2 element can simply
> > > allocate a new buffer for each dequeued buffer.
> > 
> > What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ?
> 
> Does CREATE_BUFS always work while already streaming has already started? If 
> it depends on the driver, it's kinda helpless.

Yes, it does. It's one of the reasons it exists in the first place. But there
are currently only a handful of drivers that implement it. I hope that as
more and more drivers are converted to vb2 that the availability of create_bufs
will increase.

> What's the guaranteed minimum buffer count? It seems in any case, MMAP has a 
> hard limit of 32 buffers (at least videobuf2 has), though one might argue 
> this 
> should be more than enough.

Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
Although drivers may force a lower maximum if they want. I have no idea
whether there are drivers that do that. There probably are.

The minimum is usually between 1 and 3, depending on hardware limitations.

Regards,

Hans

Massive power regression going 3.4->3.5

[James Bottomley]

On Wed, 2012-08-01 at 22:08 -0700, bwidawsk wrote:
> On 2012-08-01 03:06, Chris Wilson wrote:
> > On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley
> >  wrote:
> >> On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote:
> >> > On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley 
> >>  wrote:
> >> > > On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote:
> >> > > > On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley 
> >>  wrote:
> >> > > > > I got the attached to apply and it doesn't really improve 
> >> the idle power
> >> > > > > much (12.5W).
> >> > > >
> >> > > > That's good to know. Next step is to try overriding 
> >> i915.semaphores.
> >> > > > Can you please test with i915.semaphores=0 and 
> >> i915.semaphores=1?
> >> > >
> >> > > There's not much point doing i915_semaphores=1 since that's the 
> >> default
> >> > > on gen 6 hardware, but i915_semaphores=0 recovers and idle power 
> >> of
> >> > > ~6.5W
> >> >
> >> > It is only the default if iommu is off, and changing the default
> >> > was one of the side-effects of the patch you bisected.
> >> >
> >> > Can you please login to the desktop, let it idle, record
> >> > /sys/kernel/debug/dri/0/i915_cur_delayinfo and .../i915_drpc_info.
> >> > Then trace-cmd record -e i915 sleep 10s,
> >>
> >> OK, what is trace-cmd?  It looks similar to perf tools ... is that 
> >> it?
> >
> > Yes, it is roughly equivalent and you should be able to achieve the 
> > same
> > with perf trace - except I haven't done it before so I don't have 
> > quick
> > advice on how to drive it. :)
> > -Chris
> 
> Should be something like:
> perf record -f -g -e i915:* -a

I already sent the output of trace-cmd ... is that enough?

James

[RFC 0/3] solving omapdrm/omapdss layering issues

[Rob Clark]

On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen  wrote:
> On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote:
>> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen  
>> wrote:
>
>> > I guess the fact is that DRM concepts do not really match the OMAP DSS
>> > hardware, and we'll have to use whatever gives us least problems.
>>
>> Actually, I think it does map fairly well to the hardware.. at least
>> more so than to omapdss ;-)
>
> Hm, I'm not sure I understand, omapdss concepts map directly to the
> hardware.

I think it is mainly exposing the encoder and panel as two separate
entities.. which seems to be what Archit is working on

in case of something like DVI bridge from DPI, this seems pretty
straightforward.. only the connector needs to know about DDC stuff,
which i2c to use and that sort of thing.  So at kms level we would
have (for example) an omap_dpi_encoder which would be the same for DPI
panel (connector) or DPI->DVI bridge.  For HDMI I'm still looking
through the code to see how this would work.  Honestly I've looked
less at this part of code and encoder related registers in the TRM,
compared to the ovl/mgr parts, but at least from the 'DSS overview'
picture in the TRM it seems to make sense ;-)

KMS even exposes the idea that certain crtcs can connect to only
certain encoders.  Or that you could you could have certain connectors
switched between encoders.  For example if you had a hw w/ DPI out,
and some mux to switch that back and forth between a DPI lcd panel and
a DPI->DVI bridge.  (Ok, I'm not aware of any board that actually does
this, but it is in theory possible.)  So we could expose possible
video chain topologies to userspace in this way.

The other thing is that we don't need to propagate timings from the
panel up to the mgr at the dss level.. kms is already handling this
for us.  In my latest version, which I haven't pushed, I removed the
'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'.

>> The one area that kms mismatches a bit is decoupling of ovl from mgr
>> that we have in our hw..  I've partially solved that a while back w/
>
> What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't
> the drm_plane stuff separate these?

yes and no.. it is in our omapdrm implementation, because each crtc
has it's own private plane assigned.  Basically the purpose is that we
can't break the interface to existing KMS userspace, which expects a
CRTC to include the dma scanout engine.  But it means at the moment we
can't re-use these planes from crtc's that are not in use.

I have some ideas about how to expose this to userspace in a backwards
compatible way, so a userspace that is aware of this can re-use planes
from crtcs that are not in use.  There is at least one other SoC
platform (STE, IIRC?) that has similar flexibility in hw, so I think
this is a worthwhile thing to do.. but just haven't gotten to it yet.

>> For enabling/disabling an output (manager+encoder), this is relatively
>> infrequent, so it can afford to block to avoid races.  (Like userspace
>> enabling and then rapidly disabling an output part way through the
>> enable.)  But enabling/disabling an overlay, or adjusting position or
>> scanout address must not block.  And ideally, if possible, switching
>> an overlay between two managers should not block.
>
> Adjusting the position or address of the buffer are simple, they can be
> easily done without any blocking.
>
> But ovl enable/disable and switching an ovl to another mgr do (possibly)
> take multiple vsyncs (and in the switch case, vsyncs of two separate
> outputs). So if those do not block, we'll need to handle them as a state
> machine and try to avoid races etc. It'll be "interesting".

ok, I see the problem.  Really the one thing I'm not handling properly
is disconnecting a plane from one crtc and connecting to another.  The
disconnect should synchronize on the outgoing crtc's vblank/GO, and
connect on the incoming crtc.  But this is not a frequent operation,
so I think the easy solution here is to block on the
connect-to-new-crtc if the disconnect is still pending.  I'd prefer
this to introducing intermediate states.

A simple enable/disable without changing crtc does not need to block.
If usespace disables and then re-enables before the vblank, we just
apply whatever is the most recent state at the vblank.  Meaning
enable->disable->enable, the middle disable might just get skipped.
This is fine, and actually desirable.

> However, we can sometimes do those operations immediately. So I think we
> should have these conditional fast-paths in the code, and do them in
> non-blocking manner when possible.
>
>> I'll look again, but as far as I remember that at least wasn't
>> addressing the performance issues from making overlay enable/disable
>
> Right, it wasn't addressing those issues. But your branch doesn't really
> address those issues either, as it doesn't handle the problems related
> to ovl enable/disable.
>
>> synchronous.  And fixing that 

Massive power regression going 3.4->3.5

[Ben Widawsky]

On 2012-08-02 00:20, James Bottomley wrote:
> On Wed, 2012-08-01 at 22:08 -0700, bwidawsk wrote:
>> On 2012-08-01 03:06, Chris Wilson wrote:
>> > On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley
>> >  wrote:
>> >> On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote:
>> >> > On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley
>> >>  wrote:
>> >> > > On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote:
>> >> > > > On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley
>> >>  wrote:
>> >> > > > > I got the attached to apply and it doesn't really improve
>> >> the idle power
>> >> > > > > much (12.5W).
>> >> > > >
>> >> > > > That's good to know. Next step is to try overriding
>> >> i915.semaphores.
>> >> > > > Can you please test with i915.semaphores=0 and
>> >> i915.semaphores=1?
>> >> > >
>> >> > > There's not much point doing i915_semaphores=1 since that's 
>> the
>> >> default
>> >> > > on gen 6 hardware, but i915_semaphores=0 recovers and idle 
>> power
>> >> of
>> >> > > ~6.5W
>> >> >
>> >> > It is only the default if iommu is off, and changing the 
>> default
>> >> > was one of the side-effects of the patch you bisected.
>> >> >
>> >> > Can you please login to the desktop, let it idle, record
>> >> > /sys/kernel/debug/dri/0/i915_cur_delayinfo and 
>> .../i915_drpc_info.
>> >> > Then trace-cmd record -e i915 sleep 10s,
>> >>
>> >> OK, what is trace-cmd?  It looks similar to perf tools ... is 
>> that
>> >> it?
>> >
>> > Yes, it is roughly equivalent and you should be able to achieve 
>> the
>> > same
>> > with perf trace - except I haven't done it before so I don't have
>> > quick
>> > advice on how to drive it. :)
>> > -Chris
>>
>> Should be something like:
>> perf record -f -g -e i915:* -a
>
> I already sent the output of trace-cmd ... is that enough?
>
> James

Yes, should do. Have we already eliminated the obvious? GPU semaphores 
will
give time back to the GPU clients normally waiting on such things, X, 
XFCE,
whatever else. It'd probably be handy to begin investigating what those 
guys
are doing with their new extra time. Chris, this is what I was getting 
at on
IRC the other day. What do you think?

[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=45018

--- Comment #83 from Jerome Glisse  2012-08-02 
03:46:43 UTC ---
How do you trigger the va issue ? piglit ? I was not able to reproduce. It's
kind of painful to debug in the dark.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=45018

--- Comment #82 from Alexandre Demers  
2012-08-02 01:02:42 UTC ---
(In reply to comment #80)
> I tried both patches, the one from comment 76 and the one from comment 70,
> neither fixed the issue with the rendering regression or the va conflict.

Same here, I was rebuilding my kernel from scratch just in case.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=45018

--- Comment #81 from Anthony Waters  2012-08-02 00:47:06 
UTC ---
Created attachment 65051
  --> https://bugs.freedesktop.org/attachment.cgi?id=65051
fixes to wait on the bo and to free the va after the kernel

These are the changes I made to make it work in mesa, the first change,
inserting radeon_bo_wait was so that the va wouldn't be immediately reallocated
for a different va while the GPU was still using it causing the rendering
regression.

The second change was to move the freeing of the va in mesa after the kernel
was freed so that the kernel's list would be updated before mesa's list.

Hopefully this provides more insight to the issue/cause

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=45018

--- Comment #80 from Anthony Waters  2012-08-02 00:41:23 
UTC ---
I tried both patches, the one from comment 76 and the one from comment 70,
neither fixed the issue with the rendering regression or the va conflict.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

[PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

Le mercredi 1 ao?t 2012 14:35:03 Laurent Pinchart, vous avez ?crit :
> > But in general, the V4L element in the pipeline does not know how fast
> > the downstream element(s) will consume the buffers. Thus it has to copy
> > from the MMAP buffers into anonymous user memory pending processing.
> > Then any dequeued buffer can be requeued as soon as possible. In theory,
> > it might also be that, even though the latency is known, the number of
> > required buffers exceeds the maximum MMAP buffers count of the V4L
> > device. Either way, user space ends up doing memory copy from MMAP to
> > custom buffers.
> > 
> > This problem does not exist with USERBUF - the V4L2 element can simply
> > allocate a new buffer for each dequeued buffer.
> 
> What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ?

Does CREATE_BUFS always work while already streaming has already started? If 
it depends on the driver, it's kinda helpless.

What's the guaranteed minimum buffer count? It seems in any case, MMAP has a 
hard limit of 32 buffers (at least videobuf2 has), though one might argue this 
should be more than enough.

-- 
R?mi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

Re: [PATCH V3 11/16] drm/radeon: Make radeon card usable for Loongson.

[Huacai Chen]

Hi, Lucas, sorry for so long a delay because I have a holiday for one month.

I found Loongson-3 must turn on SWIOTLB if the system memory has
addresses above 4G. As I know, there are two ways to get a DMA addr,
the first way is use dma_alloc_coherent(), and the other one is use
map_page()/map_sg() on an exsisting address. The first way can make
sure DMA addr below 4G, but the second way cannot (unless the memory
is so little that all the address space is below 4G).

Take SATA driver as an example, during initialization an 'ata_device'
struct is allocated and its address is probably above 4G (because this
is first used by CPU, not for DMA). 'ata_device' has a member 'id' and
'id' will be use for DMA in such a call path:

ata_host_register() -- ata_scsi_add_hosts() -- async_port_probe()
-- ata_port_probe() -- ata_bus_probe() -- ata_dev_read_id() --
ata_exec_internal() -- ata_exec_internal_sg() -- ata_qc_issue() --
ata_sg_setup() -- dma_map_sg()

Here,  dma_map_sg() will get a DMA addr above 4G, then SATA init failed.

In OHCI driver, there are similar situations.

P.S.:  With recently drm changes, I found radeon with SWIOTLB can
already work after suspend/resume, so my next version of Loongson
patches will not modify radeon_ttm.c.

On Fri, Jun 22, 2012 at 1:59 PM, Huacai Chen chenhua...@gmail.com wrote:
 On Fri, Jun 22, 2012 at 1:25 PM, Lucas Stach d...@lynxeye.de wrote:
 Hello Huacai,

 Am Freitag, den 22.06.2012, 11:01 +0800 schrieb Huacai Chen:
 1, Handle io prot correctly for MIPS.
 2, Define SAREA_MAX as the size of one page.
 3, Don't use swiotlb on Loongson machines (Loonson need swioitlb, but
when use swiotlb, GPU reset occurs at resume from suspend).

 I still think this is wrong. You say Loongson needs SWIOTLB, but when
 it's actually used you ignore it in the radeon driver code.

 I looked up why you are using SWIOTLB and I don't agree with you that it
 is needed. SWIOTLB just gives you bounce pages for DMA memory above
 DMA32 and therefore papers over your 4GB DMA platform bug in some
 cases, while hurting performance.

 Please fix your DMA platform code so that region DMA is an alias for
 region DMA32. It should allow you to drop all those ugly workarounds.

 Hmm, you are probably right, I think I should have a discuss with the
 original author of this part of code.

 Signed-off-by: Huacai Chen che...@lemote.com
 Signed-off-by: Hongliang Tao ta...@lemote.com
 Signed-off-by: Hua Yan y...@lemote.com
 Reviewed-by: Michel Dänzer mic...@daenzer.net
 Reviewed-by: Alex Deucher alexdeuc...@gmail.com
 Reviewed-by: Lucas Stach d...@lynxeye.de
 Reviewed-by: j.glisse j.gli...@gmail.com

 You should probably only stick this tag on your patches after the people
 you are naming explicitly gave their r-b for a specific version of a
 patch.

 Thanks,
 Lucas
 Cc: dri-devel@lists.freedesktop.org
 ---
  drivers/gpu/drm/drm_vm.c|2 +-
  drivers/gpu/drm/radeon/radeon_ttm.c |6 +++---
  drivers/gpu/drm/ttm/ttm_bo_util.c   |2 +-
  include/drm/drm_sarea.h |2 ++
  4 files changed, 7 insertions(+), 5 deletions(-)

 diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
 index 961ee08..3f06166 100644
 --- a/drivers/gpu/drm/drm_vm.c
 +++ b/drivers/gpu/drm/drm_vm.c
 @@ -62,7 +62,7 @@ static pgprot_t drm_io_prot(uint32_t map_type, struct 
 vm_area_struct *vma)
   tmp = pgprot_writecombine(tmp);
   else
   tmp = pgprot_noncached(tmp);
 -#elif defined(__sparc__) || defined(__arm__)
 +#elif defined(__sparc__) || defined(__arm__) || defined(__mips__)
   tmp = pgprot_noncached(tmp);
  #endif
   return tmp;
 diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
 b/drivers/gpu/drm/radeon/radeon_ttm.c
 index c94a225..f49bdd1 100644
 --- a/drivers/gpu/drm/radeon/radeon_ttm.c
 +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
 @@ -630,7 +630,7 @@ static int radeon_ttm_tt_populate(struct ttm_tt *ttm)
   }
  #endif

 -#ifdef CONFIG_SWIOTLB
 +#if defined(CONFIG_SWIOTLB)  !defined(CONFIG_CPU_LOONGSON3)
   if (swiotlb_nr_tbl()) {
   return ttm_dma_populate(gtt-ttm, rdev-dev);
   }
 @@ -676,7 +676,7 @@ static void radeon_ttm_tt_unpopulate(struct ttm_tt *ttm)
   }
  #endif

 -#ifdef CONFIG_SWIOTLB
 +#if defined(CONFIG_SWIOTLB)  !defined(CONFIG_CPU_LOONGSON3)
   if (swiotlb_nr_tbl()) {
   ttm_dma_unpopulate(gtt-ttm, rdev-dev);
   return;
 @@ -906,7 +906,7 @@ static int radeon_ttm_debugfs_init(struct radeon_device 
 *rdev)
   radeon_mem_types_list[i].show = ttm_page_alloc_debugfs;
   radeon_mem_types_list[i].driver_features = 0;
   radeon_mem_types_list[i++].data = NULL;
 -#ifdef CONFIG_SWIOTLB
 +#if defined(CONFIG_SWIOTLB)  !defined(CONFIG_CPU_LOONGSON3)
   if (swiotlb_nr_tbl()) {
   sprintf(radeon_mem_types_names[i], ttm_dma_page_pool);
   radeon_mem_types_list[i].name = radeon_mem_types_names[i];
 diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
 

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
 Le mercredi 1 août 2012 14:35:03 Laurent Pinchart, vous avez écrit :
   But in general, the V4L element in the pipeline does not know how fast
   the downstream element(s) will consume the buffers. Thus it has to copy
   from the MMAP buffers into anonymous user memory pending processing.
   Then any dequeued buffer can be requeued as soon as possible. In theory,
   it might also be that, even though the latency is known, the number of
   required buffers exceeds the maximum MMAP buffers count of the V4L
   device. Either way, user space ends up doing memory copy from MMAP to
   custom buffers.
   
   This problem does not exist with USERBUF - the V4L2 element can simply
   allocate a new buffer for each dequeued buffer.
  
  What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ?
 
 Does CREATE_BUFS always work while already streaming has already started? If 
 it depends on the driver, it's kinda helpless.

Yes, it does. It's one of the reasons it exists in the first place. But there
are currently only a handful of drivers that implement it. I hope that as
more and more drivers are converted to vb2 that the availability of create_bufs
will increase.

 What's the guaranteed minimum buffer count? It seems in any case, MMAP has a 
 hard limit of 32 buffers (at least videobuf2 has), though one might argue 
 this 
 should be more than enough.

Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
Although drivers may force a lower maximum if they want. I have no idea
whether there are drivers that do that. There probably are.

The minimum is usually between 1 and 3, depending on hardware limitations.

Regards,

Hans
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Thu August 2 2012 08:56:43 Rémi Denis-Courmont wrote:
 Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit :
  On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
What about using the CREATE_BUFS ioctl to add new MMAP buffers at
runtime ?
   
   Does CREATE_BUFS always work while already streaming has already started?
   If it depends on the driver, it's kinda helpless.
  
  Yes, it does. It's one of the reasons it exists in the first place. But
  there are currently only a handful of drivers that implement it. I hope
  that as more and more drivers are converted to vb2 that the availability
  of create_bufs will increase.
 
 That's contradictory. If most drivers do not support it, then it won't work 
 during streaming.

IF create_bufs is implemented in the driver, THEN you can use it during 
streaming.
I.e., it will never return EBUSY as an error due to the fact that streaming is 
in
progress.

Obviously it won't work if the driver didn't implement it in the first place.

 
   What's the guaranteed minimum buffer count? It seems in any case, MMAP
   has a hard limit of 32 buffers (at least videobuf2 has), though one
   might argue this should be more than enough.
  
  Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
  Although drivers may force a lower maximum if they want. I have no idea
  whether there are drivers that do that. There probably are.
 
 The smallest of the maxima of all drivers.

I've no idea. Most will probably abide by the 32 maximum, but without analyzing
all drivers I can't guarantee it.

  The minimum is usually between 1 and 3, depending on hardware limitations.
 
 And that's clearly insufficient without memory copy to userspace buffers.
 
 It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for 
 REQBUFS+USERBUF then.

Just to put your mind at rest: USERPTR mode will *not* disappear or be 
deprecated
in any way. It's been there for a long time, it's in heavy use, it's easy to use
and it will not be turned into a second class citizen, because it isn't. Just
because there is a new dmabuf mode available doesn't mean that everything should
be done as a mmap+dmabuf thing.

Regards,

Hans
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/3] solving omapdrm/omapdss layering issues

[Tomi Valkeinen]

On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote:
 On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:

  I guess the fact is that DRM concepts do not really match the OMAP DSS
  hardware, and we'll have to use whatever gives us least problems.
 
 Actually, I think it does map fairly well to the hardware.. at least
 more so than to omapdss ;-)

Hm, I'm not sure I understand, omapdss concepts map directly to the
hardware.

 The one area that kms mismatches a bit is decoupling of ovl from mgr
 that we have in our hw..  I've partially solved that a while back w/

What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't
the drm_plane stuff separate these?

 For enabling/disabling an output (manager+encoder), this is relatively
 infrequent, so it can afford to block to avoid races.  (Like userspace
 enabling and then rapidly disabling an output part way through the
 enable.)  But enabling/disabling an overlay, or adjusting position or
 scanout address must not block.  And ideally, if possible, switching
 an overlay between two managers should not block.

Adjusting the position or address of the buffer are simple, they can be
easily done without any blocking.

But ovl enable/disable and switching an ovl to another mgr do (possibly)
take multiple vsyncs (and in the switch case, vsyncs of two separate
outputs). So if those do not block, we'll need to handle them as a state
machine and try to avoid races etc. It'll be interesting.

However, we can sometimes do those operations immediately. So I think we
should have these conditional fast-paths in the code, and do them in
non-blocking manner when possible.

 I'll look again, but as far as I remember that at least wasn't
 addressing the performance issues from making overlay enable/disable

Right, it wasn't addressing those issues. But your branch doesn't really
address those issues either, as it doesn't handle the problems related
to ovl enable/disable.

 synchronous.  And fixing that would, I expect, trigger the same
 problems that I already spent a few days debugging before switching
 over to handle apply in omapdrm.

I was under the impression that the apply mechanism, the caching and
setting of the configs, was the major issue you had. But you're hinting
that the actual problem is related to ovl enable/disable? I haven't
tried fixing the ovl enable/disable, as I didn't know it's an issue for
omapdrm. Or are they both as big issues?

 Tomi



signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Massive power regression going 3.4-3.5

[James Bottomley]

On Wed, 2012-08-01 at 22:08 -0700, bwidawsk wrote:
 On 2012-08-01 03:06, Chris Wilson wrote:
  On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley
  james.bottom...@hansenpartnership.com wrote:
  On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote:
   On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley 
  james.bottom...@hansenpartnership.com wrote:
On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote:
 On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley 
  james.bottom...@hansenpartnership.com wrote:
  I got the attached to apply and it doesn't really improve 
  the idle power
  much (12.5W).

 That's good to know. Next step is to try overriding 
  i915.semaphores.
 Can you please test with i915.semaphores=0 and 
  i915.semaphores=1?
   
There's not much point doing i915_semaphores=1 since that's the 
  default
on gen 6 hardware, but i915_semaphores=0 recovers and idle power 
  of
~6.5W
  
   It is only the default if iommu is off, and changing the default
   was one of the side-effects of the patch you bisected.
  
   Can you please login to the desktop, let it idle, record
   /sys/kernel/debug/dri/0/i915_cur_delayinfo and .../i915_drpc_info.
   Then trace-cmd record -e i915 sleep 10s,
 
  OK, what is trace-cmd?  It looks similar to perf tools ... is that 
  it?
 
  Yes, it is roughly equivalent and you should be able to achieve the 
  same
  with perf trace - except I haven't done it before so I don't have 
  quick
  advice on how to drive it. :)
  -Chris
 
 Should be something like:
 perf record -f -g -e i915:* -a

I already sent the output of trace-cmd ... is that enough?

James


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Massive power regression going 3.4-3.5

[Ben Widawsky]

On 2012-08-02 00:20, James Bottomley wrote:

On Wed, 2012-08-01 at 22:08 -0700, bwidawsk wrote:

On 2012-08-01 03:06, Chris Wilson wrote:
 On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley
 james.bottom...@hansenpartnership.com wrote:
 On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote:
  On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley
 james.bottom...@hansenpartnership.com wrote:
   On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote:
On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley
 james.bottom...@hansenpartnership.com wrote:
 I got the attached to apply and it doesn't really improve
 the idle power
 much (12.5W).
   
That's good to know. Next step is to try overriding
 i915.semaphores.
Can you please test with i915.semaphores=0 and
 i915.semaphores=1?
  
   There's not much point doing i915_semaphores=1 since that's 
the

 default
   on gen 6 hardware, but i915_semaphores=0 recovers and idle 
power

 of
   ~6.5W
 
  It is only the default if iommu is off, and changing the 
default

  was one of the side-effects of the patch you bisected.
 
  Can you please login to the desktop, let it idle, record
  /sys/kernel/debug/dri/0/i915_cur_delayinfo and 
.../i915_drpc_info.

  Then trace-cmd record -e i915 sleep 10s,

 OK, what is trace-cmd?  It looks similar to perf tools ... is 
that

 it?

 Yes, it is roughly equivalent and you should be able to achieve 
the

 same
 with perf trace - except I haven't done it before so I don't have
 quick
 advice on how to drive it. :)
 -Chris

Should be something like:
perf record -f -g -e i915:* -a


I already sent the output of trace-cmd ... is that enough?

James


Yes, should do. Have we already eliminated the obvious? GPU semaphores 
will
give time back to the GPU clients normally waiting on such things, X, 
XFCE,
whatever else. It'd probably be handy to begin investigating what those 
guys
are doing with their new extra time. Chris, this is what I was getting 
at on

IRC the other day. What do you think?

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/3] solving omapdrm/omapdss layering issues

[Rob Clark]

On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote:
 On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:

  I guess the fact is that DRM concepts do not really match the OMAP DSS
  hardware, and we'll have to use whatever gives us least problems.

 Actually, I think it does map fairly well to the hardware.. at least
 more so than to omapdss ;-)

 Hm, I'm not sure I understand, omapdss concepts map directly to the
 hardware.

I think it is mainly exposing the encoder and panel as two separate
entities.. which seems to be what Archit is working on

in case of something like DVI bridge from DPI, this seems pretty
straightforward.. only the connector needs to know about DDC stuff,
which i2c to use and that sort of thing.  So at kms level we would
have (for example) an omap_dpi_encoder which would be the same for DPI
panel (connector) or DPI-DVI bridge.  For HDMI I'm still looking
through the code to see how this would work.  Honestly I've looked
less at this part of code and encoder related registers in the TRM,
compared to the ovl/mgr parts, but at least from the 'DSS overview'
picture in the TRM it seems to make sense ;-)

KMS even exposes the idea that certain crtcs can connect to only
certain encoders.  Or that you could you could have certain connectors
switched between encoders.  For example if you had a hw w/ DPI out,
and some mux to switch that back and forth between a DPI lcd panel and
a DPI-DVI bridge.  (Ok, I'm not aware of any board that actually does
this, but it is in theory possible.)  So we could expose possible
video chain topologies to userspace in this way.

The other thing is that we don't need to propagate timings from the
panel up to the mgr at the dss level.. kms is already handling this
for us.  In my latest version, which I haven't pushed, I removed the
'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'.

 The one area that kms mismatches a bit is decoupling of ovl from mgr
 that we have in our hw..  I've partially solved that a while back w/

 What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't
 the drm_plane stuff separate these?

yes and no.. it is in our omapdrm implementation, because each crtc
has it's own private plane assigned.  Basically the purpose is that we
can't break the interface to existing KMS userspace, which expects a
CRTC to include the dma scanout engine.  But it means at the moment we
can't re-use these planes from crtc's that are not in use.

I have some ideas about how to expose this to userspace in a backwards
compatible way, so a userspace that is aware of this can re-use planes
from crtcs that are not in use.  There is at least one other SoC
platform (STE, IIRC?) that has similar flexibility in hw, so I think
this is a worthwhile thing to do.. but just haven't gotten to it yet.

 For enabling/disabling an output (manager+encoder), this is relatively
 infrequent, so it can afford to block to avoid races.  (Like userspace
 enabling and then rapidly disabling an output part way through the
 enable.)  But enabling/disabling an overlay, or adjusting position or
 scanout address must not block.  And ideally, if possible, switching
 an overlay between two managers should not block.

 Adjusting the position or address of the buffer are simple, they can be
 easily done without any blocking.

 But ovl enable/disable and switching an ovl to another mgr do (possibly)
 take multiple vsyncs (and in the switch case, vsyncs of two separate
 outputs). So if those do not block, we'll need to handle them as a state
 machine and try to avoid races etc. It'll be interesting.

ok, I see the problem.  Really the one thing I'm not handling properly
is disconnecting a plane from one crtc and connecting to another.  The
disconnect should synchronize on the outgoing crtc's vblank/GO, and
connect on the incoming crtc.  But this is not a frequent operation,
so I think the easy solution here is to block on the
connect-to-new-crtc if the disconnect is still pending.  I'd prefer
this to introducing intermediate states.

A simple enable/disable without changing crtc does not need to block.
If usespace disables and then re-enables before the vblank, we just
apply whatever is the most recent state at the vblank.  Meaning
enable-disable-enable, the middle disable might just get skipped.
This is fine, and actually desirable.

 However, we can sometimes do those operations immediately. So I think we
 should have these conditional fast-paths in the code, and do them in
 non-blocking manner when possible.

 I'll look again, but as far as I remember that at least wasn't
 addressing the performance issues from making overlay enable/disable

 Right, it wasn't addressing those issues. But your branch doesn't really
 address those issues either, as it doesn't handle the problems related
 to ovl enable/disable.

 synchronous.  And fixing that would, I expect, trigger the same

Re: [RFC 0/3] solving omapdrm/omapdss layering issues

[Tomi Valkeinen]

On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote:
 On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote:
  On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
 
   I guess the fact is that DRM concepts do not really match the OMAP DSS
   hardware, and we'll have to use whatever gives us least problems.
 
  Actually, I think it does map fairly well to the hardware.. at least
  more so than to omapdss ;-)
 
  Hm, I'm not sure I understand, omapdss concepts map directly to the
  hardware.
 
 I think it is mainly exposing the encoder and panel as two separate
 entities.. which seems to be what Archit is working on

I still don't follow =) They are separate entities. Omapdss models the
HW quite directly, I think. It doesn't expose everything, though, as the
output drivers (dsi.c, dpi.c etc) are used via the panel drivers.

 in case of something like DVI bridge from DPI, this seems pretty
 straightforward.. only the connector needs to know about DDC stuff,
 which i2c to use and that sort of thing.  So at kms level we would
 have (for example) an omap_dpi_encoder which would be the same for DPI
 panel (connector) or DPI-DVI bridge.  For HDMI I'm still looking
 through the code to see how this would work.  Honestly I've looked
 less at this part of code and encoder related registers in the TRM,
 compared to the ovl/mgr parts, but at least from the 'DSS overview'
 picture in the TRM it seems to make sense ;-)
 
 KMS even exposes the idea that certain crtcs can connect to only
 certain encoders.  Or that you could you could have certain connectors
 switched between encoders.  For example if you had a hw w/ DPI out,
 and some mux to switch that back and forth between a DPI lcd panel and
 a DPI-DVI bridge.  (Ok, I'm not aware of any board that actually does
 this, but it is in theory possible.)  So we could expose possible
 video chain topologies to userspace in this way.

OMAP3 SDP board has such a setup, with manual switch to select between
LCD and DVI.

 The other thing is that we don't need to propagate timings from the
 panel up to the mgr at the dss level.. kms is already handling this
 for us.  In my latest version, which I haven't pushed, I removed the
 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'.

You're thinking only about simple DPI cases. Consider this example, with
a DSI-to-DP bridge chip. What we have is the following flow of data:

DISPC - DSI - DSI-2-DP - DP monitor

The timings you are thinking about are in the DISPC, but here they are
only one part of the link. And here the DISPC timings are not actually
the timings what the user is interested about. The user wants his
timings to be between DSI-2-DP chip and the DP monitor.

Timings programmed to DISPC are not the same. The timings for DISPC come
from the DSI driver, and they may be very different than the user's
timings. With DSI video mode, the DISPC timings would have some
resemblance to the user's timings, mainly the time to send one line
would be the same. With DSI cmd mode, the DISPC timings would be
something totally different, most likely with 0 blank times and as fast
pixel clock as possible.

What omapdss does currently is that you set the user's timings to the
right side of the chain, which propagate back to DSS. This allows the
DSI-2-DP bridge use DSI timings that work optimally for the bridge, and
DSI driver will use DISPC timings that work optimally for it.

And it's not only about timings above, but also other settings related
to the busses between the components. Clock dividers, polarities, stuff
like that.

 I think the problem was there were some cases, like ovl updates before
 setting the mgr, where the user_info_dirty flag would be cleared but
 the registers not updated.  This is what I meant by sequence of

Hmm, I'd appreciate more info about this if you can give. Sounds like a
bug, that shouldn't happen.

So you mean that you have an ovl, with no manager set. And you change
the settings of the ovl before assigning it to a mgr? That's something I
haven't really tested, so it could bug, true.

 operations at KMS level confusing omapdss.  This should be fixable
 with some debugging.  Although getting rid of the state tracking at
 omapdss level altogether was a much simpler solution, and is the one I
 prefer :-)

Yes, I don't prefer the state tracking either (we didn't have it in
earlier versions of omapdss), but I still don't see an option to it if
we want to support all the stuff we have.

 Tomi



signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

[Luca Tettamanti]

On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote:
 On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote:
  AMD ACPI interface may overload the standard event
  ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such
  cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the
  userspace because the user did not press the mode switch key (the
  spurious keypress confuses the DE which usually changes the
  display configuration and messes up a dual-screen setup).
  This patch gives the radeon driver the chance to examine the event and
  block the keypress if the event is an AMD event.
  
  Signed-off-by: Luca Tettamanti kronos...@gmail.com
  ---
  Any comment from ACPI front?
  
 it looks good to me.
 But I'm wondering if we can use the following code for ACPI part, which
 looks cleaner.
 I know this may change the behavior of other events, but in theory, we
 should not send any input event if we know something wrong in kernel.
 
 what do you think?

I like it, it's cleaner.
I've split the patch in two pieces (one for video, the other for
radeon) and adopted your suggestion.

BTW, I'm leaving for vacation in a few hours, I'll be offline till mid
August.

Luca
From acce30c96b90d1bc550e82a9e7f19226fa194d5e Mon Sep 17 00:00:00 2001
From: Luca Tettamanti kronos...@gmail.com
Date: Thu, 2 Aug 2012 15:30:27 +0200
Subject: [PATCH 1/2] ACPI video: allow events handlers to veto the keypress

The standard video events may be overloaded for device specific
purposes. For example AMD ACPI interface overloads
ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such
cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the
userspace because the user did not press the mode switch key (the
spurious keypress confuses the DE which usually changes the
display configuration and messes up a dual-screen setup).
This patch gives the handlers the chance to examine the event and
block the keypress if the event is device specific.
v2: refactor as suggested by Zhang Rui rui.zh...@intel.com

Signed-off-by: Luca Tettamanti kronos...@gmail.com
---
 drivers/acpi/video.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 1e0a9e1..d75642a 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
 * most likely via hotkey. */
acpi_bus_generate_proc_event(device, event, 0);
-   if (!acpi_notifier_call_chain(device, event, 0))
-   keycode = KEY_SWITCHVIDEOMODE;
+   keycode = KEY_SWITCHVIDEOMODE;
break;
 
case ACPI_VIDEO_NOTIFY_PROBE:   /* User plugged in or removed a video
@@ -1479,8 +1478,9 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
break;
}
 
-   if (event != ACPI_VIDEO_NOTIFY_SWITCH)
-   acpi_notifier_call_chain(device, event, 0);
+   if (acpi_notifier_call_chain(device, event, 0))
+   /* Something vetoed the keypress. */
+   keycode = 0;
 
if (keycode) {
input_report_key(input, keycode, 1);
-- 
1.7.10.4

From def5119d8f617eef0fac2cd35d7bb18c176ff8f6 Mon Sep 17 00:00:00 2001
From: Luca Tettamanti kronos...@gmail.com
Date: Thu, 2 Aug 2012 15:33:03 +0200
Subject: [PATCH 2/2] drm/radeon: block the keypress on ATIF events

The AMD ACPI interface may use ACPI_VIDEO_NOTIFY_PROBE to signal SBIOS
requests; block the keypress in this case since the user did not
actually press the mode switch key.

Signed-off-by: Luca Tettamanti kronos...@gmail.com
---
 drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
b/drivers/gpu/drm/radeon/radeon_acpi.c
index 96de08d..ee0d29e 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
}
/* TODO: check other events */
 
-   return NOTIFY_OK;
+   /* We've handled the event, stop the notifier chain. The ACPI interface
+* overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
+* userspace if the event was generated only to signal a SBIOS
+* request.
+*/
+   return NOTIFY_BAD;
 }
 
 /* Call all ACPI methods here */
-- 
1.7.10.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/3] solving omapdrm/omapdss layering issues

[Rob Clark]

On Thu, Aug 2, 2012 at 8:15 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote:
 On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote:
  On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
 
   I guess the fact is that DRM concepts do not really match the OMAP DSS
   hardware, and we'll have to use whatever gives us least problems.
 
  Actually, I think it does map fairly well to the hardware.. at least
  more so than to omapdss ;-)
 
  Hm, I'm not sure I understand, omapdss concepts map directly to the
  hardware.

 I think it is mainly exposing the encoder and panel as two separate
 entities.. which seems to be what Archit is working on

 I still don't follow =) They are separate entities. Omapdss models the
 HW quite directly, I think. It doesn't expose everything, though, as the
 output drivers (dsi.c, dpi.c etc) are used via the panel drivers.

right.. so we just need to expose the output drivers as separate
entities, and let omapdrm propagate information such as timings
between them

 in case of something like DVI bridge from DPI, this seems pretty
 straightforward.. only the connector needs to know about DDC stuff,
 which i2c to use and that sort of thing.  So at kms level we would
 have (for example) an omap_dpi_encoder which would be the same for DPI
 panel (connector) or DPI-DVI bridge.  For HDMI I'm still looking
 through the code to see how this would work.  Honestly I've looked
 less at this part of code and encoder related registers in the TRM,
 compared to the ovl/mgr parts, but at least from the 'DSS overview'
 picture in the TRM it seems to make sense ;-)

 KMS even exposes the idea that certain crtcs can connect to only
 certain encoders.  Or that you could you could have certain connectors
 switched between encoders.  For example if you had a hw w/ DPI out,
 and some mux to switch that back and forth between a DPI lcd panel and
 a DPI-DVI bridge.  (Ok, I'm not aware of any board that actually does
 this, but it is in theory possible.)  So we could expose possible
 video chain topologies to userspace in this way.

 OMAP3 SDP board has such a setup, with manual switch to select between
 LCD and DVI.

ahh, good to know.. so I'm not crazy for wanting to expose this
possibility to userspace

 The other thing is that we don't need to propagate timings from the
 panel up to the mgr at the dss level.. kms is already handling this
 for us.  In my latest version, which I haven't pushed, I removed the
 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'.

 You're thinking only about simple DPI cases. Consider this example, with
 a DSI-to-DP bridge chip. What we have is the following flow of data:

 DISPC - DSI - DSI-2-DP - DP monitor

 The timings you are thinking about are in the DISPC, but here they are
 only one part of the link. And here the DISPC timings are not actually
 the timings what the user is interested about. The user wants his
 timings to be between DSI-2-DP chip and the DP monitor.

 Timings programmed to DISPC are not the same. The timings for DISPC come
 from the DSI driver, and they may be very different than the user's
 timings. With DSI video mode, the DISPC timings would have some
 resemblance to the user's timings, mainly the time to send one line
 would be the same. With DSI cmd mode, the DISPC timings would be
 something totally different, most likely with 0 blank times and as fast
 pixel clock as possible.

hmm, well kms already has a concept of adjusted_timings, which could
perhaps be used here to propagate the timings between crtc-encoder..
although the order is probably backwards from what we want (it comes
from the crtc to the encoder.. and if I understand properly we want it
the other way and actually possibly from the connector).  But that
isn't to say that internally in omapdrm the crtc couldn't get the
adjusted timings from the connector.  So I still think the parameter
flow doesn't need to be 'under the hood' in omapdss.

And fwiw, the adjusted_timings stuff is handled by drm_crtc_helper
fxns, so if the way the core kms handles it isn't what we want, we can
just plug in our own fxn instead of using drm_crtc_helper_set_mode(),
so that isn't really a big problem.

 What omapdss does currently is that you set the user's timings to the
 right side of the chain, which propagate back to DSS. This allows the
 DSI-2-DP bridge use DSI timings that work optimally for the bridge, and
 DSI driver will use DISPC timings that work optimally for it.

 And it's not only about timings above, but also other settings related
 to the busses between the components. Clock dividers, polarities, stuff
 like that.

I expect we could handle other settings in the same way as the timings.

 I think the problem was there were some cases, like ovl updates before
 setting the mgr, where the user_info_dirty flag would be cleared but
 the 

Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Alex Deucher]

I admit I'm not really an ACPI expert, but thinking about this more,
I'm wondering if maybe we should just send the appropriate brightness
change, switch display, etc. event to userspace rather than handling
it directly in the radeon driver, then let userspace callback down via
the bl interface, etc.  With backlight for example, does handling it
in the kernel driver as per your patch prevent userspace from seeing
the brightness up/down event?  Wouldn't that break things like OSD
brightness displays and such?

Alex

On Wed, Aug 1, 2012 at 9:56 AM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Wed, Aug 1, 2012 at 4:57 AM, Luca Tettamanti kronos...@gmail.com wrote:
 On Tue, Jul 31, 2012 at 05:33:16PM -0400, Alex Deucher wrote:
 Patches look good.  I picked them up and combined them with may
 patches plus a few other small fixes.  They are available here:
 http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches
 Let me know what you think.

 Looks ok, I lost one fix along the road though, I'm attaching the patch.

 Thanks, I rolled it into your original patch.  New tree:
 http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches

 Alex


 Luca
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/radeon: fix virtual memory locking in case of reset

[j . glisse]

From: Jerome Glisse jgli...@redhat.com

Lock/unlock mutex in proper order to avoid deadlock in case
of GPU reset triggered from VM code path.

Cc: sta...@vger.kernel.org [3.5]
Signed-off-by: Jerome Glisse jgli...@redhat.com
---
 drivers/gpu/drm/radeon/radeon_gart.c |   11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gart.c 
b/drivers/gpu/drm/radeon/radeon_gart.c
index b372005..7eabb59 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -508,14 +508,19 @@ static void radeon_vm_unbind_locked(struct radeon_device 
*rdev,
while (vm-fence) {
int r;
r = radeon_fence_wait(vm-fence, false);
-   if (r)
+   if (r) {
DRM_ERROR(error while waiting for fence: %d\n, r);
+   }
if (r == -EDEADLK) {
+   /* release mutex and lock in right order */
mutex_unlock(rdev-vm_manager.lock);
+   mutex_unlock(vm-mutex);
r = radeon_gpu_reset(rdev);
mutex_lock(rdev-vm_manager.lock);
-   if (!r)
+   mutex_lock(vm-mutex);
+   if (!r) {
continue;
+   }
}
break;
}
@@ -551,7 +556,9 @@ void radeon_vm_manager_fini(struct radeon_device *rdev)
mutex_lock(rdev-vm_manager.lock);
/* unbind all active vm */
list_for_each_entry_safe(vm, tmp, rdev-vm_manager.lru_vm, list) {
+   mutex_lock(vm-mutex);
radeon_vm_unbind_locked(rdev, vm);
+   mutex_unlock(vm-mutex);
}
rdev-vm_manager.funcs-fini(rdev);
mutex_unlock(rdev-vm_manager.lock);
-- 
1.7.10.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: fix virtual memory locking in case of reset

[Alex Deucher]

On Thu, Aug 2, 2012 at 12:05 PM,  j.gli...@gmail.com wrote:
 From: Jerome Glisse jgli...@redhat.com

 Lock/unlock mutex in proper order to avoid deadlock in case
 of GPU reset triggered from VM code path.

 Cc: sta...@vger.kernel.org [3.5]
 Signed-off-by: Jerome Glisse jgli...@redhat.com

Reviewed-by: Alex Deucher alexander.deuc...@amd.com

 ---
  drivers/gpu/drm/radeon/radeon_gart.c |   11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/radeon/radeon_gart.c 
 b/drivers/gpu/drm/radeon/radeon_gart.c
 index b372005..7eabb59 100644
 --- a/drivers/gpu/drm/radeon/radeon_gart.c
 +++ b/drivers/gpu/drm/radeon/radeon_gart.c
 @@ -508,14 +508,19 @@ static void radeon_vm_unbind_locked(struct 
 radeon_device *rdev,
 while (vm-fence) {
 int r;
 r = radeon_fence_wait(vm-fence, false);
 -   if (r)
 +   if (r) {
 DRM_ERROR(error while waiting for fence: %d\n, r);
 +   }
 if (r == -EDEADLK) {
 +   /* release mutex and lock in right order */
 mutex_unlock(rdev-vm_manager.lock);
 +   mutex_unlock(vm-mutex);
 r = radeon_gpu_reset(rdev);
 mutex_lock(rdev-vm_manager.lock);
 -   if (!r)
 +   mutex_lock(vm-mutex);
 +   if (!r) {
 continue;
 +   }
 }
 break;
 }
 @@ -551,7 +556,9 @@ void radeon_vm_manager_fini(struct radeon_device *rdev)
 mutex_lock(rdev-vm_manager.lock);
 /* unbind all active vm */
 list_for_each_entry_safe(vm, tmp, rdev-vm_manager.lru_vm, list) {
 +   mutex_lock(vm-mutex);
 radeon_vm_unbind_locked(rdev, vm);
 +   mutex_unlock(vm-mutex);
 }
 rdev-vm_manager.funcs-fini(rdev);
 mutex_unlock(rdev-vm_manager.lock);
 --
 1.7.10.4

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv7 03/15] v4l: vb2: add support for shared buffer (dma_buf)

[Tomasz Stanislawski]

Hi Laurent, Hi Dima,

On 06/27/2012 10:40 PM, Laurent Pinchart wrote:
 Hi Dima,
 
 On Tuesday 26 June 2012 13:53:34 Dima Zavin wrote:
 On Tue, Jun 26, 2012 at 2:40 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote:
 On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote:
 Hi Dima Zavin,
 Thank you for the patch and for a ping remainder :).

 You are right. The unmap is missing in __vb2_queue_cancel.
 I will apply your fix into next version of V4L2 support for dmabuf.

 Please refer to some comments below.

 On 06/20/2012 08:12 AM, Dima Zavin wrote:
 Tomasz,

 I've encountered an issue with this patch when userspace does several
 stream_on/stream_off cycles. When the user tries to qbuf a buffer
 after doing stream_off, we trigger the dmabuf already pinned
 warning since we didn't unmap the buffer as dqbuf was never called.

 The below patch adds calls to unmap in queue_cancel, but my feeling
 is that we probably should be calling detach too (i.e. put_dmabuf).

 According to the V4L2 specification, the VIDIOC_STREAMOFF ioctl, apart
 of aborting or finishing any DMA in progress, unlocks any user pointer
 buffers locked in physical memory, and it removes all buffers from the
 incoming and outgoing queues.

 Correct. And what that means in practice is that after a streamoff all
 buffers are returned to the state they had just before STREAMON was
 called.

 That can't be right. The buffers had to have been returned to the
 state just *after REQBUFS*, not just *before STREAMON*. You need to
 re-enqueue buffers before calling STREAMON. I assume that's what you
 meant?
 
 Your interpretation is correct.
 

So now we should decide what should be changed: the spec or vb2 ?
Bringing the queue state back to *after REQBUFS* may make the
next (STREAMON + QBUFs) very costly operations.

Reattaching and mapping a DMABUF might trigger DMA allocation and
*will* trigger creation of IOMMU mappings. In case of a user pointer,
calling next get_user_pages may cause numerous fault events and
will *create* new IOMMU mapping.

Is there any need to do such a cleanup if the destruction of buffers
and all caches can be explicitly executed by REQBUFS(count = 0) ?

Maybe it would be easier to change the spec by removing
apart of ... in physical memory part?

STREAMOFF should mean stopping streaming, and that resources are no
longer used. DMABUFs are unmapped but unmapping does not mean releasing.
The exporter may keep the resource in its caches.

Currently, vb2 does not follow the policy from the spec.
The put_userptr ops is called on:
- VIDIOC_REBUFS
- VIDIOC_CREATE_BUFS
- vb2_queue_release() which is usually called on close() syscall

The put_userptr is not called and streamoff therefore the user pages
are locked after STREAMOFF.

BTW. What does 'buffer locked' mean?

Does it mean that a buffer is pinned or referenced by a driver/HW?

Regards,
Tomasz Stanislawski


 So after STREAMOFF you can immediately queue all buffers again with QBUF
 and call STREAMON to restart streaming. No mmap or other operations
 should be required. This behavior must be kept.

 VIDIOC_REQBUFS() or a close() are the only two operations that will
 actually free the buffers completely.

 In practice, a STREAMOFF is either followed by a STREAMON at a later time,
 or almost immediately followed by REQBUFS or close() to tear down the
 buffers. So I don't think the buffers should be detached at streamoff.

 I agree. I was leaning this way which is why I left it out of my patch
 and wanted to hear your guys' opinion as you are much more familiar
 with the intended behavior than I am.

 Thanks!
 
 You're welcome. Thank you for reporting the problem and providing a patch.
 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Luca Tettamanti]

On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 I admit I'm not really an ACPI expert, but thinking about this more,
 I'm wondering if maybe we should just send the appropriate brightness
 change, switch display, etc. event to userspace rather than handling
 it directly in the radeon driver, then let userspace callback down via
 the bl interface, etc.  With backlight for example, does handling it
 in the kernel driver as per your patch prevent userspace from seeing
 the brightness up/down event?  Wouldn't that break things like OSD
 brightness displays and such?

No, the event is sent to userspace by the standard ACPI video driver,
it works as before.
Changing brightness usually goes like this:
1) user presses a hotkey
2) a notification is generated (0x86 or 0x87)
3) video.ko handles the notification and calls into ACPI to change the
level (_BCM) and firmware does its magic
4) a key press (brightness up/down) is sent to userspace

With ATIF step 3 does not actually change the brightness, it just send
out another event (VIDEO_PROBE, or one of the device specific ones) so
we need to take care of that too. The rest of the process, including
the delivery of the key presses, stays the same.

Luca
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Alex Deucher]

On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti kronos...@gmail.com wrote:
 On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 I admit I'm not really an ACPI expert, but thinking about this more,
 I'm wondering if maybe we should just send the appropriate brightness
 change, switch display, etc. event to userspace rather than handling
 it directly in the radeon driver, then let userspace callback down via
 the bl interface, etc.  With backlight for example, does handling it
 in the kernel driver as per your patch prevent userspace from seeing
 the brightness up/down event?  Wouldn't that break things like OSD
 brightness displays and such?

 No, the event is sent to userspace by the standard ACPI video driver,
 it works as before.
 Changing brightness usually goes like this:
 1) user presses a hotkey
 2) a notification is generated (0x86 or 0x87)
 3) video.ko handles the notification and calls into ACPI to change the
 level (_BCM) and firmware does its magic
 4) a key press (brightness up/down) is sent to userspace

 With ATIF step 3 does not actually change the brightness, it just send
 out another event (VIDEO_PROBE, or one of the device specific ones) so
 we need to take care of that too. The rest of the process, including
 the delivery of the key presses, stays the same.

Excellent!  thanks for clarifying.

Alex
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm/nouveau: crash regression in 3.5

[Marcin Slusarz]

On Thu, Aug 02, 2012 at 01:26:55PM +0200, Ortwin Glück wrote:
 I have managed to turn the crash into a WARN_ON, by adding this to the 
 begin of nouveau_software_vblank():
 
  if (!psw) {
  WARN_ON(1);
  return;
  }

Yes, I know about it, but this change fixes only a symptom. We should not get
into this code at all without enabling vblank.

 And I have also managed to load the module manually instead by udev. So 
 I am happy to attach a full log of what's going on here. See also my 
 added printk's starting with XXX that mark some interesting points in 
 the initialization.
 
 This should give you enough information to track down the cause of the 
 problem. To my non-expert eyes it looks like noaccel prevents 
 registration of NVOBJ_ENGINE_SW or at least delays it too much.

Yes, that's what I wrote in my last patch - with noaccel it's not registered,
which leads to NULL pointer derefence.

I'm currently out of ideas why my patch does not work. But I have some ideas
how to debug it. Can you come to nouveau IRC channel at freenode and do some
additional tests? My nick is joi and I'm available on IRC between 6pm and
11pm CEST.

Marcin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code

[Alex Deucher]

On Thu, Aug 2, 2012 at 12:33 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti kronos...@gmail.com wrote:
 On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 I admit I'm not really an ACPI expert, but thinking about this more,
 I'm wondering if maybe we should just send the appropriate brightness
 change, switch display, etc. event to userspace rather than handling
 it directly in the radeon driver, then let userspace callback down via
 the bl interface, etc.  With backlight for example, does handling it
 in the kernel driver as per your patch prevent userspace from seeing
 the brightness up/down event?  Wouldn't that break things like OSD
 brightness displays and such?

 No, the event is sent to userspace by the standard ACPI video driver,
 it works as before.
 Changing brightness usually goes like this:
 1) user presses a hotkey
 2) a notification is generated (0x86 or 0x87)
 3) video.ko handles the notification and calls into ACPI to change the
 level (_BCM) and firmware does its magic
 4) a key press (brightness up/down) is sent to userspace

 With ATIF step 3 does not actually change the brightness, it just send
 out another event (VIDEO_PROBE, or one of the device specific ones) so
 we need to take care of that too. The rest of the process, including
 the delivery of the key presses, stays the same.

Updated tree with fixes to a few existing patches and improved support
for ATPX and initial support for ATCS:
http://cgit.freedesktop.org/~agd5f/linux/?h=acpi_patches

Alex
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Rémi,

On Thursday 02 August 2012 09:56:43 Rémi Denis-Courmont wrote:
 Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit :
  On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
What about using the CREATE_BUFS ioctl to add new MMAP buffers at
runtime ?
   
   Does CREATE_BUFS always work while already streaming has already
   started?
   If it depends on the driver, it's kinda helpless.
  
  Yes, it does. It's one of the reasons it exists in the first place. But
  there are currently only a handful of drivers that implement it. I hope
  that as more and more drivers are converted to vb2 that the availability
  of create_bufs will increase.
 
 That's contradictory. If most drivers do not support it, then it won't work
 during streaming.
 
   What's the guaranteed minimum buffer count? It seems in any case, MMAP
   has a hard limit of 32 buffers (at least videobuf2 has), though one
   might argue this should be more than enough.
  
  Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
  Although drivers may force a lower maximum if they want. I have no idea
  whether there are drivers that do that. There probably are.
 
 The smallest of the maxima of all drivers.
 
  The minimum is usually between 1 and 3, depending on hardware limitations.
 
 And that's clearly insufficient without memory copy to userspace buffers.

That's the minimum number of buffers *required* by the hardware. You can add 
up to 32 buffers, I'm not aware of any driver that would prevent that.

 It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for
 REQBUFS+USERBUF then.

-- 
Regards,

Laurent Pinchart

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Hans,

On Thursday 02 August 2012 09:08:18 Hans Verkuil wrote:
 On Thu August 2 2012 08:56:43 Rémi Denis-Courmont wrote:
  Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit :
   On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
 What about using the CREATE_BUFS ioctl to add new MMAP buffers at
 runtime ?

Does CREATE_BUFS always work while already streaming has already
started? If it depends on the driver, it's kinda helpless.
   
   Yes, it does. It's one of the reasons it exists in the first place. But
   there are currently only a handful of drivers that implement it. I hope
   that as more and more drivers are converted to vb2 that the availability
   of create_bufs will increase.
  
  That's contradictory. If most drivers do not support it, then it won't
  work during streaming.
 
 IF create_bufs is implemented in the driver, THEN you can use it during
 streaming. I.e., it will never return EBUSY as an error due to the fact
 that streaming is in progress.
 
 Obviously it won't work if the driver didn't implement it in the first
 place.

What's the guaranteed minimum buffer count? It seems in any case, MMAP
has a hard limit of 32 buffers (at least videobuf2 has), though one
might argue this should be more than enough.
   
   Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2
   core. Although drivers may force a lower maximum if they want. I have no
   idea whether there are drivers that do that. There probably are.
  
  The smallest of the maxima of all drivers.
 
 I've no idea. Most will probably abide by the 32 maximum, but without
 analyzing all drivers I can't guarantee it.
 
   The minimum is usually between 1 and 3, depending on hardware
   limitations.
  
  And that's clearly insufficient without memory copy to userspace buffers.
  
  It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for
  REQBUFS+USERBUF then.
 
 Just to put your mind at rest: USERPTR mode will *not* disappear or be
 deprecated in any way. It's been there for a long time, it's in heavy use,
 it's easy to use and it will not be turned into a second class citizen,
 because it isn't. Just because there is a new dmabuf mode available doesn't
 mean that everything should be done as a mmap+dmabuf thing.

I disagree with this. Not everything should obviously be done with MMAP + 
DMABUF, but for buffer sharing between devices, we should encourage 
application developers to use DMABUF instead of USERPTR.

-- 
Regards,

Laurent Pinchart

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 51787] performance regression with llvm shader compiler in ut2004

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=51787

--- Comment #2 from almos aaalmo...@gmail.com 2012-08-02 22:04:09 UTC ---
It might not help in fixing this, but I found that the framerate is much more
consistent if I load all cpu cores with something while playing ut2004.
Normally, the framerate counter is green, but flashes into yellow and purple,
which makes movements very choppy. When something is running in the background,
the framerate counter stays green, and all movements are smooth.

With the llvm compiler the game is unplayable either way.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 51787] performance regression with llvm shader compiler in ut2004

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=51787

--- Comment #3 from Andy Furniss li...@andyfurniss.entadsl.com 2012-08-02 
22:32:43 UTC ---
(In reply to comment #2)
 It might not help in fixing this, but I found that the framerate is much more
 consistent if I load all cpu cores with something while playing ut2004.
 Normally, the framerate counter is green, but flashes into yellow and purple,
 which makes movements very choppy. When something is running in the 
 background,
 the framerate counter stays green, and all movements are smooth.

Maybe cpufreq is causing this here's what I get on the demo on an HD4890 +
4x3.4GHz phenom II.

R600_LLVM=0 ut2004
as-convoy?spectatoronly=1?numbots=8?quickstart=1?attractcam=1 -benchmark
-seconds=120 -nosound

cpufreq ondemand - 

30.027578 / 76.497787 / 162.457611 fps

cpufreq set to performance

37.627441 / 93.523949 / 197.051376 fps

 
 With the llvm compiler the game is unplayable either way.

I see the stalling in the demo benchmark.

2.030273 / 70.589706 / 196.047073 fps

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Massive power regression going 3.4-3.5

[bwidawsk]

On 2012-08-01 03:06, Chris Wilson wrote:

On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley
james.bottom...@hansenpartnership.com wrote:

On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote:
 On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley 
james.bottom...@hansenpartnership.com wrote:

  On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote:
   On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley 
james.bottom...@hansenpartnership.com wrote:
I got the attached to apply and it doesn't really improve 
the idle power

much (12.5W).
  
   That's good to know. Next step is to try overriding 
i915.semaphores.
   Can you please test with i915.semaphores=0 and 
i915.semaphores=1?

 
  There's not much point doing i915_semaphores=1 since that's the 
default
  on gen 6 hardware, but i915_semaphores=0 recovers and idle power 
of

  ~6.5W

 It is only the default if iommu is off, and changing the default
 was one of the side-effects of the patch you bisected.

 Can you please login to the desktop, let it idle, record
 /sys/kernel/debug/dri/0/i915_cur_delayinfo and .../i915_drpc_info.
 Then trace-cmd record -e i915 sleep 10s,

OK, what is trace-cmd?  It looks similar to perf tools ... is that 
it?


Yes, it is roughly equivalent and you should be able to achieve the 
same
with perf trace - except I haven't done it before so I don't have 
quick

advice on how to drive it. :)
-Chris


Should be something like:
perf record -f -g -e i915:* -a
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: ignore disconnected - unknown status changes

[Knut Petersen]

On an AOpen i915GMm-hfs the hotplug events generated
by transitions between connector_status_unknown and
connector_status_disconnected cause screen distortions.

The attached patch cures the problem by disabling the
generation of hotplug events in those cases. That should
be safe for everybody as the only relevant changes are
those from / to connector_status_connected.

cu,
 Knut
From f631128c46f916eb58bbdabf867248a04a0d2fc5 Mon Sep 17 00:00:00 2001
From: Knut Petersen knut_peter...@t-online.de
Date: Thu, 2 Aug 2012 08:52:04 +0200
Subject: [PATCH] drm: ignore disconnected - unknown status changes

Only generate a hotplug event if the status changed
to / from connector_status_connected.

On some hardware the connector status is oscillating
between disconnected and unknown, and the hotplug
events mirroring these transitions cause screen
distortions. As the only reasonable action on such
a status change is to ignore it, it also is safe
not to genereate a hotplug event at all.

Needed for / tested on AOpen i915GMm-hfs mobo.

Signed-off-by: Knut Petersen knut_peter...@t-online.de
---
 drivers/gpu/drm/drm_crtc_helper.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 3252e70..fb6160b 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -954,7 +954,11 @@ static void output_poll_execute(struct work_struct *work)
 			  connector-base.id,
 			  drm_get_connector_name(connector),
 			  old_status, connector-status);
-		if (old_status != connector-status)
+		/* changes are only relevant if previous or
+		current status is connector_status_connected */
+		if (old_status != connector-status 
+		(old_status == connector_status_connected ||
+		connector-status == connector_status_connected))
 			changed = true;
 	}
 
-- 
1.7.7

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit :
 On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
   What about using the CREATE_BUFS ioctl to add new MMAP buffers at
   runtime ?
  
  Does CREATE_BUFS always work while already streaming has already started?
  If it depends on the driver, it's kinda helpless.
 
 Yes, it does. It's one of the reasons it exists in the first place. But
 there are currently only a handful of drivers that implement it. I hope
 that as more and more drivers are converted to vb2 that the availability
 of create_bufs will increase.

That's contradictory. If most drivers do not support it, then it won't work 
during streaming.

  What's the guaranteed minimum buffer count? It seems in any case, MMAP
  has a hard limit of 32 buffers (at least videobuf2 has), though one
  might argue this should be more than enough.
 
 Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
 Although drivers may force a lower maximum if they want. I have no idea
 whether there are drivers that do that. There probably are.

The smallest of the maxima of all drivers.

 The minimum is usually between 1 and 3, depending on hardware limitations.

And that's clearly insufficient without memory copy to userspace buffers.

It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for 
REQBUFS+USERBUF then.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] of: Add videomode helper

[Stephen Warren]

On 07/04/2012 01:56 AM, Sascha Hauer wrote:
 This patch adds a helper function for parsing videomodes from the devicetree.
 The videomode can be either converted to a struct drm_display_mode or a
 struct fb_videomode.

 diff --git a/Documentation/devicetree/bindings/video/displaymode 
 b/Documentation/devicetree/bindings/video/displaymode

 +Required properties:
 + - xres, yres: Display resolution
 + - left-margin, right-margin, hsync-len: Horizontal Display timing parameters
 +   in pixels
 +   upper-margin, lower-margin, vsync-len: Vertical display timing parameters 
 in
 +   lines

Perhaps bike-shedding, but...

For the margin property names, wouldn't it be better to use terminology
more commonly used for video timings rather than Linux FB naming. In
other words naming like:

hactive, hsync-len, hfront-porch, hback-porch?

 + - clock: displayclock in Hz
 +
 +Optional properties:
 + - width-mm, height-mm: Display dimensions in mm
 + - hsync-active-high (bool): Hsync pulse is active high
 + - vsync-active-high (bool): Vsync pulse is active high
 + - interlaced (bool): This is an interlaced mode
 + - doublescan (bool): This is a doublescan mode
 +
 +There are different ways of describing a display mode. The devicetree 
 representation
 +corresponds to the one used by the Linux Framebuffer framework described 
 here in
 +Documentation/fb/framebuffer.txt. This representation has been chosen 
 because it's
 +the only format which does not allow for inconsistent parameters.Unlike the 
 Framebuffer
 +framework the devicetree has the clock in Hz instead of ps.

As Rob mentioned, I think there shouldn't be any mention of Linux FB here.

 +
 +Example:
 +
 + display@0 {

This node appears to describe a video mode, not a display, hence the
node name seems wrong.

Many displays can support multiple different video modes. As mentioned
elsewhere, properties like display width/height are properties of the
display not the mode.

So, I might expect something more like the following (various overhead
properties like reg/#address-cells etc. elided for simplicity):

disp: display {
width-mm = ...;
height-mm = ...;
modes {
mode@0 {
/* 1920x1080p24 */
clock = 5200;
xres = 1920;
yres = 1080;
left-margin = 25;
right-margin = 25;
hsync-len = 25;
lower-margin = 2;
upper-margin = 2;
vsync-len = 2;
hsync-active-high;
};
mode@1 {
};
};
};

display-connector {
display = disp;
// interface-specific properties such as pixel format here
};

Finally, have you considered just using an EDID instead of creating
something custom? I know that creating an EDID is harder than writing a
few simple properties into a DT, but EDIDs have the following advantages:

a) They're already standardized and very common.

b) They allow other information such as a display's HDMI audio
capabilities to be represented, which can then feed into an ALSA driver.

c) The few LCD panel datasheets I've seen actually quote their
specification as an EDID already, so deriving the EDID may actually be easy.

d) People familiar with displays are almost certainly familiar with
EDID's mode representation. There are many ways of representing display
modes (sync position vs. porch widths, htotal specified rather than
specifying all the components and hence htotal being calculated etc.).
Not everyone will be familiar with all representations. Conversion
errors are less likely if the target is EDID's familiar format.

e) You'll end up with exactly the same data as if you have a DDC-based
external monitor rather than an internal panel, so you end up getting to
a common path in display handling code much more quickly.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] of: Add videomode helper

[Stephen Warren]

On 07/05/2012 08:51 AM, Rob Herring wrote:
 On 07/04/2012 02:56 AM, Sascha Hauer wrote:
 This patch adds a helper function for parsing videomodes from the devicetree.
 The videomode can be either converted to a struct drm_display_mode or a
 struct fb_videomode.

 diff --git a/Documentation/devicetree/bindings/video/displaymode 
 b/Documentation/devicetree/bindings/video/displaymode

 +Example:
 +
 + display@0 {
 + /* 1920x1080p24 */
 + clock = 5200;
 
 Should this use the clock binding? You probably need both constraints
 and clock binding though.

I don't think the clock binding is appropriate here. This binding
specifies a desired video mode, and should specify just the expected
clock frequency for that mode. Perhaps any tolerance imposed by the
specification used to calculate the mode timing could be specified too,
but the allowed tolerance (for a mode to be recognized by the dispaly)
is more driven by the receiving device than the transmitting device.

The clock bindings should come into play in the display controller that
sends a signal in that display mode. Something like:

mode: hd1080p {
clock = 5200;
xres = 1920;
yres = 1080;

};

display-controller {
pixel-clock-source = clk ...; // common clock bindings here
default-mode = mode;

 Often you don't know the frequency up front and/or have limited control
 of the frequency (i.e. integer dividers). Then you have to adjust the
 margins to get the desired refresh rate. To do that, you need to know
 the ranges of values a panel can support. Perhaps you just assume you
 can increase the right-margin and lower-margins as I think you will hit
 pixel clock frequency max before any limit on margins.

I think this is more usually dealt with in HW design and matching
components.

The PLL/... feeding the display controller is going to have some known
specifications that imply which pixel clocks it can generate. HW
designers will pick a panel that accepts a clock the display controller
can generate. The driver for the display controller will just generate
as near of a pixel clock as it can to the rate specified in the mode
definition. I believe it'd be unusual for a display driver to start
fiddling with front-back porch (or margin) widths to munge the timing;
that kind of thing probably worked OK with analog displays, but with
digital displays where each pixel is clocked even in the margins, I
think that would cause more problems than it solved.

Similarly for external displays, the display controller will just pick
the nearest clock it can. If it can't generate a close enough clock, it
will just refuse to set the mode. In fact, a display controller driver
would typically validate this when the set of legal modes was enumerated
when initially detecting the display, so user-space typically wouldn't
even request invalid modes.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=45018

--- Comment #84 from Anthony Waters awate...@gmail.com 2012-08-03 01:28:25 
UTC ---
I randomly saw it when I was playing a game of Warcraft 3, the terrain textures
would blink.  I'll check the piglit tests and mesa demos to see if I can
reproduce the issue with them.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

[Alex Deucher]

On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui rui.zh...@intel.com wrote:
 On 四, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote:
 On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote:
  On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote:
   AMD ACPI interface may overload the standard event
   ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such
   cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the
   userspace because the user did not press the mode switch key (the
   spurious keypress confuses the DE which usually changes the
   display configuration and messes up a dual-screen setup).
   This patch gives the radeon driver the chance to examine the event and
   block the keypress if the event is an AMD event.
  
   Signed-off-by: Luca Tettamanti kronos...@gmail.com
   ---
   Any comment from ACPI front?
  
  it looks good to me.
  But I'm wondering if we can use the following code for ACPI part, which
  looks cleaner.
  I know this may change the behavior of other events, but in theory, we
  should not send any input event if we know something wrong in kernel.
 
  what do you think?

 I like it, it's cleaner.
 I've split the patch in two pieces (one for video, the other for
 radeon) and adopted your suggestion.

 Great.
 Acked-by: Zhang Rui rui.zh...@intel.com

 hmm, who should take these two patches?

I'm happy to take the patches.

 and which tree the second patch is based on?

I've got a tree with all the radeon ACPI patches on the acpi_patches
branches of my git tree:
git://people.freedesktop.org/~agd5f/linux

Alex


 thanks,
 rui

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=45018

--- Comment #85 from Anthony Waters awate...@gmail.com 2012-08-03 02:07:06 
UTC ---
I found a demo that has the issue, in the demos repository for mesa within the
src/demo folder the program 'reflect'.  After I start it up and press 's' to
see the stencil buffer the white plan blinks continuously.  Applying the patch
'fixes to wait on the bo and to free the va after the kernel' removes the
blinking, as does disabling va through the variable
ws-info.r600_virtual_address.

The other issue with the kernel reporting a va conflict is going to be a little
harder to reproduce because it appears to be caused by a race condition.

I'll still look for other demos that have the issue.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=45018

--- Comment #86 from Alexandre Demers alexandre.f.dem...@gmail.com 2012-08-03 
03:00:00 UTC ---
(In reply to comment #85)
 I found a demo that has the issue, in the demos repository for mesa within the
 src/demo folder the program 'reflect'.  After I start it up and press 's' to
 see the stencil buffer the white plan blinks continuously.  Applying the patch
 'fixes to wait on the bo and to free the va after the kernel' removes the
 blinking, as does disabling va through the variable
 ws-info.r600_virtual_address.
 
 The other issue with the kernel reporting a va conflict is going to be a 
 little
 harder to reproduce because it appears to be caused by a race condition.
 
 I'll still look for other demos that have the issue.

Yes, I understand it can be hard to track for you Jerome. Well for the va
issue, on my side, it is as simple as logging in KDE or Gnome 3. Before logging
in, there is no va error in dmesg. Once I'm in, there are usually 3 or
sometimes 6 errors (they are written in block of 3, so I suspect it tries a
first time and for some reason it fails and try again second time).

I also experience the issue when watching some movies. With Anthony's patch, va
issues are gone and I watched a couple of shows yesterday without any problem.
Before the patch, it would blink and get corrupted after about 16 minutes and
then crash. So, Anthony has put a finger on something.

However, I also run piglit tests and some other applications like
RendererFeatTest64 (which is an application released before Amnesia went out to
test OpenGL performances if I recall recorrectly). With Anthony's patch, I'm
still able to lock the display everytime (if I play music at the same time, it
will continue to play but I won't be able to change terminal even if sometimes
my mouse pointer can still be moved). RendererFeatTest64 will always lock at
the same test, but it is not the same for piglit tests (even if it happens
often at the same or near the same).

I'm installing a freshly compiled kernel 3.5.0 with both Alex and your patches
(by the way, they can't be applied on latest drm-next branch) and I'll tell you
if I'm still experiencing the lockups. I'll also try Anthony's test to see if I
get the same results (blinking without his patch, OK with it)

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel