[Intel-gfx] [PATCH 2/2] Check if the bus is valid prior to discovering edid.
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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