Re: [PATCH] amdgpu/drm: Refine the way to check atom bios

[Ken Xue <kxue-5C7GfCeVMHo@public.gmane.org>]

Th 12/20/2016 12:09, Alex Deucher wrote:
> On Tue, Dec 20, 2016 at 2:20 AM, Xue, Ken <Ken.Xue@amd.com> wrote:
> > There are several ways to check out a ATOMBIOS. In previous codes, try
> > a new way to fetch out vbios/rom, until current vbios/rom is started with
> > 0x55aa, then check if this vbios is ATOMBIOS. Now, try a new way to fetch
> > out vbios until all flags of ATOMBIOS are verified.
> >
> > Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |   2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c   | 152 ++++++++++++++++-------------
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  10 +-
> >  3 files changed, 89 insertions(+), 75 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index b7f521a..dd8acb9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -305,7 +305,7 @@ struct amdgpu_ih_funcs {
> >  /*
> >   * BIOS.
> >   */
> > -bool amdgpu_get_bios(struct amdgpu_device *adev);
> > +bool amdgpu_get_atom_bios(struct amdgpu_device *adev);
> 
> No need to change the name of this function in my opinion, but I'm ok
> either way.
> 

Ok. I will revert the name of the function.

> >  bool amdgpu_read_bios(struct amdgpu_device *adev);
> >
> >  /*
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> > index 4f973a9..86e7b43 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> > @@ -42,6 +42,44 @@
> >  #define AMD_IS_VALID_VBIOS(p) ((p)[0] == 0x55 && (p)[1] == 0xAA)
> >  #define AMD_VBIOS_LENGTH(p) ((p)[2] << 9)
> >
> > +/* Check if current bios is an ATOM BIOS.
> > + * Return true if it is ATOM BIOS. Otherwise, return false.
> > + */
> > +static bool check_atom_bios(uint8_t __iomem *bios, size_t size)
> > +{
> > +       uint16_t tmp, bios_header_start;
> > +
> > +       if (!bios || size < 0x49) {
> > +               DRM_INFO("vbios mem is null or mem size is wrong\n");
> > +               return false;
> > +       }
> > +
> > +       if (!AMD_IS_VALID_VBIOS(bios)) {
> > +               DRM_INFO("BIOS signature incorrect %x %x\n", bios[0], bios[1]);
> > +               return false;
> > +       }
> > +
> > +       tmp = bios[0x18] | (bios[0x19] << 8);
> > +       if (bios[tmp + 0x14] != 0x0) {
> > +               DRM_INFO("Not an x86 BIOS ROM\n");
> > +               return false;
> > +       }
> > +
> > +       bios_header_start = bios[0x48] | (bios[0x49] << 8);
> > +       if (!bios_header_start) {
> > +               DRM_INFO("Can't locate bios header\n");
> > +               return false;
> > +       }
> > +       tmp = bios_header_start + 4;
> > +       if (!memcmp(bios + tmp, "ATOM", 4) ||
> > +           !memcmp(bios + tmp, "MOTA", 4) ||
> > +           (size < tmp))
> > +               return true;
> > +
> > +       return false;
> > +}
> > +
> > +
> >  /* If you boot an IGP board with a discrete card as the primary,
> >   * the IGP rom is not accessible via the rom bar as the IGP rom is
> >   * part of the system bios.  On boot, the system bios puts a
> > @@ -65,7 +103,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
> >                 return false;
> >         }
> >
> > -       if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
> > +       if (!check_atom_bios(bios, size)) {
> >                 iounmap(bios);
> >                 return false;
> >         }
> > @@ -82,7 +120,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
> >
> >  bool amdgpu_read_bios(struct amdgpu_device *adev)
> >  {
> > -       uint8_t __iomem *bios, val[2];
> > +       uint8_t __iomem *bios;
> >         size_t size;
> >
> >         adev->bios = NULL;
> > @@ -92,10 +130,7 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)
> >                 return false;
> >         }
> >
> > -       val[0] = readb(&bios[0]);
> > -       val[1] = readb(&bios[1]);
> 
> We need to use io accessors on some platforms rather than accessing
> memory directly for pcie resources.  This should be preseved.
> 
> Alex
> 

Sure, Understand it. And I can see that some part of the original file also
does not use "readb" to access iomem, I will send a new patch for keep same
coding style for the file amdgpu_bios.c.

Regards,
Ken
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx