Subject: Re: [patch 5/6] PARISC: move PERR & SERR enables out of pcibios_enable_resources()

From: Grant Grundler <[email-masked]>
Date: Thu, 28 Feb 2008 10:31:25 -0700

On Wed, Feb 27, 2008 at 05:04:42PM -0700, Bjorn Helgaas wrote:
> Move PERR and SERR enables from pcibios_enable_resources() to
> platform_pci_enable_device() so the former matches other
> architectures and can be shared.
> 
> Signed-off-by: Bjorn Helgaas <[email-masked]>

Ack-By: Grant Grundler <[email-masked]>

This patch sequence is heading in the right direction.
I've not tested this particular one yet but I'm pretty sure it's ok.
I'll fixup any breakage for parisc.

...
> +/*
> + * A driver is enabling the device.  We enable the PERR and SERR bits
> + * unconditionally.  Drivers that do not need parity (eg graphics and
> + * possibly networking) can clear these bits if they want.
> + */
> +static int platform_pci_enable_device(struct pci_dev *dev)

Thanks for preserving this comment.

In general, I'm wondering if the check for device class would be
sufficient here to NOT enable PERR/SERR for graphics automatically.
While disabling PERR was "the right thing" for older "mostly write"
devices of the 1990's and early 2000, it might not be correct for
current 3-D graphics devices which use host mem to buffer processed
results. I'm thinking of Intel graphics controllers in particular
but I don't know any details of how they actually work.

I'm also a bit concerned about this now becuase (IIRC) AGP didn't
implement parity though it looked like PCI protocol. PCI-e certainly
does but it's possible BIOS/Firmware disable parity generation
on the host bridge when connected to a gfx device.
We wouldn't want to enable parity checking on a PCI-e gfx device in this
case and I hope someone (perhaps at Intel) could double check this.

thanks,
grant