[PATCH] DRM: bug: RADEON_DEBUGFS_MAX_{NUM_FILES => COMPONENTS}

[Michael Witten]

Date: Fri, 16 Sep 2011 20:45:30 +

The value of RADEON_DEBUGFS_MAX_NUM_FILES has been used to
specify the size of an array, each element of which looks
like this:

  struct radeon_debugfs {
  struct drm_info_list*files;
  unsignednum_files;
  };

Consequently, the number of debugfs files may be much greater
than RADEON_DEBUGFS_MAX_NUM_FILES, something that the current
code ignores:

  if ((_radeon_debugfs_count + nfiles) > RADEON_DEBUGFS_MAX_NUM_FILES) {
  DRM_ERROR("Reached maximum number of debugfs files.\n");
  DRM_ERROR("Report so we increase RADEON_DEBUGFS_MAX_NUM_FILES.\n");
  return -EINVAL;
  }

This commit fixes this mistake, and accordingly renames:

  RADEON_DEBUGFS_MAX_NUM_FILES

to:

  RADEON_DEBUGFS_MAX_COMPONENTS

Signed-off-by: Michael Witten 
---
 drivers/gpu/drm/radeon/radeon.h|2 +-
 drivers/gpu/drm/radeon/radeon_device.c |   13 -
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index c1e056b..dd7bab9 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -102,7 +102,7 @@ extern int radeon_pcie_gen2;
 #define RADEON_FENCE_JIFFIES_TIMEOUT   (HZ / 2)
 /* RADEON_IB_POOL_SIZE must be a power of 2 */
 #define RADEON_IB_POOL_SIZE16
-#define RADEON_DEBUGFS_MAX_NUM_FILES   32
+#define RADEON_DEBUGFS_MAX_COMPONENTS  32
 #define RADEONFB_CONN_LIMIT4
 #define RADEON_BIOS_NUM_SCRATCH8

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index b51e157..31b1f4b 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -981,7 +981,7 @@ struct radeon_debugfs {
struct drm_info_list*files;
unsignednum_files;
 };
-static struct radeon_debugfs _radeon_debugfs[RADEON_DEBUGFS_MAX_NUM_FILES];
+static struct radeon_debugfs _radeon_debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
 static unsigned _radeon_debugfs_count = 0;

 int radeon_debugfs_add_files(struct radeon_device *rdev,
@@ -996,14 +996,17 @@ int radeon_debugfs_add_files(struct radeon_device *rdev,
return 0;
}
}
-   if ((_radeon_debugfs_count + nfiles) > RADEON_DEBUGFS_MAX_NUM_FILES) {
-   DRM_ERROR("Reached maximum number of debugfs files.\n");
-   DRM_ERROR("Report so we increase 
RADEON_DEBUGFS_MAX_NUM_FILES.\n");
+
+   i = _radeon_debugfs_count + 1;
+   if (i > RADEON_DEBUGFS_MAX_COMPONENTS) {
+   DRM_ERROR("Reached maximum number of debugfs components.\n");
+   DRM_ERROR("Report so we increase "
+ "RADEON_DEBUGFS_MAX_COMPONENTS.\n");
return -EINVAL;
}
_radeon_debugfs[_radeon_debugfs_count].files = files;
_radeon_debugfs[_radeon_debugfs_count].num_files = nfiles;
-   _radeon_debugfs_count++;
+   _radeon_debugfs_count = i;
 #if defined(CONFIG_DEBUG_FS)
drm_debugfs_create_files(files, nfiles,
 rdev->ddev->control->debugfs_root,
-- 
1.7.6.409.ge7a85

[PATCH 2/2] Check if the bus is valid prior to discovering edid.

[Eugeni Dodonov]

This adds a new function intel_drm_get_valid_edid, which is used instead
of drm_get_edid within the i915 driver.

It does a similar check to the one in previous patch, but it is limited to
i915 driver.

The check is similar to the first part of EDID discovery performed by the
drm_do_probe_ddc_edid. In case the i2c_transfer fails with the -ENXIO
result, we know that the i2c_algo_bit was unable to locate the hardware,
so we give up on further edid discovery.

They should also fix https://bugs.freedesktop.org/show_bug.cgi?id=41059

v2: change printk level to KERN_DEBUG to avoid filling up dmesg

Signed-off-by: Eugeni Dodonov 
---
 drivers/gpu/drm/i915/intel_dp.c|4 ++--
 drivers/gpu/drm/i915/intel_drv.h   |2 ++
 drivers/gpu/drm/i915/intel_hdmi.c  |4 ++--
 drivers/gpu/drm/i915/intel_i2c.c   |   33 +
 drivers/gpu/drm/i915/intel_lvds.c  |2 +-
 drivers/gpu/drm/i915/intel_modes.c |2 +-
 drivers/gpu/drm/i915/intel_sdvo.c  |4 ++--
 7 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44fef5e..dd0d8b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1715,7 +1715,7 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
if (intel_dp->force_audio) {
intel_dp->has_audio = intel_dp->force_audio > 0;
} else {
-   edid = drm_get_edid(connector, _dp->adapter);
+   edid = intel_drm_get_valid_edid(connector, _dp->adapter);
if (edid) {
intel_dp->has_audio = drm_detect_monitor_audio(edid);
connector->display_info.raw_edid = NULL;
@@ -1772,7 +1772,7 @@ intel_dp_detect_audio(struct drm_connector *connector)
struct edid *edid;
bool has_audio = false;

-   edid = drm_get_edid(connector, _dp->adapter);
+   edid = intel_drm_get_valid_edid(connector, _dp->adapter);
if (edid) {
has_audio = drm_detect_monitor_audio(edid);

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fe1099d..d80a9b0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -264,6 +264,8 @@ struct intel_fbc_work {
int interval;
 };

+struct edid * intel_drm_get_valid_edid(struct drm_connector *connector,
+   struct i2c_adapter *adapter);
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 extern bool intel_ddc_probe(struct intel_encoder *intel_encoder, int ddc_bus);

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 226ba83..714f2fb 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -329,7 +329,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
force)

intel_hdmi->has_hdmi_sink = false;
intel_hdmi->has_audio = false;
-   edid = drm_get_edid(connector,
+   edid = intel_drm_get_valid_edid(connector,
_priv->gmbus[intel_hdmi->ddc_bus].adapter);

if (edid) {
@@ -371,7 +371,7 @@ intel_hdmi_detect_audio(struct drm_connector *connector)
struct edid *edid;
bool has_audio = false;

-   edid = drm_get_edid(connector,
+   edid = intel_drm_get_valid_edid(connector,
_priv->gmbus[intel_hdmi->ddc_bus].adapter);
if (edid) {
if (edid->input & DRM_EDID_INPUT_DIGITAL)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index d98cee6..77115b9 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -470,3 +470,36 @@ void intel_teardown_gmbus(struct drm_device *dev)
kfree(dev_priv->gmbus);
dev_priv->gmbus = NULL;
 }
+
+/**
+ * intel_drm_get_valid_edid - gets edid from existent adapters only
+ * @connector: DRM connector device to use
+ * @adapter: i2c adapter
+ *
+ * Verifies if the i2c adapter is responding to our queries before
+ * attempting to do proper communication with it. If it does,
+ * retreive the EDID with help of drm_get_edid
+ */
+struct edid *
+intel_drm_get_valid_edid(struct drm_connector *connector,
+   struct i2c_adapter *adapter)
+{
+   unsigned char out;
+   int ret;
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = 0x50,
+   .flags  = 0,
+   .len= 1,
+   .buf= ,
+   }
+   };
+   /* Let's try one edid transfer */
+   ret = i2c_transfer(adapter, msgs, 1);
+   if (ret == -ENXIO) {
+   printk(KERN_DEBUG "i915: adapter %s not responding: %d\n",
+   adapter->name, ret);
+   return NULL;
+   }
+   return drm_get_edid(connector, adapter);
+}
diff --git a/drivers/gpu/drm/i915/intel_lvds.c 

[PATCH 1/2] Give up on edid retries when i2c tells us that bus is not there

[Eugeni Dodonov]

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.

As the disadvantage comes the fact that I only tested it on Intel cards,
so I am not sure whether it would work on nouveau and radeon.

This change should potentially fix
https://bugs.freedesktop.org/show_bug.cgi?id=41059

v2: change printk level to KERN_DEBUG to avoid filling up dmesg

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..5ed34f2 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) {
+   printk(KERN_DEBUG "drm: skipping non-existent adapter 
%s\n",
+   adapter->name);
+   break;
+   }
} while (ret != 2 && --retries);

return ret == 2 ? 0 : -1;
-- 
1.7.6.4

[PATCH 0/2] RFC: Potential improvements in edid detection timings (v2)

[Eugeni Dodonov]

(Resending with small improvements - also sending to dri-devel for feedback).

This is the the forth iteration of potential fixes for slow edid detection
issues over non-existent outputs
(https://bugs.freedesktop.org/show_bug.cgi?id=41059) - the previous versions
were posted to the bug and were used mostly for debugging the problem.

The problem I attempted to fix here is that some systems would take a very
long time trying to locate all the possible video outputs - in in cases
where such outputs were bogus, this lead to timing out after all the
possible delays were done.

After investigation, I came to think on two different ways to fix the issue:
 - The first patch does a one-line fix in drm_edid.c. I added a check for the
   return value of i2c_transfer - and, if it is -ENXIO, we give up on further
   attempts as the bus is not there. A drawback to this approach is that it
   affects all the devices out there which use drm_get_edid.  From my testing,
   the -ENXIO gave no false positives, but I haven't tested it on non-Intel
   cards. So I'd like to hear what other drm developers think about that.

 - The second patch does a similar procedure within the i915 driver, in case
   the proposed change to drm_edid.c won't be adequate for other drivers. It
   adds a new function - intel_drm_get_valid_edid - which attempts to do a
   simple i2c transfer over the bus prior to calling drm_get_edid. In case such
   transfer fails with -ENXIO, it is a signal that the bus is not there, so we
   shouldn't waste any time trying to communicate with it further.

Note that those patches provide lots of dmesg pollution - for the final
version those printk'ss should probably be removed, but I left them
intentionally with KERN_DEBUG in order to see when the adapters come and go
during the debugging and testing.

According to testing performed at
https://bugs.freedesktop.org/show_bug.cgi?id=41059, the following results
were obtained with those patches:

System1:
 (testing all possible outputs)
 ubuntu 3.0.0-12.19:   840ms
 3.0.0-12.19 +  drm patch: 290ms
 3.0.0-12.19 + i915 patch: 290ms
 (manually ignoring phantom outputs)
 ubuntu 3.0.0-12.19:   690ms
 3.0.0-12.19 +  drm patch: 140ms
 3.0.0-12.19 + i915 patch: 140ms

System2:
 (testing all possible outputs)
 ubuntu 3.0.0-12.19:   315ms
 3.0.0-12.19 +  drm patch: 280ms
 3.0.0-12.19 + i915 patch: 280ms
 (manually ignoring phantom outputs)
 ubuntu 3.0.0-12.19:   175ms
 3.0.0-12.19 +  drm patch: 140ms
 3.0.0-12.19 + i915 patch: 140ms



Eugeni Dodonov (2):
  Give up on edid retries when i2c tells us that bus is not there
  Check if the bus is valid prior to discovering edid.

 drivers/gpu/drm/drm_edid.c |5 +
 drivers/gpu/drm/i915/intel_dp.c|4 ++--
 drivers/gpu/drm/i915/intel_drv.h   |2 ++
 drivers/gpu/drm/i915/intel_hdmi.c  |4 ++--
 drivers/gpu/drm/i915/intel_i2c.c   |   33 +
 drivers/gpu/drm/i915/intel_lvds.c  |2 +-
 drivers/gpu/drm/i915/intel_modes.c |2 +-
 drivers/gpu/drm/i915/intel_sdvo.c  |4 ++--
 8 files changed, 48 insertions(+), 8 deletions(-)

-- 
1.7.6.3

[Bug 41579] New: R300 Segfaults when using mupen64plus

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

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

   Summary: R300 Segfaults when using mupen64plus
   Product: Mesa
   Version: 7.11
  Platform: x86 (IA32)
OS/Version: Linux (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: Drivers/DRI/r300
AssignedTo: dri-devel at lists.freedesktop.org
ReportedBy: krthurow at gmail.com


Created an attachment (id=52103)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52103)
Backtrace from r300 segfault when running mupen64plus.

When starting up the emulator mupen64plus with a ROM, it crashes before loading
the ROM, showing a segfault. Video card is a Mobility Radeon X1700.

Using gdb I traced the segfault back to r300_dri.so
The issues seems to be at line 864 of src/gallium/drivers/r300/r300_emit.c
'buf' is getting set to 0 instead of a proper address.

My backtrace is attached.

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

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Thomas Hellstrom]

On 10/07/2011 03:38 PM, Jerome Glisse wrote:
> On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom  
> wrote:
>
>> On 10/07/2011 12:42 AM, Marek Ol??k wrote:
>>  
>>> On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstrom
>>>   wrote:
>>>
>>>
 In any case, I'm not saying fences is the best way to flush but since the
 bo
 code assumes that signaling a sync object means "make the buffer contents
 available for CPU read / write", it's usually a good way to do it;
 there's
 even a sync_obj_flush() method that gets called when a potential flush
 needs
 to happen.

  
>>> I don't think we use it like that. To my knowledge, the purpose of the
>>> sync obj (to Radeon Gallium drivers at least) is to be able to wait
>>> for the last use of a buffer. Whether the contents can or cannot be
>>> available to the CPU is totally irrelevant.
>>>
>>> Currently (and it's a very important performance optimization),
>>> buffers stay mapped and available for CPU read/write during their
>>> first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
>>> on buffer destruction. We only call bo_wait when we want to wait for
>>> the GPU until it's done with the buffer (we don't always want that,
>>> sometimes we want to use the unsychronized flag). Otherwise the
>>> contents of buffers are available at *any time*.
>>>
>>> We could probably implement bo_wait privately in the kernel driver and
>>> not use ttm_bo_wait. I preferred code sharing though.
>>>
>>> Textures (especially the tiled ones) are never mapped directly and a
>>> temporary staging resource is used instead, so we don't actually
>>> pollute address space that much. (in case you would have such a
>>> remark) We will use staging resources for buffers too, but it's really
>>> the last resort to avoid waiting when direct access can't be used.
>>>
>>>
>>>
>>>
>> 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
>> difference between those two? I think we should remove the
>> write_sync_obj
>> bo
>> member.
>>
>>
>>  
> Okay, but I think we should remove sync_obj instead, and keep write
> and read sync objs. In the case of READWRITE usage, read_sync_obj
> would be equal to write_sync_obj.
>
>
>
>
 Sure, I'm fine with that.

 One other thing, though, that makes me a little puzzled:

 Let's assume you don't allow readers and writers at the same time, which
 is
 my perception of how read- and write fences should work; you either have
 a
 list of read fences or a single write fence (in the same way a read-write
 lock works).

 Now, if you only allow a single read fence, like in this patch. That
 implies
 that you can only have either a single read fence or a single write fence
 at
 any one time. We'd only need a single fence pointer on the bo, and
 sync_obj_arg would tell us whether to signal the fence for read or for
 write
 (assuming that sync_obj_arg was set up to indicate read / write at
 validation time), then the patch really isn't necessary at all, as it
 only
 allows a single read fence?

 Or is it that you want to allow read- and write fences co-existing? In
 that
 case, what's the use case?

  
>>> There are lots of read-write use cases which don't need any barriers
>>> or flushing. The obvious ones are color blending and depth-stencil
>>> buffering. The OpenGL application is also allowed to use a subrange of
>>> a buffer as a vertex buffer (read-only) and another disjoint subrange
>>> of the same buffer for transform feedback (write-only), which kinda
>>> makes me think about whether we should track subranges instead of
>>> treating a whole buffer as "busy". It gets even more funky with
>>> ARB_shader_image_load_store, which supports atomic read-modify-write
>>> operations on textures, not to mention atomic memory operations in
>>> compute shaders (wait, isn't that also exposed in GL as
>>> GL_ARB_shader_atomic_counters?).
>>>
>>> I was thinking whether the two sync objs should be "read" and
>>> "readwrite", or "read" and "write". I chose the latter, because it's
>>> more fine-grained and we have to keep at least two of them around
>>> anyway.
>>>
>>> So now that you know what we use sync objs for, what are your ideas on
>>> re-implementing that patch in a way that is okay with you? Besides
>>> removing the third sync objs of course.
>>>
>>> Marek
>>>
>>>
>> OK. First I think we need to make a distinction: bo sync objects vs driver
>> fences. The bo sync obj api is there to strictly provide functionality that
>> the ttm bo subsystem is using, and that follows a simple set of rules:
>>
>> 1) the bo subsystem does never assume sync objects are ordered. That means
>> the bo subsystem needs to wait on a sync object before removing it from a
>> buffer. Any other assumption 

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Thomas Hellstrom]

On 10/07/2011 03:24 PM, Alex Deucher wrote:
> On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom  
> wrote:
>
>> On 10/07/2011 12:42 AM, Marek Ol??k wrote:
>>  
>>> On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstrom
>>>   wrote:
>>>
>>>
 In any case, I'm not saying fences is the best way to flush but since the
 bo
 code assumes that signaling a sync object means "make the buffer contents
 available for CPU read / write", it's usually a good way to do it;
 there's
 even a sync_obj_flush() method that gets called when a potential flush
 needs
 to happen.

  
>>> I don't think we use it like that. To my knowledge, the purpose of the
>>> sync obj (to Radeon Gallium drivers at least) is to be able to wait
>>> for the last use of a buffer. Whether the contents can or cannot be
>>> available to the CPU is totally irrelevant.
>>>
>>> Currently (and it's a very important performance optimization),
>>> buffers stay mapped and available for CPU read/write during their
>>> first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
>>> on buffer destruction. We only call bo_wait when we want to wait for
>>> the GPU until it's done with the buffer (we don't always want that,
>>> sometimes we want to use the unsychronized flag). Otherwise the
>>> contents of buffers are available at *any time*.
>>>
>>> We could probably implement bo_wait privately in the kernel driver and
>>> not use ttm_bo_wait. I preferred code sharing though.
>>>
>>> Textures (especially the tiled ones) are never mapped directly and a
>>> temporary staging resource is used instead, so we don't actually
>>> pollute address space that much. (in case you would have such a
>>> remark) We will use staging resources for buffers too, but it's really
>>> the last resort to avoid waiting when direct access can't be used.
>>>
>>>
>>>
>>>
>> 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
>> difference between those two? I think we should remove the
>> write_sync_obj
>> bo
>> member.
>>
>>
>>  
> Okay, but I think we should remove sync_obj instead, and keep write
> and read sync objs. In the case of READWRITE usage, read_sync_obj
> would be equal to write_sync_obj.
>
>
>
>
 Sure, I'm fine with that.

 One other thing, though, that makes me a little puzzled:

 Let's assume you don't allow readers and writers at the same time, which
 is
 my perception of how read- and write fences should work; you either have
 a
 list of read fences or a single write fence (in the same way a read-write
 lock works).

 Now, if you only allow a single read fence, like in this patch. That
 implies
 that you can only have either a single read fence or a single write fence
 at
 any one time. We'd only need a single fence pointer on the bo, and
 sync_obj_arg would tell us whether to signal the fence for read or for
 write
 (assuming that sync_obj_arg was set up to indicate read / write at
 validation time), then the patch really isn't necessary at all, as it
 only
 allows a single read fence?

 Or is it that you want to allow read- and write fences co-existing? In
 that
 case, what's the use case?

  
>>> There are lots of read-write use cases which don't need any barriers
>>> or flushing. The obvious ones are color blending and depth-stencil
>>> buffering. The OpenGL application is also allowed to use a subrange of
>>> a buffer as a vertex buffer (read-only) and another disjoint subrange
>>> of the same buffer for transform feedback (write-only), which kinda
>>> makes me think about whether we should track subranges instead of
>>> treating a whole buffer as "busy". It gets even more funky with
>>> ARB_shader_image_load_store, which supports atomic read-modify-write
>>> operations on textures, not to mention atomic memory operations in
>>> compute shaders (wait, isn't that also exposed in GL as
>>> GL_ARB_shader_atomic_counters?).
>>>
>>> I was thinking whether the two sync objs should be "read" and
>>> "readwrite", or "read" and "write". I chose the latter, because it's
>>> more fine-grained and we have to keep at least two of them around
>>> anyway.
>>>
>>> So now that you know what we use sync objs for, what are your ideas on
>>> re-implementing that patch in a way that is okay with you? Besides
>>> removing the third sync objs of course.
>>>
>>> Marek
>>>
>>>
>> OK. First I think we need to make a distinction: bo sync objects vs driver
>> fences. The bo sync obj api is there to strictly provide functionality that
>> the ttm bo subsystem is using, and that follows a simple set of rules:
>>
>> 1) the bo subsystem does never assume sync objects are ordered. That means
>> the bo subsystem needs to wait on a sync object before removing it from a
>> buffer. Any other assumption 

[PATCH 2/2] vmwgfx: Don't use virtual coords when using screen objects

[Thomas Hellstrom]

From: Jakob Bornecrantz 

Signed-off-by: Jakob Bornecrantz 
Signed-off-by: Thomas Hellstrom 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |  272 +++
 1 files changed, 215 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index fc62c87..2421d0c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -358,49 +358,109 @@ static int do_surface_dirty_sou(struct vmw_private 
*dev_priv,
struct drm_clip_rect *clips,
unsigned num_clips, int inc)
 {
-   int left = clips->x2, right = clips->x1;
-   int top = clips->y2, bottom = clips->y1;
+   struct drm_clip_rect *clips_ptr;
+   struct vmw_display_unit *units[VMWGFX_NUM_DISPLAY_UNITS];
+   struct drm_crtc *crtc;
size_t fifo_size;
-   int i, ret;
+   int i, num_units;
+   int ret = 0; /* silence warning */
+   int left, right, top, bottom;

struct {
SVGA3dCmdHeader header;
SVGA3dCmdBlitSurfaceToScreen body;
} *cmd;
+   SVGASignedRect *blits;


-   fifo_size = sizeof(*cmd);
+   num_units = 0;
+   list_for_each_entry(crtc, _priv->dev->mode_config.crtc_list,
+   head) {
+   if (crtc->fb != >base)
+   continue;
+   units[num_units++] = vmw_crtc_to_du(crtc);
+   }
+
+   BUG_ON(surf == NULL);
+   BUG_ON(!clips || !num_clips);
+
+   fifo_size = sizeof(*cmd) + sizeof(SVGASignedRect) * num_clips;
cmd = kzalloc(fifo_size, GFP_KERNEL);
if (unlikely(cmd == NULL)) {
DRM_ERROR("Temporary fifo memory alloc failed.\n");
return -ENOMEM;
}

-   cmd->header.id = cpu_to_le32(SVGA_3D_CMD_BLIT_SURFACE_TO_SCREEN);
-   cmd->header.size = cpu_to_le32(sizeof(cmd->body));
-
-   cmd->body.srcImage.sid = cpu_to_le32(framebuffer->user_handle);
-   cmd->body.destScreenId = SVGA_ID_INVALID; /* virtual coords */
-
-   for (i = 0; i < num_clips; i++, clips += inc) {
-   left = min_t(int, left, (int)clips->x1);
-   right = max_t(int, right, (int)clips->x2);
-   top = min_t(int, top, (int)clips->y1);
-   bottom = max_t(int, bottom, (int)clips->y2);
+   left = clips->x1;
+   right = clips->x2;
+   top = clips->y1;
+   bottom = clips->y2;
+
+   clips_ptr = clips;
+   for (i = 1; i < num_clips; i++, clips_ptr += inc) {
+   left = min_t(int, left, (int)clips_ptr->x1);
+   right = max_t(int, right, (int)clips_ptr->x2);
+   top = min_t(int, top, (int)clips_ptr->y1);
+   bottom = max_t(int, bottom, (int)clips_ptr->y2);
}

+   /* only need to do this once */
+   memset(cmd, 0, fifo_size);
+   cmd->header.id = cpu_to_le32(SVGA_3D_CMD_BLIT_SURFACE_TO_SCREEN);
+   cmd->header.size = cpu_to_le32(fifo_size - sizeof(cmd->header));
+
cmd->body.srcRect.left = left;
cmd->body.srcRect.right = right;
cmd->body.srcRect.top = top;
cmd->body.srcRect.bottom = bottom;

-   cmd->body.destRect.left = left;
-   cmd->body.destRect.right = right;
-   cmd->body.destRect.top = top;
-   cmd->body.destRect.bottom = bottom;
+   clips_ptr = clips;
+   blits = (SVGASignedRect *)[1];
+   for (i = 0; i < num_clips; i++, clips_ptr += inc) {
+   blits[i].left   = clips_ptr->x1 - left;
+   blits[i].right  = clips_ptr->x2 - left;
+   blits[i].top= clips_ptr->y1 - top;
+   blits[i].bottom = clips_ptr->y2 - top;
+   }
+
+   /* do per unit writing, reuse fifo for each */
+   for (i = 0; i < num_units; i++) {
+   struct vmw_display_unit *unit = units[i];
+   int clip_x1 = left - unit->crtc.x;
+   int clip_y1 = top - unit->crtc.y;
+   int clip_x2 = right - unit->crtc.x;
+   int clip_y2 = bottom - unit->crtc.y;
+
+   /* skip any crtcs that misses the clip region */
+   if (clip_x1 >= unit->crtc.mode.hdisplay ||
+   clip_y1 >= unit->crtc.mode.vdisplay ||
+   clip_x2 <= 0 || clip_y2 <= 0)
+   continue;
+
+   /* need to reset sid as it is changed by execbuf */
+   cmd->body.srcImage.sid = cpu_to_le32(framebuffer->user_handle);
+
+   cmd->body.destScreenId = unit->unit;
+
+   /*
+* The blit command is a lot more resilient then the
+* readback command when it comes to clip rects. So its
+* okay to go out of bounds.
+*/
+
+   cmd->body.destRect.left = clip_x1;
+   cmd->body.destRect.right = clip_x2;
+   cmd->body.destRect.top = clip_y1;

[PATCH 1/2] vmwgfx: Implement memory accounting for resources

[Thomas Hellstrom]

Contexts, surfaces and streams allocate persistent kernel memory as the
direct result of user-space requests. Make sure this memory is
accounted as graphics memory, to avoid DOS vulnerabilities.

Also take the TTM read lock around resource creation to block
switched-out dri clients from allocating resources.

Signed-off-by: Thomas Hellstrom 
Reviewed-by: Jakob Bornecrantz 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c |  172 +-
 1 files changed, 146 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 93a68a6..c7cff3d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -39,6 +39,7 @@ struct vmw_user_context {
 struct vmw_user_surface {
struct ttm_base_object base;
struct vmw_surface srf;
+   uint32_t size;
 };

 struct vmw_user_dma_buffer {
@@ -67,6 +68,11 @@ struct vmw_surface_offset {
uint32_t bo_offset;
 };

+
+static uint64_t vmw_user_context_size;
+static uint64_t vmw_user_surface_size;
+static uint64_t vmw_user_stream_size;
+
 static inline struct vmw_dma_buffer *
 vmw_dma_buffer(struct ttm_buffer_object *bo)
 {
@@ -343,8 +349,11 @@ static void vmw_user_context_free(struct vmw_resource *res)
 {
struct vmw_user_context *ctx =
container_of(res, struct vmw_user_context, res);
+   struct vmw_private *dev_priv = res->dev_priv;

kfree(ctx);
+   ttm_mem_global_free(vmw_mem_glob(dev_priv),
+   vmw_user_context_size);
 }

 /**
@@ -398,23 +407,56 @@ int vmw_context_define_ioctl(struct drm_device *dev, void 
*data,
 struct drm_file *file_priv)
 {
struct vmw_private *dev_priv = vmw_priv(dev);
-   struct vmw_user_context *ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+   struct vmw_user_context *ctx;
struct vmw_resource *res;
struct vmw_resource *tmp;
struct drm_vmw_context_arg *arg = (struct drm_vmw_context_arg *)data;
struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
+   struct vmw_master *vmaster = vmw_master(file_priv->master);
int ret;

-   if (unlikely(ctx == NULL))
-   return -ENOMEM;
+
+   /*
+* Approximate idr memory usage with 128 bytes. It will be limited
+* by maximum number_of contexts anyway.
+*/
+
+   if (unlikely(vmw_user_context_size == 0))
+   vmw_user_context_size = ttm_round_pot(sizeof(*ctx)) + 128;
+
+   ret = ttm_read_lock(>lock, true);
+   if (unlikely(ret != 0))
+   return ret;
+
+   ret = ttm_mem_global_alloc(vmw_mem_glob(dev_priv),
+  vmw_user_context_size,
+  false, true);
+   if (unlikely(ret != 0)) {
+   if (ret != -ERESTARTSYS)
+   DRM_ERROR("Out of graphics memory for context"
+ " creation.\n");
+   goto out_unlock;
+   }
+
+   ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+   if (unlikely(ctx == NULL)) {
+   ttm_mem_global_free(vmw_mem_glob(dev_priv),
+   vmw_user_context_size);
+   ret = -ENOMEM;
+   goto out_unlock;
+   }

res = >res;
ctx->base.shareable = false;
ctx->base.tfile = NULL;

+   /*
+* From here on, the destructor takes over resource freeing.
+*/
+
ret = vmw_context_init(dev_priv, res, vmw_user_context_free);
if (unlikely(ret != 0))
-   return ret;
+   goto out_unlock;

tmp = vmw_resource_reference(>res);
ret = ttm_base_object_init(tfile, >base, false, VMW_RES_CONTEXT,
@@ -428,6 +470,8 @@ int vmw_context_define_ioctl(struct drm_device *dev, void 
*data,
arg->cid = res->id;
 out_err:
vmw_resource_unreference();
+out_unlock:
+   ttm_read_unlock(>lock);
return ret;

 }
@@ -1095,6 +1139,8 @@ static void vmw_user_surface_free(struct vmw_resource 
*res)
struct vmw_surface *srf = container_of(res, struct vmw_surface, res);
struct vmw_user_surface *user_srf =
container_of(srf, struct vmw_user_surface, srf);
+   struct vmw_private *dev_priv = srf->res.dev_priv;
+   uint32_t size = user_srf->size;

if (srf->backup)
ttm_bo_unref(>backup);
@@ -1102,6 +1148,7 @@ static void vmw_user_surface_free(struct vmw_resource 
*res)
kfree(srf->sizes);
kfree(srf->snooper.image);
kfree(user_srf);
+   ttm_mem_global_free(vmw_mem_glob(dev_priv), size);
 }

 /**
@@ -1226,9 +1273,45 @@ int vmw_surface_define_ioctl(struct drm_device *dev, 
void *data,
struct vmw_surface_offset *cur_offset;
uint32_t stride_bpp;
uint32_t bpp;
+   uint32_t num_sizes;
+   uint32_t size;
+   struct vmw_master *vmaster = 

[PATCH -next 0/2] more vmwgfx updates

[Thomas Hellstrom]

Two fixes on top of previous patches.
The first removes a DOS vulnerabilty.
The second one implements some screen object fixes.

[PATCH 2/2] drm/radeon/kms: handle !force case in connector detect more gracefully

[alexdeuc...@gmail.com]

From: Alex Deucher 

When force == false, we don't do load detection in the connector
detect functions.  Unforunately, we also return the previous
connector state so we never get disconnect events for DVI-I, DVI-A,
or VGA.  Save whether we detected the monitor via load detection
previously and use that to determine whether we return the previous
state or not.

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

Signed-off-by: Alex Deucher 
Cc: stable at kernel.org
---
 drivers/gpu/drm/radeon/radeon_connectors.c |   23 ---
 drivers/gpu/drm/radeon/radeon_mode.h   |1 +
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 103eb1b..dec6cbe 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -724,6 +724,7 @@ radeon_vga_detect(struct drm_connector *connector, bool 
force)
dret = radeon_ddc_probe(radeon_connector,

radeon_connector->requires_extended_probe);
if (dret) {
+   radeon_connector->detected_by_load = false;
if (radeon_connector->edid) {
kfree(radeon_connector->edid);
radeon_connector->edid = NULL;
@@ -750,12 +751,21 @@ radeon_vga_detect(struct drm_connector *connector, bool 
force)
} else {

/* if we aren't forcing don't do destructive polling */
-   if (!force)
-   return connector->status;
+   if (!force) {
+   /* only return the previous status if we last
+* detected a monitor via load.
+*/
+   if (radeon_connector->detected_by_load)
+   return connector->status;
+   else
+   return ret;
+   }

if (radeon_connector->dac_load_detect && encoder) {
encoder_funcs = encoder->helper_private;
ret = encoder_funcs->detect(encoder, connector);
+   if (ret == connector_status_connected)
+   radeon_connector->detected_by_load = true;
}
}

@@ -897,6 +907,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool 
force)
dret = radeon_ddc_probe(radeon_connector,

radeon_connector->requires_extended_probe);
if (dret) {
+   radeon_connector->detected_by_load = false;
if (radeon_connector->edid) {
kfree(radeon_connector->edid);
radeon_connector->edid = NULL;
@@ -964,8 +975,13 @@ radeon_dvi_detect(struct drm_connector *connector, bool 
force)
(connector->connector_type == DRM_MODE_CONNECTOR_HDMIA))
goto out;

+   /* if we aren't forcing don't do destructive polling */
if (!force) {
-   ret = connector->status;
+   /* only return the previous status if we last
+* detected a monitor via load.
+*/
+   if (radeon_connector->detected_by_load)
+   ret = connector->status;
goto out;
}

@@ -989,6 +1005,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool 
force)
ret = encoder_funcs->detect(encoder, 
connector);
if (ret == connector_status_connected) {
radeon_connector->use_digital = 
false;
+   
radeon_connector->detected_by_load = true;
}
}
break;
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h 
b/drivers/gpu/drm/radeon/radeon_mode.h
index 68820f5..ed0178f 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -447,6 +447,7 @@ struct radeon_connector {
struct edid *edid;
void *con_priv;
bool dac_load_detect;
+   bool detected_by_load; /* if the connection status was determined by 
load */
uint16_t connector_object_id;
struct radeon_hpd hpd;
struct radeon_router router;
-- 
1.7.1.1

[PATCH 1/2] drm/radeon/kms: bail early in dvi_detect for digital only connectors

[alexdeuc...@gmail.com]

From: Alex Deucher 

DVI-D and HDMI-A are digital only, so there's no need to
attempt analog load detect.  Also, skip bail before the
!force check, or we fail to get a disconnect events.
The next patches in the series attempt to fix disconnect
events for connectors with analog support (DVI-I, HDMI-B,
DVI-A).

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

Signed-off-by: Alex Deucher 
Cc: stable at kernel.org
---
 drivers/gpu/drm/radeon/radeon_connectors.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 449c3d8..103eb1b 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -959,6 +959,11 @@ radeon_dvi_detect(struct drm_connector *connector, bool 
force)
if ((ret == connector_status_connected) && 
(radeon_connector->use_digital == true))
goto out;

+   /* DVI-D and HDMI-A are digital only */
+   if ((connector->connector_type == DRM_MODE_CONNECTOR_DVID) ||
+   (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA))
+   goto out;
+
if (!force) {
ret = connector->status;
goto out;
-- 
1.7.1.1

[Bug 41569] [r600 KMS] Asus A53T A4-3400

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

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

Alex Deucher  changed:

   What|Removed |Added

  Attachment #52097|application/octet-stream|text/plain
  mime type||
  Attachment #52097|0   |1
   is patch||

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

[Bug 41569] [r600 KMS] Asus A53T A4-3400

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

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

Alex Deucher  changed:

   What|Removed |Added

  Attachment #52096|text/x-log  |text/plain
  mime type||
  Attachment #52096|0   |1
   is patch||

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

[Bug 41569] [r600 KMS] Asus A53T A4-3400

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

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

--- Comment #3 from Robert Nelson  2011-10-07 
11:26:21 PDT ---
Created an attachment (id=52097)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52097)
lspci

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

[Bug 41569] [r600 KMS] Asus A53T A4-3400

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

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

Alex Deucher  changed:

   What|Removed |Added

  Attachment #52095|application/octet-stream|text/plain
  mime type||
  Attachment #52095|0   |1
   is patch||

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

[Bug 41569] [r600 KMS] Asus A53T A4-3400

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

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

--- Comment #2 from Robert Nelson  2011-10-07 
11:26:04 PDT ---
Created an attachment (id=52096)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52096)
Xorg log

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

[Bug 41569] [r600 KMS] Asus A53T A4-3400

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

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

--- Comment #1 from Robert Nelson  2011-10-07 
11:25:48 PDT ---
Created an attachment (id=52095)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52095)
dmesg

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

[Bug 41569] [r600 KMS] Asus A53T A4-3400

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

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

Robert Nelson  changed:

   What|Removed |Added

Summary|[r600 KMS] Asus A35T|[r600 KMS] Asus A53T
   |A4-3400 |A4-3400

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

[Bug 41569] New: [r600 KMS] Asus A35T A4-3400

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

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

   Summary: [r600 KMS] Asus A35T A4-3400
   Product: DRI
   Version: XOrg CVS
  Platform: x86-64 (AMD64)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/Radeon
AssignedTo: dri-devel at lists.freedesktop.org
ReportedBy: robertcnelson at gmail.com


This laptop, has only one standard video connector (VGA) along with the lcd
screen.  With 3.1-rc9+ plus airlied's drm-next on bootup bios init the screen,
kernel boots in text mode fine, but when KMS takes over the screen goes blank.
(the external VGA connector also goes blank). (running latest git of
xorg-ati/mesa)

interesting from dmesg;

[   32.534186] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid,
remainder is 104
[   32.534191] Raw EDID:
[   32.534194]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534196]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534198]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534199]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 70
[   32.534201]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534203]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534205]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534207]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.589038] [drm:radeon_dp_i2c_aux_ch] *ERROR* aux_ch invalid native reply
0x70

voodoo at a53t:~$ ls -lh /lib/firmware/radeon/TURKS_*
-rw-r--r-- 1 root root  24K 2011-10-06 22:26 /lib/firmware/radeon/TURKS_mc.bin
-rw-r--r-- 1 root root 5.4K 2011-10-06 22:26 /lib/firmware/radeon/TURKS_me.bin
-rw-r--r-- 1 root root 4.4K 2011-10-06 22:26 /lib/firmware/radeon/TURKS_pfp.bin

Regards,

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

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

--- Comment #8 from Alex Deucher  2011-10-07 11:16:14 PDT 
---
(In reply to comment #6)
> Results:
> 
> DVI-D hotplug works fine. There is a delay on the first DVI-D hotplug after 
> the
> connector has been used for VGA, but I get an event within 10 seconds.

good.

> 
> No events when I plug in the adapter with no load and wait 20 seconds.
> 

As expected.

> No events when I plug in the adapter with load but not DDC and wait 20 
> seconds.
> 

There's no way to get events for this case without doing load detection in the
hotplug polling.  We don't currently support that case as load detection has
the potential to cause flicker on connectors that share resources.  If you need
it, you can remove the !force checks in the connector detect functions, but you
may get flicker on other connectors if they share resources.

> Events on connect only when I plug in the adapter with DDC.

As expected.

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

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

Alex Deucher  changed:

   What|Removed |Added

 CC||qjn at gmx.de

--- Comment #7 from Alex Deucher  2011-10-07 11:14:49 PDT 
---
*** Bug 40252 has been marked as a duplicate of this bug. ***

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

[Bug 40252] No udev event for disconnecting VGA/HDMI cable

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

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

Alex Deucher  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||DUPLICATE

--- Comment #6 from Alex Deucher  2011-10-07 11:14:49 PDT 
---


*** This bug has been marked as a duplicate of bug 41561 ***

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

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Thomas Hellstrom]

Oh, and one more style comment below:

On 08/07/2011 10:39 PM, Marek Ol??k wrote:
>
> +enum ttm_buffer_usage {
> +TTM_USAGE_READ = 1,
> +TTM_USAGE_WRITE = 2,
> +TTM_USAGE_READWRITE = TTM_USAGE_READ | TTM_USAGE_WRITE
> +};
>
>

Please don't use enums for bit operations.

#define TTM_USAGE_FLAG_READ   (1 << 0)
#define TTM_USAGE_FLAG_WRITE  (1 << 1)

/Thomas

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

--- Comment #6 from Simon Farnsworth  
2011-10-07 10:53:04 PDT ---
I've applied both patches, and am testing on a machine with DVI-I and
DisplayPort.

Test sequence:

1) Boot system with no outputs connected.
2) Attach a DVI-D monitor, then remove it.
3) Attach a DVI-A to VGA adapter that has no load, then remove it.
4) Attach a DVI-A to VGA adapter that has load but no DDC, then remove it.
5) Attach a DVI-A to VGA adapter that has load and DDC, then remove it.
6) Repeat steps 2 to 5, watching for uevents.

Note that the DVI-A to VGA adapter asserts HPD when connected.

Results:

DVI-D hotplug works fine. There is a delay on the first DVI-D hotplug after the
connector has been used for VGA, but I get an event within 10 seconds.

No events when I plug in the adapter with no load and wait 20 seconds.

No events when I plug in the adapter with load but not DDC and wait 20 seconds.

Events on connect only when I plug in the adapter with DDC.

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

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

--- Comment #5 from Alex Deucher  2011-10-07 10:35:43 PDT 
---
Created an attachment (id=52094)
 View: https://bugs.freedesktop.org/attachment.cgi?id=52094
 Review: https://bugs.freedesktop.org/review?bug=41561=52094

2/2: fix the mixed and analog cases

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

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

--- Comment #4 from Alex Deucher  2011-10-07 10:35:06 PDT 
---
Created an attachment (id=52093)
 View: https://bugs.freedesktop.org/attachment.cgi?id=52093
 Review: https://bugs.freedesktop.org/review?bug=41561=52093

1/2: fix the digital only cases

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

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Alex Deucher]

On Fri, Oct 7, 2011 at 10:05 AM, Thomas Hellstrom  
wrote:
> On 10/07/2011 03:24 PM, Alex Deucher wrote:
>>
>> On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom
>> ?wrote:
>>
>>>
>>> On 10/07/2011 12:42 AM, Marek Ol??k wrote:
>>>

 On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstrom
 ?wrote:


>
> In any case, I'm not saying fences is the best way to flush but since
> the
> bo
> code assumes that signaling a sync object means "make the buffer
> contents
> available for CPU read / write", it's usually a good way to do it;
> there's
> even a sync_obj_flush() method that gets called when a potential flush
> needs
> to happen.
>
>

 I don't think we use it like that. To my knowledge, the purpose of the
 sync obj (to Radeon Gallium drivers at least) is to be able to wait
 for the last use of a buffer. Whether the contents can or cannot be
 available to the CPU is totally irrelevant.

 Currently (and it's a very important performance optimization),
 buffers stay mapped and available for CPU read/write during their
 first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
 on buffer destruction. We only call bo_wait when we want to wait for
 the GPU until it's done with the buffer (we don't always want that,
 sometimes we want to use the unsychronized flag). Otherwise the
 contents of buffers are available at *any time*.

 We could probably implement bo_wait privately in the kernel driver and
 not use ttm_bo_wait. I preferred code sharing though.

 Textures (especially the tiled ones) are never mapped directly and a
 temporary staging resource is used instead, so we don't actually
 pollute address space that much. (in case you would have such a
 remark) We will use staging resources for buffers too, but it's really
 the last resort to avoid waiting when direct access can't be used.




>>>
>>> 2) Can't we say that a write_sync_obj is simply a sync_obj? What's
>>> the
>>> difference between those two? I think we should remove the
>>> write_sync_obj
>>> bo
>>> member.
>>>
>>>
>>>
>>
>> Okay, but I think we should remove sync_obj instead, and keep write
>> and read sync objs. In the case of READWRITE usage, read_sync_obj
>> would be equal to write_sync_obj.
>>
>>
>>
>>
>
> Sure, I'm fine with that.
>
> One other thing, though, that makes me a little puzzled:
>
> Let's assume you don't allow readers and writers at the same time,
> which
> is
> my perception of how read- and write fences should work; you either
> have
> a
> list of read fences or a single write fence (in the same way a
> read-write
> lock works).
>
> Now, if you only allow a single read fence, like in this patch. That
> implies
> that you can only have either a single read fence or a single write
> fence
> at
> any one time. We'd only need a single fence pointer on the bo, and
> sync_obj_arg would tell us whether to signal the fence for read or for
> write
> (assuming that sync_obj_arg was set up to indicate read / write at
> validation time), then the patch really isn't necessary at all, as it
> only
> allows a single read fence?
>
> Or is it that you want to allow read- and write fences co-existing? In
> that
> case, what's the use case?
>
>

 There are lots of read-write use cases which don't need any barriers
 or flushing. The obvious ones are color blending and depth-stencil
 buffering. The OpenGL application is also allowed to use a subrange of
 a buffer as a vertex buffer (read-only) and another disjoint subrange
 of the same buffer for transform feedback (write-only), which kinda
 makes me think about whether we should track subranges instead of
 treating a whole buffer as "busy". It gets even more funky with
 ARB_shader_image_load_store, which supports atomic read-modify-write
 operations on textures, not to mention atomic memory operations in
 compute shaders (wait, isn't that also exposed in GL as
 GL_ARB_shader_atomic_counters?).

 I was thinking whether the two sync objs should be "read" and
 "readwrite", or "read" and "write". I chose the latter, because it's
 more fine-grained and we have to keep at least two of them around
 anyway.

 So now that you know what we use sync objs for, what are your ideas on
 re-implementing that patch in a way that is okay with you? Besides
 removing the third sync objs of course.

 Marek


>>>
>>> OK. First I think we need to make a distinction: bo sync objects vs
>>> driver
>>> fences. The bo sync obj api is there to strictly provide functionality
>>> that
>>> the ttm bo subsystem is using, and that follows a simple set of 

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Thomas Hellstrom]

On 10/07/2011 12:42 AM, Marek Ol??k wrote:
> On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstrom  
> wrote:
>
>> In any case, I'm not saying fences is the best way to flush but since the bo
>> code assumes that signaling a sync object means "make the buffer contents
>> available for CPU read / write", it's usually a good way to do it; there's
>> even a sync_obj_flush() method that gets called when a potential flush needs
>> to happen.
>>  
> I don't think we use it like that. To my knowledge, the purpose of the
> sync obj (to Radeon Gallium drivers at least) is to be able to wait
> for the last use of a buffer. Whether the contents can or cannot be
> available to the CPU is totally irrelevant.
>
> Currently (and it's a very important performance optimization),
> buffers stay mapped and available for CPU read/write during their
> first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
> on buffer destruction. We only call bo_wait when we want to wait for
> the GPU until it's done with the buffer (we don't always want that,
> sometimes we want to use the unsychronized flag). Otherwise the
> contents of buffers are available at *any time*.
>
> We could probably implement bo_wait privately in the kernel driver and
> not use ttm_bo_wait. I preferred code sharing though.
>
> Textures (especially the tiled ones) are never mapped directly and a
> temporary staging resource is used instead, so we don't actually
> pollute address space that much. (in case you would have such a
> remark) We will use staging resources for buffers too, but it's really
> the last resort to avoid waiting when direct access can't be used.
>
>
>
 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
 difference between those two? I think we should remove the write_sync_obj
 bo
 member.

  
>>> Okay, but I think we should remove sync_obj instead, and keep write
>>> and read sync objs. In the case of READWRITE usage, read_sync_obj
>>> would be equal to write_sync_obj.
>>>
>>>
>>>
>> Sure, I'm fine with that.
>>
>> One other thing, though, that makes me a little puzzled:
>>
>> Let's assume you don't allow readers and writers at the same time, which is
>> my perception of how read- and write fences should work; you either have a
>> list of read fences or a single write fence (in the same way a read-write
>> lock works).
>>
>> Now, if you only allow a single read fence, like in this patch. That implies
>> that you can only have either a single read fence or a single write fence at
>> any one time. We'd only need a single fence pointer on the bo, and
>> sync_obj_arg would tell us whether to signal the fence for read or for write
>> (assuming that sync_obj_arg was set up to indicate read / write at
>> validation time), then the patch really isn't necessary at all, as it only
>> allows a single read fence?
>>
>> Or is it that you want to allow read- and write fences co-existing? In that
>> case, what's the use case?
>>  
> There are lots of read-write use cases which don't need any barriers
> or flushing. The obvious ones are color blending and depth-stencil
> buffering. The OpenGL application is also allowed to use a subrange of
> a buffer as a vertex buffer (read-only) and another disjoint subrange
> of the same buffer for transform feedback (write-only), which kinda
> makes me think about whether we should track subranges instead of
> treating a whole buffer as "busy". It gets even more funky with
> ARB_shader_image_load_store, which supports atomic read-modify-write
> operations on textures, not to mention atomic memory operations in
> compute shaders (wait, isn't that also exposed in GL as
> GL_ARB_shader_atomic_counters?).
>
> I was thinking whether the two sync objs should be "read" and
> "readwrite", or "read" and "write". I chose the latter, because it's
> more fine-grained and we have to keep at least two of them around
> anyway.
>
> So now that you know what we use sync objs for, what are your ideas on
> re-implementing that patch in a way that is okay with you? Besides
> removing the third sync objs of course.
>
> Marek
>
OK. First I think we need to make a distinction: bo sync objects vs 
driver fences. The bo sync obj api is there to strictly provide 
functionality that the ttm bo subsystem is using, and that follows a 
simple set of rules:

1) the bo subsystem does never assume sync objects are ordered. That 
means the bo subsystem needs to wait on a sync object before removing it 
from a buffer. Any other assumption is buggy and must be fixed. BUT, if 
that assumption takes place in the driver unknowingly from the ttm bo 
subsystem (which is usually the case), it's OK.

2) When the sync object(s) attached to the bo are signaled the ttm bo 
subsystem is free to copy the bo contents and to unbind the bo.

3) The ttm bo system allows sync objects to be signaled in different 
ways opaque to the subsystem using sync_obj_arg. The driver is 

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Jerome Glisse]

On Fri, Oct 7, 2011 at 9:38 AM, Jerome Glisse  wrote:
> On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom  
> wrote:
>> On 10/07/2011 12:42 AM, Marek Ol??k wrote:
>>>
>>> On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstrom
>>> ?wrote:
>>>

 In any case, I'm not saying fences is the best way to flush but since the
 bo
 code assumes that signaling a sync object means "make the buffer contents
 available for CPU read / write", it's usually a good way to do it;
 there's
 even a sync_obj_flush() method that gets called when a potential flush
 needs
 to happen.

>>>
>>> I don't think we use it like that. To my knowledge, the purpose of the
>>> sync obj (to Radeon Gallium drivers at least) is to be able to wait
>>> for the last use of a buffer. Whether the contents can or cannot be
>>> available to the CPU is totally irrelevant.
>>>
>>> Currently (and it's a very important performance optimization),
>>> buffers stay mapped and available for CPU read/write during their
>>> first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
>>> on buffer destruction. We only call bo_wait when we want to wait for
>>> the GPU until it's done with the buffer (we don't always want that,
>>> sometimes we want to use the unsychronized flag). Otherwise the
>>> contents of buffers are available at *any time*.
>>>
>>> We could probably implement bo_wait privately in the kernel driver and
>>> not use ttm_bo_wait. I preferred code sharing though.
>>>
>>> Textures (especially the tiled ones) are never mapped directly and a
>>> temporary staging resource is used instead, so we don't actually
>>> pollute address space that much. (in case you would have such a
>>> remark) We will use staging resources for buffers too, but it's really
>>> the last resort to avoid waiting when direct access can't be used.
>>>
>>>
>>>
>>
>> 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
>> difference between those two? I think we should remove the
>> write_sync_obj
>> bo
>> member.
>>
>>
>
> Okay, but I think we should remove sync_obj instead, and keep write
> and read sync objs. In the case of READWRITE usage, read_sync_obj
> would be equal to write_sync_obj.
>
>
>

 Sure, I'm fine with that.

 One other thing, though, that makes me a little puzzled:

 Let's assume you don't allow readers and writers at the same time, which
 is
 my perception of how read- and write fences should work; you either have
 a
 list of read fences or a single write fence (in the same way a read-write
 lock works).

 Now, if you only allow a single read fence, like in this patch. That
 implies
 that you can only have either a single read fence or a single write fence
 at
 any one time. We'd only need a single fence pointer on the bo, and
 sync_obj_arg would tell us whether to signal the fence for read or for
 write
 (assuming that sync_obj_arg was set up to indicate read / write at
 validation time), then the patch really isn't necessary at all, as it
 only
 allows a single read fence?

 Or is it that you want to allow read- and write fences co-existing? In
 that
 case, what's the use case?

>>>
>>> There are lots of read-write use cases which don't need any barriers
>>> or flushing. The obvious ones are color blending and depth-stencil
>>> buffering. The OpenGL application is also allowed to use a subrange of
>>> a buffer as a vertex buffer (read-only) and another disjoint subrange
>>> of the same buffer for transform feedback (write-only), which kinda
>>> makes me think about whether we should track subranges instead of
>>> treating a whole buffer as "busy". It gets even more funky with
>>> ARB_shader_image_load_store, which supports atomic read-modify-write
>>> operations on textures, not to mention atomic memory operations in
>>> compute shaders (wait, isn't that also exposed in GL as
>>> GL_ARB_shader_atomic_counters?).
>>>
>>> I was thinking whether the two sync objs should be "read" and
>>> "readwrite", or "read" and "write". I chose the latter, because it's
>>> more fine-grained and we have to keep at least two of them around
>>> anyway.
>>>
>>> So now that you know what we use sync objs for, what are your ideas on
>>> re-implementing that patch in a way that is okay with you? Besides
>>> removing the third sync objs of course.
>>>
>>> Marek
>>>
>>
>> OK. First I think we need to make a distinction: bo sync objects vs driver
>> fences. The bo sync obj api is there to strictly provide functionality that
>> the ttm bo subsystem is using, and that follows a simple set of rules:
>>
>> 1) the bo subsystem does never assume sync objects are ordered. That means
>> the bo subsystem needs to wait on a sync object before removing it from a
>> buffer. Any other assumption is buggy and must be fixed. BUT, if that
>> assumption takes place in the 

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Jerome Glisse]

On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom  wrote:
> On 10/07/2011 12:42 AM, Marek Ol??k wrote:
>>
>> On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstrom
>> ?wrote:
>>
>>>
>>> In any case, I'm not saying fences is the best way to flush but since the
>>> bo
>>> code assumes that signaling a sync object means "make the buffer contents
>>> available for CPU read / write", it's usually a good way to do it;
>>> there's
>>> even a sync_obj_flush() method that gets called when a potential flush
>>> needs
>>> to happen.
>>>
>>
>> I don't think we use it like that. To my knowledge, the purpose of the
>> sync obj (to Radeon Gallium drivers at least) is to be able to wait
>> for the last use of a buffer. Whether the contents can or cannot be
>> available to the CPU is totally irrelevant.
>>
>> Currently (and it's a very important performance optimization),
>> buffers stay mapped and available for CPU read/write during their
>> first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
>> on buffer destruction. We only call bo_wait when we want to wait for
>> the GPU until it's done with the buffer (we don't always want that,
>> sometimes we want to use the unsychronized flag). Otherwise the
>> contents of buffers are available at *any time*.
>>
>> We could probably implement bo_wait privately in the kernel driver and
>> not use ttm_bo_wait. I preferred code sharing though.
>>
>> Textures (especially the tiled ones) are never mapped directly and a
>> temporary staging resource is used instead, so we don't actually
>> pollute address space that much. (in case you would have such a
>> remark) We will use staging resources for buffers too, but it's really
>> the last resort to avoid waiting when direct access can't be used.
>>
>>
>>
>
> 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
> difference between those two? I think we should remove the
> write_sync_obj
> bo
> member.
>
>

 Okay, but I think we should remove sync_obj instead, and keep write
 and read sync objs. In the case of READWRITE usage, read_sync_obj
 would be equal to write_sync_obj.



>>>
>>> Sure, I'm fine with that.
>>>
>>> One other thing, though, that makes me a little puzzled:
>>>
>>> Let's assume you don't allow readers and writers at the same time, which
>>> is
>>> my perception of how read- and write fences should work; you either have
>>> a
>>> list of read fences or a single write fence (in the same way a read-write
>>> lock works).
>>>
>>> Now, if you only allow a single read fence, like in this patch. That
>>> implies
>>> that you can only have either a single read fence or a single write fence
>>> at
>>> any one time. We'd only need a single fence pointer on the bo, and
>>> sync_obj_arg would tell us whether to signal the fence for read or for
>>> write
>>> (assuming that sync_obj_arg was set up to indicate read / write at
>>> validation time), then the patch really isn't necessary at all, as it
>>> only
>>> allows a single read fence?
>>>
>>> Or is it that you want to allow read- and write fences co-existing? In
>>> that
>>> case, what's the use case?
>>>
>>
>> There are lots of read-write use cases which don't need any barriers
>> or flushing. The obvious ones are color blending and depth-stencil
>> buffering. The OpenGL application is also allowed to use a subrange of
>> a buffer as a vertex buffer (read-only) and another disjoint subrange
>> of the same buffer for transform feedback (write-only), which kinda
>> makes me think about whether we should track subranges instead of
>> treating a whole buffer as "busy". It gets even more funky with
>> ARB_shader_image_load_store, which supports atomic read-modify-write
>> operations on textures, not to mention atomic memory operations in
>> compute shaders (wait, isn't that also exposed in GL as
>> GL_ARB_shader_atomic_counters?).
>>
>> I was thinking whether the two sync objs should be "read" and
>> "readwrite", or "read" and "write". I chose the latter, because it's
>> more fine-grained and we have to keep at least two of them around
>> anyway.
>>
>> So now that you know what we use sync objs for, what are your ideas on
>> re-implementing that patch in a way that is okay with you? Besides
>> removing the third sync objs of course.
>>
>> Marek
>>
>
> OK. First I think we need to make a distinction: bo sync objects vs driver
> fences. The bo sync obj api is there to strictly provide functionality that
> the ttm bo subsystem is using, and that follows a simple set of rules:
>
> 1) the bo subsystem does never assume sync objects are ordered. That means
> the bo subsystem needs to wait on a sync object before removing it from a
> buffer. Any other assumption is buggy and must be fixed. BUT, if that
> assumption takes place in the driver unknowingly from the ttm bo subsystem
> (which is usually the case), it's OK.
>
> 2) When the sync object(s) attached to the bo are signaled the ttm bo
> subsystem is 

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

--- Comment #3 from Simon Farnsworth  
2011-10-07 09:26:08 PDT ---
So, I think I see the proximate cause of the bug, but not the reason for it:

In radeon_dvi_detect at radeon_connectors.c:962, I see:

if (!force) {
ret = connector->status;
goto out;
}

A HPD interrupt causes output_poll_execute at drm_crtc_helper.c:897 to execute 
   connector->status = connector->funcs->detect(connector, false); as a
result, I cannot see how a hotplug interrupt will ever result in a connector
state changing.

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

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Alex Deucher]

On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom  wrote:
> On 10/07/2011 12:42 AM, Marek Ol??k wrote:
>>
>> On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstrom
>> ?wrote:
>>
>>>
>>> In any case, I'm not saying fences is the best way to flush but since the
>>> bo
>>> code assumes that signaling a sync object means "make the buffer contents
>>> available for CPU read / write", it's usually a good way to do it;
>>> there's
>>> even a sync_obj_flush() method that gets called when a potential flush
>>> needs
>>> to happen.
>>>
>>
>> I don't think we use it like that. To my knowledge, the purpose of the
>> sync obj (to Radeon Gallium drivers at least) is to be able to wait
>> for the last use of a buffer. Whether the contents can or cannot be
>> available to the CPU is totally irrelevant.
>>
>> Currently (and it's a very important performance optimization),
>> buffers stay mapped and available for CPU read/write during their
>> first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
>> on buffer destruction. We only call bo_wait when we want to wait for
>> the GPU until it's done with the buffer (we don't always want that,
>> sometimes we want to use the unsychronized flag). Otherwise the
>> contents of buffers are available at *any time*.
>>
>> We could probably implement bo_wait privately in the kernel driver and
>> not use ttm_bo_wait. I preferred code sharing though.
>>
>> Textures (especially the tiled ones) are never mapped directly and a
>> temporary staging resource is used instead, so we don't actually
>> pollute address space that much. (in case you would have such a
>> remark) We will use staging resources for buffers too, but it's really
>> the last resort to avoid waiting when direct access can't be used.
>>
>>
>>
>
> 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
> difference between those two? I think we should remove the
> write_sync_obj
> bo
> member.
>
>

 Okay, but I think we should remove sync_obj instead, and keep write
 and read sync objs. In the case of READWRITE usage, read_sync_obj
 would be equal to write_sync_obj.



>>>
>>> Sure, I'm fine with that.
>>>
>>> One other thing, though, that makes me a little puzzled:
>>>
>>> Let's assume you don't allow readers and writers at the same time, which
>>> is
>>> my perception of how read- and write fences should work; you either have
>>> a
>>> list of read fences or a single write fence (in the same way a read-write
>>> lock works).
>>>
>>> Now, if you only allow a single read fence, like in this patch. That
>>> implies
>>> that you can only have either a single read fence or a single write fence
>>> at
>>> any one time. We'd only need a single fence pointer on the bo, and
>>> sync_obj_arg would tell us whether to signal the fence for read or for
>>> write
>>> (assuming that sync_obj_arg was set up to indicate read / write at
>>> validation time), then the patch really isn't necessary at all, as it
>>> only
>>> allows a single read fence?
>>>
>>> Or is it that you want to allow read- and write fences co-existing? In
>>> that
>>> case, what's the use case?
>>>
>>
>> There are lots of read-write use cases which don't need any barriers
>> or flushing. The obvious ones are color blending and depth-stencil
>> buffering. The OpenGL application is also allowed to use a subrange of
>> a buffer as a vertex buffer (read-only) and another disjoint subrange
>> of the same buffer for transform feedback (write-only), which kinda
>> makes me think about whether we should track subranges instead of
>> treating a whole buffer as "busy". It gets even more funky with
>> ARB_shader_image_load_store, which supports atomic read-modify-write
>> operations on textures, not to mention atomic memory operations in
>> compute shaders (wait, isn't that also exposed in GL as
>> GL_ARB_shader_atomic_counters?).
>>
>> I was thinking whether the two sync objs should be "read" and
>> "readwrite", or "read" and "write". I chose the latter, because it's
>> more fine-grained and we have to keep at least two of them around
>> anyway.
>>
>> So now that you know what we use sync objs for, what are your ideas on
>> re-implementing that patch in a way that is okay with you? Besides
>> removing the third sync objs of course.
>>
>> Marek
>>
>
> OK. First I think we need to make a distinction: bo sync objects vs driver
> fences. The bo sync obj api is there to strictly provide functionality that
> the ttm bo subsystem is using, and that follows a simple set of rules:
>
> 1) the bo subsystem does never assume sync objects are ordered. That means
> the bo subsystem needs to wait on a sync object before removing it from a
> buffer. Any other assumption is buggy and must be fixed. BUT, if that
> assumption takes place in the driver unknowingly from the ttm bo subsystem
> (which is usually the case), it's OK.
>
> 2) When the sync object(s) attached to the bo are signaled the ttm bo
> subsystem is 

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

--- Comment #2 from Simon Farnsworth  
2011-10-07 09:07:40 PDT ---
drm_helper_hpd_irq_event() isn't generating uevents, because connector->status
is not being updated somewhere, and is remaining at connector_status_connected,
when it should be being updated to connector_status_disconnected.

If I do cat /sys/class/drm/card0-HDMI-A-1/status, I see the status is
"disconnected", but something resets it by the time the helper comes back
round. Possibly connected to the following from dmesg:

# cat /sys/class/drm/card0-HDMI-A-1/status ; dmesg -c
[16791.042128] [drm:radeon_atombios_connected_scratch_regs], DFP1 disconnected

# sleep 10 ; dmesg -c
[16799.522394] [drm:radeon_atombios_connected_scratch_regs], DFP1 connected
[16799.522407] [drm:output_poll_execute], [CONNECTOR:15:HDMI-A-1] status
updated from 1 to 1
[16799.524465] [drm:output_poll_execute], [CONNECTOR:17:VGA-1] status updated
from 2 to 2

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

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

--- Comment #1 from Alex Deucher  2011-10-07 08:59:22 PDT 
---
If you are getting HPD interrupts, radeon_hotplug_work_func() should be getting
scheduled.  From there drm_helper_hpd_irq_event() gets called which actually
generates the uevent.

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

[Bug 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

Simon Farnsworth  changed:

   What|Removed |Added

Summary|Hotplug detect does not |[r600 KMS] Hotplug detect
   |work for HDMI monitor on|does not work for HDMI
   |Fusion E-350 board  |monitor on Fusion E-350
   ||board

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

[Bug 41561] New: Hotplug detect does not work for HDMI monitor on Fusion E-350 board

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

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

   Summary: Hotplug detect does not work for HDMI monitor on
Fusion E-350 board
   Product: DRI
   Version: unspecified
  Platform: x86 (IA32)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/Radeon
AssignedTo: dri-devel at lists.freedesktop.org
ReportedBy: simon.farnsworth at onelan.co.uk


Using airlied-drm-fixes kernel as of commit
6777a4f6898a53974ef7fe7ce09ec41fae0f32db with Alex Deucher's patch
"drm/radeon/kms: use hardcoded dig encoder to transmitter mapping for DCE4.1"
on top, I'm not seeing uevents from the kernel when I plug and unplug a HDMI
monitor.

dmesg with drm.debug=0xf shows me the following when I connect a monitor:

# dmesg -c
[15202.939581] [drm:evergreen_irq_process], r600_irq_process start: rptr 5872,
wptr 5888
[15202.939609] [drm:evergreen_irq_process], IH: HPD2
[15203.050597] [drm:radeon_atombios_connected_scratch_regs], DFP1 connected
[15203.050605] [drm:output_poll_execute], [CONNECTOR:15:HDMI-A-1] status
updated from 1 to 1
[15203.052661] [drm:output_poll_execute], [CONNECTOR:17:VGA-1] status updated
from 2 to 2

radeonreg suggests that the HPD sense bit is correctly set

# ./radeonreg regs all | grep 6028 # DC_HPD2_INT_STATUS
6028ff0ff012 (-15732718)

When I remove the monitor, I get the following from dmesg and radeonreg:

# dmesg -c
[15307.075271] [drm:evergreen_irq_process], r600_irq_process start: rptr 5888,
wptr 5904
[15307.075300] [drm:evergreen_irq_process], IH: HPD2
[15307.131727] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid,
remainder is 192
[15307.131733] Raw EDID:
[15307.131738]  3f ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131742]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131745]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131749]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131752]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131756]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131759]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131763]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.141965] [drm:radeon_dvi_detect] *ERROR* HDMI-A-1: probed a monitor but
no|invalid EDID
[15307.141975] [drm:radeon_atombios_connected_scratch_regs], DFP1 connected
[15307.141981] [drm:output_poll_execute], [CONNECTOR:15:HDMI-A-1] status
updated from 1 to 1
[15307.144028] [drm:output_poll_execute], [CONNECTOR:17:VGA-1] status updated
from 2 to 2

# ./radeonreg regs all | grep 6028 # DC_HPD2_INT_STATUS
6028ff0ff000 (-15732736)

suggesting that HPD sense bits are being updated correctly by the hardware, but
that this isn't resulting in uevents following through.

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

[PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Marek Olšák]

On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstrom  wrote:
> In any case, I'm not saying fences is the best way to flush but since the bo
> code assumes that signaling a sync object means "make the buffer contents
> available for CPU read / write", it's usually a good way to do it; there's
> even a sync_obj_flush() method that gets called when a potential flush needs
> to happen.

I don't think we use it like that. To my knowledge, the purpose of the
sync obj (to Radeon Gallium drivers at least) is to be able to wait
for the last use of a buffer. Whether the contents can or cannot be
available to the CPU is totally irrelevant.

Currently (and it's a very important performance optimization),
buffers stay mapped and available for CPU read/write during their
first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
on buffer destruction. We only call bo_wait when we want to wait for
the GPU until it's done with the buffer (we don't always want that,
sometimes we want to use the unsychronized flag). Otherwise the
contents of buffers are available at *any time*.

We could probably implement bo_wait privately in the kernel driver and
not use ttm_bo_wait. I preferred code sharing though.

Textures (especially the tiled ones) are never mapped directly and a
temporary staging resource is used instead, so we don't actually
pollute address space that much. (in case you would have such a
remark) We will use staging resources for buffers too, but it's really
the last resort to avoid waiting when direct access can't be used.


>>> 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
>>> difference between those two? I think we should remove the write_sync_obj
>>> bo
>>> member.
>>>
>>
>> Okay, but I think we should remove sync_obj instead, and keep write
>> and read sync objs. In the case of READWRITE usage, read_sync_obj
>> would be equal to write_sync_obj.
>>
>>
>
> Sure, I'm fine with that.
>
> One other thing, though, that makes me a little puzzled:
>
> Let's assume you don't allow readers and writers at the same time, which is
> my perception of how read- and write fences should work; you either have a
> list of read fences or a single write fence (in the same way a read-write
> lock works).
>
> Now, if you only allow a single read fence, like in this patch. That implies
> that you can only have either a single read fence or a single write fence at
> any one time. We'd only need a single fence pointer on the bo, and
> sync_obj_arg would tell us whether to signal the fence for read or for write
> (assuming that sync_obj_arg was set up to indicate read / write at
> validation time), then the patch really isn't necessary at all, as it only
> allows a single read fence?
>
> Or is it that you want to allow read- and write fences co-existing? In that
> case, what's the use case?

There are lots of read-write use cases which don't need any barriers
or flushing. The obvious ones are color blending and depth-stencil
buffering. The OpenGL application is also allowed to use a subrange of
a buffer as a vertex buffer (read-only) and another disjoint subrange
of the same buffer for transform feedback (write-only), which kinda
makes me think about whether we should track subranges instead of
treating a whole buffer as "busy". It gets even more funky with
ARB_shader_image_load_store, which supports atomic read-modify-write
operations on textures, not to mention atomic memory operations in
compute shaders (wait, isn't that also exposed in GL as
GL_ARB_shader_atomic_counters?).

I was thinking whether the two sync objs should be "read" and
"readwrite", or "read" and "write". I chose the latter, because it's
more fine-grained and we have to keep at least two of them around
anyway.

So now that you know what we use sync objs for, what are your ideas on
re-implementing that patch in a way that is okay with you? Besides
removing the third sync objs of course.

Marek

[PATCH] drm/radeon/kms: Fix I2C mask definitions

[Jean Delvare]

Commit 9b9fe724 accidentally used RADEON_GPIO_EN_* where
RADEON_GPIO_MASK_* was intended. This caused improper initialization
of I2C buses, mostly visible when setting i2c_algo_bit.bit_test=1.
Using the right constants fixes the problem.

Signed-off-by: Jean Delvare jdelv...@suse.de
Cc: Alex Deucher alexander.deuc...@amd.com
Cc: Jerome Glisse j.gli...@gmail.com
Cc: sta...@kernel.org
---
This needs testing on more combios-based Radeon cards, please. I
could only test it on one Radeon 9200 (RV280) card.

 drivers/gpu/drm/radeon/radeon_combios.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-3.0.orig/drivers/gpu/drm/radeon/radeon_combios.c  2011-10-06 
14:52:59.0 +0200
+++ linux-3.0/drivers/gpu/drm/radeon/radeon_combios.c   2011-10-06 
14:53:23.0 +0200
@@ -620,8 +620,8 @@ static struct radeon_i2c_bus_rec combios
i2c.y_data_mask = 0x80;
} else {
/* default masks for ddc pads */
-   i2c.mask_clk_mask = RADEON_GPIO_EN_1;
-   i2c.mask_data_mask = RADEON_GPIO_EN_0;
+   i2c.mask_clk_mask = RADEON_GPIO_MASK_1;
+   i2c.mask_data_mask = RADEON_GPIO_MASK_0;
i2c.a_clk_mask = RADEON_GPIO_A_1;
i2c.a_data_mask = RADEON_GPIO_A_0;
i2c.en_clk_mask = RADEON_GPIO_EN_1;

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

RE: [PATCH] drm/radeon/kms: Fix I2C mask definitions

[Deucher, Alexander]

 -Original Message-
 From: Jean Delvare [mailto:jdelv...@suse.de]
 Sent: Thursday, October 06, 2011 12:16 PM
 To: David Airlie
 Cc: Deucher, Alexander; dri-devel@lists.freedesktop.org; Jerome Glisse
 Subject: [PATCH] drm/radeon/kms: Fix I2C mask definitions
 
 Commit 9b9fe724 accidentally used RADEON_GPIO_EN_* where
 RADEON_GPIO_MASK_* was intended. This caused improper initialization of
 I2C buses, mostly visible when setting i2c_algo_bit.bit_test=1.
 Using the right constants fixes the problem.
 
 Signed-off-by: Jean Delvare jdelv...@suse.de
 Cc: Alex Deucher alexander.deuc...@amd.com
 Cc: Jerome Glisse j.gli...@gmail.com
 Cc: sta...@kernel.org

The patch is correct.  Looks like a copy paste error.

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

 ---
 This needs testing on more combios-based Radeon cards, please. I could only
 test it on one Radeon 9200 (RV280) card.
 
  drivers/gpu/drm/radeon/radeon_combios.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 --- linux-3.0.orig/drivers/gpu/drm/radeon/radeon_combios.c2011-10-06
 14:52:59.0 +0200
 +++ linux-3.0/drivers/gpu/drm/radeon/radeon_combios.c 2011-10-06
 14:53:23.0 +0200
 @@ -620,8 +620,8 @@ static struct radeon_i2c_bus_rec combios
   i2c.y_data_mask = 0x80;
   } else {
   /* default masks for ddc pads */
 - i2c.mask_clk_mask = RADEON_GPIO_EN_1;
 - i2c.mask_data_mask = RADEON_GPIO_EN_0;
 + i2c.mask_clk_mask = RADEON_GPIO_MASK_1;
 + i2c.mask_data_mask = RADEON_GPIO_MASK_0;
   i2c.a_clk_mask = RADEON_GPIO_A_1;
   i2c.a_data_mask = RADEON_GPIO_A_0;
   i2c.en_clk_mask = RADEON_GPIO_EN_1;
 
 --
 Jean Delvare
 Suse L3


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

Re: [PULL] drm-intel-fixes (drm/i915 driver)

[Woody Suwalski]

Keith Packard wrote:

On Thu, 6 Oct 2011 10:12:57 -0700, Linus 
Torvaldstorva...@linux-foundation.org  wrote:


   [drm:ironlake_update_pch_refclk] *ERROR* enabling SSC on PCH

Thanks. I've got a patch series that fixes a pile of refclk bugs which
is still out for review that should fix this. This error should be
harmless, but still..


And what about blanking (black screen) issue reported Sep21?
I confirm that disabling the blanking e.g. commenting out
/* intel_panel_set_backlight(dev, 0); */
in intel_panel.c is somehow working on EeePC as well as Dell machine.

I guess Linus has stopped using EeePCs ;-)

Woody

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

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Thomas Hellstrom]

On 10/07/2011 12:42 AM, Marek Olšák wrote:

On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstromtho...@shipmail.org  wrote:
   

In any case, I'm not saying fences is the best way to flush but since the bo
code assumes that signaling a sync object means make the buffer contents
available for CPU read / write, it's usually a good way to do it; there's
even a sync_obj_flush() method that gets called when a potential flush needs
to happen.
 

I don't think we use it like that. To my knowledge, the purpose of the
sync obj (to Radeon Gallium drivers at least) is to be able to wait
for the last use of a buffer. Whether the contents can or cannot be
available to the CPU is totally irrelevant.

Currently (and it's a very important performance optimization),
buffers stay mapped and available for CPU read/write during their
first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
on buffer destruction. We only call bo_wait when we want to wait for
the GPU until it's done with the buffer (we don't always want that,
sometimes we want to use the unsychronized flag). Otherwise the
contents of buffers are available at *any time*.

We could probably implement bo_wait privately in the kernel driver and
not use ttm_bo_wait. I preferred code sharing though.

Textures (especially the tiled ones) are never mapped directly and a
temporary staging resource is used instead, so we don't actually
pollute address space that much. (in case you would have such a
remark) We will use staging resources for buffers too, but it's really
the last resort to avoid waiting when direct access can't be used.


   

2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
difference between those two? I think we should remove the write_sync_obj
bo
member.

 

Okay, but I think we should remove sync_obj instead, and keep write
and read sync objs. In the case of READWRITE usage, read_sync_obj
would be equal to write_sync_obj.


   

Sure, I'm fine with that.

One other thing, though, that makes me a little puzzled:

Let's assume you don't allow readers and writers at the same time, which is
my perception of how read- and write fences should work; you either have a
list of read fences or a single write fence (in the same way a read-write
lock works).

Now, if you only allow a single read fence, like in this patch. That implies
that you can only have either a single read fence or a single write fence at
any one time. We'd only need a single fence pointer on the bo, and
sync_obj_arg would tell us whether to signal the fence for read or for write
(assuming that sync_obj_arg was set up to indicate read / write at
validation time), then the patch really isn't necessary at all, as it only
allows a single read fence?

Or is it that you want to allow read- and write fences co-existing? In that
case, what's the use case?
 

There are lots of read-write use cases which don't need any barriers
or flushing. The obvious ones are color blending and depth-stencil
buffering. The OpenGL application is also allowed to use a subrange of
a buffer as a vertex buffer (read-only) and another disjoint subrange
of the same buffer for transform feedback (write-only), which kinda
makes me think about whether we should track subranges instead of
treating a whole buffer as busy. It gets even more funky with
ARB_shader_image_load_store, which supports atomic read-modify-write
operations on textures, not to mention atomic memory operations in
compute shaders (wait, isn't that also exposed in GL as
GL_ARB_shader_atomic_counters?).

I was thinking whether the two sync objs should be read and
readwrite, or read and write. I chose the latter, because it's
more fine-grained and we have to keep at least two of them around
anyway.

So now that you know what we use sync objs for, what are your ideas on
re-implementing that patch in a way that is okay with you? Besides
removing the third sync objs of course.

Marek
   
OK. First I think we need to make a distinction: bo sync objects vs 
driver fences. The bo sync obj api is there to strictly provide 
functionality that the ttm bo subsystem is using, and that follows a 
simple set of rules:


1) the bo subsystem does never assume sync objects are ordered. That 
means the bo subsystem needs to wait on a sync object before removing it 
from a buffer. Any other assumption is buggy and must be fixed. BUT, if 
that assumption takes place in the driver unknowingly from the ttm bo 
subsystem (which is usually the case), it's OK.


2) When the sync object(s) attached to the bo are signaled the ttm bo 
subsystem is free to copy the bo contents and to unbind the bo.


3) The ttm bo system allows sync objects to be signaled in different 
ways opaque to the subsystem using sync_obj_arg. The driver is 
responsible for setting up that argument.


4) Driver fences may be used for or expose other functionality or 
adaptions to APIs as long as the sync obj api exported to the bo 
sybsystem follows the above 

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Thomas Hellstrom]

Oh, and one more style comment below:

On 08/07/2011 10:39 PM, Marek Olšák wrote:


+enum ttm_buffer_usage {
+TTM_USAGE_READ = 1,
+TTM_USAGE_WRITE = 2,
+TTM_USAGE_READWRITE = TTM_USAGE_READ | TTM_USAGE_WRITE
+};

   


Please don't use enums for bit operations.

#define TTM_USAGE_FLAG_READ   (1  0)
#define TTM_USAGE_FLAG_WRITE  (1  1)

/Thomas



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

[PATCH -next 0/2] more vmwgfx updates

[Thomas Hellstrom]

Two fixes on top of previous patches.
The first removes a DOS vulnerabilty.
The second one implements some screen object fixes.

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

[PATCH 1/2] vmwgfx: Implement memory accounting for resources

[Thomas Hellstrom]

Contexts, surfaces and streams allocate persistent kernel memory as the
direct result of user-space requests. Make sure this memory is
accounted as graphics memory, to avoid DOS vulnerabilities.

Also take the TTM read lock around resource creation to block
switched-out dri clients from allocating resources.

Signed-off-by: Thomas Hellstrom thellst...@vmware.com
Reviewed-by: Jakob Bornecrantz ja...@vmware.com
---
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c |  172 +-
 1 files changed, 146 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 93a68a6..c7cff3d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -39,6 +39,7 @@ struct vmw_user_context {
 struct vmw_user_surface {
struct ttm_base_object base;
struct vmw_surface srf;
+   uint32_t size;
 };
 
 struct vmw_user_dma_buffer {
@@ -67,6 +68,11 @@ struct vmw_surface_offset {
uint32_t bo_offset;
 };
 
+
+static uint64_t vmw_user_context_size;
+static uint64_t vmw_user_surface_size;
+static uint64_t vmw_user_stream_size;
+
 static inline struct vmw_dma_buffer *
 vmw_dma_buffer(struct ttm_buffer_object *bo)
 {
@@ -343,8 +349,11 @@ static void vmw_user_context_free(struct vmw_resource *res)
 {
struct vmw_user_context *ctx =
container_of(res, struct vmw_user_context, res);
+   struct vmw_private *dev_priv = res-dev_priv;
 
kfree(ctx);
+   ttm_mem_global_free(vmw_mem_glob(dev_priv),
+   vmw_user_context_size);
 }
 
 /**
@@ -398,23 +407,56 @@ int vmw_context_define_ioctl(struct drm_device *dev, void 
*data,
 struct drm_file *file_priv)
 {
struct vmw_private *dev_priv = vmw_priv(dev);
-   struct vmw_user_context *ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+   struct vmw_user_context *ctx;
struct vmw_resource *res;
struct vmw_resource *tmp;
struct drm_vmw_context_arg *arg = (struct drm_vmw_context_arg *)data;
struct ttm_object_file *tfile = vmw_fpriv(file_priv)-tfile;
+   struct vmw_master *vmaster = vmw_master(file_priv-master);
int ret;
 
-   if (unlikely(ctx == NULL))
-   return -ENOMEM;
+
+   /*
+* Approximate idr memory usage with 128 bytes. It will be limited
+* by maximum number_of contexts anyway.
+*/
+
+   if (unlikely(vmw_user_context_size == 0))
+   vmw_user_context_size = ttm_round_pot(sizeof(*ctx)) + 128;
+
+   ret = ttm_read_lock(vmaster-lock, true);
+   if (unlikely(ret != 0))
+   return ret;
+
+   ret = ttm_mem_global_alloc(vmw_mem_glob(dev_priv),
+  vmw_user_context_size,
+  false, true);
+   if (unlikely(ret != 0)) {
+   if (ret != -ERESTARTSYS)
+   DRM_ERROR(Out of graphics memory for context
+  creation.\n);
+   goto out_unlock;
+   }
+
+   ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+   if (unlikely(ctx == NULL)) {
+   ttm_mem_global_free(vmw_mem_glob(dev_priv),
+   vmw_user_context_size);
+   ret = -ENOMEM;
+   goto out_unlock;
+   }
 
res = ctx-res;
ctx-base.shareable = false;
ctx-base.tfile = NULL;
 
+   /*
+* From here on, the destructor takes over resource freeing.
+*/
+
ret = vmw_context_init(dev_priv, res, vmw_user_context_free);
if (unlikely(ret != 0))
-   return ret;
+   goto out_unlock;
 
tmp = vmw_resource_reference(ctx-res);
ret = ttm_base_object_init(tfile, ctx-base, false, VMW_RES_CONTEXT,
@@ -428,6 +470,8 @@ int vmw_context_define_ioctl(struct drm_device *dev, void 
*data,
arg-cid = res-id;
 out_err:
vmw_resource_unreference(res);
+out_unlock:
+   ttm_read_unlock(vmaster-lock);
return ret;
 
 }
@@ -1095,6 +1139,8 @@ static void vmw_user_surface_free(struct vmw_resource 
*res)
struct vmw_surface *srf = container_of(res, struct vmw_surface, res);
struct vmw_user_surface *user_srf =
container_of(srf, struct vmw_user_surface, srf);
+   struct vmw_private *dev_priv = srf-res.dev_priv;
+   uint32_t size = user_srf-size;
 
if (srf-backup)
ttm_bo_unref(srf-backup);
@@ -1102,6 +1148,7 @@ static void vmw_user_surface_free(struct vmw_resource 
*res)
kfree(srf-sizes);
kfree(srf-snooper.image);
kfree(user_srf);
+   ttm_mem_global_free(vmw_mem_glob(dev_priv), size);
 }
 
 /**
@@ -1226,9 +1273,45 @@ int vmw_surface_define_ioctl(struct drm_device *dev, 
void *data,
struct vmw_surface_offset *cur_offset;
uint32_t stride_bpp;
uint32_t bpp;
+   uint32_t num_sizes;
+   

[PATCH 2/2] vmwgfx: Don't use virtual coords when using screen objects

[Thomas Hellstrom]

From: Jakob Bornecrantz ja...@vmware.com

Signed-off-by: Jakob Bornecrantz ja...@vmware.com
Signed-off-by: Thomas Hellstrom thellst...@vmware.com
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |  272 +++
 1 files changed, 215 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index fc62c87..2421d0c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -358,49 +358,109 @@ static int do_surface_dirty_sou(struct vmw_private 
*dev_priv,
struct drm_clip_rect *clips,
unsigned num_clips, int inc)
 {
-   int left = clips-x2, right = clips-x1;
-   int top = clips-y2, bottom = clips-y1;
+   struct drm_clip_rect *clips_ptr;
+   struct vmw_display_unit *units[VMWGFX_NUM_DISPLAY_UNITS];
+   struct drm_crtc *crtc;
size_t fifo_size;
-   int i, ret;
+   int i, num_units;
+   int ret = 0; /* silence warning */
+   int left, right, top, bottom;
 
struct {
SVGA3dCmdHeader header;
SVGA3dCmdBlitSurfaceToScreen body;
} *cmd;
+   SVGASignedRect *blits;
 
 
-   fifo_size = sizeof(*cmd);
+   num_units = 0;
+   list_for_each_entry(crtc, dev_priv-dev-mode_config.crtc_list,
+   head) {
+   if (crtc-fb != framebuffer-base)
+   continue;
+   units[num_units++] = vmw_crtc_to_du(crtc);
+   }
+
+   BUG_ON(surf == NULL);
+   BUG_ON(!clips || !num_clips);
+
+   fifo_size = sizeof(*cmd) + sizeof(SVGASignedRect) * num_clips;
cmd = kzalloc(fifo_size, GFP_KERNEL);
if (unlikely(cmd == NULL)) {
DRM_ERROR(Temporary fifo memory alloc failed.\n);
return -ENOMEM;
}
 
-   cmd-header.id = cpu_to_le32(SVGA_3D_CMD_BLIT_SURFACE_TO_SCREEN);
-   cmd-header.size = cpu_to_le32(sizeof(cmd-body));
-
-   cmd-body.srcImage.sid = cpu_to_le32(framebuffer-user_handle);
-   cmd-body.destScreenId = SVGA_ID_INVALID; /* virtual coords */
-
-   for (i = 0; i  num_clips; i++, clips += inc) {
-   left = min_t(int, left, (int)clips-x1);
-   right = max_t(int, right, (int)clips-x2);
-   top = min_t(int, top, (int)clips-y1);
-   bottom = max_t(int, bottom, (int)clips-y2);
+   left = clips-x1;
+   right = clips-x2;
+   top = clips-y1;
+   bottom = clips-y2;
+
+   clips_ptr = clips;
+   for (i = 1; i  num_clips; i++, clips_ptr += inc) {
+   left = min_t(int, left, (int)clips_ptr-x1);
+   right = max_t(int, right, (int)clips_ptr-x2);
+   top = min_t(int, top, (int)clips_ptr-y1);
+   bottom = max_t(int, bottom, (int)clips_ptr-y2);
}
 
+   /* only need to do this once */
+   memset(cmd, 0, fifo_size);
+   cmd-header.id = cpu_to_le32(SVGA_3D_CMD_BLIT_SURFACE_TO_SCREEN);
+   cmd-header.size = cpu_to_le32(fifo_size - sizeof(cmd-header));
+
cmd-body.srcRect.left = left;
cmd-body.srcRect.right = right;
cmd-body.srcRect.top = top;
cmd-body.srcRect.bottom = bottom;
 
-   cmd-body.destRect.left = left;
-   cmd-body.destRect.right = right;
-   cmd-body.destRect.top = top;
-   cmd-body.destRect.bottom = bottom;
+   clips_ptr = clips;
+   blits = (SVGASignedRect *)cmd[1];
+   for (i = 0; i  num_clips; i++, clips_ptr += inc) {
+   blits[i].left   = clips_ptr-x1 - left;
+   blits[i].right  = clips_ptr-x2 - left;
+   blits[i].top= clips_ptr-y1 - top;
+   blits[i].bottom = clips_ptr-y2 - top;
+   }
+
+   /* do per unit writing, reuse fifo for each */
+   for (i = 0; i  num_units; i++) {
+   struct vmw_display_unit *unit = units[i];
+   int clip_x1 = left - unit-crtc.x;
+   int clip_y1 = top - unit-crtc.y;
+   int clip_x2 = right - unit-crtc.x;
+   int clip_y2 = bottom - unit-crtc.y;
+
+   /* skip any crtcs that misses the clip region */
+   if (clip_x1 = unit-crtc.mode.hdisplay ||
+   clip_y1 = unit-crtc.mode.vdisplay ||
+   clip_x2 = 0 || clip_y2 = 0)
+   continue;
+
+   /* need to reset sid as it is changed by execbuf */
+   cmd-body.srcImage.sid = cpu_to_le32(framebuffer-user_handle);
+
+   cmd-body.destScreenId = unit-unit;
+
+   /*
+* The blit command is a lot more resilient then the
+* readback command when it comes to clip rects. So its
+* okay to go out of bounds.
+*/
+
+   cmd-body.destRect.left = clip_x1;
+   cmd-body.destRect.right = clip_x2;
+   cmd-body.destRect.top = clip_y1;
+ 

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Alex Deucher]

On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom tho...@shipmail.org wrote:
 On 10/07/2011 12:42 AM, Marek Olšák wrote:

 On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstromtho...@shipmail.org
  wrote:


 In any case, I'm not saying fences is the best way to flush but since the
 bo
 code assumes that signaling a sync object means make the buffer contents
 available for CPU read / write, it's usually a good way to do it;
 there's
 even a sync_obj_flush() method that gets called when a potential flush
 needs
 to happen.


 I don't think we use it like that. To my knowledge, the purpose of the
 sync obj (to Radeon Gallium drivers at least) is to be able to wait
 for the last use of a buffer. Whether the contents can or cannot be
 available to the CPU is totally irrelevant.

 Currently (and it's a very important performance optimization),
 buffers stay mapped and available for CPU read/write during their
 first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
 on buffer destruction. We only call bo_wait when we want to wait for
 the GPU until it's done with the buffer (we don't always want that,
 sometimes we want to use the unsychronized flag). Otherwise the
 contents of buffers are available at *any time*.

 We could probably implement bo_wait privately in the kernel driver and
 not use ttm_bo_wait. I preferred code sharing though.

 Textures (especially the tiled ones) are never mapped directly and a
 temporary staging resource is used instead, so we don't actually
 pollute address space that much. (in case you would have such a
 remark) We will use staging resources for buffers too, but it's really
 the last resort to avoid waiting when direct access can't be used.




 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
 difference between those two? I think we should remove the
 write_sync_obj
 bo
 member.



 Okay, but I think we should remove sync_obj instead, and keep write
 and read sync objs. In the case of READWRITE usage, read_sync_obj
 would be equal to write_sync_obj.




 Sure, I'm fine with that.

 One other thing, though, that makes me a little puzzled:

 Let's assume you don't allow readers and writers at the same time, which
 is
 my perception of how read- and write fences should work; you either have
 a
 list of read fences or a single write fence (in the same way a read-write
 lock works).

 Now, if you only allow a single read fence, like in this patch. That
 implies
 that you can only have either a single read fence or a single write fence
 at
 any one time. We'd only need a single fence pointer on the bo, and
 sync_obj_arg would tell us whether to signal the fence for read or for
 write
 (assuming that sync_obj_arg was set up to indicate read / write at
 validation time), then the patch really isn't necessary at all, as it
 only
 allows a single read fence?

 Or is it that you want to allow read- and write fences co-existing? In
 that
 case, what's the use case?


 There are lots of read-write use cases which don't need any barriers
 or flushing. The obvious ones are color blending and depth-stencil
 buffering. The OpenGL application is also allowed to use a subrange of
 a buffer as a vertex buffer (read-only) and another disjoint subrange
 of the same buffer for transform feedback (write-only), which kinda
 makes me think about whether we should track subranges instead of
 treating a whole buffer as busy. It gets even more funky with
 ARB_shader_image_load_store, which supports atomic read-modify-write
 operations on textures, not to mention atomic memory operations in
 compute shaders (wait, isn't that also exposed in GL as
 GL_ARB_shader_atomic_counters?).

 I was thinking whether the two sync objs should be read and
 readwrite, or read and write. I chose the latter, because it's
 more fine-grained and we have to keep at least two of them around
 anyway.

 So now that you know what we use sync objs for, what are your ideas on
 re-implementing that patch in a way that is okay with you? Besides
 removing the third sync objs of course.

 Marek


 OK. First I think we need to make a distinction: bo sync objects vs driver
 fences. The bo sync obj api is there to strictly provide functionality that
 the ttm bo subsystem is using, and that follows a simple set of rules:

 1) the bo subsystem does never assume sync objects are ordered. That means
 the bo subsystem needs to wait on a sync object before removing it from a
 buffer. Any other assumption is buggy and must be fixed. BUT, if that
 assumption takes place in the driver unknowingly from the ttm bo subsystem
 (which is usually the case), it's OK.

 2) When the sync object(s) attached to the bo are signaled the ttm bo
 subsystem is free to copy the bo contents and to unbind the bo.

 3) The ttm bo system allows sync objects to be signaled in different ways
 opaque to the subsystem using sync_obj_arg. The driver is responsible for
 setting up that argument.

 4) Driver fences may be used for or expose 

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Jerome Glisse]

On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom tho...@shipmail.org wrote:
 On 10/07/2011 12:42 AM, Marek Olšák wrote:

 On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstromtho...@shipmail.org
  wrote:


 In any case, I'm not saying fences is the best way to flush but since the
 bo
 code assumes that signaling a sync object means make the buffer contents
 available for CPU read / write, it's usually a good way to do it;
 there's
 even a sync_obj_flush() method that gets called when a potential flush
 needs
 to happen.


 I don't think we use it like that. To my knowledge, the purpose of the
 sync obj (to Radeon Gallium drivers at least) is to be able to wait
 for the last use of a buffer. Whether the contents can or cannot be
 available to the CPU is totally irrelevant.

 Currently (and it's a very important performance optimization),
 buffers stay mapped and available for CPU read/write during their
 first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
 on buffer destruction. We only call bo_wait when we want to wait for
 the GPU until it's done with the buffer (we don't always want that,
 sometimes we want to use the unsychronized flag). Otherwise the
 contents of buffers are available at *any time*.

 We could probably implement bo_wait privately in the kernel driver and
 not use ttm_bo_wait. I preferred code sharing though.

 Textures (especially the tiled ones) are never mapped directly and a
 temporary staging resource is used instead, so we don't actually
 pollute address space that much. (in case you would have such a
 remark) We will use staging resources for buffers too, but it's really
 the last resort to avoid waiting when direct access can't be used.




 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
 difference between those two? I think we should remove the
 write_sync_obj
 bo
 member.



 Okay, but I think we should remove sync_obj instead, and keep write
 and read sync objs. In the case of READWRITE usage, read_sync_obj
 would be equal to write_sync_obj.




 Sure, I'm fine with that.

 One other thing, though, that makes me a little puzzled:

 Let's assume you don't allow readers and writers at the same time, which
 is
 my perception of how read- and write fences should work; you either have
 a
 list of read fences or a single write fence (in the same way a read-write
 lock works).

 Now, if you only allow a single read fence, like in this patch. That
 implies
 that you can only have either a single read fence or a single write fence
 at
 any one time. We'd only need a single fence pointer on the bo, and
 sync_obj_arg would tell us whether to signal the fence for read or for
 write
 (assuming that sync_obj_arg was set up to indicate read / write at
 validation time), then the patch really isn't necessary at all, as it
 only
 allows a single read fence?

 Or is it that you want to allow read- and write fences co-existing? In
 that
 case, what's the use case?


 There are lots of read-write use cases which don't need any barriers
 or flushing. The obvious ones are color blending and depth-stencil
 buffering. The OpenGL application is also allowed to use a subrange of
 a buffer as a vertex buffer (read-only) and another disjoint subrange
 of the same buffer for transform feedback (write-only), which kinda
 makes me think about whether we should track subranges instead of
 treating a whole buffer as busy. It gets even more funky with
 ARB_shader_image_load_store, which supports atomic read-modify-write
 operations on textures, not to mention atomic memory operations in
 compute shaders (wait, isn't that also exposed in GL as
 GL_ARB_shader_atomic_counters?).

 I was thinking whether the two sync objs should be read and
 readwrite, or read and write. I chose the latter, because it's
 more fine-grained and we have to keep at least two of them around
 anyway.

 So now that you know what we use sync objs for, what are your ideas on
 re-implementing that patch in a way that is okay with you? Besides
 removing the third sync objs of course.

 Marek


 OK. First I think we need to make a distinction: bo sync objects vs driver
 fences. The bo sync obj api is there to strictly provide functionality that
 the ttm bo subsystem is using, and that follows a simple set of rules:

 1) the bo subsystem does never assume sync objects are ordered. That means
 the bo subsystem needs to wait on a sync object before removing it from a
 buffer. Any other assumption is buggy and must be fixed. BUT, if that
 assumption takes place in the driver unknowingly from the ttm bo subsystem
 (which is usually the case), it's OK.

 2) When the sync object(s) attached to the bo are signaled the ttm bo
 subsystem is free to copy the bo contents and to unbind the bo.

 3) The ttm bo system allows sync objects to be signaled in different ways
 opaque to the subsystem using sync_obj_arg. The driver is responsible for
 setting up that argument.

 4) Driver fences may be used for or expose 

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Jerome Glisse]

On Fri, Oct 7, 2011 at 9:38 AM, Jerome Glisse j.gli...@gmail.com wrote:
 On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstrom tho...@shipmail.org wrote:
 On 10/07/2011 12:42 AM, Marek Olšák wrote:

 On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstromtho...@shipmail.org
  wrote:


 In any case, I'm not saying fences is the best way to flush but since the
 bo
 code assumes that signaling a sync object means make the buffer contents
 available for CPU read / write, it's usually a good way to do it;
 there's
 even a sync_obj_flush() method that gets called when a potential flush
 needs
 to happen.


 I don't think we use it like that. To my knowledge, the purpose of the
 sync obj (to Radeon Gallium drivers at least) is to be able to wait
 for the last use of a buffer. Whether the contents can or cannot be
 available to the CPU is totally irrelevant.

 Currently (and it's a very important performance optimization),
 buffers stay mapped and available for CPU read/write during their
 first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
 on buffer destruction. We only call bo_wait when we want to wait for
 the GPU until it's done with the buffer (we don't always want that,
 sometimes we want to use the unsychronized flag). Otherwise the
 contents of buffers are available at *any time*.

 We could probably implement bo_wait privately in the kernel driver and
 not use ttm_bo_wait. I preferred code sharing though.

 Textures (especially the tiled ones) are never mapped directly and a
 temporary staging resource is used instead, so we don't actually
 pollute address space that much. (in case you would have such a
 remark) We will use staging resources for buffers too, but it's really
 the last resort to avoid waiting when direct access can't be used.




 2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
 difference between those two? I think we should remove the
 write_sync_obj
 bo
 member.



 Okay, but I think we should remove sync_obj instead, and keep write
 and read sync objs. In the case of READWRITE usage, read_sync_obj
 would be equal to write_sync_obj.




 Sure, I'm fine with that.

 One other thing, though, that makes me a little puzzled:

 Let's assume you don't allow readers and writers at the same time, which
 is
 my perception of how read- and write fences should work; you either have
 a
 list of read fences or a single write fence (in the same way a read-write
 lock works).

 Now, if you only allow a single read fence, like in this patch. That
 implies
 that you can only have either a single read fence or a single write fence
 at
 any one time. We'd only need a single fence pointer on the bo, and
 sync_obj_arg would tell us whether to signal the fence for read or for
 write
 (assuming that sync_obj_arg was set up to indicate read / write at
 validation time), then the patch really isn't necessary at all, as it
 only
 allows a single read fence?

 Or is it that you want to allow read- and write fences co-existing? In
 that
 case, what's the use case?


 There are lots of read-write use cases which don't need any barriers
 or flushing. The obvious ones are color blending and depth-stencil
 buffering. The OpenGL application is also allowed to use a subrange of
 a buffer as a vertex buffer (read-only) and another disjoint subrange
 of the same buffer for transform feedback (write-only), which kinda
 makes me think about whether we should track subranges instead of
 treating a whole buffer as busy. It gets even more funky with
 ARB_shader_image_load_store, which supports atomic read-modify-write
 operations on textures, not to mention atomic memory operations in
 compute shaders (wait, isn't that also exposed in GL as
 GL_ARB_shader_atomic_counters?).

 I was thinking whether the two sync objs should be read and
 readwrite, or read and write. I chose the latter, because it's
 more fine-grained and we have to keep at least two of them around
 anyway.

 So now that you know what we use sync objs for, what are your ideas on
 re-implementing that patch in a way that is okay with you? Besides
 removing the third sync objs of course.

 Marek


 OK. First I think we need to make a distinction: bo sync objects vs driver
 fences. The bo sync obj api is there to strictly provide functionality that
 the ttm bo subsystem is using, and that follows a simple set of rules:

 1) the bo subsystem does never assume sync objects are ordered. That means
 the bo subsystem needs to wait on a sync object before removing it from a
 buffer. Any other assumption is buggy and must be fixed. BUT, if that
 assumption takes place in the driver unknowingly from the ttm bo subsystem
 (which is usually the case), it's OK.

 2) When the sync object(s) attached to the bo are signaled the ttm bo
 subsystem is free to copy the bo contents and to unbind the bo.

 3) The ttm bo system allows sync objects to be signaled in different ways
 opaque to the subsystem using sync_obj_arg. The driver is responsible for
 

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Thomas Hellstrom]

On 10/07/2011 03:24 PM, Alex Deucher wrote:

On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstromtho...@shipmail.org  wrote:
   

On 10/07/2011 12:42 AM, Marek Olšák wrote:
 

On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstromtho...@shipmail.org
  wrote:

   

In any case, I'm not saying fences is the best way to flush but since the
bo
code assumes that signaling a sync object means make the buffer contents
available for CPU read / write, it's usually a good way to do it;
there's
even a sync_obj_flush() method that gets called when a potential flush
needs
to happen.

 

I don't think we use it like that. To my knowledge, the purpose of the
sync obj (to Radeon Gallium drivers at least) is to be able to wait
for the last use of a buffer. Whether the contents can or cannot be
available to the CPU is totally irrelevant.

Currently (and it's a very important performance optimization),
buffers stay mapped and available for CPU read/write during their
first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
on buffer destruction. We only call bo_wait when we want to wait for
the GPU until it's done with the buffer (we don't always want that,
sometimes we want to use the unsychronized flag). Otherwise the
contents of buffers are available at *any time*.

We could probably implement bo_wait privately in the kernel driver and
not use ttm_bo_wait. I preferred code sharing though.

Textures (especially the tiled ones) are never mapped directly and a
temporary staging resource is used instead, so we don't actually
pollute address space that much. (in case you would have such a
remark) We will use staging resources for buffers too, but it's really
the last resort to avoid waiting when direct access can't be used.



   

2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
difference between those two? I think we should remove the
write_sync_obj
bo
member.


 

Okay, but I think we should remove sync_obj instead, and keep write
and read sync objs. In the case of READWRITE usage, read_sync_obj
would be equal to write_sync_obj.



   

Sure, I'm fine with that.

One other thing, though, that makes me a little puzzled:

Let's assume you don't allow readers and writers at the same time, which
is
my perception of how read- and write fences should work; you either have
a
list of read fences or a single write fence (in the same way a read-write
lock works).

Now, if you only allow a single read fence, like in this patch. That
implies
that you can only have either a single read fence or a single write fence
at
any one time. We'd only need a single fence pointer on the bo, and
sync_obj_arg would tell us whether to signal the fence for read or for
write
(assuming that sync_obj_arg was set up to indicate read / write at
validation time), then the patch really isn't necessary at all, as it
only
allows a single read fence?

Or is it that you want to allow read- and write fences co-existing? In
that
case, what's the use case?

 

There are lots of read-write use cases which don't need any barriers
or flushing. The obvious ones are color blending and depth-stencil
buffering. The OpenGL application is also allowed to use a subrange of
a buffer as a vertex buffer (read-only) and another disjoint subrange
of the same buffer for transform feedback (write-only), which kinda
makes me think about whether we should track subranges instead of
treating a whole buffer as busy. It gets even more funky with
ARB_shader_image_load_store, which supports atomic read-modify-write
operations on textures, not to mention atomic memory operations in
compute shaders (wait, isn't that also exposed in GL as
GL_ARB_shader_atomic_counters?).

I was thinking whether the two sync objs should be read and
readwrite, or read and write. I chose the latter, because it's
more fine-grained and we have to keep at least two of them around
anyway.

So now that you know what we use sync objs for, what are your ideas on
re-implementing that patch in a way that is okay with you? Besides
removing the third sync objs of course.

Marek

   

OK. First I think we need to make a distinction: bo sync objects vs driver
fences. The bo sync obj api is there to strictly provide functionality that
the ttm bo subsystem is using, and that follows a simple set of rules:

1) the bo subsystem does never assume sync objects are ordered. That means
the bo subsystem needs to wait on a sync object before removing it from a
buffer. Any other assumption is buggy and must be fixed. BUT, if that
assumption takes place in the driver unknowingly from the ttm bo subsystem
(which is usually the case), it's OK.

2) When the sync object(s) attached to the bo are signaled the ttm bo
subsystem is free to copy the bo contents and to unbind the bo.

3) The ttm bo system allows sync objects to be signaled in different ways
opaque to the subsystem using sync_obj_arg. The driver is responsible for
setting up that argument.

4) Driver 

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Thomas Hellstrom]

On 10/07/2011 03:38 PM, Jerome Glisse wrote:

On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstromtho...@shipmail.org  wrote:
   

On 10/07/2011 12:42 AM, Marek Olšák wrote:
 

On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstromtho...@shipmail.org
  wrote:

   

In any case, I'm not saying fences is the best way to flush but since the
bo
code assumes that signaling a sync object means make the buffer contents
available for CPU read / write, it's usually a good way to do it;
there's
even a sync_obj_flush() method that gets called when a potential flush
needs
to happen.

 

I don't think we use it like that. To my knowledge, the purpose of the
sync obj (to Radeon Gallium drivers at least) is to be able to wait
for the last use of a buffer. Whether the contents can or cannot be
available to the CPU is totally irrelevant.

Currently (and it's a very important performance optimization),
buffers stay mapped and available for CPU read/write during their
first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
on buffer destruction. We only call bo_wait when we want to wait for
the GPU until it's done with the buffer (we don't always want that,
sometimes we want to use the unsychronized flag). Otherwise the
contents of buffers are available at *any time*.

We could probably implement bo_wait privately in the kernel driver and
not use ttm_bo_wait. I preferred code sharing though.

Textures (especially the tiled ones) are never mapped directly and a
temporary staging resource is used instead, so we don't actually
pollute address space that much. (in case you would have such a
remark) We will use staging resources for buffers too, but it's really
the last resort to avoid waiting when direct access can't be used.



   

2) Can't we say that a write_sync_obj is simply a sync_obj? What's the
difference between those two? I think we should remove the
write_sync_obj
bo
member.


 

Okay, but I think we should remove sync_obj instead, and keep write
and read sync objs. In the case of READWRITE usage, read_sync_obj
would be equal to write_sync_obj.



   

Sure, I'm fine with that.

One other thing, though, that makes me a little puzzled:

Let's assume you don't allow readers and writers at the same time, which
is
my perception of how read- and write fences should work; you either have
a
list of read fences or a single write fence (in the same way a read-write
lock works).

Now, if you only allow a single read fence, like in this patch. That
implies
that you can only have either a single read fence or a single write fence
at
any one time. We'd only need a single fence pointer on the bo, and
sync_obj_arg would tell us whether to signal the fence for read or for
write
(assuming that sync_obj_arg was set up to indicate read / write at
validation time), then the patch really isn't necessary at all, as it
only
allows a single read fence?

Or is it that you want to allow read- and write fences co-existing? In
that
case, what's the use case?

 

There are lots of read-write use cases which don't need any barriers
or flushing. The obvious ones are color blending and depth-stencil
buffering. The OpenGL application is also allowed to use a subrange of
a buffer as a vertex buffer (read-only) and another disjoint subrange
of the same buffer for transform feedback (write-only), which kinda
makes me think about whether we should track subranges instead of
treating a whole buffer as busy. It gets even more funky with
ARB_shader_image_load_store, which supports atomic read-modify-write
operations on textures, not to mention atomic memory operations in
compute shaders (wait, isn't that also exposed in GL as
GL_ARB_shader_atomic_counters?).

I was thinking whether the two sync objs should be read and
readwrite, or read and write. I chose the latter, because it's
more fine-grained and we have to keep at least two of them around
anyway.

So now that you know what we use sync objs for, what are your ideas on
re-implementing that patch in a way that is okay with you? Besides
removing the third sync objs of course.

Marek

   

OK. First I think we need to make a distinction: bo sync objects vs driver
fences. The bo sync obj api is there to strictly provide functionality that
the ttm bo subsystem is using, and that follows a simple set of rules:

1) the bo subsystem does never assume sync objects are ordered. That means
the bo subsystem needs to wait on a sync object before removing it from a
buffer. Any other assumption is buggy and must be fixed. BUT, if that
assumption takes place in the driver unknowingly from the ttm bo subsystem
(which is usually the case), it's OK.

2) When the sync object(s) attached to the bo are signaled the ttm bo
subsystem is free to copy the bo contents and to unbind the bo.

3) The ttm bo system allows sync objects to be signaled in different ways
opaque to the subsystem using sync_obj_arg. The driver is responsible for
setting up that argument.

4) Driver 

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Alex Deucher]

On Fri, Oct 7, 2011 at 10:05 AM, Thomas Hellstrom tho...@shipmail.org wrote:
 On 10/07/2011 03:24 PM, Alex Deucher wrote:

 On Fri, Oct 7, 2011 at 4:00 AM, Thomas Hellstromtho...@shipmail.org
  wrote:


 On 10/07/2011 12:42 AM, Marek Olšák wrote:


 On Wed, Oct 5, 2011 at 7:54 AM, Thomas Hellstromtho...@shipmail.org
  wrote:



 In any case, I'm not saying fences is the best way to flush but since
 the
 bo
 code assumes that signaling a sync object means make the buffer
 contents
 available for CPU read / write, it's usually a good way to do it;
 there's
 even a sync_obj_flush() method that gets called when a potential flush
 needs
 to happen.



 I don't think we use it like that. To my knowledge, the purpose of the
 sync obj (to Radeon Gallium drivers at least) is to be able to wait
 for the last use of a buffer. Whether the contents can or cannot be
 available to the CPU is totally irrelevant.

 Currently (and it's a very important performance optimization),
 buffers stay mapped and available for CPU read/write during their
 first map_buffer call. Unmap_buffer is a no-op. The unmapping happens
 on buffer destruction. We only call bo_wait when we want to wait for
 the GPU until it's done with the buffer (we don't always want that,
 sometimes we want to use the unsychronized flag). Otherwise the
 contents of buffers are available at *any time*.

 We could probably implement bo_wait privately in the kernel driver and
 not use ttm_bo_wait. I preferred code sharing though.

 Textures (especially the tiled ones) are never mapped directly and a
 temporary staging resource is used instead, so we don't actually
 pollute address space that much. (in case you would have such a
 remark) We will use staging resources for buffers too, but it's really
 the last resort to avoid waiting when direct access can't be used.





 2) Can't we say that a write_sync_obj is simply a sync_obj? What's
 the
 difference between those two? I think we should remove the
 write_sync_obj
 bo
 member.




 Okay, but I think we should remove sync_obj instead, and keep write
 and read sync objs. In the case of READWRITE usage, read_sync_obj
 would be equal to write_sync_obj.





 Sure, I'm fine with that.

 One other thing, though, that makes me a little puzzled:

 Let's assume you don't allow readers and writers at the same time,
 which
 is
 my perception of how read- and write fences should work; you either
 have
 a
 list of read fences or a single write fence (in the same way a
 read-write
 lock works).

 Now, if you only allow a single read fence, like in this patch. That
 implies
 that you can only have either a single read fence or a single write
 fence
 at
 any one time. We'd only need a single fence pointer on the bo, and
 sync_obj_arg would tell us whether to signal the fence for read or for
 write
 (assuming that sync_obj_arg was set up to indicate read / write at
 validation time), then the patch really isn't necessary at all, as it
 only
 allows a single read fence?

 Or is it that you want to allow read- and write fences co-existing? In
 that
 case, what's the use case?



 There are lots of read-write use cases which don't need any barriers
 or flushing. The obvious ones are color blending and depth-stencil
 buffering. The OpenGL application is also allowed to use a subrange of
 a buffer as a vertex buffer (read-only) and another disjoint subrange
 of the same buffer for transform feedback (write-only), which kinda
 makes me think about whether we should track subranges instead of
 treating a whole buffer as busy. It gets even more funky with
 ARB_shader_image_load_store, which supports atomic read-modify-write
 operations on textures, not to mention atomic memory operations in
 compute shaders (wait, isn't that also exposed in GL as
 GL_ARB_shader_atomic_counters?).

 I was thinking whether the two sync objs should be read and
 readwrite, or read and write. I chose the latter, because it's
 more fine-grained and we have to keep at least two of them around
 anyway.

 So now that you know what we use sync objs for, what are your ideas on
 re-implementing that patch in a way that is okay with you? Besides
 removing the third sync objs of course.

 Marek



 OK. First I think we need to make a distinction: bo sync objects vs
 driver
 fences. The bo sync obj api is there to strictly provide functionality
 that
 the ttm bo subsystem is using, and that follows a simple set of rules:

 1) the bo subsystem does never assume sync objects are ordered. That
 means
 the bo subsystem needs to wait on a sync object before removing it from a
 buffer. Any other assumption is buggy and must be fixed. BUT, if that
 assumption takes place in the driver unknowingly from the ttm bo
 subsystem
 (which is usually the case), it's OK.

 2) When the sync object(s) attached to the bo are signaled the ttm bo
 subsystem is free to copy the bo contents and to unbind the bo.

 3) The ttm bo system allows sync objects to be signaled in different ways
 

[Bug 41561] New: Hotplug detect does not work for HDMI monitor on Fusion E-350 board

[bugzilla-daemon]

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

   Summary: Hotplug detect does not work for HDMI monitor on
Fusion E-350 board
   Product: DRI
   Version: unspecified
  Platform: x86 (IA32)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/Radeon
AssignedTo: dri-devel@lists.freedesktop.org
ReportedBy: simon.farnswo...@onelan.co.uk


Using airlied-drm-fixes kernel as of commit
6777a4f6898a53974ef7fe7ce09ec41fae0f32db with Alex Deucher's patch
drm/radeon/kms: use hardcoded dig encoder to transmitter mapping for DCE4.1
on top, I'm not seeing uevents from the kernel when I plug and unplug a HDMI
monitor.

dmesg with drm.debug=0xf shows me the following when I connect a monitor:

# dmesg -c
[15202.939581] [drm:evergreen_irq_process], r600_irq_process start: rptr 5872,
wptr 5888
[15202.939609] [drm:evergreen_irq_process], IH: HPD2
[15203.050597] [drm:radeon_atombios_connected_scratch_regs], DFP1 connected
[15203.050605] [drm:output_poll_execute], [CONNECTOR:15:HDMI-A-1] status
updated from 1 to 1
[15203.052661] [drm:output_poll_execute], [CONNECTOR:17:VGA-1] status updated
from 2 to 2

radeonreg suggests that the HPD sense bit is correctly set

# ./radeonreg regs all | grep 6028 # DC_HPD2_INT_STATUS
6028ff0ff012 (-15732718)

When I remove the monitor, I get the following from dmesg and radeonreg:

# dmesg -c
[15307.075271] [drm:evergreen_irq_process], r600_irq_process start: rptr 5888,
wptr 5904
[15307.075300] [drm:evergreen_irq_process], IH: HPD2
[15307.131727] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid,
remainder is 192
[15307.131733] Raw EDID:
[15307.131738]  3f ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131742]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131745]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131749]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131752]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131756]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131759]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.131763]  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[15307.141965] [drm:radeon_dvi_detect] *ERROR* HDMI-A-1: probed a monitor but
no|invalid EDID
[15307.141975] [drm:radeon_atombios_connected_scratch_regs], DFP1 connected
[15307.141981] [drm:output_poll_execute], [CONNECTOR:15:HDMI-A-1] status
updated from 1 to 1
[15307.144028] [drm:output_poll_execute], [CONNECTOR:17:VGA-1] status updated
from 2 to 2

# ./radeonreg regs all | grep 6028 # DC_HPD2_INT_STATUS
6028ff0ff000 (-15732736)

suggesting that HPD sense bits are being updated correctly by the hardware, but
that this isn't resulting in uevents following through.

-- 
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 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

[bugzilla-daemon]

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

Simon Farnsworth simon.farnswo...@onelan.co.uk changed:

   What|Removed |Added

Summary|Hotplug detect does not |[r600 KMS] Hotplug detect
   |work for HDMI monitor on|does not work for HDMI
   |Fusion E-350 board  |monitor on Fusion E-350
   ||board

-- 
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 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

[bugzilla-daemon]

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

--- Comment #1 from Alex Deucher ag...@yahoo.com 2011-10-07 08:59:22 PDT ---
If you are getting HPD interrupts, radeon_hotplug_work_func() should be getting
scheduled.  From there drm_helper_hpd_irq_event() gets called which actually
generates the uevent.

-- 
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 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

[bugzilla-daemon]

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

--- Comment #2 from Simon Farnsworth simon.farnswo...@onelan.co.uk 2011-10-07 
09:07:40 PDT ---
drm_helper_hpd_irq_event() isn't generating uevents, because connector-status
is not being updated somewhere, and is remaining at connector_status_connected,
when it should be being updated to connector_status_disconnected.

If I do cat /sys/class/drm/card0-HDMI-A-1/status, I see the status is
disconnected, but something resets it by the time the helper comes back
round. Possibly connected to the following from dmesg:

# cat /sys/class/drm/card0-HDMI-A-1/status ; dmesg -c
[16791.042128] [drm:radeon_atombios_connected_scratch_regs], DFP1 disconnected

# sleep 10 ; dmesg -c
[16799.522394] [drm:radeon_atombios_connected_scratch_regs], DFP1 connected
[16799.522407] [drm:output_poll_execute], [CONNECTOR:15:HDMI-A-1] status
updated from 1 to 1
[16799.524465] [drm:output_poll_execute], [CONNECTOR:17:VGA-1] status updated
from 2 to 2

-- 
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 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

[bugzilla-daemon]

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

--- Comment #6 from Simon Farnsworth simon.farnswo...@onelan.co.uk 2011-10-07 
10:53:04 PDT ---
I've applied both patches, and am testing on a machine with DVI-I and
DisplayPort.

Test sequence:

1) Boot system with no outputs connected.
2) Attach a DVI-D monitor, then remove it.
3) Attach a DVI-A to VGA adapter that has no load, then remove it.
4) Attach a DVI-A to VGA adapter that has load but no DDC, then remove it.
5) Attach a DVI-A to VGA adapter that has load and DDC, then remove it.
6) Repeat steps 2 to 5, watching for uevents.

Note that the DVI-A to VGA adapter asserts HPD when connected.

Results:

DVI-D hotplug works fine. There is a delay on the first DVI-D hotplug after the
connector has been used for VGA, but I get an event within 10 seconds.

No events when I plug in the adapter with no load and wait 20 seconds.

No events when I plug in the adapter with load but not DDC and wait 20 seconds.

Events on connect only when I plug in the adapter with DDC.

-- 
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 40252] No udev event for disconnecting VGA/HDMI cable

[bugzilla-daemon]

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

Alex Deucher ag...@yahoo.com changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||DUPLICATE

--- Comment #6 from Alex Deucher ag...@yahoo.com 2011-10-07 11:14:49 PDT ---


*** This bug has been marked as a duplicate of bug 41561 ***

-- 
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 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

[bugzilla-daemon]

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

Alex Deucher ag...@yahoo.com changed:

   What|Removed |Added

 CC||q...@gmx.de

--- Comment #7 from Alex Deucher ag...@yahoo.com 2011-10-07 11:14:49 PDT ---
*** Bug 40252 has been marked as a duplicate of this bug. ***

-- 
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 41561] [r600 KMS] Hotplug detect does not work for HDMI monitor on Fusion E-350 board

[bugzilla-daemon]

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

--- Comment #8 from Alex Deucher ag...@yahoo.com 2011-10-07 11:16:14 PDT ---
(In reply to comment #6)
 Results:
 
 DVI-D hotplug works fine. There is a delay on the first DVI-D hotplug after 
 the
 connector has been used for VGA, but I get an event within 10 seconds.

good.

 
 No events when I plug in the adapter with no load and wait 20 seconds.
 

As expected.

 No events when I plug in the adapter with load but not DDC and wait 20 
 seconds.
 

There's no way to get events for this case without doing load detection in the
hotplug polling.  We don't currently support that case as load detection has
the potential to cause flicker on connectors that share resources.  If you need
it, you can remove the !force checks in the connector detect functions, but you
may get flicker on other connectors if they share resources.

 Events on connect only when I plug in the adapter with DDC.

As expected.

-- 
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

[PATCH 1/2] drm/radeon/kms: bail early in dvi_detect for digital only connectors

[alexdeucher]

From: Alex Deucher alexander.deuc...@amd.com

DVI-D and HDMI-A are digital only, so there's no need to
attempt analog load detect.  Also, skip bail before the
!force check, or we fail to get a disconnect events.
The next patches in the series attempt to fix disconnect
events for connectors with analog support (DVI-I, HDMI-B,
DVI-A).

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

Signed-off-by: Alex Deucher alexander.deuc...@amd.com
Cc: sta...@kernel.org
---
 drivers/gpu/drm/radeon/radeon_connectors.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 449c3d8..103eb1b 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -959,6 +959,11 @@ radeon_dvi_detect(struct drm_connector *connector, bool 
force)
if ((ret == connector_status_connected)  
(radeon_connector-use_digital == true))
goto out;
 
+   /* DVI-D and HDMI-A are digital only */
+   if ((connector-connector_type == DRM_MODE_CONNECTOR_DVID) ||
+   (connector-connector_type == DRM_MODE_CONNECTOR_HDMIA))
+   goto out;
+
if (!force) {
ret = connector-status;
goto out;
-- 
1.7.1.1

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

[PATCH 2/2] drm/radeon/kms: handle !force case in connector detect more gracefully

[alexdeucher]

From: Alex Deucher alexander.deuc...@amd.com

When force == false, we don't do load detection in the connector
detect functions.  Unforunately, we also return the previous
connector state so we never get disconnect events for DVI-I, DVI-A,
or VGA.  Save whether we detected the monitor via load detection
previously and use that to determine whether we return the previous
state or not.

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

Signed-off-by: Alex Deucher alexander.deuc...@amd.com
Cc: sta...@kernel.org
---
 drivers/gpu/drm/radeon/radeon_connectors.c |   23 ---
 drivers/gpu/drm/radeon/radeon_mode.h   |1 +
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 103eb1b..dec6cbe 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -724,6 +724,7 @@ radeon_vga_detect(struct drm_connector *connector, bool 
force)
dret = radeon_ddc_probe(radeon_connector,

radeon_connector-requires_extended_probe);
if (dret) {
+   radeon_connector-detected_by_load = false;
if (radeon_connector-edid) {
kfree(radeon_connector-edid);
radeon_connector-edid = NULL;
@@ -750,12 +751,21 @@ radeon_vga_detect(struct drm_connector *connector, bool 
force)
} else {
 
/* if we aren't forcing don't do destructive polling */
-   if (!force)
-   return connector-status;
+   if (!force) {
+   /* only return the previous status if we last
+* detected a monitor via load.
+*/
+   if (radeon_connector-detected_by_load)
+   return connector-status;
+   else
+   return ret;
+   }
 
if (radeon_connector-dac_load_detect  encoder) {
encoder_funcs = encoder-helper_private;
ret = encoder_funcs-detect(encoder, connector);
+   if (ret == connector_status_connected)
+   radeon_connector-detected_by_load = true;
}
}
 
@@ -897,6 +907,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool 
force)
dret = radeon_ddc_probe(radeon_connector,

radeon_connector-requires_extended_probe);
if (dret) {
+   radeon_connector-detected_by_load = false;
if (radeon_connector-edid) {
kfree(radeon_connector-edid);
radeon_connector-edid = NULL;
@@ -964,8 +975,13 @@ radeon_dvi_detect(struct drm_connector *connector, bool 
force)
(connector-connector_type == DRM_MODE_CONNECTOR_HDMIA))
goto out;
 
+   /* if we aren't forcing don't do destructive polling */
if (!force) {
-   ret = connector-status;
+   /* only return the previous status if we last
+* detected a monitor via load.
+*/
+   if (radeon_connector-detected_by_load)
+   ret = connector-status;
goto out;
}
 
@@ -989,6 +1005,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool 
force)
ret = encoder_funcs-detect(encoder, 
connector);
if (ret == connector_status_connected) {
radeon_connector-use_digital = 
false;
+   
radeon_connector-detected_by_load = true;
}
}
break;
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h 
b/drivers/gpu/drm/radeon/radeon_mode.h
index 68820f5..ed0178f 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -447,6 +447,7 @@ struct radeon_connector {
struct edid *edid;
void *con_priv;
bool dac_load_detect;
+   bool detected_by_load; /* if the connection status was determined by 
load */
uint16_t connector_object_id;
struct radeon_hpd hpd;
struct radeon_router router;
-- 
1.7.1.1

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

[Bug 41569] New: [r600 KMS] Asus A35T A4-3400

[bugzilla-daemon]

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

   Summary: [r600 KMS] Asus A35T A4-3400
   Product: DRI
   Version: XOrg CVS
  Platform: x86-64 (AMD64)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/Radeon
AssignedTo: dri-devel@lists.freedesktop.org
ReportedBy: robertcnel...@gmail.com


This laptop, has only one standard video connector (VGA) along with the lcd
screen.  With 3.1-rc9+ plus airlied's drm-next on bootup bios init the screen,
kernel boots in text mode fine, but when KMS takes over the screen goes blank.
(the external VGA connector also goes blank). (running latest git of
xorg-ati/mesa)

interesting from dmesg;

[   32.534186] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid,
remainder is 104
[   32.534191] Raw EDID:
[   32.534194]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534196]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534198]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534199]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 70
[   32.534201]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534203]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534205]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.534207]  08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08
[   32.589038] [drm:radeon_dp_i2c_aux_ch] *ERROR* aux_ch invalid native reply
0x70

voodoo@a53t:~$ ls -lh /lib/firmware/radeon/TURKS_*
-rw-r--r-- 1 root root  24K 2011-10-06 22:26 /lib/firmware/radeon/TURKS_mc.bin
-rw-r--r-- 1 root root 5.4K 2011-10-06 22:26 /lib/firmware/radeon/TURKS_me.bin
-rw-r--r-- 1 root root 4.4K 2011-10-06 22:26 /lib/firmware/radeon/TURKS_pfp.bin

Regards,

-- 
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 41569] [r600 KMS] Asus A53T A4-3400

[bugzilla-daemon]

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

Robert Nelson robertcnel...@gmail.com changed:

   What|Removed |Added

Summary|[r600 KMS] Asus A35T|[r600 KMS] Asus A53T
   |A4-3400 |A4-3400

-- 
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 41569] [r600 KMS] Asus A53T A4-3400

[bugzilla-daemon]

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

--- Comment #1 from Robert Nelson robertcnel...@gmail.com 2011-10-07 11:25:48 
PDT ---
Created an attachment (id=52095)
 -- (https://bugs.freedesktop.org/attachment.cgi?id=52095)
dmesg

-- 
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 41569] [r600 KMS] Asus A53T A4-3400

[bugzilla-daemon]

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

--- Comment #2 from Robert Nelson robertcnel...@gmail.com 2011-10-07 11:26:04 
PDT ---
Created an attachment (id=52096)
 -- (https://bugs.freedesktop.org/attachment.cgi?id=52096)
Xorg log

-- 
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 41569] [r600 KMS] Asus A53T A4-3400

[bugzilla-daemon]

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

Alex Deucher ag...@yahoo.com changed:

   What|Removed |Added

  Attachment #52095|application/octet-stream|text/plain
  mime type||
  Attachment #52095|0   |1
   is patch||

-- 
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 41569] [r600 KMS] Asus A53T A4-3400

[bugzilla-daemon]

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

--- Comment #3 from Robert Nelson robertcnel...@gmail.com 2011-10-07 11:26:21 
PDT ---
Created an attachment (id=52097)
 -- (https://bugs.freedesktop.org/attachment.cgi?id=52097)
lspci

-- 
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

[PATCH] DRM: bug: RADEON_DEBUGFS_MAX_{NUM_FILES = COMPONENTS}

[Michael Witten]

Date: Fri, 16 Sep 2011 20:45:30 +

The value of RADEON_DEBUGFS_MAX_NUM_FILES has been used to
specify the size of an array, each element of which looks
like this:

  struct radeon_debugfs {
  struct drm_info_list*files;
  unsignednum_files;
  };

Consequently, the number of debugfs files may be much greater
than RADEON_DEBUGFS_MAX_NUM_FILES, something that the current
code ignores:

  if ((_radeon_debugfs_count + nfiles)  RADEON_DEBUGFS_MAX_NUM_FILES) {
  DRM_ERROR(Reached maximum number of debugfs files.\n);
  DRM_ERROR(Report so we increase RADEON_DEBUGFS_MAX_NUM_FILES.\n);
  return -EINVAL;
  }

This commit fixes this mistake, and accordingly renames:

  RADEON_DEBUGFS_MAX_NUM_FILES

to:

  RADEON_DEBUGFS_MAX_COMPONENTS

Signed-off-by: Michael Witten mfwit...@gmail.com
---
 drivers/gpu/drm/radeon/radeon.h|2 +-
 drivers/gpu/drm/radeon/radeon_device.c |   13 -
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index c1e056b..dd7bab9 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -102,7 +102,7 @@ extern int radeon_pcie_gen2;
 #define RADEON_FENCE_JIFFIES_TIMEOUT   (HZ / 2)
 /* RADEON_IB_POOL_SIZE must be a power of 2 */
 #define RADEON_IB_POOL_SIZE16
-#define RADEON_DEBUGFS_MAX_NUM_FILES   32
+#define RADEON_DEBUGFS_MAX_COMPONENTS  32
 #define RADEONFB_CONN_LIMIT4
 #define RADEON_BIOS_NUM_SCRATCH8
 
diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index b51e157..31b1f4b 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -981,7 +981,7 @@ struct radeon_debugfs {
struct drm_info_list*files;
unsignednum_files;
 };
-static struct radeon_debugfs _radeon_debugfs[RADEON_DEBUGFS_MAX_NUM_FILES];
+static struct radeon_debugfs _radeon_debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
 static unsigned _radeon_debugfs_count = 0;
 
 int radeon_debugfs_add_files(struct radeon_device *rdev,
@@ -996,14 +996,17 @@ int radeon_debugfs_add_files(struct radeon_device *rdev,
return 0;
}
}
-   if ((_radeon_debugfs_count + nfiles)  RADEON_DEBUGFS_MAX_NUM_FILES) {
-   DRM_ERROR(Reached maximum number of debugfs files.\n);
-   DRM_ERROR(Report so we increase 
RADEON_DEBUGFS_MAX_NUM_FILES.\n);
+
+   i = _radeon_debugfs_count + 1;
+   if (i  RADEON_DEBUGFS_MAX_COMPONENTS) {
+   DRM_ERROR(Reached maximum number of debugfs components.\n);
+   DRM_ERROR(Report so we increase 
+ RADEON_DEBUGFS_MAX_COMPONENTS.\n);
return -EINVAL;
}
_radeon_debugfs[_radeon_debugfs_count].files = files;
_radeon_debugfs[_radeon_debugfs_count].num_files = nfiles;
-   _radeon_debugfs_count++;
+   _radeon_debugfs_count = i;
 #if defined(CONFIG_DEBUG_FS)
drm_debugfs_create_files(files, nfiles,
 rdev-ddev-control-debugfs_root,
-- 
1.7.6.409.ge7a85

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

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Marek Olšák]

On Fri, Oct 7, 2011 at 3:38 PM, Jerome Glisse j.gli...@gmail.com wrote:
 I should have look at the patch long ago ... anyway i think a better
 approach would be to expose fence id as 64bits unsigned to each
 userspace client. I was thinking of mapping a page readonly (same page
 as the one gpu write back) at somespecific offset in drm file (bit
 like sarea but readonly so no lock). Each time userspace submit a
 command stream it would get the fence id associated with the command
 stream. It would then be up to userspace to track btw read or write
 and do appropriate things. The kernel code would be simple (biggest
 issue is finding an offset we can use for that), it would be fast as
 no round trip to kernel to know the last fence.

 Each fence seq would be valid only for a specific ring (only future
 gpu impacted here, maybe cayman).

 So no change to ttm, just change to radeon to return fence seq and to
 move to an unsigned 64. Issue would be when gpu write back is
 disabled, then we would either need userspace to call somethings like
 bo wait or to other ioctl to get the kernel to update the copy, copy
 would be updated in the irq handler too so at least it get updated
 anytime something enable irq.

I am having a hard time understanding what you are saying.

Anyway, I had some read and write usage tracking in the radeon winsys.
That worked well for driver-private resources, but it was a total fail
for the ones shared with the DDX. I needed this bo_wait optimization
to work with multiple processes. That's the whole point why I am doing
this in the kernel.

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

[PATCH 0/2] RFC: Potential improvements in edid detection timings (v2)

[Eugeni Dodonov]

(Resending with small improvements - also sending to dri-devel for feedback).

This is the the forth iteration of potential fixes for slow edid detection
issues over non-existent outputs
(https://bugs.freedesktop.org/show_bug.cgi?id=41059) - the previous versions
were posted to the bug and were used mostly for debugging the problem.

The problem I attempted to fix here is that some systems would take a very
long time trying to locate all the possible video outputs - in in cases
where such outputs were bogus, this lead to timing out after all the
possible delays were done.

After investigation, I came to think on two different ways to fix the issue:
 - The first patch does a one-line fix in drm_edid.c. I added a check for the
   return value of i2c_transfer - and, if it is -ENXIO, we give up on further
   attempts as the bus is not there. A drawback to this approach is that it
   affects all the devices out there which use drm_get_edid.  From my testing,
   the -ENXIO gave no false positives, but I haven't tested it on non-Intel
   cards. So I'd like to hear what other drm developers think about that.

 - The second patch does a similar procedure within the i915 driver, in case
   the proposed change to drm_edid.c won't be adequate for other drivers. It
   adds a new function - intel_drm_get_valid_edid - which attempts to do a
   simple i2c transfer over the bus prior to calling drm_get_edid. In case such
   transfer fails with -ENXIO, it is a signal that the bus is not there, so we
   shouldn't waste any time trying to communicate with it further.

Note that those patches provide lots of dmesg pollution - for the final
version those printk'ss should probably be removed, but I left them
intentionally with KERN_DEBUG in order to see when the adapters come and go
during the debugging and testing.

According to testing performed at
https://bugs.freedesktop.org/show_bug.cgi?id=41059, the following results
were obtained with those patches:

System1:
 (testing all possible outputs)
 ubuntu 3.0.0-12.19:   840ms
 3.0.0-12.19 +  drm patch: 290ms
 3.0.0-12.19 + i915 patch: 290ms
 (manually ignoring phantom outputs)
 ubuntu 3.0.0-12.19:   690ms
 3.0.0-12.19 +  drm patch: 140ms
 3.0.0-12.19 + i915 patch: 140ms

System2:
 (testing all possible outputs)
 ubuntu 3.0.0-12.19:   315ms
 3.0.0-12.19 +  drm patch: 280ms
 3.0.0-12.19 + i915 patch: 280ms
 (manually ignoring phantom outputs)
 ubuntu 3.0.0-12.19:   175ms
 3.0.0-12.19 +  drm patch: 140ms
 3.0.0-12.19 + i915 patch: 140ms



Eugeni Dodonov (2):
  Give up on edid retries when i2c tells us that bus is not there
  Check if the bus is valid prior to discovering edid.

 drivers/gpu/drm/drm_edid.c |5 +
 drivers/gpu/drm/i915/intel_dp.c|4 ++--
 drivers/gpu/drm/i915/intel_drv.h   |2 ++
 drivers/gpu/drm/i915/intel_hdmi.c  |4 ++--
 drivers/gpu/drm/i915/intel_i2c.c   |   33 +
 drivers/gpu/drm/i915/intel_lvds.c  |2 +-
 drivers/gpu/drm/i915/intel_modes.c |2 +-
 drivers/gpu/drm/i915/intel_sdvo.c  |4 ++--
 8 files changed, 48 insertions(+), 8 deletions(-)

-- 
1.7.6.3

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

[PATCH 1/2] Give up on edid retries when i2c tells us that bus is not there

[Eugeni Dodonov]

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.

As the disadvantage comes the fact that I only tested it on Intel cards,
so I am not sure whether it would work on nouveau and radeon.

This change should potentially fix
https://bugs.freedesktop.org/show_bug.cgi?id=41059

v2: change printk level to KERN_DEBUG to avoid filling up dmesg

Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
 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..5ed34f2 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) {
+   printk(KERN_DEBUG drm: skipping non-existent adapter 
%s\n,
+   adapter-name);
+   break;
+   }
} while (ret != 2  --retries);
 
return ret == 2 ? 0 : -1;
-- 
1.7.6.4

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

Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Marek Olšák]

On Fri, Oct 7, 2011 at 10:00 AM, Thomas Hellstrom tho...@shipmail.org wrote:
 OK. First I think we need to make a distinction: bo sync objects vs driver
 fences. The bo sync obj api is there to strictly provide functionality that
 the ttm bo subsystem is using, and that follows a simple set of rules:

 1) the bo subsystem does never assume sync objects are ordered. That means
 the bo subsystem needs to wait on a sync object before removing it from a
 buffer. Any other assumption is buggy and must be fixed. BUT, if that
 assumption takes place in the driver unknowingly from the ttm bo subsystem
 (which is usually the case), it's OK.

 2) When the sync object(s) attached to the bo are signaled the ttm bo
 subsystem is free to copy the bo contents and to unbind the bo.

 3) The ttm bo system allows sync objects to be signaled in different ways
 opaque to the subsystem using sync_obj_arg. The driver is responsible for
 setting up that argument.

 4) Driver fences may be used for or expose other functionality or adaptions
 to APIs as long as the sync obj api exported to the bo sybsystem follows the
 above rules.

 This means the following w r t the patch.

 A) it violates 1). This is a bug that must be fixed. Assumptions that if one
 sync object is singnaled, another sync object is also signaled must be done
 in the driver and not in the bo subsystem. Hence we need to explicitly wait
 for a fence to remove it from the bo.

 B) the sync_obj_arg carries *per-sync-obj* information on how it should be
 signaled. If we need to attach multiple sync objects to a buffer object, we
 also need multiple sync_obj_args. This is a bug and needs to be fixed.

 C) There is really only one reason that the ttm bo subsystem should care
 about multiple sync objects, and that is because the driver can't order them
 efficiently. A such example would be hardware with multiple pipes reading
 simultaneously from the same texture buffer. Currently we don't support this
 so only the *last* sync object needs to be know by the bo subsystem. Keeping
 track of multiple fences generates a lot of completely unnecessary code in
 the ttm_bo_util file, the ttm_bo_vm file, and will be a nightmare if / when
 we truly support pipelined moves.

 As I understand it from your patches, you want to keep multiple fences
 around only to track rendering history. If we want to do that generically, i
 suggest doing it in the execbuf util code in the following way:

 struct ttm_eu_rendering_history {
    void *last_read_sync_obj;
    void *last_read_sync_obj_arg;
    void *last_write_sync_obj;
    void *last_write_sync_obj_arg;
 }

 Embed this structure in the radeon_bo, and build a small api around it,
 including *optionally* passing it to the existing execbuf utilities, and you
 should be done. The bo_util code and bo_vm code doesn't care about the
 rendering history. Only that the bo is completely idle.

 Note also that when an accelerated bo move is scheduled, the driver needs to
 update this struct

OK, sounds good. I'll fix what should be fixed and send a patch when
it's ready. I am now not so sure whether doing this generically is a
good idea. :)

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

[PATCH 2/2] Check if the bus is valid prior to discovering edid.

[Eugeni Dodonov]

This adds a new function intel_drm_get_valid_edid, which is used instead
of drm_get_edid within the i915 driver.

It does a similar check to the one in previous patch, but it is limited to
i915 driver.

The check is similar to the first part of EDID discovery performed by the
drm_do_probe_ddc_edid. In case the i2c_transfer fails with the -ENXIO
result, we know that the i2c_algo_bit was unable to locate the hardware,
so we give up on further edid discovery.

They should also fix https://bugs.freedesktop.org/show_bug.cgi?id=41059

v2: change printk level to KERN_DEBUG to avoid filling up dmesg

Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
 drivers/gpu/drm/i915/intel_dp.c|4 ++--
 drivers/gpu/drm/i915/intel_drv.h   |2 ++
 drivers/gpu/drm/i915/intel_hdmi.c  |4 ++--
 drivers/gpu/drm/i915/intel_i2c.c   |   33 +
 drivers/gpu/drm/i915/intel_lvds.c  |2 +-
 drivers/gpu/drm/i915/intel_modes.c |2 +-
 drivers/gpu/drm/i915/intel_sdvo.c  |4 ++--
 7 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44fef5e..dd0d8b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1715,7 +1715,7 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
if (intel_dp-force_audio) {
intel_dp-has_audio = intel_dp-force_audio  0;
} else {
-   edid = drm_get_edid(connector, intel_dp-adapter);
+   edid = intel_drm_get_valid_edid(connector, intel_dp-adapter);
if (edid) {
intel_dp-has_audio = drm_detect_monitor_audio(edid);
connector-display_info.raw_edid = NULL;
@@ -1772,7 +1772,7 @@ intel_dp_detect_audio(struct drm_connector *connector)
struct edid *edid;
bool has_audio = false;
 
-   edid = drm_get_edid(connector, intel_dp-adapter);
+   edid = intel_drm_get_valid_edid(connector, intel_dp-adapter);
if (edid) {
has_audio = drm_detect_monitor_audio(edid);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fe1099d..d80a9b0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -264,6 +264,8 @@ struct intel_fbc_work {
int interval;
 };
 
+struct edid * intel_drm_get_valid_edid(struct drm_connector *connector,
+   struct i2c_adapter *adapter);
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 extern bool intel_ddc_probe(struct intel_encoder *intel_encoder, int ddc_bus);
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 226ba83..714f2fb 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -329,7 +329,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
force)
 
intel_hdmi-has_hdmi_sink = false;
intel_hdmi-has_audio = false;
-   edid = drm_get_edid(connector,
+   edid = intel_drm_get_valid_edid(connector,
dev_priv-gmbus[intel_hdmi-ddc_bus].adapter);
 
if (edid) {
@@ -371,7 +371,7 @@ intel_hdmi_detect_audio(struct drm_connector *connector)
struct edid *edid;
bool has_audio = false;
 
-   edid = drm_get_edid(connector,
+   edid = intel_drm_get_valid_edid(connector,
dev_priv-gmbus[intel_hdmi-ddc_bus].adapter);
if (edid) {
if (edid-input  DRM_EDID_INPUT_DIGITAL)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index d98cee6..77115b9 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -470,3 +470,36 @@ void intel_teardown_gmbus(struct drm_device *dev)
kfree(dev_priv-gmbus);
dev_priv-gmbus = NULL;
 }
+
+/**
+ * intel_drm_get_valid_edid - gets edid from existent adapters only
+ * @connector: DRM connector device to use
+ * @adapter: i2c adapter
+ *
+ * Verifies if the i2c adapter is responding to our queries before
+ * attempting to do proper communication with it. If it does,
+ * retreive the EDID with help of drm_get_edid
+ */
+struct edid *
+intel_drm_get_valid_edid(struct drm_connector *connector,
+   struct i2c_adapter *adapter)
+{
+   unsigned char out;
+   int ret;
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = 0x50,
+   .flags  = 0,
+   .len= 1,
+   .buf= out,
+   }
+   };
+   /* Let's try one edid transfer */
+   ret = i2c_transfer(adapter, msgs, 1);
+   if (ret == -ENXIO) {
+   printk(KERN_DEBUG i915: adapter %s not responding: %d\n,
+   adapter-name, ret);
+   return NULL;
+   }
+   return drm_get_edid(connector, adapter);
+}
diff --git 

[Bug 41579] New: R300 Segfaults when using mupen64plus

[bugzilla-daemon]

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

   Summary: R300 Segfaults when using mupen64plus
   Product: Mesa
   Version: 7.11
  Platform: x86 (IA32)
OS/Version: Linux (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: Drivers/DRI/r300
AssignedTo: dri-devel@lists.freedesktop.org
ReportedBy: krthu...@gmail.com


Created an attachment (id=52103)
 -- (https://bugs.freedesktop.org/attachment.cgi?id=52103)
Backtrace from r300 segfault when running mupen64plus.

When starting up the emulator mupen64plus with a ROM, it crashes before loading
the ROM, showing a segfault. Video card is a Mobility Radeon X1700.

Using gdb I traced the segfault back to r300_dri.so
The issues seems to be at line 864 of src/gallium/drivers/r300/r300_emit.c
'buf' is getting set to 0 instead of a proper address.

My backtrace is attached.

-- 
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