diff mbox

only try to find radeon IGP ROM on X86

Message ID alpine.SOC.1.00.1310101444090.15296@math.ut.ee
State Rejected
Delegated to: David Miller
Headers show

Commit Message

Meelis Roos Oct. 10, 2013, 11:51 a.m. UTC
To prevent hangs on non-PC machines (e.g. sparc64), probe Radeon ROM 
from ATI IGP only on X86. Fixes hang in this place and allows PCI radeon 
detection to move on to next problem.

Signed-off-by: Meelis Roos <mroos@linux.ee>

Comments

Alex Deucher Oct. 10, 2013, 2:38 p.m. UTC | #1
On Thu, Oct 10, 2013 at 7:51 AM, Meelis Roos <mroos@linux.ee> wrote:
> To prevent hangs on non-PC machines (e.g. sparc64), probe Radeon ROM
> from ATI IGP only on X86. Fixes hang in this place and allows PCI radeon
> detection to move on to next problem.

NACK.  All this function does it attempt to read the rom from the
framebuffer PCI BAR.  If you get hangs reading the BAR, then you have
larger problems on your platform.  Also, there are non-x86 platforms
that IGP chips which this may break.

Alex

>
> Signed-off-by: Meelis Roos <mroos@linux.ee>
>
> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
> index 061b227..48ef97c 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -49,6 +49,11 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
>         resource_size_t vram_base;
>         resource_size_t size = 256 * 1024; /* ??? */
>
> +#ifndef CONFIG_X86
> +       /* IGP only exists on X86 (32- and 64-bit) */
> +       return false;
> +#endif
> +
>         if (!(rdev->flags & RADEON_IS_IGP))
>                 if (!radeon_card_posted(rdev))
>                         return false;
>
> --
> Meelis Roos (mroos@linux.ee)
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Meelis Roos Oct. 10, 2013, 3:26 p.m. UTC | #2
> On Thu, Oct 10, 2013 at 7:51 AM, Meelis Roos <mroos@linux.ee> wrote:
> > To prevent hangs on non-PC machines (e.g. sparc64), probe Radeon ROM
> > from ATI IGP only on X86. Fixes hang in this place and allows PCI radeon
> > detection to move on to next problem.
> 
> NACK.  All this function does it attempt to read the rom from the
> framebuffer PCI BAR.  If you get hangs reading the BAR, then you have
> larger problems on your platform.  Also, there are non-x86 platforms
> that IGP chips which this may break.

OK. But the code seems to just dereference addresses returned from 
ioremap, but to the best of my knowledge, these are not universally 
(like on each arch) safe to read without readb/readw/readl. Should I 
convert these to readb for test?

> > Signed-off-by: Meelis Roos <mroos@linux.ee>
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
> > index 061b227..48ef97c 100644
> > --- a/drivers/gpu/drm/radeon/radeon_bios.c
> > +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> > @@ -49,6 +49,11 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
> >         resource_size_t vram_base;
> >         resource_size_t size = 256 * 1024; /* ??? */
> >
> > +#ifndef CONFIG_X86
> > +       /* IGP only exists on X86 (32- and 64-bit) */
> > +       return false;
> > +#endif
> > +
> >         if (!(rdev->flags & RADEON_IS_IGP))
> >                 if (!radeon_card_posted(rdev))
> >                         return false;
> >
> > --
> > Meelis Roos (mroos@linux.ee)
>
David Miller Oct. 10, 2013, 5:02 p.m. UTC | #3
From: Meelis Roos <mroos@linux.ee>
Date: Thu, 10 Oct 2013 18:26:39 +0300 (EEST)

>> On Thu, Oct 10, 2013 at 7:51 AM, Meelis Roos <mroos@linux.ee> wrote:
>> > To prevent hangs on non-PC machines (e.g. sparc64), probe Radeon ROM
>> > from ATI IGP only on X86. Fixes hang in this place and allows PCI radeon
>> > detection to move on to next problem.
>> 
>> NACK.  All this function does it attempt to read the rom from the
>> framebuffer PCI BAR.  If you get hangs reading the BAR, then you have
>> larger problems on your platform.  Also, there are non-x86 platforms
>> that IGP chips which this may break.
> 
> OK. But the code seems to just dereference addresses returned from 
> ioremap, but to the best of my knowledge, these are not universally 
> (like on each arch) safe to read without readb/readw/readl. Should I 
> convert these to readb for test?

That's correct, ioremap() returned pointers must not be directly
dereferenced, and must be accessed through the appropriate accessors.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Deucher Oct. 10, 2013, 5:10 p.m. UTC | #4
On Thu, Oct 10, 2013 at 11:26 AM, Meelis Roos <mroos@linux.ee> wrote:
>> On Thu, Oct 10, 2013 at 7:51 AM, Meelis Roos <mroos@linux.ee> wrote:
>> > To prevent hangs on non-PC machines (e.g. sparc64), probe Radeon ROM
>> > from ATI IGP only on X86. Fixes hang in this place and allows PCI radeon
>> > detection to move on to next problem.
>>
>> NACK.  All this function does it attempt to read the rom from the
>> framebuffer PCI BAR.  If you get hangs reading the BAR, then you have
>> larger problems on your platform.  Also, there are non-x86 platforms
>> that IGP chips which this may break.
>
> OK. But the code seems to just dereference addresses returned from
> ioremap, but to the best of my knowledge, these are not universally
> (like on each arch) safe to read without readb/readw/readl. Should I
> convert these to readb for test?

Yeah, that would be great.

Thanks,

Alex

>
>> > Signed-off-by: Meelis Roos <mroos@linux.ee>
>> >
>> > diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
>> > index 061b227..48ef97c 100644
>> > --- a/drivers/gpu/drm/radeon/radeon_bios.c
>> > +++ b/drivers/gpu/drm/radeon/radeon_bios.c
>> > @@ -49,6 +49,11 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
>> >         resource_size_t vram_base;
>> >         resource_size_t size = 256 * 1024; /* ??? */
>> >
>> > +#ifndef CONFIG_X86
>> > +       /* IGP only exists on X86 (32- and 64-bit) */
>> > +       return false;
>> > +#endif
>> > +
>> >         if (!(rdev->flags & RADEON_IS_IGP))
>> >                 if (!radeon_card_posted(rdev))
>> >                         return false;
>> >
>> > --
>> > Meelis Roos (mroos@linux.ee)
>>
>
> --
> Meelis Roos (mroos@linux.ee)
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index 061b227..48ef97c 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -49,6 +49,11 @@  static bool igp_read_bios_from_vram(struct radeon_device *rdev)
 	resource_size_t vram_base;
 	resource_size_t size = 256 * 1024; /* ??? */
 
+#ifndef CONFIG_X86
+	/* IGP only exists on X86 (32- and 64-bit) */
+	return false;
+#endif
+
 	if (!(rdev->flags & RADEON_IS_IGP))
 		if (!radeon_card_posted(rdev))
 			return false;