Re: [Nouveau] Bug or not?
I sent the file to you and the list but it appears to have bounced off the list server. Did you get the direct copy? On Thu, Mar 12, 2015 at 6:46 PM, Evan Foss wrote: > Hi Pierre, > > Yes well acpidump is free software island wildlife. As yet it has not > predators so the only thing keeping it in check is forkers loosing > interest in maintaining their version. (is forker a word?!) > > Attached is the file. > > Thanks, > Evan > > On Thu, Mar 12, 2015 at 1:37 AM, Pierre Moreau wrote: >> Hi Evan, >> >> I didn't know there were so many different versions of acpidump in the wild! >> :D >> So apparently, you'll need to replace the first command by 'acpidump -b -t >> DSDT -o dsdt.dat' - we need the DSDT table in binary format for iasl to >> disassemble. >> >> Regards, >> >> Pierre >> >> >> >>> On 12 Mar 2015, at 06:47, Evan Foss wrote: >>> >>> Hi Pierre, >>> >>> No worries about the response time. I remember when the features >>> matrix was mostly todo's. Clearly you folks have put in a ton of work. >>> I would feel bad about trying to shove a developer into my problem and >>> I do not have the time right now to join the development team. Besides >>> my day job I am currently writing a library to interpret BSDL files >>> (JTAG). >>> >>> I am having a bit of trouble with acpidump which is telling me there >>> is no -n option. The acpidump I am running is from a package called >>> pmtools. Is that the correct thing? I am in gentoo and used the >>> following ebuild to do the install. >>> >>> http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/sys-power/pmtools/pmtools-20110323-r1.ebuild?view=markup >>> which pulls this source >>> https://github.com/anyc/pmtools/ >>> >>> I see your point about using the intel instead of the nVidia GPU. >>> >>> Many thanks for your help, >>> -Evan >>> On Wed, Mar 11, 2015 at 3:52 PM, Pierre Moreau wrote: Hi Evan! Sorry for taking so long before answering... Thanks for the greps! It appears you have a newer version of apple_gmux than I have, so let's see if something also changed in the _DSM method. Could you please run - as root - `acpidump -b -n DSDT` which will create a file named dsdt.dat containing the DSDT table, and then run `iasl -d dsdt.dat` and send me the resulting dsdt.dsl file? That way I will have the uuid, the available functions and their version for your _DSM method. Regarding bringing the clocks down, you could add a fake performance level in the vbios but I'm not sure if it would be a good move. You should rather power off the NVidia card and only use the Intel one - using vgaswitcheroo, see [1]; once I have that dsdt.dsl file, I should be able to add support for auto-powering down the NVidia card. Or you could try to motivate some developers - or if you want to give it a try - to investigate / add support for power and/or clock gating. Regards, Pierre [1]: http://nouveau.freedesktop.org/wiki/Optimus/#checkingthecurrentpowerstate - Mail original - > The first grep of dmesg is for the apple info so you know exactly > what > box I have. The second is to show that I am booting in EFI mode and > not some bogus bios compatibility thing. The third is to show the > gmux > version. The fourth is to answer the question you actually asked. > > $ cat /proc/version > Linux version 3.19.0 (root@turingatlarge) (gcc version 4.7.3 (Gentoo > 4.7.3-r1 p1.4, pie-0.5.5) ) #3 SMP Sun Feb 15 01:09:59 EST 2015 > > -- $ dmesg|grep Apple > [0.00] efi: EFI v1.10 by Apple > [0.00] DMI: Apple Inc. MacBookPro9,1/Mac-4B7AC7E43945597E, > BIOS MBP91.88Z.00D3.B08.1208081132 08/08/2012 > [0.00] ACPI: XSDT 0x8CD8E1C0 B4 (v01 APPLE > Apple00 0113) > [0.00] ACPI: FACP 0x8CD8C000 F4 (v04 APPLE > Apple00 Loki 005F) > [0.00] ACPI: HPET 0x8CD8B000 38 (v01 APPLE > Apple00 0001 Loki 005F) > [0.00] ACPI: APIC 0x8CD8A000 BC (v02 APPLE > Apple00 0001 Loki 005F) > [0.00] ACPI: SBST 0x8CD88000 30 (v01 APPLE > Apple00 0001 Loki 005F) > [0.00] ACPI: ECDT 0x8CD87000 53 (v01 APPLE > Apple00 0001 Loki 005F) > [0.00] ACPI: MCFG 0x8CD89000 3C (v01 APPLE > Apple00 0001 Loki 005F) > [3.091740] usb 3-1.1: Manufacturer: Apple Inc. > [3.410516] usb 4-1.8.1: Manufacturer: Apple Inc. > [3.581141] usb 4-1.8.2: Manufacturer: Apple Computer, Inc. > [3.777265] usb 4-1.8.3: Product: Apple Internal Keyboard / > Trackpad > [3.777267] usb 4-1.8.3: Manufacturer: Apple Inc. > [3.782615] input: Apple Inc. Apple Internal Keyboard / Trackpad > as > /devi
[Nouveau] [Bug 85570] DPMS does not turn off LCD backlight on G73 (NV4B) [GeForce 7600 GS]
https://bugs.freedesktop.org/show_bug.cgi?id=85570 --- Comment #3 from Erik van Pienbroek --- Okay my apologies for the noise then. Would it still make sense to file a separate bug report for my backlight issue (next to the one already on Fedora Bugzilla) ? -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 85570] DPMS does not turn off LCD backlight on G73 (NV4B) [GeForce 7600 GS]
https://bugs.freedesktop.org/show_bug.cgi?id=85570 --- Comment #2 from Ilia Mirkin --- (In reply to Erik van Pienbroek from comment #1) > Hi, > > I'm also on Fedora and have been observing similar issues on my Dell XPS > 17/GeForce GT 555M where the backlight of the internal display doesn't turn > off any more (xset dpms force off) starting with early kernel 3.16 snapshots > (git tag v3.15-9837-g682b7c1c8ea8 was the first one to be broken). This was > reported to Red Hat bugzilla @ > https://bugzilla.redhat.com/show_bug.cgi?id=1123661 which is currently still > open. The patch referenced in that bug report relates to DP outputs. This bug report is about a NV4B which came out in ~2005, and has no DP outputs. It also uses the 'dispnv04' path rather than the nv50_display path and is very different. -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 85570] DPMS does not turn off LCD backlight on G73 (NV4B) [GeForce 7600 GS]
https://bugs.freedesktop.org/show_bug.cgi?id=85570 Erik van Pienbroek changed: What|Removed |Added CC||erik-freedesktop-bugzilla@v ||anpienbroek.nl --- Comment #1 from Erik van Pienbroek --- Hi, I'm also on Fedora and have been observing similar issues on my Dell XPS 17/GeForce GT 555M where the backlight of the internal display doesn't turn off any more (xset dpms force off) starting with early kernel 3.16 snapshots (git tag v3.15-9837-g682b7c1c8ea8 was the first one to be broken). This was reported to Red Hat bugzilla @ https://bugzilla.redhat.com/show_bug.cgi?id=1123661 which is currently still open. In this bug report I've found out which exact commit is likely to be the cause of my backlight issue and I've also prepared a patch which resolves the issue for me. Perhaps it will resolve your backlight issue as well. If you want to try it out yourself, you can find a test kernel build @ https://koji.fedoraproject.org/koji/taskinfo?taskID=9172516 which can be installed with 'yum install http://kojipkgs.fedoraproject.org//work/tasks/2518/9172518/kernel-core-4.0.0-0.rc2.git2.1.bug1123661.fc22.x86_64.rpm http://kojipkgs.fedoraproject.org//work/tasks/2518/9172518/kernel-modules-4.0.0-0.rc2.git2.1.bug1123661.fc22.x86_64.rpm' This kernel is targeted for Fedora 22 (currently in Alpha) but should also work fine on Fedora 20 and 21. To make a fair comparison you should also try the regular kernel (without the patch) which can be installed with 'yum install http://kojipkgs.fedoraproject.org//packages/kernel/4.0.0/0.rc2.git2.1.fc22/x86_64/kernel-core-4.0.0-0.rc2.git2.1.fc22.x86_64.rpm http://kojipkgs.fedoraproject.org//packages/kernel/4.0.0/0.rc2.git2.1.fc22/x86_64/kernel-modules-4.0.0-0.rc2.git2.1.fc22.x86_64.rpm' Could you test if this also resolves your backlight issue? -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 80901] [NVCF] PWM fan speed too high
https://bugs.freedesktop.org/show_bug.cgi?id=80901 --- Comment #45 from poma --- (In reply to K.-P. Schrage from comment #44) > Now my 10-...rules file in /etc/udev/rules.d/ looks like this: > RUN+="/bin/sh -c '/bin/echo 1 > > /sys/devices/pci:00/:00:01.0/:01:00.0/hwmon/hwmon1/pwm1_enable'", > RUN+="/usr/local/bin/nvapoke e118 8002" > > During boot, it sounds as if it gets triggered several times > (noise-silence-noise-silence), but it ends up with a silent gpu fan when the > graphical desktop has started. > Startup logs are flooded with messages like: > > nvapoke:473 conflicting memory types e800-f000 > uncached-minus<->write-combining > reserve_memtype failed [mem 0xe800-0xefff], track uncached-minus, > req uncached-minus # rm /etc/udev/rules.d/10-gpu-fan.rules And try this: /etc/systemd/system/gpu-fan.service ~~~ [Unit] Description=GPU fan lower speed [Service] Type=oneshot ExecStart=/bin/sh -c '/bin/echo 1 > /sys/devices/pci:00/:00:01.0/:01:00.0/hwmon/hwmon1/pwm1_enable' ExecStart=/usr/local/bin/nvapoke e118 8002 StandardOutput=null StandardError=null [Install] WantedBy=basic.target ~~ # systemctl enable gpu-fan.service # systemctl start gpu-fan.service # systemctl status gpu-fan.service If everything is OK: # systemctl reboot ... BOOT ... # systemctl status gpu-fan.service -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] nouveau: add coherent BO attribute
Hey, Op 13-03-15 om 07:27 schreef Alexandre Courbot: > Add a flag allowing Nouveau to specify that an object should be coherent > at allocation time. This is required for some class of objects like > fences which are randomly-accessed by both the CPU and GPU. This flag > instructs the kernel driver to make sure the object remains coherent > even on architectures for which coherency is not guaranteed by the bus. > > Signed-off-by: Alexandre Courbot I don't see a problem with this patch, but similar patches to intel to libdrm have been shot down when the changes weren't in an official kernel yet, so I think this should wait until the change is at least in drm-next. ;-) ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 80901] [NVCF] PWM fan speed too high
https://bugs.freedesktop.org/show_bug.cgi?id=80901 --- Comment #44 from K.-P. Schrage --- (In reply to poma from comment #43) > There is no the GPU fan here, so I tested the CPU fan, and this is how it > works: > /etc/udev/rules.d/10-cpu-fan-manual-mode.rules > RUN+="/bin/sh -c '/bin/echo 1 > /sys/devices/platform/it87.656/pwm1_enable'" > > Therefore remove ACTION & ATTRS part, so it runs unconditionally. > > Before you reboot, check with: > # udevadm trigger Now my 10-...rules files in /etc/udev/rules.d/ looks like this: RUN+="/bin/sh -c '/bin/echo 1 > /sys/devices/pci:00/:00:01.0/:01:00.0/hwmon/hwmon1/pwm1_enable'", RUN+="/usr/local/bin/nvapoke e118 8002" During boot, it sounds as if it gets triggered several times (noise-silence-noise-silence), but it ends up with a silent gpu fan when the graphical desktop has started. Startup logs are flooded with messages like: nvapoke:473 conflicting memory types e800-f000 uncached-minus<->write-combining reserve_memtype failed [mem 0xe800-0xefff], track uncached-minus, req uncached-minus -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89572] [NV86] Video playback corruption with nouveau, vdpau
https://bugs.freedesktop.org/show_bug.cgi?id=89572
--- Comment #5 from Ilia Mirkin ---
Unlikely that the errors are related to the corruption, but... possible. There
has always been some corruption in some h264 videos with VP2, but your
screenshot is rather extreme -- the one I'm thinking of is a few macroblocks
being wrong. For you it's like *all* macroblocks being wrong :) FTR your sample
video displays fine on my VP4.2 GF108 (nvc1).
Unfortunately I don't really have the
{time,motivation,interest,your-choice-of-excuse} to track this down, but if
that's something you'd be interested in, come over to #nouveau on freenode for
some advice on how to proceed.
--
You are receiving this mail because:
You are the assignee for the bug.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89572] [NV86] Video playback corruption with nouveau, vdpau
https://bugs.freedesktop.org/show_bug.cgi?id=89572 --- Comment #4 from Matthias Blankertz --- The machine on which the error occurs is a Thinkpad T61 with a Nvidia Quadro NVS140M GPU w/ 128 MiB of video memory. -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89572] [NV86] Video playback corruption with nouveau, vdpau
https://bugs.freedesktop.org/show_bug.cgi?id=89572 --- Comment #3 from Matthias Blankertz --- Created attachment 114293 --> https://bugs.freedesktop.org/attachment.cgi?id=114293&action=edit contents of Xorg.0.log -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89572] [NV86] Video playback corruption with nouveau, vdpau
https://bugs.freedesktop.org/show_bug.cgi?id=89572 --- Comment #2 from Matthias Blankertz --- Created attachment 114292 --> https://bugs.freedesktop.org/attachment.cgi?id=114292&action=edit output of vdpauinfo -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89572] [NV86] Video playback corruption with nouveau, vdpau
https://bugs.freedesktop.org/show_bug.cgi?id=89572 Ilia Mirkin changed: What|Removed |Added Attachment #114290|text/plain |image/png mime type|| -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89572] [NV86] Video playback corruption with nouveau, vdpau
https://bugs.freedesktop.org/show_bug.cgi?id=89572 --- Comment #1 from Matthias Blankertz --- Created attachment 114291 --> https://bugs.freedesktop.org/attachment.cgi?id=114291&action=edit dmesg w/ errors -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89572] New: [NV86] Video playback corruption with nouveau, vdpau
https://bugs.freedesktop.org/show_bug.cgi?id=89572
Bug ID: 89572
Summary: [NV86] Video playback corruption with nouveau, vdpau
Product: xorg
Version: unspecified
Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
Severity: normal
Priority: medium
Component: Driver/nouveau
Assignee: nouveau@lists.freedesktop.org
Reporter: matth...@blankertz.org
QA Contact: xorg-t...@lists.x.org
Created attachment 114290
--> https://bugs.freedesktop.org/attachment.cgi?id=114290&action=edit
screenshot of corrupted video
Using archlinux x86_64 with the current mainline kernel (4.0rc3), xorg 1.17.1,
mesa-vdpau 10.4.6, nouveau-fw 325.15, xf86-video-nouveau 1.0.11.
Playing back a h264 video file (test file big buck bunny,
http://blender-mirror.kino3d.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov)
causes major graphical corruption in the video playback (see attachment).
Several errors appear in the kernel log. The playback is done using "mplayer
-vo vdpau -vc ffh264vdpau -fs". (mplayer 37379)
Playing back the same file without acceleration ("mplayer -vo vdpau -fs") works
perfectly.
I have had similiar problems in the past, so this is (probably) not a new
problem of the current 4.0 development kernel.
--
You are receiving this mail because:
You are the assignee for the bug.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 80901] [NVCF] PWM fan speed too high
https://bugs.freedesktop.org/show_bug.cgi?id=80901
--- Comment #43 from poma ---
(In reply to K.-P. Schrage from comment #42)
> (In reply to poma from comment #40)
>
> > /etc/udev/rules.d/10-unladen-swallow.rules
> > ACTION=="add", ATTRS{vendor}=="0x1234", ATTRS{device}=="0xabcd",
> > RUN+="/bin/sh -c '/bin/echo 1 >
> > /sys/devices/pci:00/:00:01.0/:01:00.0/hwmon/hwmon1/pwm1_enable'",
> > RUN+="/usr/local/bin/nvapoke e118 8002"
>
> Thanks, Poma, this rule (all in one line, with my appropriate device and
> vendor id's) seems to work correctly, but it only reduces fan speed for a
> second or so during the boot process, then speed is up again, and the value
> that nvapoke writes is overwritten (80d8 instead of 8002).
> Perhaps this rule comes up too early, but changing the prefix number from 10
> to e. g. 99 doesn't help.
Exactly, these are oneliners.
There is no the GPU fan here, so I tested the CPU fan, and this is how it
works:
/etc/udev/rules.d/10-cpu-fan-manual-mode.rules
RUN+="/bin/sh -c '/bin/echo 1 > /sys/devices/platform/it87.656/pwm1_enable'"
Therefore remove ACTION & ATTRS part, so it runs unconditionally.
Before you reboot, check with:
# udevadm trigger
--
You are receiving this mail because:
You are the assignee for the bug.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89571] [NV46] XFX 7300 GS - Black screen with DVI when KMS(nouveau) starts - VGA working
https://bugs.freedesktop.org/show_bug.cgi?id=89571 Ilia Mirkin changed: What|Removed |Added Summary|XFX 7300 GS - Black screen |[NV46] XFX 7300 GS - Black |with DVI when KMS(nouveau) |screen with DVI when |starts - VGA working|KMS(nouveau) starts - VGA ||working --- Comment #1 from Ilia Mirkin --- Please provide dmesg from a boot with nouveau.debug=trace drm.debug=0xe Both with VGA plugged in, and with DVI plugged in. Also, please attach files to the bug, not pastebins (which expire). Include the full dmesg, not a grep for 'nouveau'. There are other things that are interesting in there. -- You are receiving this mail because: You are the assignee for the bug. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 89571] New: XFX 7300 GS - Black screen with DVI when KMS(nouveau) starts - VGA working
https://bugs.freedesktop.org/show_bug.cgi?id=89571
Bug ID: 89571
Summary: XFX 7300 GS - Black screen with DVI when KMS(nouveau)
starts - VGA working
Product: xorg
Version: unspecified
Hardware: x86-64 (AMD64)
OS: All
Status: NEW
Severity: blocker
Priority: medium
Component: Driver/nouveau
Assignee: nouveau@lists.freedesktop.org
Reporter: bxmix...@sharklasers.com
QA Contact: xorg-t...@lists.x.org
Created attachment 114287
--> https://bugs.freedesktop.org/attachment.cgi?id=114287&action=edit
vbios XFX Nvidia 7300 GS 256MB
I already talked in IRC #nouveau before reporting this bug.
Card:
http://images10.newegg.com/NeweggImage/ProductImageCompressAll300/14-150-410-02.jpg
Problem: Card connected to an 24" 1920x1200 monitor via DVI. Everything is
working fine with windows and DVI at full resolution.
When booting any Linux live, i get a black screen directly when KMS starts.
BIOS post and VESA works fine with DVI.
To get a picture with nouveau, i had to connect the same monitor via VGA cable
to the nvidia card. I get then an 1024x768 resolution.
After boot with VGA i remove the VGA cable and connect DVI to get some logfiles
for debugging.
first:
for p in /sys/class/drm/*/status; do con=${p%/status}; echo -n
"${con#*/card?-}: "; cat $p; done
reports in VGA-mode:
DVI-I-1: disconnected
TV-1: connected
TV-2: disconnected
reports in DVI-mode (no working screen):
DVI-I-1: disconnected
TV-1: disconnected
TV-2: disconnected
The xorg log:
https://pastebin.mozilla.org/8825681
the dmesg:
https://pastebin.mozilla.org/8825682
The vbios from /sys/kernel/debug/dri/0/ :
http://www.file-upload.net/download-10414782/vbios.rom.html
(also attached to this bug as attachment)
The IRC log with some additional information that are maybe usefull:
https://pastebin.mozilla.org/8825683
Would be nice to get able to use this computer with nouveau and DVI with normal
resolution.
--
You are receiving this mail because:
You are the assignee for the bug.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 80901] [NVCF] PWM fan speed too high
https://bugs.freedesktop.org/show_bug.cgi?id=80901
--- Comment #42 from K.-P. Schrage ---
(In reply to poma from comment #40)
> /etc/udev/rules.d/10-unladen-swallow.rules
> ACTION=="add", ATTRS{vendor}=="0x1234", ATTRS{device}=="0xabcd",
> RUN+="/bin/sh -c '/bin/echo 1 >
> /sys/devices/pci:00/:00:01.0/:01:00.0/hwmon/hwmon1/pwm1_enable'",
> RUN+="/usr/local/bin/nvapoke e118 8002"
Thanks, Poma, this rule (all in one line, with my appropriate device and vendor
id's) seems to work correctly, but it only reduces fan speed for a second or so
during the boot process, then speed is up again, and the value that nvapoke
writes is overwritten (80d8 instead of 8002).
Perhaps this rule comes up too early, but changing the prefix number from 10 to
e. g. 99 doesn't help.
--
You are receiving this mail because:
You are the assignee for the bug.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] pmu/gk20a: PMU boot support.
Due to the length of the patch there are many things to fix. This review
alone won't cover all of them, but is mainly an attempt to reduce the
amount of code and to split this.
On Wed, Mar 11, 2015 at 3:33 PM, Deepak Goyal wrote:
> It adds PMU boot support.It loads PMU
> firmware into PMU falcon.RM/Kernel driver
> receives INIT ack (through interrupt mechanism)
> from PMU when PMU boots with success.
This commit log is strangely formatted. You want to break lines of git
commit lots around column 70, not 50. Also don't forget the space after
the end of your sentences.
The log itself also lacks informative value, especially considering the
length of this patch. Please assume your reader is completely unfamiliar
with your work, and explain in detail what your patch does, even the
parts that seem obvious. Some questions that come to mind when reading
the log:
- What is the PMU firmware do?
- What is RM? (this is not the terminology used by Nouveau, so better to
avoid using it altogether)
- What value does this patch add to the project?
I understand that this patch clears the way for follow-up patches that
will actually add features. Please state this clearly in the log, and
explain what these features are. No code can be merged upstream until
its benefits are clearly understood.
Review follows, I have changed the order of files to comment on the
structures before the code.
> diff --git a/drm/nouveau/nvkm/subdev/pmu/priv.h
b/drm/nouveau/nvkm/subdev/pmu/priv.h
> index 998410563bfd..c4686e418582 100644
> --- a/drm/nouveau/nvkm/subdev/pmu/priv.h
> +++ b/drm/nouveau/nvkm/subdev/pmu/priv.h
> @@ -2,7 +2,91 @@
> #define __NVKM_PMU_PRIV_H__
> #include
> #include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
>
> +static inline u32 u64_hi32(u64 n)
> +{
> + return (u32)((n >> 32) & ~(u32)0);
> +}
> +
> +static inline u32 u64_lo32(u64 n)
> +{
> + return (u32)(n & ~(u32)0);
> +}
Use the lower_32_bits() and upper_32_bits() macros instead.
> +
> +/* #define ALLOCATOR_DEBUG */
This line is useless...
> +
> +/* main struct */
... and this comment uninformative.
> +struct nvkm_pmu_allocator {
> +
> + char name[32]; /* name for allocator */
> +/*struct rb_root rb_root;*//* rb tree root for blocks */
Do not comment out members that we don't need. If it's unneeded, just
remove it.
> +
> + u32 base; /* min value of this linear
space */
> + u32 limit; /* max value = limit - 1 */
> +
> + unsigned long *bitmap; /* bitmap */
> +
> + struct gk20a_alloc_block *block_first; /* first block in list */
> + struct gk20a_alloc_block *block_recent; /* last visited block */
> +
> + u32 first_free_addr;/* first free addr, non-contigous
> + allocation preferred start,
> + in order to pick up small
holes */
> + u32 last_free_addr; /* last free addr, contiguous
> + allocation preferred start */
> + u32 cached_hole_size; /* max free hole size up to
> + last_free_addr */
> + u32 block_count;/* number of blocks */
> +
> + struct rw_semaphore rw_sema;/* lock */
> + struct kmem_cache *block_cache; /* slab cache */
> +
> + /* if enabled, constrain to [base, limit) */
> + struct {
> + bool enable;
> + u32 base;
> + u32 limit;
> + } constraint;
> +
> + int (*alloc)(struct nvkm_pmu_allocator *allocator,
> + u32 *addr, u32 len, u32 align);
> + int (*free)(struct nvkm_pmu_allocator *allocator,
> + u32 addr, u32 len, u32 align);
> +
> +};
> +
> +int nvkm_pmu_allocator_init(struct nvkm_pmu_allocator *allocator,
> + const char *name, u32 base, u32 size);
> +void nvkm_pmu_allocator_destroy(struct nvkm_pmu_allocator *allocator);
> +
> +int nvkm_pmu_allocator_block_alloc(struct nvkm_pmu_allocator *allocator,
> + u32 *addr, u32 len, u32 align);
> +
> +int nvkm_pmu_allocator_block_free(struct nvkm_pmu_allocator *allocator,
> + u32 addr, u32 len, u32 align);
So from the nvkm_pmu_allocator struct and these function prototypes,
this looks like a pretty casual address space allocator. Nouveau already
has such an allocator: nvkm_mm. Check it out, it will do all that you
need and you can remove a lot of code from this patch.
> +
> +#if defined(ALLOCATOR_DEBUG)
> +
> +#define allocator_dbg(alloctor, format, arg...)
\
> +do { \
> + if (1) \
> + pr_debug("nvkm_pmu_allocator (%s) %s: " format
Re: [Nouveau] [PATCH] nouveau: add coherent BO attribute
On Fri, Mar 13, 2015 at 3:39 PM, Alexandre Courbot wrote: > On Fri, Mar 13, 2015 at 3:36 PM, Ilia Mirkin wrote: >> Doesn't this require a kernel version that has your other patch? What >> happens when this runs on an older kernel? Does it get silently >> ignored, or does it end up erroring out? If it errors out, that's >> fine. Otherwise some sort of version check should be put in, no? > > The corresponding kernel patch is already merged in Ben's tree. If > running with an older kernel, this flag will be a no-op, which is fine > since GK20A's GPU (the reason for this patch as you have guessed :)) > is not enabled for these kernels. > > I am fine with adding a version check if you think it is necessary. So as discussed on IRC, I will check the DRM version from Mesa. This patch should be good to go - it will require a version increase in libdrm though. Let me know if you want me to send a patch for this too. ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
