Re: [PATCH] drm: serialize access to debugs_nodes.list
On Sun, Oct 30, 2011 at 11:04:48PM +0100, Marcin Slusarz wrote:
> Nouveau, when configured with debugfs, creates debugfs files for every
> channel, so structure holding list of files needs to be protected from
> simultaneous changes by multiple threads.
>
> Without this patch it's possible to hit kernel oops in
> drm_debugfs_remove_files just by running a couple of xterms with
> looped glxinfo.
>
> Signed-off-by: Marcin Slusarz
> ---
> drivers/gpu/drm/drm_debugfs.c |5 +
> include/drm/drmP.h|1 +
> 2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 9d2668a..1144fbe 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -120,7 +120,9 @@ int drm_debugfs_create_files(struct drm_info_list *files,
> int count,
> tmp->minor = minor;
> tmp->dent = ent;
> tmp->info_ent = &files[i];
> + mutex_lock(&minor->debugfs_nodes.mutex);
> list_add(&(tmp->list), &(minor->debugfs_nodes.list));
> + mutex_unlock(&minor->debugfs_nodes.mutex);
> }
> return 0;
>
> @@ -149,6 +151,7 @@ int drm_debugfs_init(struct drm_minor *minor, int
> minor_id,
> int ret;
>
> INIT_LIST_HEAD(&minor->debugfs_nodes.list);
> + mutex_init(&minor->debugfs_nodes.mutex);
> sprintf(name, "%d", minor_id);
> minor->debugfs_root = debugfs_create_dir(name, root);
> if (!minor->debugfs_root) {
> @@ -194,6 +197,7 @@ int drm_debugfs_remove_files(struct drm_info_list *files,
> int count,
> struct drm_info_node *tmp;
> int i;
>
> + mutex_lock(&minor->debugfs_nodes.mutex);
> for (i = 0; i < count; i++) {
> list_for_each_safe(pos, q, &minor->debugfs_nodes.list) {
> tmp = list_entry(pos, struct drm_info_node, list);
> @@ -204,6 +208,7 @@ int drm_debugfs_remove_files(struct drm_info_list *files,
> int count,
> }
> }
> }
> + mutex_unlock(&minor->debugfs_nodes.mutex);
> return 0;
> }
> EXPORT_SYMBOL(drm_debugfs_remove_files);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 9b7c2bb..c70c943 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -970,6 +970,7 @@ struct drm_info_list {
> * debugfs node structure. This structure represents a debugfs file.
> */
> struct drm_info_node {
> + struct mutex mutex;
> struct list_head list;
> struct drm_minor *minor;
> struct drm_info_list *info_ent;
This is just ugly, you're adding a mutex to every drm_info_node, but only
use the one embedded into the minor. On a quick grep we're only ever using
the list in there, so I suggest to
- replace minor->debugfs_node.list with minor->debugfs_list and kill
->debugfs_node
- add the mutex as minor->debugfs_lock
That way it's clear what's going on.
Also, you've forgotten to add the locking to i915/i915_debugfs.c
Yours, Daniel
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 42409] Mesa 7.12-devel /dri/r600 compilation error: no makefile found
https://bugs.freedesktop.org/show_bug.cgi?id=42409 Christian König changed: What|Removed |Added Status|NEW |RESOLVED Resolution||NOTABUG --- Comment #2 from Christian König 2011-10-31 02:04:32 PDT --- That is correct, r600 has been removed. It looks like that git didn't remove the directory probably because of some stale file in it. Please remove it manually, to make compiling work again. -- 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 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #11 from miran...@orange.fr 2011-10-31 03:42:13 PDT --- My system don't freeze when i (In reply to comment #10) > Bugzilla Gnome 661898 > Posted some days ago. > > Because it can be just a bug around the radeon driver in kernel I opened a new > bug here. > https://bugs.freedesktop.org/show_bug.cgi?id=41838 > > Would be intersting to see if it freezes your system in the same way, if you > visit the website I mentioned in my bug. So we can be sure, that is the same > issue. My system do not freeze when I visit http://piratpix.com/bpt2011.1/index.html and browse pictures with Epiphany, so I don't think it's the same 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 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #12 from pete...@hottemptation.org 2011-10-31 04:50:56 PDT --- Thanks! That is bad, or good. Did you clicked on the thumbnails or just viewed the slideshow? Clicking on the thumbnails is causing the issue, the slideshow not. -- 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 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #13 from pete...@hottemptation.org 2011-10-31 05:14:25 PDT --- Forgot something important! Are you using the last official xf86-video-radeon driver or the one from git? -- 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 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #14 from miran...@orange.fr 2011-10-31 05:19:17 PDT --- Yes, I have clicked on the thumbnails, and I use the git version of the driver. -- 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 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 Nicolas Peninguy changed: What|Removed |Added CC||npenin...@gmail.com --- Comment #15 from Nicolas Peninguy 2011-10-31 06:14:45 PDT --- May be this is related to https://bugzilla.gnome.org/show_bug.cgi?id=650857 ? (But Intel and NVidia owners are also affected) -- 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 42435] New: Suspected loss of varyings between shaders
https://bugs.freedesktop.org/show_bug.cgi?id=42435 Bug #: 42435 Summary: Suspected loss of varyings between shaders Classification: Unclassified Product: Mesa Version: git Platform: x86-64 (AMD64) OS/Version: Linux (All) Status: NEW Severity: major Priority: medium Component: Drivers/Gallium/r600 AssignedTo: dri-devel@lists.freedesktop.org ReportedBy: screwdri...@lxnt.info Longer description, the test case, screenshots, and core from i965 crash are in attached archive. In short: I try to draw a 2D screenful of point sprites, emulating an ansi-colored terminal. Fragment shader receives texture coordinates for the character, its color and size of graphic in the texture via varyings. R600g draws a black screen, while llvmpipe and fglrx draw what is supposed to be drawn. i965 crashes. non-llvm swrast fails to set gl_PointSize. R600c also draws a black screen. Drawing varying values themselves I get bogus values for some of them. Mesa version: 7.12.0~git20111027.0fbc8d30-0ubuntu0sarvatt~natty -- 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 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #16 from miran...@orange.fr 2011-10-31 07:43:07 PDT --- Thanks for the link. You are right, this problem seems very very close to mine. -- 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 42435] Suspected loss of varyings between shaders
https://bugs.freedesktop.org/show_bug.cgi?id=42435 --- Comment #1 from Alexander Sabourenkov 2011-10-31 07:43:44 PDT --- Created attachment 52954 --> https://bugs.freedesktop.org/attachment.cgi?id=52954 Test case and stuff -- 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 42435] Suspected loss of varyings between shaders
https://bugs.freedesktop.org/show_bug.cgi?id=42435 --- Comment #2 from Alexander Sabourenkov 2011-10-31 07:50:58 PDT --- core did not fit: >3000K size -- 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: RFC: Radeon multi ring support branch
On Sat, Oct 29, 2011 at 03:00:28PM +0200, Christian König wrote: > Hello everybody, > > to support multiple compute rings, async DMA engines and UVD we need > to teach the radeon kernel module how to sync buffers between > different rings and make some changes to the command submission > ioctls. > > Since we can't release any documentation about async DMA or UVD > (yet), my current branch concentrates on getting the additional > compute rings on cayman running. Unfortunately those rings have > hardware bugs that can't be worked around, so they are actually not > very useful in a production environment, but they should do quite > well for this testing purpose. > > The branch can be found here: > http://cgit.freedesktop.org/~deathsimple/linux/log/ > > Since some of the patches are quite intrusive, constantly rebaseing > them could get a bit painful. So I would like to see most of the > stuff included into drm-next, even if we don't make use of the new > functionality right now. > > Comments welcome, > Christian. So for all patches except the interface change see below Reviewed-by: Jerome Glisse For the interface change, as discussed previously, i believe prio should be a userspace argument, kernel could override it. Cheers, Jerome ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
Dear Alex,
your reply confuses me:
> On Sat, Oct 29, 2011 at 8:55 AM, Thomas Reim wrote:
> > Dear Alex,
> >
> > but we use DDC probing e. g. to identify connectors with improperly
> > terminated i2c bus. Instead of flooding logs and terminals with EDID dumps,
> > we decided some months ago to use this function during module loading to
> > inform the user once (and only once!), which connector has a monitor
> > connected with valid EDID and which connector has not.
>
> There's nothing in that function that actually prevents the printing
> of bad EDIDs.
That's true.
> All it does is call drm_get_edid() and report whether
> it found an EDID or not. radeon_dvi_detect() already takes into
> account the requires_extended_probe flag.
The extended probe flag will prevent the radeon driver from further
useless printing of bad EDIDs every ten seconds:
in radeon_connectors.c:
if (radeon_connector->ddc_bus)
dret = radeon_ddc_probe(radeon_connector,
radeon_connector->requires_extended_probe);
if (dret) {
if (radeon_connector->edid) {
kfree(radeon_connector->edid);
radeon_connector->edid = NULL;
}
radeon_connector->edid = drm_get_edid(&radeon_connector->base,
&radeon_connector->ddc_bus->adapter);
But the now to be removed function radeon_ddc_dump() took care during
connector setup that at least one (!) dump was in the logs, accompagnied
by the info, that no monitor is connected, or there is a wrong/buggy
EDID.
in radeon_display.c:
radeon_print_display_setup(dev);
list_for_each_entry(drm_connector,
&dev->mode_config.connector_list, head)
radeon_ddc_dump(drm_connector);
I cannot imagine why this should confuse users. Having retrieved and
configured connectors plus info plus EDID dump in case of problems
logged next to each other was perfect.
> The connectors are probed by
> the fb code for the console as well so it just adds to the module load
> time. If we want to print what connectors are connected, it should be
> done from the fb code so we only have to do it once.
>
I briefly checked the code, but couldn't find a promising connector
probing function in the fb code. Did you mean function
drm_fb_helper_probe_connector_modes?
> > Graphic solutions in general have several connectors integrated, and it's
> > sometimes really difficult to identify, which one of the does not work as
> > expected, based on the logs. From above log we see, that a monitor is
> > connected to DVI connector, nothing is connected to the VGA connector, and
> > we have a problem with the HDMI connector. Without this fuinction, nothing
> > helpful from radeon module would be in the logs.
>
> We should print the connector name in the generic drm error code then
> if we want to print this info at boot time.
Is there a place that is not called every ten seconds?
>
> >
> > Maybe we can keep this function, but call it only for connectors, for which
> > it works, i. e. not for DP, eDP and DP bridge connectors.
>
> That's just as bad. Users won't understand why only certain
> connectors are probed.
>
> >
> > Best regards
> >
> > Thomas Reim
> >
> > Am Dienstag, den 25.10.2011, 19:24 -0400 schrieb alexdeuc...@gmail.com:
> >
> > From: Alex Deucher
> >
> > The function didn't work with DP, eDP, or DP bridge
> > connectors and thus confused users as it lead them to
> > believe nothing was connected or the EDID was invalid
> > when in fact is was, just on the aux bus rather an i2c.
> >
> > It should also speed up module loading as it avoids a
> > bunch of extra DDC probing.
> >
> > Signed-off-by: Alex Deucher
> > ---
> > drivers/gpu/drm/radeon/radeon_display.c | 33
> > ---
> > 1 files changed, 0 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_display.c
> > b/drivers/gpu/drm/radeon/radeon_display.c
> > index 6adb3e5..4998736 100644
> > --- a/drivers/gpu/drm/radeon/radeon_display.c
> > +++ b/drivers/gpu/drm/radeon/radeon_display.c
> > @@ -33,8 +33,6 @@
> > #include "drm_crtc_helper.h"
> > #include "drm_edid.h"
> >
> > -static int radeon_ddc_dump(struct drm_connector *connector);
> > -
> > static void avivo_crtc_load_lut(struct drm_crtc *crtc)
> > {
> > struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> > @@ -669,7 +667,6 @@ static void radeon_print_display_setup(struct drm_device
> > *dev)
> > static bool radeon_setup_enc_conn(struct drm_device *dev)
> > {
> > struct radeon_device *rdev = dev->dev_private;
> > - struct drm_connector *drm_connector;
> > bool ret = false;
> >
> > if (rdev->bios) {
> > @@ -689,8 +686,6 @@ static bool radeon_setup_enc_conn(struct drm_device
> > *dev)
> > if (ret) {
> > radeon_setup_encoder_clones(dev);
> > radeon_print_display_setup(dev);
> > - list_for_each_entry(dr
Re: [PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
On Mon, Oct 31, 2011 at 11:15 AM, Thomas Reim wrote:
> Dear Alex,
>
> your reply confuses me:
>
>> On Sat, Oct 29, 2011 at 8:55 AM, Thomas Reim wrote:
>> > Dear Alex,
>> >
>> > but we use DDC probing e. g. to identify connectors with improperly
>> > terminated i2c bus. Instead of flooding logs and terminals with EDID dumps,
>> > we decided some months ago to use this function during module loading to
>> > inform the user once (and only once!), which connector has a monitor
>> > connected with valid EDID and which connector has not.
>>
>> There's nothing in that function that actually prevents the printing
>> of bad EDIDs.
>
> That's true.
>
>> All it does is call drm_get_edid() and report whether
>> it found an EDID or not. radeon_dvi_detect() already takes into
>> account the requires_extended_probe flag.
>
> The extended probe flag will prevent the radeon driver from further
> useless printing of bad EDIDs every ten seconds:
Yes, the extended probe check will prevent the spewing of probe
failures, so why do we need to fetch all the edids again at load? It
just adds latency.
>
> in radeon_connectors.c:
> if (radeon_connector->ddc_bus)
> dret = radeon_ddc_probe(radeon_connector,
>
> radeon_connector->requires_extended_probe);
> if (dret) {
> if (radeon_connector->edid) {
> kfree(radeon_connector->edid);
> radeon_connector->edid = NULL;
> }
> radeon_connector->edid = drm_get_edid(&radeon_connector->base,
> &radeon_connector->ddc_bus->adapter);
>
>
>
>
> But the now to be removed function radeon_ddc_dump() took care during
> connector setup that at least one (!) dump was in the logs, accompagnied
> by the info, that no monitor is connected, or there is a wrong/buggy
> EDID.
Ok, I see what you are saying. I'm not sure how important it is that
we print that out. If we have an improperly terminated i2c line,
you'll either get an edid or garbage and we already cover that with
the extended edid probe check.
>
> in radeon_display.c:
> radeon_print_display_setup(dev);
> list_for_each_entry(drm_connector,
> &dev->mode_config.connector_list, head)
> radeon_ddc_dump(drm_connector);
>
>
> I cannot imagine why this should confuse users. Having retrieved and
> configured connectors plus info plus EDID dump in case of problems
> logged next to each other was perfect.
It's confusing because on systems with eDP, DP, or DP bridge chips
(VGA and LVDS bridges) users get messages saying that the connectors
are disconnected or have a bad edid and then they file bugs that the
driver can''t detect their monitor when the monitor is detected just
fine, it's just on aux rather than i2c. It also adds latency to boot
since you now have two rounds of connector probing; once for the
ddc_dump, and once for the fb code. If we only dump information for
certain connectors it is also confusing since users will not see some
of the connectors checked at boot while others will be. We could add
additional logic for the DP cases, but you'd need to drag in most of
the logic from the dp detect functions in order to determine whether
the DP port is in DP or legacy mode and then decide whether or not to
use aux or i2c; all of which just adds more boot latency considering
we already have to do this for the fb setup.
>
>> The connectors are probed by
>> the fb code for the console as well so it just adds to the module load
>> time. If we want to print what connectors are connected, it should be
>> done from the fb code so we only have to do it once.
>>
>
> I briefly checked the code, but couldn't find a promising connector
> probing function in the fb code. Did you mean function
> drm_fb_helper_probe_connector_modes?
yes, via drm_fb_helper_initial_config()
>
>
>> > Graphic solutions in general have several connectors integrated, and it's
>> > sometimes really difficult to identify, which one of the does not work as
>> > expected, based on the logs. From above log we see, that a monitor is
>> > connected to DVI connector, nothing is connected to the VGA connector, and
>> > we have a problem with the HDMI connector. Without this fuinction, nothing
>> > helpful from radeon module would be in the logs.
>>
>> We should print the connector name in the generic drm error code then
>> if we want to print this info at boot time.
>
> Is there a place that is not called every ten seconds?
You could add logic to the fb probe code.
>
>>
>> >
>> > Maybe we can keep this function, but call it only for connectors, for which
>> > it works, i. e. not for DP, eDP and DP bridge connectors.
>>
>> That's just as bad. Users won't understand why only certain
>> connectors are probed.
>>
>> >
>> > Best regards
>> >
>> > Thomas Reim
>> >
>> > Am Dienstag, den 25.10.2011, 19:24 -0400 schrieb alexdeuc...@gmail.com:
>> >
>> > From: Alex Deucher
>> >
>> > The function didn
Re: [PATCH] drm: serialize access to debugs_nodes.list
On Mon, Oct 31, 2011 at 09:06:57AM +0100, Daniel Vetter wrote: > This is just ugly, you're adding a mutex to every drm_info_node, but only > use the one embedded into the minor. On a quick grep we're only ever using > the list in there, so I suggest to > - replace minor->debugfs_node.list with minor->debugfs_list and kill > ->debugfs_node > - add the mutex as minor->debugfs_lock > That way it's clear what's going on. Yes, you are right. I don't know what the heck I was thinking. > Also, you've forgotten to add the locking to i915/i915_debugfs.c Yeah. Will fix it too. Marcin ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm: do not sleep on vblank while holding a mutex
drm_wait_vblank must be DRM_UNLOCKED because otherwise it
will grab the drm_global_mutex and then go to sleep until the vblank
event it is waiting for. That can wreck havoc in the windowing system
because if one process issues this ioctl, it will block all other
processes for the duration of all vblanks between the current and the
one it is waiting for. In some cases it can block the entire windowing
system.
v2: incorporate comments received from Daniel Vetter and
Michel Daenzer.
v3: after a lengty discussion with Daniel Vetter, it was concluded
we should not worry about any locking, within drm_wait_vblank
function so this patch becomes a rather trivial removal
of drm_global_mutex from drm_wait_vblank
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_drv.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index dbabcb0..dc0eb0b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc_ioctl,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, 0),
+ DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: add some comments to drm_wait_vblank and drm_queue_vblank_event
during the review of the fix for locks problems in drm_wait_vblank,
a couple of false concerns were raised about how the drm_vblank_get
and drm_vblank_put are used in this function; it turned out that the
code is correct and that it cannot be simplified
add a few comments to explain non-obvious flows in the code,
to prevent "false alarms" in the future
v2: incorporate comments received from Daniel Vetter
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_irq.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 3830e9e..79c02da 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1124,6 +1124,7 @@ static int drm_queue_vblank_event(struct drm_device *dev,
int pipe,
trace_drm_vblank_event_delivered(current->pid, pipe,
vblwait->request.sequence);
} else {
+ /* drm_handle_vblank_events will call drm_vblank_put */
list_add_tail(&e->base.link, &dev->vblank_event_list);
vblwait->reply.sequence = vblwait->request.sequence;
}
@@ -1204,8 +1205,12 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
goto done;
}
- if (flags & _DRM_VBLANK_EVENT)
+ if (flags & _DRM_VBLANK_EVENT) {
+ /* must hold on to the vblank ref until the event fires
+* drm_vblank_put will be called asynchronously
+*/
return drm_queue_vblank_event(dev, crtc, vblwait, file_priv);
+ }
if ((flags & _DRM_VBLANK_NEXTONMISS) &&
(seq - vblwait->request.sequence) <= (1<<23)) {
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: serialize access to list of debugfs files
Nouveau, when configured with debugfs, creates debugfs files for every
channel, so structure holding list of files needs to be protected from
simultaneous changes by multiple threads.
Without this patch it's possible to hit kernel oops in
drm_debugfs_remove_files just by running a couple of xterms with
looped glxinfo.
Signed-off-by: Marcin Slusarz
---
drivers/gpu/drm/drm_debugfs.c | 12 +---
drivers/gpu/drm/i915/i915_debugfs.c |5 -
include/drm/drmP.h |4 +++-
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 9d2668a..d34d4fc 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -120,7 +120,10 @@ int drm_debugfs_create_files(struct drm_info_list *files,
int count,
tmp->minor = minor;
tmp->dent = ent;
tmp->info_ent = &files[i];
- list_add(&(tmp->list), &(minor->debugfs_nodes.list));
+
+ mutex_lock(&minor->debugfs_lock);
+ list_add(&tmp->list, &minor->debugfs_list);
+ mutex_unlock(&minor->debugfs_lock);
}
return 0;
@@ -148,7 +151,8 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
char name[64];
int ret;
- INIT_LIST_HEAD(&minor->debugfs_nodes.list);
+ INIT_LIST_HEAD(&minor->debugfs_list);
+ mutex_init(&minor->debugfs_lock);
sprintf(name, "%d", minor_id);
minor->debugfs_root = debugfs_create_dir(name, root);
if (!minor->debugfs_root) {
@@ -194,8 +198,9 @@ int drm_debugfs_remove_files(struct drm_info_list *files,
int count,
struct drm_info_node *tmp;
int i;
+ mutex_lock(&minor->debugfs_lock);
for (i = 0; i < count; i++) {
- list_for_each_safe(pos, q, &minor->debugfs_nodes.list) {
+ list_for_each_safe(pos, q, &minor->debugfs_list) {
tmp = list_entry(pos, struct drm_info_node, list);
if (tmp->info_ent == &files[i]) {
debugfs_remove(tmp->dent);
@@ -204,6 +209,7 @@ int drm_debugfs_remove_files(struct drm_info_list *files,
int count,
}
}
}
+ mutex_unlock(&minor->debugfs_lock);
return 0;
}
EXPORT_SYMBOL(drm_debugfs_remove_files);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3c395a5..fdad293 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1505,7 +1505,10 @@ drm_add_fake_info_node(struct drm_minor *minor,
node->minor = minor;
node->dent = ent;
node->info_ent = (void *) key;
- list_add(&node->list, &minor->debugfs_nodes.list);
+
+ mutex_lock(&minor->debugfs_lock);
+ list_add(&node->list, &minor->debugfs_list);
+ mutex_unlock(&minor->debugfs_lock);
return 0;
}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9b7c2bb..40e9ea3 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -989,7 +989,9 @@ struct drm_minor {
struct proc_dir_entry *proc_root; /**< proc directory entry */
struct drm_info_node proc_nodes;
struct dentry *debugfs_root;
- struct drm_info_node debugfs_nodes;
+
+ struct list_head debugfs_list;
+ struct mutex debugfs_lock; /* Protects debugfs_list. */
struct drm_master *master; /* currently active master for this node */
struct list_head master_list;
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/11] ttm: Provide DMA aware TTM page pool code.
On Wed, Oct 19, 2011 at 06:19:29PM -0400, Konrad Rzeszutek Wilk wrote: > In TTM world the pages for the graphic drivers are kept in three different > pools: write combined, uncached, and cached (write-back). When the pages > are used by the graphic driver the graphic adapter via its built in MMU > (or AGP) programs these pages in. The programming requires the virtual address > (from the graphic adapter perspective) and the physical address (either > System RAM > or the memory on the card) which is obtained using the pci_map_* calls (which > does the > virtual to physical - or bus address translation). During the graphic > application's > "life" those pages can be shuffled around, swapped out to disk, moved from the > VRAM to System RAM or vice-versa. This all works with the existing TTM pool > code > - except when we want to use the software IOTLB (SWIOTLB) code to "map" the > physical > addresses to the graphic adapter MMU. We end up programming the bounce > buffer's > physical address instead of the TTM pool memory's and get a non-worky driver. > There are two solutions: > 1) using the DMA API to allocate pages that are screened by the DMA API, or > 2) using the pci_sync_* calls to copy the pages from the bounce-buffer and > back. > > This patch fixes the issue by allocating pages using the DMA API. The second > is a viable option - but it has performance drawbacks and potential > correctness > issues - think of the write cache page being bounced (SWIOTLB->TTM), the > WC is set on the TTM page and the copy from SWIOTLB not making it to the TTM > page until the page has been recycled in the pool (and used by another > application). > > The bounce buffer does not get activated often - only in cases where we have > a 32-bit capable card and we want to use a page that is allocated above the > 4GB limit. The bounce buffer offers the solution of copying the contents > of that 4GB page to an location below 4GB and then back when the operation > has been > completed (or vice-versa). This is done by using the 'pci_sync_*' calls. > Note: If you look carefully enough in the existing TTM page pool code you will > notice the GFP_DMA32 flag is used - which should guarantee that the provided > page > is under 4GB. It certainly is the case, except this gets ignored in two cases: > - If user specifies 'swiotlb=force' which bounces _every_ page. > - If user is using a Xen's PV Linux guest (which uses the SWIOTLB and the >underlaying PFN's aren't necessarily under 4GB). > > To not have this extra copying done the other option is to allocate the pages > using the DMA API so that there is not need to map the page and perform the > expensive 'pci_sync_*' calls. > > This DMA API capable TTM pool requires for this the 'struct device' to > properly call the DMA API. It also has to track the virtual and bus address of > the page being handed out in case it ends up being swapped out or > de-allocated - > to make sure it is de-allocated using the proper's 'struct device'. > > Implementation wise the code keeps two lists: one that is attached to the > 'struct device' (via the dev->dma_pools list) and a global one to be used when > the 'struct device' is unavailable (think shrinker code). The global list can > iterate over all of the 'struct device' and its associated dma_pool. The list > in dev->dma_pools can only iterate the device's dma_pool. > /[struct > device_pool]\ > /---| dev >| >/+---| dma_pool >| > /-+--\/ > \/ > |struct device| /-->[struct dma_pool for WC] device_pool]\ > | dma_pools ++ /-| dev >| > | ...|\--->[struct dma_pool for uncached]<-/--| dma_pool >| > \-+--/ / > \/ > \--/ > [Two pools associated with the device (WC and UC), and the parallel list > containing the 'struct dev' and 'struct dma_pool' entries] > > The maximum amount of dma pools a device can have is six: write-combined, > uncached, and cached; then there are the DMA32 variants which are: > write-combined dma32, uncached dma32, and cached dma32. > > Currently this code only gets activated when any variant of the SWIOTLB IOMMU > code is running (Intel without VT-d, AMD without GART, IBM Calgary and Xen PV > with PCI devices). > > Tested-by: Michel Dänzer > [v1: Using swiotlb_nr_tbl instead of swiotlb_enabled] > [v2: Major overhaul - added 'inuse_list' to seperate used from inuse and > reorder > the order of lists to get better performance.] > Signed-off-by: Konrad Rzeszutek Wilk > --- > drivers/gpu/drm/ttm/Makefile |3 + > driv
[PATCH] drm: Make the per-driver file_operations struct const
>From fdf1fdebaa00f81de18c227f32f8074c8b352d50 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven
Date: Sun, 30 Oct 2011 19:06:07 -0700
Subject: [PATCH] drm: Make the per-driver file_operations struct const
The DRM layer keeps a copy of struct file_operations inside its
big driver struct... which prevents it from being consistent and static.
For consistency (and the general security objective of having such things
static), it's desirable to get this fixed.
This patch splits out the file_operations field to its own struct,
which is then "static const", and just stick a pointer to this into
the driver struct, making it more consistent with how the rest of the
kernel does this.
Signed-off-by: Arjan van de Ven
---
drivers/gpu/drm/drm_fops.c|2 +-
drivers/gpu/drm/i810/i810_drv.c | 23 ++--
drivers/gpu/drm/i915/i915_drv.c | 31 +
drivers/gpu/drm/mga/mga_drv.c | 29
drivers/gpu/drm/nouveau/nouveau_drv.c | 31 +
drivers/gpu/drm/r128/r128_drv.c | 30
drivers/gpu/drm/radeon/radeon_drv.c | 60 +
drivers/gpu/drm/savage/savage_drv.c | 23 ++--
drivers/gpu/drm/sis/sis_drv.c | 23 ++--
drivers/gpu/drm/tdfx/tdfx_drv.c | 23 ++--
drivers/gpu/drm/via/via_drv.c | 23 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 28 ---
drivers/staging/gma500/psb_drv.c | 23 ++--
include/drm/drmP.h|2 +-
14 files changed, 182 insertions(+), 169 deletions(-)
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 2ec7d48..3d7a75e 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -181,7 +181,7 @@ int drm_stub_open(struct inode *inode, struct file *filp)
goto out;
old_fops = filp->f_op;
- filp->f_op = fops_get(&dev->driver->fops);
+ filp->f_op = fops_get(dev->driver->fops);
if (filp->f_op == NULL) {
filp->f_op = old_fops;
goto out;
diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
index 6f98d05..c923956 100644
--- a/drivers/gpu/drm/i810/i810_drv.c
+++ b/drivers/gpu/drm/i810/i810_drv.c
@@ -41,6 +41,17 @@ static struct pci_device_id pciidlist[] = {
i810_PCI_IDS
};
+static const struct file_operations i810_driver_fops = {
+ .owner = THIS_MODULE,
+ .open = drm_open,
+ .release = drm_release,
+ .unlocked_ioctl = drm_ioctl,
+ .mmap = drm_mmap,
+ .poll = drm_poll,
+ .fasync = drm_fasync,
+ .llseek = noop_llseek,
+};
+
static struct drm_driver driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | DRIVER_USE_MTRR |
@@ -53,17 +64,7 @@ static struct drm_driver driver = {
.reclaim_buffers_locked = i810_driver_reclaim_buffers_locked,
.dma_quiescent = i810_driver_dma_quiescent,
.ioctls = i810_ioctls,
- .fops = {
-.owner = THIS_MODULE,
-.open = drm_open,
-.release = drm_release,
-.unlocked_ioctl = drm_ioctl,
-.mmap = drm_mmap,
-.poll = drm_poll,
-.fasync = drm_fasync,
-.llseek = noop_llseek,
- },
-
+ .fops = &i810_driver_fops,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
.date = DRIVER_DATE,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f07e4252..c395713 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -784,6 +784,21 @@ static struct vm_operations_struct i915_gem_vm_ops = {
.close = drm_gem_vm_close,
};
+static const struct file_operations i915_driver_fops = {
+ .owner = THIS_MODULE,
+ .open = drm_open,
+ .release = drm_release,
+ .unlocked_ioctl = drm_ioctl,
+ .mmap = drm_gem_mmap,
+ .poll = drm_poll,
+ .fasync = drm_fasync,
+ .read = drm_read,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = i915_compat_ioctl,
+#endif
+ .llseek = noop_llseek,
+};
+
static struct drm_driver driver = {
/* don't use mtrr's here, the Xserver or user space app should
* deal with them for intel hardware.
@@ -817,21 +832,7 @@ static struct drm_driver driver = {
.dumb_map_offset = i915_gem_mmap_gtt,
.dumb_destroy = i915_gem_dumb_destroy,
.ioctls = i915_ioctls,
- .fops = {
-.owner = THIS_MODULE,
-.open = drm_open,
-.release = drm_release,
-.unlocked_ioctl = drm_ioctl,
-.mmap = drm_gem_mmap,
-.poll = drm_poll,
-.fasync = drm_fasync,
-.read = drm_read,
-#ifdef CONFIG_COMPAT
-.compat_ioctl = i915_compat_ioctl,
-#endif
-.llseek = noop_llseek
Re: [Intel-gfx] [PATCH 1/2] Give up on edid retries when i2c tells us that bus is not there
On Mon, 17 Oct 2011 11:12:29 -0200, Eugeni Dodonov wrote: > This allows to avoid talking to a non-existent bus repeatedly until we > finally timeout. The non-existent bus is signalled by -ENXIO error, > provided by i2c_algo_bit:bit_doAddress call. > > As the advantage of such change, all the other routines which use > drm_get_edid would benefit for this timeout. > > This change should fix > https://bugs.freedesktop.org/show_bug.cgi?id=41059 and improve overall > edid detection timing by 10-30% in most cases, and by a much larger margin > in case of phantom outputs. > > Signed-off-by: Eugeni Dodonov Looks like we have reached the conclusion that this simple patch is the least likely to cause problems and easiest to fix if it does. :) Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Proposal for a low-level Linux display framework
On Sat, 17 Sep 2011 21:25:29 +0100 Alan Cox wrote: > > Just tell the X driver to not use acceleration, and it you won't get > > any acceleration used, then you get complete stability. If a driver > > writer wants to turn off all accel in the kernel driver, it can, its > > In fact one thing we actually need really is a "dumb" KMS X server to > replace the fbdev X server that unaccel stuff depends upon and which > can't do proper mode handling, multi-head or resizing as a result. A dumb > fb generic request for a back to front copy might also be useful for > shadowfb, or at least indicators so you know what the cache behaviour is > so the X server can pick the right policy. > > > We've fixed this in KMS, we don't pass direct mappings to userspace > > that we can't tear down and refault. We only provide objects via > > handles. The only place its a problem is where we expose fbdev legacy > > emulation, since we have to fix the pages. > > Which is doable. Horrible but doable. The usb framebuffer code has to > play games like this with the virtual framebuffer in order to track > changes by faulting. > > There are still some architectural screwups however. DRM continues the > fbdev worldview that outputs, memory and accelerators are tied together > in lumps we call video cards. That isn't really true for all cases and > with capture/overlay it gets even less true. Sorry for re-opening this ancient thread; I'm catching up from the past 2 months of travel & misc. I definitely agree about the PC card centric architecture of DRM KMS (and before it, X). But we have a path out of it now, and lots of interest from vendors and developers, so I don't think it's an insurmountable problem by any means. I definitely understand Florian's worries about DRM vs fb. If nothing else, there's certainly a perception that fb is simpler and easier to get right. But really, as others have pointed out, it's solving a different set of problems than the DRM layer. The latter is actually trying to expose the features of contemporary hardware in a way that's as portable as possible. That portability comes at a cost though: the APIs we add need to get lots of review, and there's no doubt we'll need to add more as newer, weirder hardware comes along. Really, I see no reason why fb and DRM can't continue to live side by side. If a vendor really only needs the features provided by the fb layer, they're free to stick with a simple fb driver. However, I expect most vendors making phones, tablets, notebooks, etc will need and want an architecture that looks a lot like the DRM layer, with authentication for rendering clients, an command submission ioctl for acceleration, and memory management, so I expect most of the driver growth to be in DRM in the near future. And I totally agree with Dave about having a kmscon. I really wish someone would implement it so I could have my VTs spinning on a cube. -- Jesse Barnes, Intel Open Source Technology Center signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: serialize access to list of debugfs files
On Mon, Oct 31, 2011 at 06:33:57PM +0100, Marcin Slusarz wrote: > Nouveau, when configured with debugfs, creates debugfs files for every > channel, so structure holding list of files needs to be protected from > simultaneous changes by multiple threads. > > Without this patch it's possible to hit kernel oops in > drm_debugfs_remove_files just by running a couple of xterms with > looped glxinfo. > > Signed-off-by: Marcin Slusarz And v2 also includes a nice comment in the header explaining the locking. Awesome! Reviewed-by: Daniel Vetter -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: add some comments to drm_wait_vblank and drm_queue_vblank_event
On Mon, Oct 31, 2011 at 01:11:57PM -0400, Ilija Hadzic wrote: > during the review of the fix for locks problems in drm_wait_vblank, > a couple of false concerns were raised about how the drm_vblank_get > and drm_vblank_put are used in this function; it turned out that the > code is correct and that it cannot be simplified > > add a few comments to explain non-obvious flows in the code, > to prevent "false alarms" in the future > > v2: incorporate comments received from Daniel Vetter > > Signed-off-by: Ilija Hadzic Reviewed-by: Daniel Vetter -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm: do not sleep on vblank while holding a mutex
On Mon, Oct 31, 2011 at 01:10:21PM -0400, Ilija Hadzic wrote: > drm_wait_vblank must be DRM_UNLOCKED because otherwise it > will grab the drm_global_mutex and then go to sleep until the vblank > event it is waiting for. That can wreck havoc in the windowing system > because if one process issues this ioctl, it will block all other > processes for the duration of all vblanks between the current and the > one it is waiting for. In some cases it can block the entire windowing > system. > > v2: incorporate comments received from Daniel Vetter and > Michel Daenzer. > > v3: after a lengty discussion with Daniel Vetter, it was concluded > we should not worry about any locking, within drm_wait_vblank > function so this patch becomes a rather trivial removal > of drm_global_mutex from drm_wait_vblank That commit message is a bit garbage. What about ... "... it was concluded that the only think not yet protected with locks and atomic ops is the write to dev->last_vblank_wait. It's only used in a debug file in proc, and the current code already employs no correct locking: the proc file only takes dev->struct_mutex, whereas drm_wait_vblank implicitly took the drm_global_mutex. Given all this, it's not worth bothering to try to fix this at this time." I think it's important to correctly document the conclusion of this discussion, because we've worried quite a bit about correct locking. Yours, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm: do not sleep on vblank while holding a mutex
OK, v4 coming up and your suggestion will be copied verbatim. Hopefully that does it. This patch is probably going to become a record-holder in comment/code lines ratio ;-) -- Ilija On Mon, 31 Oct 2011, Daniel Vetter wrote: On Mon, Oct 31, 2011 at 01:10:21PM -0400, Ilija Hadzic wrote: drm_wait_vblank must be DRM_UNLOCKED because otherwise it will grab the drm_global_mutex and then go to sleep until the vblank event it is waiting for. That can wreck havoc in the windowing system because if one process issues this ioctl, it will block all other processes for the duration of all vblanks between the current and the one it is waiting for. In some cases it can block the entire windowing system. v2: incorporate comments received from Daniel Vetter and Michel Daenzer. v3: after a lengty discussion with Daniel Vetter, it was concluded we should not worry about any locking, within drm_wait_vblank function so this patch becomes a rather trivial removal of drm_global_mutex from drm_wait_vblank That commit message is a bit garbage. What about ... "... it was concluded that the only think not yet protected with locks and atomic ops is the write to dev->last_vblank_wait. It's only used in a debug file in proc, and the current code already employs no correct locking: the proc file only takes dev->struct_mutex, whereas drm_wait_vblank implicitly took the drm_global_mutex. Given all this, it's not worth bothering to try to fix this at this time." I think it's important to correctly document the conclusion of this discussion, because we've worried quite a bit about correct locking. Yours, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4] drm: do not sleep on vblank while holding a mutex
drm_wait_vblank must be DRM_UNLOCKED because otherwise it
will grab the drm_global_mutex and then go to sleep until the vblank
event it is waiting for. That can wreck havoc in the windowing system
because if one process issues this ioctl, it will block all other
processes for the duration of all vblanks between the current and the
one it is waiting for. In some cases it can block the entire windowing
system.
v2: incorporate comments received from Daniel Vetter and
Michel Daenzer.
v3/v4: after a lengty discussion with Daniel Vetter, it was concluded
that the only thing not yet protected with locks and atomic
ops is the write to dev->last_vblank_wait. It's only used in a
debug file in proc, and the current code already employs no
correct locking: the proc file only takes dev->struct_mutex,
whereas drm_wait_vblank implicitly took the drm_global_mutex.
Given all this, it's not worth bothering to try to fix
the locks at this time.
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_drv.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index dbabcb0..dc0eb0b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc_ioctl,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, 0),
+ DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
--
1.7.7
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] TTM DMA pool v2.1
On Wed, Oct 19, 2011 at 06:19:21PM -0400, Konrad Rzeszutek Wilk wrote: > [.. and this is what I said in v1 post]: > > Way back in January this patchset: > http://lists.freedesktop.org/archives/dri-devel/2011-January/006905.html > was merged in, but pieces of it had to be reverted b/c they did not > work properly under PowerPC, ARM, and when swapping out pages to disk. > > After a bit of discussion on the mailing list > http://marc.info/?i=4d769726.2030...@shipmail.org I started working on it, but > got waylaid by other things .. and finally I am able to post the RFC patches. > > There was a lot of discussion about it and I am not sure if I captured > everybody's thoughts - if I did not - that is _not_ intentional - it has just > been quite some time.. > > Anyhow .. the patches explore what the "lib/dmapool.c" does - which is to > have a > DMA pool that the device has associated with. I kind of married that code > along with drivers/gpu/drm/ttm/ttm_page_alloc.c to create a TTM DMA pool code. > The end result is DMA pool with extra features: can do write-combine, > uncached, > writeback (and tracks them and sets back to WB when freed); tracks "cached" > pages that don't really need to be returned to a pool; and hooks up to > the shrinker code so that the pools can be shrunk. > > If you guys think this set of patches make sense - my future plans were > 1) Get this in large crowd of testing .. and if it works for a kernel release > 2) to move a bulk of this in the lib/dmapool.c (I spoke with Matthew Wilcox > about it and he is OK as long as I don't introduce performance > regressions). > > But before I do any of that a second set of eyes taking a look at these > patches would be most welcome. > > In regards to testing, I've been running them non-stop for the last month. > (and found some issues which I've fixed up) - and been quite happy with how > they work. > > Michel (thanks!) took a spin of the patches on his PowerPC and they did not > cause any regressions (wheew). > > The patches are also located in a git tree: > Reviewed the patch series, looks good, already sent comment on one of the patch. I am on the same side of Thomas for dma stuff, lately i have been working on GPU virtual memory address space and i believe having driver allocating the ttm_tt and merging more stuff in the backend make sense, after all the backend has better knowledge on both cache preference and dma mask. So far my idea is to merge ttm_tt & ttm_backend, simplify the backend function to bind/unbind/destroy where bind is responsible to allocate or not a ttm_tt and pages that goes along with it. I will try to sketch up patches for all this in next few days. Reviewed-by: Jerome Glisse Cheers, Jerome Glisse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm: do not sleep on vblank while holding a mutex
On Mon, Oct 31, 2011 at 05:46:18PM -0400, Ilija Hadzic wrote: > drm_wait_vblank must be DRM_UNLOCKED because otherwise it > will grab the drm_global_mutex and then go to sleep until the vblank > event it is waiting for. That can wreck havoc in the windowing system > because if one process issues this ioctl, it will block all other > processes for the duration of all vblanks between the current and the > one it is waiting for. In some cases it can block the entire windowing > system. > > v2: incorporate comments received from Daniel Vetter and > Michel Daenzer. > > v3/v4: after a lengty discussion with Daniel Vetter, it was concluded >that the only thing not yet protected with locks and atomic >ops is the write to dev->last_vblank_wait. It's only used in a >debug file in proc, and the current code already employs no >correct locking: the proc file only takes dev->struct_mutex, >whereas drm_wait_vblank implicitly took the drm_global_mutex. >Given all this, it's not worth bothering to try to fix >the locks at this time. > > Signed-off-by: Ilija Hadzic Reviewed-by: Daniel Vetter -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] edid candidate for -next
So as we all agree on this patch now, I updated it with all the Tested-by and Reviewed-by. Dave, I think it is good for pulling into -next now. Thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Give up on edid retries when i2c bus is not responding
This allows to avoid talking to a non-responding bus repeatedly until we
finally timeout after 15 attempts. We can do this by catching the -ENXIO
error, provided by i2c_algo_bit:bit_doAddress call.
Within the bit_doAddress we already try 3 times to get the edid data, so
if the routine tells us that bus is not responding, it is mostly pointless
to keep re-trying those attempts over and over again until we reach final
number of retries.
This change should fix https://bugs.freedesktop.org/show_bug.cgi?id=41059
and improve overall edid detection timing by 10-30% in most cases, and by
a much larger margin in case of phantom outputs (up to 30x in one worst
case).
Timing results for i915-powered machines for 'time xrandr' command:
Machine 1: from 0.840s to 0.290s
Machine 2: from 0.315s to 0.280s
Machine 3: from +/- 4s to 0.184s
Timing results for HD5770 with 'time xrandr' command:
Machine 4: from 3.210s to 1.060s
Reviewed-by: Chris Wilson
Reviewed-by: Keith Packard
Tested-by: Sean Finney
Tested-by: Soren Hansen
Tested-by: Hernando Torque
Tested-by: Mike Lothian
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41059
Signed-off-by: Eugeni Dodonov
---
drivers/gpu/drm/drm_edid.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7425e5c..1bca6d7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -265,6 +265,11 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter,
unsigned char *buf,
}
};
ret = i2c_transfer(adapter, msgs, 2);
+ if (ret == -ENXIO) {
+ DRM_DEBUG_KMS("drm: skipping non-existent adapter %s\n",
+ adapter->name);
+ break;
+ }
} while (ret != 2 && --retries);
return ret == 2 ? 0 : -1;
--
1.7.7.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 27314] displayport link training fails on certain panels (channel equalization fails)
https://bugs.freedesktop.org/show_bug.cgi?id=27314 --- Comment #56 from Travis Glenn Hansen 2011-10-31 19:59:41 PDT --- Alex Thanks for continuing to look at this :) I have converted my amd machine to a server so I'm not using it (or rebooting it for that matter) with an external monitor. However I ran through some updates today and decided to hook it up to provide feedback. I updated to 3.1.0 (gentoo variant) and it continues to fail for me :( After looking at the patch it appears to either be in vanilla 3.1.0 or already included in the gentoo patchset. To summarize, I've tried 3.1.0 with the patch from the link and it doesn't work with my setup. -- 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 42409] New: Mesa 7.12-devel /dri/r600 compilation error: no makefile found
https://bugs.freedesktop.org/show_bug.cgi?id=42409 Bug #: 42409 Summary: Mesa 7.12-devel /dri/r600 compilation error: no makefile found Classification: Unclassified Product: Mesa Version: git Platform: x86-64 (AMD64) OS/Version: All Status: NEW Severity: normal Priority: medium Component: Drivers/DRI/R600 AssignedTo: dri-devel at lists.freedesktop.org ReportedBy: wolput at onsneteindhoven.nl git mesa 7.72-devel compilation error: drivers/dri/r600 no targets specified and no makefile found --- make[5]: Entering directory `/home/jos/src/xorg/git-master/mesa/src/mesa/drivers/dri/r600' make[5]: *** No targets specified and no makefile found. Stop. make[5]: Leaving directory `/home/jos/src/xorg/git-master/mesa/src/mesa/drivers/dri/r600' make[4]: *** [subdirs] Error 1 make[4]: Leaving directory `/home/jos/src/xorg/git-master/mesa/src/mesa/drivers/dri' make[3]: *** [default] Error 1 make[3]: Leaving directory `/home/jos/src/xorg/git-master/mesa/src/mesa/drivers' make[2]: *** [driver_subdirs] Error 2 make[2]: Leaving directory `/home/jos/src/xorg/git-master/mesa/src/mesa' make[1]: *** [subdirs] Error 1 make[1]: Leaving directory `/home/jos/src/xorg/git-master/mesa/src' make: *** [default] Error 1 --- -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 42409] Mesa 7.12-devel /dri/r600 compilation error: no makefile found
https://bugs.freedesktop.org/show_bug.cgi?id=42409 --- Comment #1 from Jos van Wolput 2011-10-30 22:27:17 PDT --- I understand the dri r600 driver has been removed, isn't it? So this issue is not a bug and can be closed. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH] drm: serialize access to debugs_nodes.list
On Sun, Oct 30, 2011 at 11:04:48PM +0100, Marcin Slusarz wrote:
> Nouveau, when configured with debugfs, creates debugfs files for every
> channel, so structure holding list of files needs to be protected from
> simultaneous changes by multiple threads.
>
> Without this patch it's possible to hit kernel oops in
> drm_debugfs_remove_files just by running a couple of xterms with
> looped glxinfo.
>
> Signed-off-by: Marcin Slusarz
> ---
> drivers/gpu/drm/drm_debugfs.c |5 +
> include/drm/drmP.h|1 +
> 2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 9d2668a..1144fbe 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -120,7 +120,9 @@ int drm_debugfs_create_files(struct drm_info_list *files,
> int count,
> tmp->minor = minor;
> tmp->dent = ent;
> tmp->info_ent = &files[i];
> + mutex_lock(&minor->debugfs_nodes.mutex);
> list_add(&(tmp->list), &(minor->debugfs_nodes.list));
> + mutex_unlock(&minor->debugfs_nodes.mutex);
> }
> return 0;
>
> @@ -149,6 +151,7 @@ int drm_debugfs_init(struct drm_minor *minor, int
> minor_id,
> int ret;
>
> INIT_LIST_HEAD(&minor->debugfs_nodes.list);
> + mutex_init(&minor->debugfs_nodes.mutex);
> sprintf(name, "%d", minor_id);
> minor->debugfs_root = debugfs_create_dir(name, root);
> if (!minor->debugfs_root) {
> @@ -194,6 +197,7 @@ int drm_debugfs_remove_files(struct drm_info_list *files,
> int count,
> struct drm_info_node *tmp;
> int i;
>
> + mutex_lock(&minor->debugfs_nodes.mutex);
> for (i = 0; i < count; i++) {
> list_for_each_safe(pos, q, &minor->debugfs_nodes.list) {
> tmp = list_entry(pos, struct drm_info_node, list);
> @@ -204,6 +208,7 @@ int drm_debugfs_remove_files(struct drm_info_list *files,
> int count,
> }
> }
> }
> + mutex_unlock(&minor->debugfs_nodes.mutex);
> return 0;
> }
> EXPORT_SYMBOL(drm_debugfs_remove_files);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 9b7c2bb..c70c943 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -970,6 +970,7 @@ struct drm_info_list {
> * debugfs node structure. This structure represents a debugfs file.
> */
> struct drm_info_node {
> + struct mutex mutex;
> struct list_head list;
> struct drm_minor *minor;
> struct drm_info_list *info_ent;
This is just ugly, you're adding a mutex to every drm_info_node, but only
use the one embedded into the minor. On a quick grep we're only ever using
the list in there, so I suggest to
- replace minor->debugfs_node.list with minor->debugfs_list and kill
->debugfs_node
- add the mutex as minor->debugfs_lock
That way it's clear what's going on.
Also, you've forgotten to add the locking to i915/i915_debugfs.c
Yours, Daniel
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
[Bug 42409] Mesa 7.12-devel /dri/r600 compilation error: no makefile found
https://bugs.freedesktop.org/show_bug.cgi?id=42409 Christian K?nig changed: What|Removed |Added Status|NEW |RESOLVED Resolution||NOTABUG --- Comment #2 from Christian K?nig 2011-10-31 02:04:32 PDT --- That is correct, r600 has been removed. It looks like that git didn't remove the directory probably because of some stale file in it. Please remove it manually, to make compiling work again. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #11 from mirandir at orange.fr 2011-10-31 03:42:13 PDT --- My system don't freeze when i (In reply to comment #10) > Bugzilla Gnome 661898 > Posted some days ago. > > Because it can be just a bug around the radeon driver in kernel I opened a new > bug here. > https://bugs.freedesktop.org/show_bug.cgi?id=41838 > > Would be intersting to see if it freezes your system in the same way, if you > visit the website I mentioned in my bug. So we can be sure, that is the same > issue. My system do not freeze when I visit http://piratpix.com/bpt2011.1/index.html and browse pictures with Epiphany, so I don't think it's the same issue. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #12 from peterle at hottemptation.org 2011-10-31 04:50:56 PDT --- Thanks! That is bad, or good. Did you clicked on the thumbnails or just viewed the slideshow? Clicking on the thumbnails is causing the issue, the slideshow not. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #13 from peterle at hottemptation.org 2011-10-31 05:14:25 PDT --- Forgot something important! Are you using the last official xf86-video-radeon driver or the one from git? -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #14 from mirandir at orange.fr 2011-10-31 05:19:17 PDT --- Yes, I have clicked on the thumbnails, and I use the git version of the driver. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 Nicolas Peninguy changed: What|Removed |Added CC||npeninguy at gmail.com --- Comment #15 from Nicolas Peninguy 2011-10-31 06:14:45 PDT --- May be this is related to https://bugzilla.gnome.org/show_bug.cgi?id=650857 ? (But Intel and NVidia owners are also affected) -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 42435] New: Suspected loss of varyings between shaders
https://bugs.freedesktop.org/show_bug.cgi?id=42435 Bug #: 42435 Summary: Suspected loss of varyings between shaders Classification: Unclassified Product: Mesa Version: git Platform: x86-64 (AMD64) OS/Version: Linux (All) Status: NEW Severity: major Priority: medium Component: Drivers/Gallium/r600 AssignedTo: dri-devel at lists.freedesktop.org ReportedBy: screwdriver at lxnt.info Longer description, the test case, screenshots, and core from i965 crash are in attached archive. In short: I try to draw a 2D screenful of point sprites, emulating an ansi-colored terminal. Fragment shader receives texture coordinates for the character, its color and size of graphic in the texture via varyings. R600g draws a black screen, while llvmpipe and fglrx draw what is supposed to be drawn. i965 crashes. non-llvm swrast fails to set gl_PointSize. R600c also draws a black screen. Drawing varying values themselves I get bogus values for some of them. Mesa version: 7.12.0~git20111027.0fbc8d30-0ubuntu0sarvatt~natty -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 41592] [Radeon] The display often freezes with gnome-shell 3.2
https://bugs.freedesktop.org/show_bug.cgi?id=41592 --- Comment #16 from mirandir at orange.fr 2011-10-31 07:43:07 PDT --- Thanks for the link. You are right, this problem seems very very close to mine. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 42435] Suspected loss of varyings between shaders
https://bugs.freedesktop.org/show_bug.cgi?id=42435 --- Comment #1 from Alexander Sabourenkov 2011-10-31 07:43:44 PDT --- Created attachment 52954 --> https://bugs.freedesktop.org/attachment.cgi?id=52954 Test case and stuff -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 42435] Suspected loss of varyings between shaders
https://bugs.freedesktop.org/show_bug.cgi?id=42435 --- Comment #2 from Alexander Sabourenkov 2011-10-31 07:50:58 PDT --- core did not fit: >3000K size -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
RFC: Radeon multi ring support branch
On Sat, Oct 29, 2011 at 03:00:28PM +0200, Christian K?nig wrote: > Hello everybody, > > to support multiple compute rings, async DMA engines and UVD we need > to teach the radeon kernel module how to sync buffers between > different rings and make some changes to the command submission > ioctls. > > Since we can't release any documentation about async DMA or UVD > (yet), my current branch concentrates on getting the additional > compute rings on cayman running. Unfortunately those rings have > hardware bugs that can't be worked around, so they are actually not > very useful in a production environment, but they should do quite > well for this testing purpose. > > The branch can be found here: > http://cgit.freedesktop.org/~deathsimple/linux/log/ > > Since some of the patches are quite intrusive, constantly rebaseing > them could get a bit painful. So I would like to see most of the > stuff included into drm-next, even if we don't make use of the new > functionality right now. > > Comments welcome, > Christian. So for all patches except the interface change see below Reviewed-by: Jerome Glisse For the interface change, as discussed previously, i believe prio should be a userspace argument, kernel could override it. Cheers, Jerome
[PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
adeon_print_display_setup(struct drm_device
> > *dev)
> > static bool radeon_setup_enc_conn(struct drm_device *dev)
> > {
> > struct radeon_device *rdev = dev->dev_private;
> > - struct drm_connector *drm_connector;
> > bool ret = false;
> >
> > if (rdev->bios) {
> > @@ -689,8 +686,6 @@ static bool radeon_setup_enc_conn(struct drm_device
> > *dev)
> > if (ret) {
> > radeon_setup_encoder_clones(dev);
> > radeon_print_display_setup(dev);
> > - list_for_each_entry(drm_connector,
> > &dev->mode_config.connector_list,
> > head)
> > - radeon_ddc_dump(drm_connector);
> > }
> >
> > return ret;
> > @@ -743,34 +738,6 @@ int radeon_ddc_get_modes(struct radeon_connector
> > *radeon_connector)
> > return 0;
> > }
> >
> > -static int radeon_ddc_dump(struct drm_connector *connector)
> > -{
> > - struct edid *edid;
> > - struct radeon_connector *radeon_connector =
> > to_radeon_connector(connector);
> > - int ret = 0;
> > -
> > - /* on hw with routers, select right port */
> > - if (radeon_connector->router.ddc_valid)
> > - radeon_router_select_ddc_port(radeon_connector);
> > -
> > - if (!radeon_connector->ddc_bus)
> > - return -1;
> > - edid = drm_get_edid(connector, &radeon_connector->ddc_bus->adapter);
> > - /* Log EDID retrieval status here. In particular with regard to
> > -* connectors with requires_extended_probe flag set, that will prevent
> > -* function radeon_dvi_detect() to fetch EDID on this connector,
> > -* as long as there is no valid EDID header found */
> > - if (edid) {
> > - DRM_INFO("Radeon display connector %s: Found valid EDID",
> > - drm_get_connector_name(connector));
> > - kfree(edid);
> > - } else {
> > - DRM_INFO("Radeon display connector %s: No monitor connected or
> > invalid
> > EDID",
> > - drm_get_connector_name(connector));
> > - }
> > - return ret;
> > -}
> > -
> > /* avivo */
> > static void avivo_get_fb_div(struct radeon_pll *pll,
> > u32 target_clock,
> >
> >
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20111031/08a726b7/attachment.pgp>
[PATCH] drm/radeon/kms: remove useless radeon_ddc_dump()
On Mon, Oct 31, 2011 at 11:15 AM, Thomas Reim wrote:
> Dear Alex,
>
> your reply confuses me:
>
>> On Sat, Oct 29, 2011 at 8:55 AM, Thomas Reim
>> wrote:
>> > Dear Alex,
>> >
>> > but we use DDC probing e. g. to identify connectors with improperly
>> > terminated i2c bus. Instead of flooding logs and terminals with EDID dumps,
>> > we decided some months ago to use this function during module loading to
>> > inform the user once (and only once!), which connector has a monitor
>> > connected with valid EDID and which connector has not.
>>
>> There's nothing in that function that actually prevents the printing
>> of bad EDIDs.
>
> That's true.
>
>> All it does is call drm_get_edid() and report whether
>> it found an EDID or not. ?radeon_dvi_detect() already takes into
>> account the requires_extended_probe flag.
>
> The extended probe flag will prevent the radeon driver from further
> useless printing of bad EDIDs every ten seconds:
Yes, the extended probe check will prevent the spewing of probe
failures, so why do we need to fetch all the edids again at load? It
just adds latency.
>
> in radeon_connectors.c:
> if (radeon_connector->ddc_bus)
> ? ? ? ? ? ? ? ?dret = radeon_ddc_probe(radeon_connector,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?
> ?radeon_connector->requires_extended_probe);
> ? ? ? ?if (dret) {
> ? ? ? ? ? ? ? ?if (radeon_connector->edid) {
> ? ? ? ? ? ? ? ? ? ? ? ?kfree(radeon_connector->edid);
> ? ? ? ? ? ? ? ? ? ? ? ?radeon_connector->edid = NULL;
> ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?radeon_connector->edid = drm_get_edid(&radeon_connector->base,
> &radeon_connector->ddc_bus->adapter);
>
>
>
>
> But the now to be removed function radeon_ddc_dump() took care during
> connector setup that at least one (!) dump was in the logs, accompagnied
> by the info, that no monitor is connected, or there is a wrong/buggy
> EDID.
Ok, I see what you are saying. I'm not sure how important it is that
we print that out. If we have an improperly terminated i2c line,
you'll either get an edid or garbage and we already cover that with
the extended edid probe check.
>
> in radeon_display.c:
> radeon_print_display_setup(dev);
> ? ? ? ? ? ? ? ?list_for_each_entry(drm_connector,
> &dev->mode_config.connector_list, head)
> ? ? ? ? ? ? ? ? ? ? ? ?radeon_ddc_dump(drm_connector);
>
>
> I cannot imagine why this should confuse users. Having retrieved and
> configured connectors plus info plus EDID dump in case of problems
> logged next to each other was perfect.
It's confusing because on systems with eDP, DP, or DP bridge chips
(VGA and LVDS bridges) users get messages saying that the connectors
are disconnected or have a bad edid and then they file bugs that the
driver can''t detect their monitor when the monitor is detected just
fine, it's just on aux rather than i2c. It also adds latency to boot
since you now have two rounds of connector probing; once for the
ddc_dump, and once for the fb code. If we only dump information for
certain connectors it is also confusing since users will not see some
of the connectors checked at boot while others will be. We could add
additional logic for the DP cases, but you'd need to drag in most of
the logic from the dp detect functions in order to determine whether
the DP port is in DP or legacy mode and then decide whether or not to
use aux or i2c; all of which just adds more boot latency considering
we already have to do this for the fb setup.
>
>> The connectors are probed by
>> the fb code for the console as well so it just adds to the module load
>> time. ?If we want to print what connectors are connected, it should be
>> done from the fb code so we only have to do it once.
>>
>
> I briefly checked the code, but couldn't find a promising connector
> probing function in the fb code. Did you mean function
> drm_fb_helper_probe_connector_modes?
yes, via drm_fb_helper_initial_config()
>
>
>> > Graphic solutions in general have several connectors integrated, and it's
>> > sometimes really difficult to identify, which one of the does not work as
>> > expected, based on the logs. From above log we see, that a monitor is
>> > connected to DVI connector, nothing is connected to the VGA connector, and
>> > we have a problem with the HDMI connector. Without this fuinction, nothing
>> > helpful from radeon module would be in the logs.
>>
>> We should print the connector name in the generic drm error code then
>> if we want to print this info at boot time.
>
> Is there a place that is not called every ten seconds?
You could add logic to the fb probe code.
>
>>
>> >
>> > Maybe we can keep this function, but call it only for connectors, for which
>> > it works, i. e. not for DP, eDP and DP bridge connectors.
>>
>> That's just as bad. ?Users won't understand why only certain
>> connectors are probed.
>>
>> >
>> > Best regards
>> >
>> > Thomas Reim
>> >
>> > Am Dienstag, den 25.10.2011, 19:24 -0400 schrieb alexdeucher at gmail.com:
>> >
>> > From: Alex Deucher
>> >
>> > The functi
[PATCH] drm: serialize access to debugs_nodes.list
On Mon, Oct 31, 2011 at 09:06:57AM +0100, Daniel Vetter wrote: > This is just ugly, you're adding a mutex to every drm_info_node, but only > use the one embedded into the minor. On a quick grep we're only ever using > the list in there, so I suggest to > - replace minor->debugfs_node.list with minor->debugfs_list and kill > ->debugfs_node > - add the mutex as minor->debugfs_lock > That way it's clear what's going on. Yes, you are right. I don't know what the heck I was thinking. > Also, you've forgotten to add the locking to i915/i915_debugfs.c Yeah. Will fix it too. Marcin
[PATCH v3] drm: do not sleep on vblank while holding a mutex
drm_wait_vblank must be DRM_UNLOCKED because otherwise it
will grab the drm_global_mutex and then go to sleep until the vblank
event it is waiting for. That can wreck havoc in the windowing system
because if one process issues this ioctl, it will block all other
processes for the duration of all vblanks between the current and the
one it is waiting for. In some cases it can block the entire windowing
system.
v2: incorporate comments received from Daniel Vetter and
Michel Daenzer.
v3: after a lengty discussion with Daniel Vetter, it was concluded
we should not worry about any locking, within drm_wait_vblank
function so this patch becomes a rather trivial removal
of drm_global_mutex from drm_wait_vblank
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_drv.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index dbabcb0..dc0eb0b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc_ioctl,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, 0),
+ DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
--
1.7.7
[PATCH v2] drm: add some comments to drm_wait_vblank and drm_queue_vblank_event
during the review of the fix for locks problems in drm_wait_vblank,
a couple of false concerns were raised about how the drm_vblank_get
and drm_vblank_put are used in this function; it turned out that the
code is correct and that it cannot be simplified
add a few comments to explain non-obvious flows in the code,
to prevent "false alarms" in the future
v2: incorporate comments received from Daniel Vetter
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_irq.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 3830e9e..79c02da 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1124,6 +1124,7 @@ static int drm_queue_vblank_event(struct drm_device *dev,
int pipe,
trace_drm_vblank_event_delivered(current->pid, pipe,
vblwait->request.sequence);
} else {
+ /* drm_handle_vblank_events will call drm_vblank_put */
list_add_tail(&e->base.link, &dev->vblank_event_list);
vblwait->reply.sequence = vblwait->request.sequence;
}
@@ -1204,8 +1205,12 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
goto done;
}
- if (flags & _DRM_VBLANK_EVENT)
+ if (flags & _DRM_VBLANK_EVENT) {
+ /* must hold on to the vblank ref until the event fires
+* drm_vblank_put will be called asynchronously
+*/
return drm_queue_vblank_event(dev, crtc, vblwait, file_priv);
+ }
if ((flags & _DRM_VBLANK_NEXTONMISS) &&
(seq - vblwait->request.sequence) <= (1<<23)) {
--
1.7.7
[PATCH] drm: serialize access to list of debugfs files
Nouveau, when configured with debugfs, creates debugfs files for every
channel, so structure holding list of files needs to be protected from
simultaneous changes by multiple threads.
Without this patch it's possible to hit kernel oops in
drm_debugfs_remove_files just by running a couple of xterms with
looped glxinfo.
Signed-off-by: Marcin Slusarz
---
drivers/gpu/drm/drm_debugfs.c | 12 +---
drivers/gpu/drm/i915/i915_debugfs.c |5 -
include/drm/drmP.h |4 +++-
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 9d2668a..d34d4fc 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -120,7 +120,10 @@ int drm_debugfs_create_files(struct drm_info_list *files,
int count,
tmp->minor = minor;
tmp->dent = ent;
tmp->info_ent = &files[i];
- list_add(&(tmp->list), &(minor->debugfs_nodes.list));
+
+ mutex_lock(&minor->debugfs_lock);
+ list_add(&tmp->list, &minor->debugfs_list);
+ mutex_unlock(&minor->debugfs_lock);
}
return 0;
@@ -148,7 +151,8 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
char name[64];
int ret;
- INIT_LIST_HEAD(&minor->debugfs_nodes.list);
+ INIT_LIST_HEAD(&minor->debugfs_list);
+ mutex_init(&minor->debugfs_lock);
sprintf(name, "%d", minor_id);
minor->debugfs_root = debugfs_create_dir(name, root);
if (!minor->debugfs_root) {
@@ -194,8 +198,9 @@ int drm_debugfs_remove_files(struct drm_info_list *files,
int count,
struct drm_info_node *tmp;
int i;
+ mutex_lock(&minor->debugfs_lock);
for (i = 0; i < count; i++) {
- list_for_each_safe(pos, q, &minor->debugfs_nodes.list) {
+ list_for_each_safe(pos, q, &minor->debugfs_list) {
tmp = list_entry(pos, struct drm_info_node, list);
if (tmp->info_ent == &files[i]) {
debugfs_remove(tmp->dent);
@@ -204,6 +209,7 @@ int drm_debugfs_remove_files(struct drm_info_list *files,
int count,
}
}
}
+ mutex_unlock(&minor->debugfs_lock);
return 0;
}
EXPORT_SYMBOL(drm_debugfs_remove_files);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3c395a5..fdad293 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1505,7 +1505,10 @@ drm_add_fake_info_node(struct drm_minor *minor,
node->minor = minor;
node->dent = ent;
node->info_ent = (void *) key;
- list_add(&node->list, &minor->debugfs_nodes.list);
+
+ mutex_lock(&minor->debugfs_lock);
+ list_add(&node->list, &minor->debugfs_list);
+ mutex_unlock(&minor->debugfs_lock);
return 0;
}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9b7c2bb..40e9ea3 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -989,7 +989,9 @@ struct drm_minor {
struct proc_dir_entry *proc_root; /**< proc directory entry */
struct drm_info_node proc_nodes;
struct dentry *debugfs_root;
- struct drm_info_node debugfs_nodes;
+
+ struct list_head debugfs_list;
+ struct mutex debugfs_lock; /* Protects debugfs_list. */
struct drm_master *master; /* currently active master for this node */
struct list_head master_list;
--
1.7.7
[PATCH 08/11] ttm: Provide DMA aware TTM page pool code.
On Wed, Oct 19, 2011 at 06:19:29PM -0400, Konrad Rzeszutek Wilk wrote: > In TTM world the pages for the graphic drivers are kept in three different > pools: write combined, uncached, and cached (write-back). When the pages > are used by the graphic driver the graphic adapter via its built in MMU > (or AGP) programs these pages in. The programming requires the virtual address > (from the graphic adapter perspective) and the physical address (either > System RAM > or the memory on the card) which is obtained using the pci_map_* calls (which > does the > virtual to physical - or bus address translation). During the graphic > application's > "life" those pages can be shuffled around, swapped out to disk, moved from the > VRAM to System RAM or vice-versa. This all works with the existing TTM pool > code > - except when we want to use the software IOTLB (SWIOTLB) code to "map" the > physical > addresses to the graphic adapter MMU. We end up programming the bounce > buffer's > physical address instead of the TTM pool memory's and get a non-worky driver. > There are two solutions: > 1) using the DMA API to allocate pages that are screened by the DMA API, or > 2) using the pci_sync_* calls to copy the pages from the bounce-buffer and > back. > > This patch fixes the issue by allocating pages using the DMA API. The second > is a viable option - but it has performance drawbacks and potential > correctness > issues - think of the write cache page being bounced (SWIOTLB->TTM), the > WC is set on the TTM page and the copy from SWIOTLB not making it to the TTM > page until the page has been recycled in the pool (and used by another > application). > > The bounce buffer does not get activated often - only in cases where we have > a 32-bit capable card and we want to use a page that is allocated above the > 4GB limit. The bounce buffer offers the solution of copying the contents > of that 4GB page to an location below 4GB and then back when the operation > has been > completed (or vice-versa). This is done by using the 'pci_sync_*' calls. > Note: If you look carefully enough in the existing TTM page pool code you will > notice the GFP_DMA32 flag is used - which should guarantee that the provided > page > is under 4GB. It certainly is the case, except this gets ignored in two cases: > - If user specifies 'swiotlb=force' which bounces _every_ page. > - If user is using a Xen's PV Linux guest (which uses the SWIOTLB and the >underlaying PFN's aren't necessarily under 4GB). > > To not have this extra copying done the other option is to allocate the pages > using the DMA API so that there is not need to map the page and perform the > expensive 'pci_sync_*' calls. > > This DMA API capable TTM pool requires for this the 'struct device' to > properly call the DMA API. It also has to track the virtual and bus address of > the page being handed out in case it ends up being swapped out or > de-allocated - > to make sure it is de-allocated using the proper's 'struct device'. > > Implementation wise the code keeps two lists: one that is attached to the > 'struct device' (via the dev->dma_pools list) and a global one to be used when > the 'struct device' is unavailable (think shrinker code). The global list can > iterate over all of the 'struct device' and its associated dma_pool. The list > in dev->dma_pools can only iterate the device's dma_pool. > /[struct > device_pool]\ > /---| dev >| >/+---| dma_pool >| > /-+--\/ > \/ > |struct device| /-->[struct dma_pool for WC] device_pool]\ > | dma_pools ++ /-| dev >| > | ...|\--->[struct dma_pool for uncached]<-/--| dma_pool >| > \-+--/ / > \/ > \--/ > [Two pools associated with the device (WC and UC), and the parallel list > containing the 'struct dev' and 'struct dma_pool' entries] > > The maximum amount of dma pools a device can have is six: write-combined, > uncached, and cached; then there are the DMA32 variants which are: > write-combined dma32, uncached dma32, and cached dma32. > > Currently this code only gets activated when any variant of the SWIOTLB IOMMU > code is running (Intel without VT-d, AMD without GART, IBM Calgary and Xen PV > with PCI devices). > > Tested-by: Michel D?nzer > [v1: Using swiotlb_nr_tbl instead of swiotlb_enabled] > [v2: Major overhaul - added 'inuse_list' to seperate used from inuse and > reorder > the order of lists to get better performance.] > Signed-off-by: Konrad Rzeszutek Wilk > --- > drivers/gpu/drm/ttm/Makefile |3 + > driv
[PATCH] drm: Make the per-driver file_operations struct const
[Intel-gfx] [PATCH 1/2] Give up on edid retries when i2c tells us that bus is not there
On Mon, 17 Oct 2011 11:12:29 -0200, Eugeni Dodonov wrote: > This allows to avoid talking to a non-existent bus repeatedly until we > finally timeout. The non-existent bus is signalled by -ENXIO error, > provided by i2c_algo_bit:bit_doAddress call. > > As the advantage of such change, all the other routines which use > drm_get_edid would benefit for this timeout. > > This change should fix > https://bugs.freedesktop.org/show_bug.cgi?id=41059 and improve overall > edid detection timing by 10-30% in most cases, and by a much larger margin > in case of phantom outputs. > > Signed-off-by: Eugeni Dodonov Looks like we have reached the conclusion that this simple patch is the least likely to cause problems and easiest to fix if it does. :) Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
Proposal for a low-level Linux display framework
On Sat, 17 Sep 2011 21:25:29 +0100 Alan Cox wrote: > > Just tell the X driver to not use acceleration, and it you won't get > > any acceleration used, then you get complete stability. If a driver > > writer wants to turn off all accel in the kernel driver, it can, its > > In fact one thing we actually need really is a "dumb" KMS X server to > replace the fbdev X server that unaccel stuff depends upon and which > can't do proper mode handling, multi-head or resizing as a result. A dumb > fb generic request for a back to front copy might also be useful for > shadowfb, or at least indicators so you know what the cache behaviour is > so the X server can pick the right policy. > > > We've fixed this in KMS, we don't pass direct mappings to userspace > > that we can't tear down and refault. We only provide objects via > > handles. The only place its a problem is where we expose fbdev legacy > > emulation, since we have to fix the pages. > > Which is doable. Horrible but doable. The usb framebuffer code has to > play games like this with the virtual framebuffer in order to track > changes by faulting. > > There are still some architectural screwups however. DRM continues the > fbdev worldview that outputs, memory and accelerators are tied together > in lumps we call video cards. That isn't really true for all cases and > with capture/overlay it gets even less true. Sorry for re-opening this ancient thread; I'm catching up from the past 2 months of travel & misc. I definitely agree about the PC card centric architecture of DRM KMS (and before it, X). But we have a path out of it now, and lots of interest from vendors and developers, so I don't think it's an insurmountable problem by any means. I definitely understand Florian's worries about DRM vs fb. If nothing else, there's certainly a perception that fb is simpler and easier to get right. But really, as others have pointed out, it's solving a different set of problems than the DRM layer. The latter is actually trying to expose the features of contemporary hardware in a way that's as portable as possible. That portability comes at a cost though: the APIs we add need to get lots of review, and there's no doubt we'll need to add more as newer, weirder hardware comes along. Really, I see no reason why fb and DRM can't continue to live side by side. If a vendor really only needs the features provided by the fb layer, they're free to stick with a simple fb driver. However, I expect most vendors making phones, tablets, notebooks, etc will need and want an architecture that looks a lot like the DRM layer, with authentication for rendering clients, an command submission ioctl for acceleration, and memory management, so I expect most of the driver growth to be in DRM in the near future. And I totally agree with Dave about having a kmscon. I really wish someone would implement it so I could have my VTs spinning on a cube. -- Jesse Barnes, Intel Open Source Technology Center -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20111031/8a7fa5bb/attachment-0001.pgp>
[PATCH] drm: serialize access to list of debugfs files
On Mon, Oct 31, 2011 at 06:33:57PM +0100, Marcin Slusarz wrote: > Nouveau, when configured with debugfs, creates debugfs files for every > channel, so structure holding list of files needs to be protected from > simultaneous changes by multiple threads. > > Without this patch it's possible to hit kernel oops in > drm_debugfs_remove_files just by running a couple of xterms with > looped glxinfo. > > Signed-off-by: Marcin Slusarz And v2 also includes a nice comment in the header explaining the locking. Awesome! Reviewed-by: Daniel Vetter -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[PATCH v2] drm: add some comments to drm_wait_vblank and drm_queue_vblank_event
On Mon, Oct 31, 2011 at 01:11:57PM -0400, Ilija Hadzic wrote: > during the review of the fix for locks problems in drm_wait_vblank, > a couple of false concerns were raised about how the drm_vblank_get > and drm_vblank_put are used in this function; it turned out that the > code is correct and that it cannot be simplified > > add a few comments to explain non-obvious flows in the code, > to prevent "false alarms" in the future > > v2: incorporate comments received from Daniel Vetter > > Signed-off-by: Ilija Hadzic Reviewed-by: Daniel Vetter -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[PATCH v3] drm: do not sleep on vblank while holding a mutex
On Mon, Oct 31, 2011 at 01:10:21PM -0400, Ilija Hadzic wrote: > drm_wait_vblank must be DRM_UNLOCKED because otherwise it > will grab the drm_global_mutex and then go to sleep until the vblank > event it is waiting for. That can wreck havoc in the windowing system > because if one process issues this ioctl, it will block all other > processes for the duration of all vblanks between the current and the > one it is waiting for. In some cases it can block the entire windowing > system. > > v2: incorporate comments received from Daniel Vetter and > Michel Daenzer. > > v3: after a lengty discussion with Daniel Vetter, it was concluded > we should not worry about any locking, within drm_wait_vblank > function so this patch becomes a rather trivial removal > of drm_global_mutex from drm_wait_vblank That commit message is a bit garbage. What about ... "... it was concluded that the only think not yet protected with locks and atomic ops is the write to dev->last_vblank_wait. It's only used in a debug file in proc, and the current code already employs no correct locking: the proc file only takes dev->struct_mutex, whereas drm_wait_vblank implicitly took the drm_global_mutex. Given all this, it's not worth bothering to try to fix this at this time." I think it's important to correctly document the conclusion of this discussion, because we've worried quite a bit about correct locking. Yours, Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[PATCH v3] drm: do not sleep on vblank while holding a mutex
OK, v4 coming up and your suggestion will be copied verbatim. Hopefully that does it. This patch is probably going to become a record-holder in comment/code lines ratio ;-) -- Ilija On Mon, 31 Oct 2011, Daniel Vetter wrote: > On Mon, Oct 31, 2011 at 01:10:21PM -0400, Ilija Hadzic wrote: >> drm_wait_vblank must be DRM_UNLOCKED because otherwise it >> will grab the drm_global_mutex and then go to sleep until the vblank >> event it is waiting for. That can wreck havoc in the windowing system >> because if one process issues this ioctl, it will block all other >> processes for the duration of all vblanks between the current and the >> one it is waiting for. In some cases it can block the entire windowing >> system. >> >> v2: incorporate comments received from Daniel Vetter and >> Michel Daenzer. >> >> v3: after a lengty discussion with Daniel Vetter, it was concluded >> we should not worry about any locking, within drm_wait_vblank >> function so this patch becomes a rather trivial removal >> of drm_global_mutex from drm_wait_vblank > > That commit message is a bit garbage. What about ... > > "... it was concluded that the only think not yet protected with locks and > atomic ops is the write to dev->last_vblank_wait. It's only used in a > debug file in proc, and the current code already employs no correct > locking: the proc file only takes dev->struct_mutex, whereas > drm_wait_vblank implicitly took the drm_global_mutex. Given all this, it's > not worth bothering to try to fix this at this time." > > I think it's important to correctly document the conclusion of this > discussion, because we've worried quite a bit about correct locking. > > Yours, Daniel > -- > Daniel Vetter > Mail: daniel at ffwll.ch > Mobile: +41 (0)79 365 57 48 >
[PATCH v4] drm: do not sleep on vblank while holding a mutex
drm_wait_vblank must be DRM_UNLOCKED because otherwise it
will grab the drm_global_mutex and then go to sleep until the vblank
event it is waiting for. That can wreck havoc in the windowing system
because if one process issues this ioctl, it will block all other
processes for the duration of all vblanks between the current and the
one it is waiting for. In some cases it can block the entire windowing
system.
v2: incorporate comments received from Daniel Vetter and
Michel Daenzer.
v3/v4: after a lengty discussion with Daniel Vetter, it was concluded
that the only thing not yet protected with locks and atomic
ops is the write to dev->last_vblank_wait. It's only used in a
debug file in proc, and the current code already employs no
correct locking: the proc file only takes dev->struct_mutex,
whereas drm_wait_vblank implicitly took the drm_global_mutex.
Given all this, it's not worth bothering to try to fix
the locks at this time.
Signed-off-by: Ilija Hadzic
---
drivers/gpu/drm/drm_drv.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index dbabcb0..dc0eb0b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc_ioctl,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, 0),
+ DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
--
1.7.7
[PATCH] TTM DMA pool v2.1
On Wed, Oct 19, 2011 at 06:19:21PM -0400, Konrad Rzeszutek Wilk wrote: > [.. and this is what I said in v1 post]: > > Way back in January this patchset: > http://lists.freedesktop.org/archives/dri-devel/2011-January/006905.html > was merged in, but pieces of it had to be reverted b/c they did not > work properly under PowerPC, ARM, and when swapping out pages to disk. > > After a bit of discussion on the mailing list > http://marc.info/?i=4D769726.2030307 at shipmail.org I started working on it, > but > got waylaid by other things .. and finally I am able to post the RFC patches. > > There was a lot of discussion about it and I am not sure if I captured > everybody's thoughts - if I did not - that is _not_ intentional - it has just > been quite some time.. > > Anyhow .. the patches explore what the "lib/dmapool.c" does - which is to > have a > DMA pool that the device has associated with. I kind of married that code > along with drivers/gpu/drm/ttm/ttm_page_alloc.c to create a TTM DMA pool code. > The end result is DMA pool with extra features: can do write-combine, > uncached, > writeback (and tracks them and sets back to WB when freed); tracks "cached" > pages that don't really need to be returned to a pool; and hooks up to > the shrinker code so that the pools can be shrunk. > > If you guys think this set of patches make sense - my future plans were > 1) Get this in large crowd of testing .. and if it works for a kernel release > 2) to move a bulk of this in the lib/dmapool.c (I spoke with Matthew Wilcox > about it and he is OK as long as I don't introduce performance > regressions). > > But before I do any of that a second set of eyes taking a look at these > patches would be most welcome. > > In regards to testing, I've been running them non-stop for the last month. > (and found some issues which I've fixed up) - and been quite happy with how > they work. > > Michel (thanks!) took a spin of the patches on his PowerPC and they did not > cause any regressions (wheew). > > The patches are also located in a git tree: > Reviewed the patch series, looks good, already sent comment on one of the patch. I am on the same side of Thomas for dma stuff, lately i have been working on GPU virtual memory address space and i believe having driver allocating the ttm_tt and merging more stuff in the backend make sense, after all the backend has better knowledge on both cache preference and dma mask. So far my idea is to merge ttm_tt & ttm_backend, simplify the backend function to bind/unbind/destroy where bind is responsible to allocate or not a ttm_tt and pages that goes along with it. I will try to sketch up patches for all this in next few days. Reviewed-by: Jerome Glisse Cheers, Jerome Glisse
[PATCH v4] drm: do not sleep on vblank while holding a mutex
On Mon, Oct 31, 2011 at 05:46:18PM -0400, Ilija Hadzic wrote: > drm_wait_vblank must be DRM_UNLOCKED because otherwise it > will grab the drm_global_mutex and then go to sleep until the vblank > event it is waiting for. That can wreck havoc in the windowing system > because if one process issues this ioctl, it will block all other > processes for the duration of all vblanks between the current and the > one it is waiting for. In some cases it can block the entire windowing > system. > > v2: incorporate comments received from Daniel Vetter and > Michel Daenzer. > > v3/v4: after a lengty discussion with Daniel Vetter, it was concluded >that the only thing not yet protected with locks and atomic >ops is the write to dev->last_vblank_wait. It's only used in a >debug file in proc, and the current code already employs no >correct locking: the proc file only takes dev->struct_mutex, >whereas drm_wait_vblank implicitly took the drm_global_mutex. >Given all this, it's not worth bothering to try to fix >the locks at this time. > > Signed-off-by: Ilija Hadzic Reviewed-by: Daniel Vetter -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[PATCH] edid candidate for -next
So as we all agree on this patch now, I updated it with all the Tested-by and Reviewed-by. Dave, I think it is good for pulling into -next now. Thanks!
[PATCH] Give up on edid retries when i2c bus is not responding
This allows to avoid talking to a non-responding bus repeatedly until we
finally timeout after 15 attempts. We can do this by catching the -ENXIO
error, provided by i2c_algo_bit:bit_doAddress call.
Within the bit_doAddress we already try 3 times to get the edid data, so
if the routine tells us that bus is not responding, it is mostly pointless
to keep re-trying those attempts over and over again until we reach final
number of retries.
This change should fix https://bugs.freedesktop.org/show_bug.cgi?id=41059
and improve overall edid detection timing by 10-30% in most cases, and by
a much larger margin in case of phantom outputs (up to 30x in one worst
case).
Timing results for i915-powered machines for 'time xrandr' command:
Machine 1: from 0.840s to 0.290s
Machine 2: from 0.315s to 0.280s
Machine 3: from +/- 4s to 0.184s
Timing results for HD5770 with 'time xrandr' command:
Machine 4: from 3.210s to 1.060s
Reviewed-by: Chris Wilson
Reviewed-by: Keith Packard
Tested-by: Sean Finney
Tested-by: Soren Hansen
Tested-by: Hernando Torque
Tested-by: Mike Lothian
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41059
Signed-off-by: Eugeni Dodonov
---
drivers/gpu/drm/drm_edid.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7425e5c..1bca6d7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -265,6 +265,11 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter,
unsigned char *buf,
}
};
ret = i2c_transfer(adapter, msgs, 2);
+ if (ret == -ENXIO) {
+ DRM_DEBUG_KMS("drm: skipping non-existent adapter %s\n",
+ adapter->name);
+ break;
+ }
} while (ret != 2 && --retries);
return ret == 2 ? 0 : -1;
--
1.7.7.1
