On Thu, 1 Apr 2010, Rafael J. Wysocki wrote:
>
> OK, I've verified that partial revert (below) is sufficient.
Hmm. Through the DRM merge I just did, this area actually conflicted, and
the resolved version is now
if ((rdev->family >= CHIP_RV380) &&
(!(rdev->flags & RADEON_IS_IGP))) {
which presumably also fixes your issue?
[ Side note: somebody in the DRM tree seems to be way too used to LISP,
and thinks that adding parenthesis always improves the code ;-]
However, I do suspect that we should probably revert the quirk regardless
as being useless (ie it probably was related to those IGP chips that
apparently don't do MSI anyway).
So the patch that reverts the quirk by Clemens (to replace it with
disabling MSI entirely when the AMD NB doesn't accept them) seems to be a
good idea regardless, since it's apparently not just about gfx. Jesse?
Linus
On Thu, Apr 1, 2010 at 12:29 PM, Linus Torvalds
<[email protected]> wrote:
>
>
> On Thu, 1 Apr 2010, Rafael J. Wysocki wrote:
>>
>> OK, I've verified that partial revert (below) is sufficient.
>
> Hmm. Through the DRM merge I just did, this area actually conflicted, and
> the resolved version is now
>
> ? ? ? ?if ((rdev->family >= CHIP_RV380) &&
> ? ? ? ? ? ?(!(rdev->flags & RADEON_IS_IGP))) {
>
> which presumably also fixes your issue?
>
> [ Side note: somebody in the DRM tree seems to be way too used to LISP,
> ?and thinks that adding parenthesis always improves the code ;-]
>
heh, that's me. habit I guess, just to be sure.
> However, I do suspect that we should probably revert the quirk regardless
> as being useless (ie it probably was related to those IGP chips that
> apparently don't do MSI anyway).
>
> So the patch that reverts the quirk by Clemens (to replace it with
> disabling MSI entirely when the AMD NB doesn't accept them) seems to be a
> good idea regardless, since it's apparently not just about gfx. Jesse?
Clemems' "PCI quirk: RS780/RS880: disable MSI completely" patch is the
right approach I think. Note that it's only devices hung off the int
gfx pci to pci bridge that have broken MSI (gfx and audio). MSI works
fine on the PCIE slots. I have a similar patch for rs400 chips on bug
15626:
https://bugzilla.kernel.org/show_bug.cgi?id=15626
Alex
>
> ? ? ? ? ? ? ? ? ? ? ? ?Linus
>
> ------------------------------------------------------------------------------
> Download Intel® Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> --
> _______________________________________________
> Dri-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/dri-devel
>
On Thu, 1 Apr 2010, Alex Deucher wrote:
>
> Clemems' "PCI quirk: RS780/RS880: disable MSI completely" patch is the
> right approach I think. Note that it's only devices hung off the int
> gfx pci to pci bridge that have broken MSI (gfx and audio). MSI works
> fine on the PCIE slots. I have a similar patch for rs400 chips on bug
> 15626:
> https://bugzilla.kernel.org/show_bug.cgi?id=15626
Hmm. Does 'pci_msi_enable' only cover regular PCI devices? Or will that
pci_no_msi() quirk disable MSI for PCIE too? I think it will trigger for
PCIE drivers too.
Put another way: it sounds like the quirk now disables MSI for all
devices. Maybe there would some more targeted mode?
Linus
Linus Torvalds wrote:
> On Thu, 1 Apr 2010, Alex Deucher wrote:
> > Clemems' "PCI quirk: RS780/RS880: disable MSI completely" patch is the
> > right approach I think. Note that it's only devices hung off the int
> > gfx pci to pci bridge that have broken MSI (gfx and audio). MSI works
> > fine on the PCIE slots. I have a similar patch for rs400 chips on bug
> > 15626:
> > https://bugzilla.kernel.org/show_bug.cgi?id=15626
>
> Hmm. Does 'pci_msi_enable' only cover regular PCI devices? Or will that
> pci_no_msi() quirk disable MSI for PCIE too?
A quirk that used pci_no_msi() would disable all MSI for all devices.
However, these patches (and that in bug 15626) use PCI_BUS_FLAGS_NO_MSI
so that only the internal GPU devices are affected.
That "completely" in my patch title should better read "unconditionally".
Regards,
Clemens
On Thu, Apr 1, 2010 at 1:24 PM, Linus Torvalds
<[email protected]> wrote:
>
>
> On Thu, 1 Apr 2010, Alex Deucher wrote:
>>
>> Clemems' "PCI quirk: RS780/RS880: disable MSI completely" patch is the
>> right approach I think. ?Note that it's only devices hung off the int
>> gfx pci to pci bridge that have broken MSI (gfx and audio). ?MSI works
>> fine on the PCIE slots. ?I have a similar patch for rs400 chips on bug
>> 15626:
>> https://bugzilla.kernel.org/show_bug.cgi?id=15626
>
> Hmm. Does 'pci_msi_enable' only cover regular PCI devices? Or will that
> pci_no_msi() quirk disable MSI for PCIE too? I think it will trigger for
> PCIE drivers too.
>
> Put another way: it sounds like the quirk now disables MSI for all
> devices. Maybe there would some more targeted mode?
>
What I meant to say was MSI works fine on bridges other than the
bridge the internal gfx lives on. quirk_disable_msi() just disables
MSI on the devices on that particular bridge as far as I understand
it, but I'm by no means an expert on the PCI code. E.g., on my RS780
board, MSIs are only problematic on the integrated gfx chip. MSIs
work fine on PCI/PCIE add-on cards and the integrated Ethernet.
Alex
> ? ? ? ? ? ? ? ?Linus
>
On Thursday 01 April 2010, Linus Torvalds wrote:
>
> On Thu, 1 Apr 2010, Rafael J. Wysocki wrote:
> >
> > OK, I've verified that partial revert (below) is sufficient.
>
> Hmm. Through the DRM merge I just did, this area actually conflicted, and
> the resolved version is now
>
> if ((rdev->family >= CHIP_RV380) &&
> (!(rdev->flags & RADEON_IS_IGP))) {
>
> which presumably also fixes your issue?
Yes, it does.
Rafael
On Thu, 1 Apr 2010, Alex Deucher wrote:
>
> What I meant to say was MSI works fine on bridges other than the
> bridge the internal gfx lives on. quirk_disable_msi() just disables
> MSI on the devices on that particular bridge as far as I understand
> it, but I'm by no means an expert on the PCI code.
Yes, it disabled MSI only on devices under that bridge. But if it's the
northbridge, that would be everything, no?
But I don't know what devices those
PCI_VENDOR_ID_AMD, 0x9602,
PCI_VENDOR_ID_ASUSTEK, 0x9602,
things are. If they are just a PCIE->PCI bridge rather than the root
bridge, then everything looks fine to me.
Linus
On Thu, Apr 1, 2010 at 4:17 PM, Linus Torvalds
<[email protected]> wrote:
>
>
> On Thu, 1 Apr 2010, Alex Deucher wrote:
>>
>> What I meant to say was MSI works fine on bridges other than the
>> bridge the internal gfx lives on. ?quirk_disable_msi() just disables
>> MSI on the devices on that particular bridge as far as I understand
>> it, but I'm by no means an expert on the PCI code.
>
> Yes, it disabled MSI only on devices under that bridge. But if it's the
> northbridge, that would be everything, no?
>
> But I don't know what devices those
>
> ? ? ? ?PCI_VENDOR_ID_AMD, 0x9602,
> ? ? ? ?PCI_VENDOR_ID_ASUSTEK, 0x9602,
>
> things are. If they are just a PCIE->PCI bridge rather than the root
> bridge, then everything looks fine to me.
>
Yup, those are just the pci to pci bridges used for the internal gfx.
Really there's only one, 0x9602, but some asus oem boards have the
vendor id wrong.
> ? ? ? ? ? ? ? ? ? ? ? ?Linus
>
On Thu, 1 Apr 2010 09:29:23 -0700 (PDT)
Linus Torvalds <[email protected]> wrote:
>
>
> On Thu, 1 Apr 2010, Rafael J. Wysocki wrote:
> >
> > OK, I've verified that partial revert (below) is sufficient.
>
> Hmm. Through the DRM merge I just did, this area actually conflicted, and
> the resolved version is now
>
> if ((rdev->family >= CHIP_RV380) &&
> (!(rdev->flags & RADEON_IS_IGP))) {
>
> which presumably also fixes your issue?
>
> [ Side note: somebody in the DRM tree seems to be way too used to LISP,
> and thinks that adding parenthesis always improves the code ;-]
>
> However, I do suspect that we should probably revert the quirk regardless
> as being useless (ie it probably was related to those IGP chips that
> apparently don't do MSI anyway).
>
> So the patch that reverts the quirk by Clemens (to replace it with
> disabling MSI entirely when the AMD NB doesn't accept them) seems to be a
> good idea regardless, since it's apparently not just about gfx. Jesse?
Yeah, that sounds fine. I can include it in my next pull req or you
can just pick it up directly.
--
Jesse Barnes, Intel Open Source Technology Center
On Friday 02 April 2010, Jesse Barnes wrote:
> On Thu, 1 Apr 2010 09:29:23 -0700 (PDT)
> Linus Torvalds <[email protected]> wrote:
>
> >
> >
> > On Thu, 1 Apr 2010, Rafael J. Wysocki wrote:
> > >
> > > OK, I've verified that partial revert (below) is sufficient.
> >
> > Hmm. Through the DRM merge I just did, this area actually conflicted, and
> > the resolved version is now
> >
> > if ((rdev->family >= CHIP_RV380) &&
> > (!(rdev->flags & RADEON_IS_IGP))) {
> >
> > which presumably also fixes your issue?
> >
> > [ Side note: somebody in the DRM tree seems to be way too used to LISP,
> > and thinks that adding parenthesis always improves the code ;-]
> >
> > However, I do suspect that we should probably revert the quirk regardless
> > as being useless (ie it probably was related to those IGP chips that
> > apparently don't do MSI anyway).
> >
> > So the patch that reverts the quirk by Clemens (to replace it with
> > disabling MSI entirely when the AMD NB doesn't accept them) seems to be a
> > good idea regardless, since it's apparently not just about gfx. Jesse?
>
> Yeah, that sounds fine. I can include it in my next pull req or you
> can just pick it up directly.
Not exactly that one, please, it's missing a quirk for the affected system.
I've just sent a corrected version, here:
https://patchwork.kernel.org/patch/90275/
Rafael