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

2011-10-08 Thread Ben Widawsky
On Fri, Oct 07, 2011 at 07:00:42PM -0300, Eugeni Dodonov wrote:
> 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 
> ---
> 
> 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);
> +}

I think you may as well optimistically try to get the edid_data here.
The problem is, in the success case you add ~10 i2c clocks because you
next call drm_get_edid. If you optimistacally try to do both you should
receive the -ENXIO after the slaves ignore the address byte, and not
lose time. (So win on exists case, lose a *slight* amount of CPU time in
fail case).

Now if you do that I think most of the code can be taken from
intel_ddc_probe. Just modify that function to take the args you need
(dev_priv, and adatper I am thinking). I only see one caller of that
function, which can easily be modified.

Ben


Power profiles low and mid are identical on Radeon HD6470M

2011-10-08 Thread Wolfgang Fritz
Hello,

I have an HP Elitebook 8560p with Radeon HD7470M graphics, running 
Debian sid with kernel 3.0.4.

I noticed that the power profiles low and mid are setting identical 
clocks and voltage, the lowest possible values:

default engine clock: 75 kHz
current engine clock: 0 kHz
default memory clock: 90 kHz
current memory clock: 149970 kHz
voltage: 900 mV

Looking at the code, this seems to be intentional at least for the 
mobility chips, but the chip provides more modes:

[9.361401] [drm] R600: Number of power states = 7
[9.361402] [drm] Is mobility = YES
[9.361403] [drm] ps #0 type 0, modes=3
[9.361404] [drm] 0: mclk=9, sclk=75000, volt=1100, vddci=0
[9.361406] [drm] 1: mclk=9, sclk=75000, volt=1100, vddci=0
[9.361407] [drm] 2: mclk=9, sclk=75000, volt=1100, vddci=0
[9.361409] [drm] ps #1 type 4, modes=3
[9.361410] [drm] 0: mclk=15000, sclk=1, volt=900, vddci=0
[9.361411] [drm] 1: mclk=9, sclk=4, volt=1000, vddci=0
[9.361413] [drm] 2: mclk=9, sclk=75000, volt=1100, vddci=0
[9.361414] [drm] ps #2 type 0, modes=3
[9.361415] [drm] 0: mclk=9, sclk=7, volt=1100, vddci=0
[9.361417] [drm] 1: mclk=9, sclk=7, volt=1100, vddci=0
[9.361418] [drm] 2: mclk=9, sclk=7, volt=1100, vddci=0
[9.361419] [drm] ps #3 type 2, modes=3
[9.361420] [drm] 0: mclk=15000, sclk=1, volt=900, vddci=0
[9.361422] [drm] 1: mclk=15000, sclk=1, volt=900, vddci=0
[9.361423] [drm] 2: mclk=3, sclk=3, volt=900, vddci=0
[9.361424] [drm] ps #4 type 2, modes=3
[9.361426] [drm] 0: mclk=65000, sclk=4, volt=900, vddci=0
[9.361427] [drm] 1: mclk=65000, sclk=4, volt=900, vddci=0
[9.361428] [drm] 2: mclk=65000, sclk=4, volt=900, vddci=0
[9.361430] [drm] ps #5 type 2, modes=3
[9.361431] [drm] 0: mclk=3, sclk=3, volt=900, vddci=0
[9.361433] [drm] 1: mclk=3, sclk=3, volt=900, vddci=0
[9.361434] [drm] 2: mclk=3, sclk=3, volt=900, vddci=0
[9.361435] [drm] ps #6 type 0, modes=3
[9.361436] [drm] 0: mclk=65000, sclk=4, volt=900, vddci=0
[9.361438] [drm] 1: mclk=65000, sclk=4, volt=900, vddci=0
[9.361439] [drm] 2: mclk=65000, sclk=4, volt=900, vddci=0
[9.361440] [drm] NOT CHIP_R600

(dmesg output from patched radeon module)

Questions:
1. Is this a bug or a feature? (I see that it is not obvious which power 
state to choose)
2. What do the 3 clock/voltage modes per power state mean?

Regards,
Wolfgang






Re: [Intel-gfx] [PATCH 2/2] Check if the bus is valid prior to discovering edid.

2011-10-08 Thread Ben Widawsky
On Fri, Oct 07, 2011 at 07:00:42PM -0300, Eugeni Dodonov wrote:
> 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 
> ---
> 
> 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);
> +}

I think you may as well optimistically try to get the edid_data here.
The problem is, in the success case you add ~10 i2c clocks because you
next call drm_get_edid. If you optimistacally try to do both you should
receive the -ENXIO after the slaves ignore the address byte, and not
lose time. (So win on exists case, lose a *slight* amount of CPU time in
fail case).

Now if you do that I think most of the code can be taken from
intel_ddc_probe. Just modify that function to take the args you need
(dev_priv, and adatper I am thinking). I only see one caller of that
function, which can easily be modified.

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


[Bug 36327] fujitsu siemens amilo li1718: ati radeon x200m does not resume from suspend

2011-10-08 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=36327

--- Comment #23 from Karesz L.  2011-10-08 14:29:26 
PDT ---
(From update of attachment 52129)
Kernel it is using: 3.0.0-12

-- 
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 36327] fujitsu siemens amilo li1718: ati radeon x200m does not resume from suspend

2011-10-08 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=36327

--- Comment #23 from Karesz L.  2011-10-08 
14:29:26 PDT ---
(From update of attachment 52129)
Kernel it is using: 3.0.0-12

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

2011-10-08 Thread Ville Syrjälä
On Sat, Oct 08, 2011 at 01:10:13PM +0200, Thomas Hellstrom wrote:
> On 10/08/2011 12:26 PM, Ville Syrj?l? wrote:
> > On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
> >
> >> 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.
> >>  
> > Now I'm curious. Why not?
> >
> >
> Because it's inconsistent with how flags are defined in the rest of the 
> TTM module.

Ah OK. I was wondering if there's some subtle technical issue involved.
I've recently gotten to the habit of using enums for pretty much all
constants. Just easier on the eye IMHO, and avoids cpp output from
looking like number soup.

-- 
Ville Syrj?l?
syrjala at sci.fi
http://www.sci.fi/~syrjala/


[Bug 36327] fujitsu siemens amilo li1718: ati radeon x200m does not resume from suspend

2011-10-08 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=36327

--- Comment #22 from Karesz L.  2011-10-08 14:26:54 
PDT ---
Created an attachment (id=52129)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52129)
dmesg output after a succesful resume

-- 
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 36327] fujitsu siemens amilo li1718: ati radeon x200m does not resume from suspend

2011-10-08 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=36327

--- Comment #22 from Karesz L.  2011-10-08 
14:26:54 PDT ---
Created an attachment (id=52129)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52129)
dmesg output after a succesful resume

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


[Bug 36327] fujitsu siemens amilo li1718: ati radeon x200m does not resume from suspend

2011-10-08 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=36327

--- Comment #21 from Karesz L.  2011-10-08 14:22:15 
PDT ---
Hey!
I've made an ubuntu 11.10 (64bit) live usb-stick.
After 4 failures, it finally resumed OK from suspend. Tried it again, and it
failed. I'm linking the dmesg output. Maybe this will help.

-- 
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 36327] fujitsu siemens amilo li1718: ati radeon x200m does not resume from suspend

2011-10-08 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=36327

--- Comment #21 from Karesz L.  2011-10-08 
14:22:15 PDT ---
Hey!
I've made an ubuntu 11.10 (64bit) live usb-stick.
After 4 failures, it finally resumed OK from suspend. Tried it again, and it
failed. I'm linking the dmesg output. Maybe this will help.

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

2011-10-08 Thread Thomas Hellstrom
On 10/08/2011 01:27 PM, Ville Syrj?l? wrote:
> On Sat, Oct 08, 2011 at 01:10:13PM +0200, Thomas Hellstrom wrote:
>
>> On 10/08/2011 12:26 PM, Ville Syrj?l? wrote:
>>  
>>> On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
>>>
>>>
 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.

  
>>> Now I'm curious. Why not?
>>>
>>>
>>>
>> Because it's inconsistent with how flags are defined in the rest of the
>> TTM module.
>>  
> Ah OK. I was wondering if there's some subtle technical issue involved.
> I've recently gotten to the habit of using enums for pretty much all
> constants. Just easier on the eye IMHO, and avoids cpp output from
> looking like number soup.
>
>

Yes, there are a number of advantages, including symbolic debugger output.
If we had flag enums that enumerated 1, 2, 4, 8 etc. I'd feel motivated 
to move
all TTM definitions over.

/Thomas





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

2011-10-08 Thread Ville Syrjälä
On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
> 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.

Now I'm curious. Why not?

-- 
Ville Syrj?l?
syrjala at sci.fi
http://www.sci.fi/~syrjala/


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

2011-10-08 Thread Thomas Hellstrom
On 10/08/2011 12:26 PM, Ville Syrj?l? wrote:
> On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
>
>> 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.
>>  
> Now I'm curious. Why not?
>
>
Because it's inconsistent with how flags are defined in the rest of the 
TTM module.

/Thomas




[Bug 41592] [Radeon] The display often freezes with gnome-shell 3.2

2011-10-08 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=41592

--- Comment #2 from miran...@orange.fr 2011-10-08 11:29:16 PDT ---
Created an attachment (id=52126)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52126)
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 41592] [Radeon] The display often freezes with gnome-shell 3.2

2011-10-08 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=41592

--- Comment #2 from mirandir at orange.fr 2011-10-08 11:29:16 PDT ---
Created an attachment (id=52126)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52126)
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 41592] [Radeon] The display often freezes with gnome-shell 3.2

2011-10-08 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=41592

--- Comment #1 from miran...@orange.fr 2011-10-08 11:28:41 PDT ---
Created an attachment (id=52125)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52125)
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 41592] [Radeon] The display often freezes with gnome-shell 3.2

2011-10-08 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=41592

--- Comment #1 from mirandir at orange.fr 2011-10-08 11:28:41 PDT ---
Created an attachment (id=52125)
 --> (https://bugs.freedesktop.org/attachment.cgi?id=52125)
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 41592] New: [Radeon] The display often freezes with gnome-shell 3.2

2011-10-08 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=41592

   Summary: [Radeon] The display often freezes with gnome-shell
3.2
   Product: Mesa
   Version: 7.11
  Platform: x86-64 (AMD64)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/r600
AssignedTo: dri-devel@lists.freedesktop.org
ReportedBy: miran...@orange.fr


Since the Gnome 3.2 update, there are many many lines like this in my Xorg.log
:

[ 4192.459] (II) RADEON(0): radeon_dri2_flip_event_handler:981
fevent[0x17e2360] width 1366 pitch 5632 (/4 1408)
[ 4193.043] (II) RADEON(0): radeon_dri2_schedule_flip:619 fevent[0x184a890]

And gnome-shell freezes very often, when i exit fullscreen games or just after
that I unlocked gnome-screensaver.
I have to switch to a VT and kill the gnome-shell process, then it restart and
I can continue to work.

I attach my dmesg and Xorg.log.


Additional info:
* Archlinux 64 bits, kernel-3.0.6, mesa-7.11, xorg-server-1.10.4 and
gnome-shell-3.2.0
* ATI Mobility Radeon HD5470 with the radeon driver.

(I can't reproduce this bug with my laptop with Intel graphics)

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


[Bug 41592] New: [Radeon] The display often freezes with gnome-shell 3.2

2011-10-08 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=41592

   Summary: [Radeon] The display often freezes with gnome-shell
3.2
   Product: Mesa
   Version: 7.11
  Platform: x86-64 (AMD64)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/r600
AssignedTo: dri-devel at lists.freedesktop.org
ReportedBy: mirandir at orange.fr


Since the Gnome 3.2 update, there are many many lines like this in my Xorg.log
:

[ 4192.459] (II) RADEON(0): radeon_dri2_flip_event_handler:981
fevent[0x17e2360] width 1366 pitch 5632 (/4 1408)
[ 4193.043] (II) RADEON(0): radeon_dri2_schedule_flip:619 fevent[0x184a890]

And gnome-shell freezes very often, when i exit fullscreen games or just after
that I unlocked gnome-screensaver.
I have to switch to a VT and kill the gnome-shell process, then it restart and
I can continue to work.

I attach my dmesg and Xorg.log.


Additional info:
* Archlinux 64 bits, kernel-3.0.6, mesa-7.11, xorg-server-1.10.4 and
gnome-shell-3.2.0
* ATI Mobility Radeon HD5470 with the radeon driver.

(I can't reproduce this bug with my laptop with Intel graphics)

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


Power profiles low and mid are identical on Radeon HD6470M

2011-10-08 Thread Wolfgang Fritz

Hello,

I have an HP Elitebook 8560p with Radeon HD7470M graphics, running 
Debian sid with kernel 3.0.4.


I noticed that the power profiles low and mid are setting identical 
clocks and voltage, the lowest possible values:


default engine clock: 75 kHz
current engine clock: 0 kHz
default memory clock: 90 kHz
current memory clock: 149970 kHz
voltage: 900 mV

Looking at the code, this seems to be intentional at least for the 
mobility chips, but the chip provides more modes:


[9.361401] [drm] R600: Number of power states = 7
[9.361402] [drm] Is mobility = YES
[9.361403] [drm] ps #0 type 0, modes=3
[9.361404] [drm] 0: mclk=9, sclk=75000, volt=1100, vddci=0
[9.361406] [drm] 1: mclk=9, sclk=75000, volt=1100, vddci=0
[9.361407] [drm] 2: mclk=9, sclk=75000, volt=1100, vddci=0
[9.361409] [drm] ps #1 type 4, modes=3
[9.361410] [drm] 0: mclk=15000, sclk=1, volt=900, vddci=0
[9.361411] [drm] 1: mclk=9, sclk=4, volt=1000, vddci=0
[9.361413] [drm] 2: mclk=9, sclk=75000, volt=1100, vddci=0
[9.361414] [drm] ps #2 type 0, modes=3
[9.361415] [drm] 0: mclk=9, sclk=7, volt=1100, vddci=0
[9.361417] [drm] 1: mclk=9, sclk=7, volt=1100, vddci=0
[9.361418] [drm] 2: mclk=9, sclk=7, volt=1100, vddci=0
[9.361419] [drm] ps #3 type 2, modes=3
[9.361420] [drm] 0: mclk=15000, sclk=1, volt=900, vddci=0
[9.361422] [drm] 1: mclk=15000, sclk=1, volt=900, vddci=0
[9.361423] [drm] 2: mclk=3, sclk=3, volt=900, vddci=0
[9.361424] [drm] ps #4 type 2, modes=3
[9.361426] [drm] 0: mclk=65000, sclk=4, volt=900, vddci=0
[9.361427] [drm] 1: mclk=65000, sclk=4, volt=900, vddci=0
[9.361428] [drm] 2: mclk=65000, sclk=4, volt=900, vddci=0
[9.361430] [drm] ps #5 type 2, modes=3
[9.361431] [drm] 0: mclk=3, sclk=3, volt=900, vddci=0
[9.361433] [drm] 1: mclk=3, sclk=3, volt=900, vddci=0
[9.361434] [drm] 2: mclk=3, sclk=3, volt=900, vddci=0
[9.361435] [drm] ps #6 type 0, modes=3
[9.361436] [drm] 0: mclk=65000, sclk=4, volt=900, vddci=0
[9.361438] [drm] 1: mclk=65000, sclk=4, volt=900, vddci=0
[9.361439] [drm] 2: mclk=65000, sclk=4, volt=900, vddci=0
[9.361440] [drm] NOT CHIP_R600

(dmesg output from patched radeon module)

Questions:
1. Is this a bug or a feature? (I see that it is not obvious which power 
state to choose)

2. What do the 3 clock/voltage modes per power state mean?

Regards,
Wolfgang




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


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

2011-10-08 Thread Thomas Hellstrom
On 10/07/2011 11:30 PM, Marek Ol??k wrote:
> On Fri, Oct 7, 2011 at 3:38 PM, Jerome Glisse  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
>
At one XDS meeting in Cambridge an IMHO questionable decision was taken to
try to keep synchronization operations like this in user-space, 
communicating necessary
info among involved components. In this case you'd then need to send 
fence sequences
down the DRI2 protocol to the winsys.

However, if you at one point want to do user-space suballocation of 
kernel buffers,
that's something you need to do anyway, because the kernel is not aware 
that user-space fences
the suballocations separately.

/Thomas





[git pull] drm fixes

2011-10-08 Thread Dave Airlie

Hi Linus,

just some more fixes from Alex for various displayport and llano output 
setup on radeon.

Dave.

The following changes since commit 976d167615b64e14bc1491ca51d424e2ba9a5e84:

  Linux 3.1-rc9 (2011-10-04 18:11:50 -0700)

are available in the git repository at:
  git://people.freedesktop.org/~airlied/linux.git drm-fixes

Alex Deucher (3):
  drm/radeon/kms: retry aux transactions if there are status flags
  drm/radeon/kms: fix dp_detect handling for DP bridge chips
  drm/radeon/kms: use hardcoded dig encoder to transmitter mapping for 
DCE4.1

 drivers/gpu/drm/radeon/atombios_dp.c   |   12 +---
 drivers/gpu/drm/radeon/radeon_connectors.c |   21 ++---
 drivers/gpu/drm/radeon/radeon_encoders.c   |9 ++---
 3 files changed, 21 insertions(+), 21 deletions(-)


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

2011-10-08 Thread Thomas Hellstrom

On 10/08/2011 01:27 PM, Ville Syrjälä wrote:

On Sat, Oct 08, 2011 at 01:10:13PM +0200, Thomas Hellstrom wrote:
   

On 10/08/2011 12:26 PM, Ville Syrjälä wrote:
 

On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:

   

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.

 

Now I'm curious. Why not?


   

Because it's inconsistent with how flags are defined in the rest of the
TTM module.
 

Ah OK. I was wondering if there's some subtle technical issue involved.
I've recently gotten to the habit of using enums for pretty much all
constants. Just easier on the eye IMHO, and avoids cpp output from
looking like number soup.

   


Yes, there are a number of advantages, including symbolic debugger output.
If we had flag enums that enumerated 1, 2, 4, 8 etc. I'd feel motivated 
to move

all TTM definitions over.

/Thomas



___
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

2011-10-08 Thread Ville Syrjälä
On Sat, Oct 08, 2011 at 01:10:13PM +0200, Thomas Hellstrom wrote:
> On 10/08/2011 12:26 PM, Ville Syrjälä wrote:
> > On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
> >
> >> 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.
> >>  
> > Now I'm curious. Why not?
> >
> >
> Because it's inconsistent with how flags are defined in the rest of the 
> TTM module.

Ah OK. I was wondering if there's some subtle technical issue involved.
I've recently gotten to the habit of using enums for pretty much all
constants. Just easier on the eye IMHO, and avoids cpp output from
looking like number soup.

-- 
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
___
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

2011-10-08 Thread Thomas Hellstrom

On 10/08/2011 12:26 PM, Ville Syrjälä wrote:

On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
   

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.
 

Now I'm curious. Why not?

   
Because it's inconsistent with how flags are defined in the rest of the 
TTM module.


/Thomas


___
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

2011-10-08 Thread Ville Syrjälä
On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
> 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.

Now I'm curious. Why not?

-- 
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
___
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

2011-10-08 Thread Thomas Hellstrom

On 10/07/2011 11:30 PM, Marek Olšák wrote:

On Fri, Oct 7, 2011 at 3:38 PM, Jerome Glisse  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
   

At one XDS meeting in Cambridge an IMHO questionable decision was taken to
try to keep synchronization operations like this in user-space, 
communicating necessary
info among involved components. In this case you'd then need to send 
fence sequences

down the DRI2 protocol to the winsys.

However, if you at one point want to do user-space suballocation of 
kernel buffers,
that's something you need to do anyway, because the kernel is not aware 
that user-space fences

the suballocations separately.

/Thomas



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


[git pull] drm fixes

2011-10-08 Thread Dave Airlie

Hi Linus,

just some more fixes from Alex for various displayport and llano output 
setup on radeon.

Dave.

The following changes since commit 976d167615b64e14bc1491ca51d424e2ba9a5e84:

  Linux 3.1-rc9 (2011-10-04 18:11:50 -0700)

are available in the git repository at:
  git://people.freedesktop.org/~airlied/linux.git drm-fixes

Alex Deucher (3):
  drm/radeon/kms: retry aux transactions if there are status flags
  drm/radeon/kms: fix dp_detect handling for DP bridge chips
  drm/radeon/kms: use hardcoded dig encoder to transmitter mapping for 
DCE4.1

 drivers/gpu/drm/radeon/atombios_dp.c   |   12 +---
 drivers/gpu/drm/radeon/radeon_connectors.c |   21 ++---
 drivers/gpu/drm/radeon/radeon_encoders.c   |9 ++---
 3 files changed, 21 insertions(+), 21 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2011-10-08 Thread Marek Olšák
On Fri, Oct 7, 2011 at 10:00 AM, Thomas Hellstrom  
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