Th 12/20/2016 12:09, Alex Deucher wrote:
> On Tue, Dec 20, 2016 at 2:20 AM, Xue, Ken <Ken.Xue at 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 at 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
