[PATCH] bios: fix OF loading

[PATCH] bios: fix OF loading

[Ilia Mirkin]

Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---

Tested on a PowerMac7,3 with a NV34 AGP adapter.

 drm/nouveau/nvkm/subdev/bios/priv.h     |  3 +++
 drm/nouveau/nvkm/subdev/bios/shadow.c   | 27 ++++++++++++++++++---------
 drm/nouveau/nvkm/subdev/bios/shadowof.c | 18 ++++++++++++++++--
 3 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/bios/priv.h b/drm/nouveau/nvkm/subdev/bios/priv.h
index e0ec2a6..212800e 100644
--- a/drm/nouveau/nvkm/subdev/bios/priv.h
+++ b/drm/nouveau/nvkm/subdev/bios/priv.h
@@ -8,7 +8,10 @@ struct nvbios_source {
 	void *(*init)(struct nvkm_bios *, const char *);
 	void  (*fini)(void *);
 	u32   (*read)(void *, u32 offset, u32 length, struct nvkm_bios *);
+	u32   (*size)(void *);
 	bool rw;
+	bool ignore_checksum;
+	bool no_pcir;
 };
 
 int nvbios_extend(struct nvkm_bios *, u32 length);
diff --git a/drm/nouveau/nvkm/subdev/bios/shadow.c b/drm/nouveau/nvkm/subdev/bios/shadow.c
index 792f017..b2557e8 100644
--- a/drm/nouveau/nvkm/subdev/bios/shadow.c
+++ b/drm/nouveau/nvkm/subdev/bios/shadow.c
@@ -45,7 +45,7 @@ shadow_fetch(struct nvkm_bios *bios, struct shadow *mthd, u32 upto)
 		u32 read = mthd->func->read(data, start, limit - start, bios);
 		bios->size = start + read;
 	}
-	return bios->size >= limit;
+	return bios->size >= upto;
 }
 
 static int
@@ -55,14 +55,22 @@ shadow_image(struct nvkm_bios *bios, int idx, u32 offset, struct shadow *mthd)
 	struct nvbios_image image;
 	int score = 1;
 
-	if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
-		nvkm_debug(subdev, "%08x: header fetch failed\n", offset);
-		return 0;
-	}
+	if (mthd->func->no_pcir) {
+		image.base = 0;
+		image.type = 0;
+		image.size = mthd->func->size(mthd->data);
+		image.last = 1;
+	} else {
+		if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
+			nvkm_debug(subdev, "%08x: header fetch failed\n",
+				   offset);
+			return 0;
+		}
 
-	if (!nvbios_image(bios, idx, &image)) {
-		nvkm_debug(subdev, "image %d invalid\n", idx);
-		return 0;
+		if (!nvbios_image(bios, idx, &image)) {
+			nvkm_debug(subdev, "image %d invalid\n", idx);
+			return 0;
+		}
 	}
 	nvkm_debug(subdev, "%08x: type %02x, %d bytes\n",
 		   image.base, image.type, image.size);
@@ -74,7 +82,8 @@ shadow_image(struct nvkm_bios *bios, int idx, u32 offset, struct shadow *mthd)
 
 	switch (image.type) {
 	case 0x00:
-		if (nvbios_checksum(&bios->data[image.base], image.size)) {
+		if (!mthd->func->ignore_checksum &&
+		    nvbios_checksum(&bios->data[image.base], image.size)) {
 			nvkm_debug(subdev, "%08x: checksum failed\n",
 				   image.base);
 			if (mthd->func->rw)
diff --git a/drm/nouveau/nvkm/subdev/bios/shadowof.c b/drm/nouveau/nvkm/subdev/bios/shadowof.c
index 29a37f0..4a20584 100644
--- a/drm/nouveau/nvkm/subdev/bios/shadowof.c
+++ b/drm/nouveau/nvkm/subdev/bios/shadowof.c
@@ -22,6 +22,7 @@
  */
 #include "priv.h"
 
+#include <core/pci.h>
 
 #if defined(__powerpc__)
 struct priv {
@@ -33,17 +34,27 @@ static u32
 of_read(void *data, u32 offset, u32 length, struct nvkm_bios *bios)
 {
 	struct priv *priv = data;
-	if (offset + length <= priv->size) {
+	if (offset < priv->size) {
+		length = min_t(u32, length, priv->size - offset);
 		memcpy_fromio(bios->data + offset, priv->data + offset, length);
 		return length;
 	}
 	return 0;
 }
 
+static u32
+of_size(void *data)
+{
+	struct priv *priv = data;
+
+	return priv->size;
+}
+
 static void *
 of_init(struct nvkm_bios *bios, const char *name)
 {
-	struct pci_dev *pdev = bios->subdev.device->pdev;
+	struct nvkm_device *device = bios->subdev.device;
+	struct pci_dev *pdev = device->func->pci(device)->pdev;
 	struct device_node *dn;
 	struct priv *priv;
 	if (!(dn = pci_device_to_OF_node(pdev)))
@@ -62,7 +73,10 @@ nvbios_of = {
 	.init = of_init,
 	.fini = (void(*)(void *))kfree,
 	.read = of_read,
+	.size = of_size,
 	.rw = false,
+	.ignore_checksum = true,
+	.no_pcir = true,
 };
 #else
 const struct nvbios_source
-- 
2.4.9

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] bios: fix OF loading

[Hans de Goede]

Hi,

On 02-10-15 05:41, Ilia Mirkin wrote:

<nothing>

As someone who has recently started following nouveau I must say that
it would greatly help me (and likely others) if patches likes this would
come with a somewhat more descriptive commit message.

Otherwise keep up the good work!

Regards,

Hans


p.s.

I've seen your message on the mesa fire demo on nv34, firing that up now.

> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
>
> Tested on a PowerMac7,3 with a NV34 AGP adapter.
>
>   drm/nouveau/nvkm/subdev/bios/priv.h     |  3 +++
>   drm/nouveau/nvkm/subdev/bios/shadow.c   | 27 ++++++++++++++++++---------
>   drm/nouveau/nvkm/subdev/bios/shadowof.c | 18 ++++++++++++++++--
>   3 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/drm/nouveau/nvkm/subdev/bios/priv.h b/drm/nouveau/nvkm/subdev/bios/priv.h
> index e0ec2a6..212800e 100644
> --- a/drm/nouveau/nvkm/subdev/bios/priv.h
> +++ b/drm/nouveau/nvkm/subdev/bios/priv.h
> @@ -8,7 +8,10 @@ struct nvbios_source {
>   	void *(*init)(struct nvkm_bios *, const char *);
>   	void  (*fini)(void *);
>   	u32   (*read)(void *, u32 offset, u32 length, struct nvkm_bios *);
> +	u32   (*size)(void *);
>   	bool rw;
> +	bool ignore_checksum;
> +	bool no_pcir;
>   };
>
>   int nvbios_extend(struct nvkm_bios *, u32 length);
> diff --git a/drm/nouveau/nvkm/subdev/bios/shadow.c b/drm/nouveau/nvkm/subdev/bios/shadow.c
> index 792f017..b2557e8 100644
> --- a/drm/nouveau/nvkm/subdev/bios/shadow.c
> +++ b/drm/nouveau/nvkm/subdev/bios/shadow.c
> @@ -45,7 +45,7 @@ shadow_fetch(struct nvkm_bios *bios, struct shadow *mthd, u32 upto)
>   		u32 read = mthd->func->read(data, start, limit - start, bios);
>   		bios->size = start + read;
>   	}
> -	return bios->size >= limit;
> +	return bios->size >= upto;
>   }
>
>   static int
> @@ -55,14 +55,22 @@ shadow_image(struct nvkm_bios *bios, int idx, u32 offset, struct shadow *mthd)
>   	struct nvbios_image image;
>   	int score = 1;
>
> -	if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
> -		nvkm_debug(subdev, "%08x: header fetch failed\n", offset);
> -		return 0;
> -	}
> +	if (mthd->func->no_pcir) {
> +		image.base = 0;
> +		image.type = 0;
> +		image.size = mthd->func->size(mthd->data);
> +		image.last = 1;
> +	} else {
> +		if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
> +			nvkm_debug(subdev, "%08x: header fetch failed\n",
> +				   offset);
> +			return 0;
> +		}
>
> -	if (!nvbios_image(bios, idx, &image)) {
> -		nvkm_debug(subdev, "image %d invalid\n", idx);
> -		return 0;
> +		if (!nvbios_image(bios, idx, &image)) {
> +			nvkm_debug(subdev, "image %d invalid\n", idx);
> +			return 0;
> +		}
>   	}
>   	nvkm_debug(subdev, "%08x: type %02x, %d bytes\n",
>   		   image.base, image.type, image.size);
> @@ -74,7 +82,8 @@ shadow_image(struct nvkm_bios *bios, int idx, u32 offset, struct shadow *mthd)
>
>   	switch (image.type) {
>   	case 0x00:
> -		if (nvbios_checksum(&bios->data[image.base], image.size)) {
> +		if (!mthd->func->ignore_checksum &&
> +		    nvbios_checksum(&bios->data[image.base], image.size)) {
>   			nvkm_debug(subdev, "%08x: checksum failed\n",
>   				   image.base);
>   			if (mthd->func->rw)
> diff --git a/drm/nouveau/nvkm/subdev/bios/shadowof.c b/drm/nouveau/nvkm/subdev/bios/shadowof.c
> index 29a37f0..4a20584 100644
> --- a/drm/nouveau/nvkm/subdev/bios/shadowof.c
> +++ b/drm/nouveau/nvkm/subdev/bios/shadowof.c
> @@ -22,6 +22,7 @@
>    */
>   #include "priv.h"
>
> +#include <core/pci.h>
>
>   #if defined(__powerpc__)
>   struct priv {
> @@ -33,17 +34,27 @@ static u32
>   of_read(void *data, u32 offset, u32 length, struct nvkm_bios *bios)
>   {
>   	struct priv *priv = data;
> -	if (offset + length <= priv->size) {
> +	if (offset < priv->size) {
> +		length = min_t(u32, length, priv->size - offset);
>   		memcpy_fromio(bios->data + offset, priv->data + offset, length);
>   		return length;
>   	}
>   	return 0;
>   }
>
> +static u32
> +of_size(void *data)
> +{
> +	struct priv *priv = data;
> +
> +	return priv->size;
> +}
> +
>   static void *
>   of_init(struct nvkm_bios *bios, const char *name)
>   {
> -	struct pci_dev *pdev = bios->subdev.device->pdev;
> +	struct nvkm_device *device = bios->subdev.device;
> +	struct pci_dev *pdev = device->func->pci(device)->pdev;
>   	struct device_node *dn;
>   	struct priv *priv;
>   	if (!(dn = pci_device_to_OF_node(pdev)))
> @@ -62,7 +73,10 @@ nvbios_of = {
>   	.init = of_init,
>   	.fini = (void(*)(void *))kfree,
>   	.read = of_read,
> +	.size = of_size,
>   	.rw = false,
> +	.ignore_checksum = true,
> +	.no_pcir = true,
>   };
>   #else
>   const struct nvbios_source
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] bios: fix OF loading

[Ilia Mirkin]

On Fri, Oct 2, 2015 at 3:18 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 02-10-15 05:41, Ilia Mirkin wrote:
>
> <nothing>
>
> As someone who has recently started following nouveau I must say that
> it would greatly help me (and likely others) if patches likes this would
> come with a somewhat more descriptive commit message.

Duly noted. I normally try to say a bit, but have gotten lazy of late.
Ben's pretty terse too :) How about something like

"""
Currently OF bios load fails for a few reasons:
 - checksum failure
 - bios size too small
 - no PCIR header
 - bios length not a multiple of 4

In this change, we resolve all of the above by ignoring any checksum
failures, and faking the PCIR data when loading from OF.
"""

>
> Otherwise keep up the good work!
>
> Regards,
>
> Hans
>
>
> p.s.
>
> I've seen your message on the mesa fire demo on nv34, firing that up now.
>
>
>> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
>> ---
>>
>> Tested on a PowerMac7,3 with a NV34 AGP adapter.
>>
>>   drm/nouveau/nvkm/subdev/bios/priv.h     |  3 +++
>>   drm/nouveau/nvkm/subdev/bios/shadow.c   | 27 ++++++++++++++++++---------
>>   drm/nouveau/nvkm/subdev/bios/shadowof.c | 18 ++++++++++++++++--
>>   3 files changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/drm/nouveau/nvkm/subdev/bios/priv.h
>> b/drm/nouveau/nvkm/subdev/bios/priv.h
>> index e0ec2a6..212800e 100644
>> --- a/drm/nouveau/nvkm/subdev/bios/priv.h
>> +++ b/drm/nouveau/nvkm/subdev/bios/priv.h
>> @@ -8,7 +8,10 @@ struct nvbios_source {
>>         void *(*init)(struct nvkm_bios *, const char *);
>>         void  (*fini)(void *);
>>         u32   (*read)(void *, u32 offset, u32 length, struct nvkm_bios *);
>> +       u32   (*size)(void *);
>>         bool rw;
>> +       bool ignore_checksum;
>> +       bool no_pcir;
>>   };
>>
>>   int nvbios_extend(struct nvkm_bios *, u32 length);
>> diff --git a/drm/nouveau/nvkm/subdev/bios/shadow.c
>> b/drm/nouveau/nvkm/subdev/bios/shadow.c
>> index 792f017..b2557e8 100644
>> --- a/drm/nouveau/nvkm/subdev/bios/shadow.c
>> +++ b/drm/nouveau/nvkm/subdev/bios/shadow.c
>> @@ -45,7 +45,7 @@ shadow_fetch(struct nvkm_bios *bios, struct shadow
>> *mthd, u32 upto)
>>                 u32 read = mthd->func->read(data, start, limit - start,
>> bios);
>>                 bios->size = start + read;
>>         }
>> -       return bios->size >= limit;
>> +       return bios->size >= upto;
>>   }
>>
>>   static int
>> @@ -55,14 +55,22 @@ shadow_image(struct nvkm_bios *bios, int idx, u32
>> offset, struct shadow *mthd)
>>         struct nvbios_image image;
>>         int score = 1;
>>
>> -       if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
>> -               nvkm_debug(subdev, "%08x: header fetch failed\n", offset);
>> -               return 0;
>> -       }
>> +       if (mthd->func->no_pcir) {
>> +               image.base = 0;
>> +               image.type = 0;
>> +               image.size = mthd->func->size(mthd->data);
>> +               image.last = 1;
>> +       } else {
>> +               if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
>> +                       nvkm_debug(subdev, "%08x: header fetch failed\n",
>> +                                  offset);
>> +                       return 0;
>> +               }
>>
>> -       if (!nvbios_image(bios, idx, &image)) {
>> -               nvkm_debug(subdev, "image %d invalid\n", idx);
>> -               return 0;
>> +               if (!nvbios_image(bios, idx, &image)) {
>> +                       nvkm_debug(subdev, "image %d invalid\n", idx);
>> +                       return 0;
>> +               }
>>         }
>>         nvkm_debug(subdev, "%08x: type %02x, %d bytes\n",
>>                    image.base, image.type, image.size);
>> @@ -74,7 +82,8 @@ shadow_image(struct nvkm_bios *bios, int idx, u32
>> offset, struct shadow *mthd)
>>
>>         switch (image.type) {
>>         case 0x00:
>> -               if (nvbios_checksum(&bios->data[image.base], image.size))
>> {
>> +               if (!mthd->func->ignore_checksum &&
>> +                   nvbios_checksum(&bios->data[image.base], image.size))
>> {
>>                         nvkm_debug(subdev, "%08x: checksum failed\n",
>>                                    image.base);
>>                         if (mthd->func->rw)
>> diff --git a/drm/nouveau/nvkm/subdev/bios/shadowof.c
>> b/drm/nouveau/nvkm/subdev/bios/shadowof.c
>> index 29a37f0..4a20584 100644
>> --- a/drm/nouveau/nvkm/subdev/bios/shadowof.c
>> +++ b/drm/nouveau/nvkm/subdev/bios/shadowof.c
>> @@ -22,6 +22,7 @@
>>    */
>>   #include "priv.h"
>>
>> +#include <core/pci.h>
>>
>>   #if defined(__powerpc__)
>>   struct priv {
>> @@ -33,17 +34,27 @@ static u32
>>   of_read(void *data, u32 offset, u32 length, struct nvkm_bios *bios)
>>   {
>>         struct priv *priv = data;
>> -       if (offset + length <= priv->size) {
>> +       if (offset < priv->size) {
>> +               length = min_t(u32, length, priv->size - offset);
>>                 memcpy_fromio(bios->data + offset, priv->data + offset,
>> length);
>>                 return length;
>>         }
>>         return 0;
>>   }
>>
>> +static u32
>> +of_size(void *data)
>> +{
>> +       struct priv *priv = data;
>> +
>> +       return priv->size;
>> +}
>> +
>>   static void *
>>   of_init(struct nvkm_bios *bios, const char *name)
>>   {
>> -       struct pci_dev *pdev = bios->subdev.device->pdev;
>> +       struct nvkm_device *device = bios->subdev.device;
>> +       struct pci_dev *pdev = device->func->pci(device)->pdev;
>>         struct device_node *dn;
>>         struct priv *priv;
>>         if (!(dn = pci_device_to_OF_node(pdev)))
>> @@ -62,7 +73,10 @@ nvbios_of = {
>>         .init = of_init,
>>         .fini = (void(*)(void *))kfree,
>>         .read = of_read,
>> +       .size = of_size,
>>         .rw = false,
>> +       .ignore_checksum = true,
>> +       .no_pcir = true,
>>   };
>>   #else
>>   const struct nvbios_source
>>
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] bios: fix OF loading

[Hans de Goede]

Hi,

On 02-10-15 09:26, Ilia Mirkin wrote:
> On Fri, Oct 2, 2015 at 3:18 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 02-10-15 05:41, Ilia Mirkin wrote:
>>
>> <nothing>
>>
>> As someone who has recently started following nouveau I must say that
>> it would greatly help me (and likely others) if patches likes this would
>> come with a somewhat more descriptive commit message.
>
> Duly noted. I normally try to say a bit, but have gotten lazy of late.
> Ben's pretty terse too :) How about something like
>
> """
> Currently OF bios load fails for a few reasons:
>   - checksum failure
>   - bios size too small
>   - no PCIR header
>   - bios length not a multiple of 4
>
> In this change, we resolve all of the above by ignoring any checksum
> failures, and faking the PCIR data when loading from OF.
> """

Much better, I must say that now that you spell it out that the
ignoring of the checksum sort of worries me, but I guess the
checksum is part of the missing PCIR header ?

Regards,

Hans
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] bios: fix OF loading

[Ilia Mirkin]

On Fri, Oct 2, 2015 at 3:35 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 02-10-15 09:26, Ilia Mirkin wrote:
>>
>> On Fri, Oct 2, 2015 at 3:18 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 02-10-15 05:41, Ilia Mirkin wrote:
>>>
>>> <nothing>
>>>
>>> As someone who has recently started following nouveau I must say that
>>> it would greatly help me (and likely others) if patches likes this would
>>> come with a somewhat more descriptive commit message.
>>
>>
>> Duly noted. I normally try to say a bit, but have gotten lazy of late.
>> Ben's pretty terse too :) How about something like
>>
>> """
>> Currently OF bios load fails for a few reasons:
>>   - checksum failure
>>   - bios size too small
>>   - no PCIR header
>>   - bios length not a multiple of 4
>>
>> In this change, we resolve all of the above by ignoring any checksum
>> failures, and faking the PCIR data when loading from OF.
>> """
>
>
> Much better, I must say that now that you spell it out that the
> ignoring of the checksum sort of worries me, but I guess the
> checksum is part of the missing PCIR header ?

Nope, the two things are unrelated. The PCIR section is normally
embedded in the vbios and tells you how long the vbios is, and allows
you to have multiple elements appended at the end (which nouveau has
recently acquired some support for). The checksum failures just
happen, pretty sure that the OF-sourced vbioses just don't have a
checksum byte at all. And they cause nouveau to try to load a vbios
from a different source, which almost invariably fails to actually
parse properly. (Why? Because you should just use the one from OF,
duh!)

  -ilia
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] bios: fix OF loading

[Hans de Goede]

Hi,

On 02-10-15 09:38, Ilia Mirkin wrote:
> On Fri, Oct 2, 2015 at 3:35 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 02-10-15 09:26, Ilia Mirkin wrote:
>>>
>>> On Fri, Oct 2, 2015 at 3:18 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 02-10-15 05:41, Ilia Mirkin wrote:
>>>>
>>>> <nothing>
>>>>
>>>> As someone who has recently started following nouveau I must say that
>>>> it would greatly help me (and likely others) if patches likes this would
>>>> come with a somewhat more descriptive commit message.
>>>
>>>
>>> Duly noted. I normally try to say a bit, but have gotten lazy of late.
>>> Ben's pretty terse too :) How about something like
>>>
>>> """
>>> Currently OF bios load fails for a few reasons:
>>>    - checksum failure
>>>    - bios size too small
>>>    - no PCIR header
>>>    - bios length not a multiple of 4
>>>
>>> In this change, we resolve all of the above by ignoring any checksum
>>> failures, and faking the PCIR data when loading from OF.
>>> """
>>
>>
>> Much better, I must say that now that you spell it out that the
>> ignoring of the checksum sort of worries me, but I guess the
>> checksum is part of the missing PCIR header ?
>
> Nope, the two things are unrelated. The PCIR section is normally
> embedded in the vbios and tells you how long the vbios is, and allows
> you to have multiple elements appended at the end (which nouveau has
> recently acquired some support for). The checksum failures just
> happen,

> "pretty sure that the OF-sourced vbioses just don't have a
>  checksum byte at all."

You may want to add something along those lines to the commit message
too...

Regards,

Hans
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] bios: fix OF loading

[Ilia Mirkin]

On Fri, Oct 2, 2015 at 3:39 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 02-10-15 09:38, Ilia Mirkin wrote:
>>
>> On Fri, Oct 2, 2015 at 3:35 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 02-10-15 09:26, Ilia Mirkin wrote:
>>>>
>>>>
>>>> On Fri, Oct 2, 2015 at 3:18 AM, Hans de Goede <hdegoede@redhat.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 02-10-15 05:41, Ilia Mirkin wrote:
>>>>>
>>>>> <nothing>
>>>>>
>>>>> As someone who has recently started following nouveau I must say that
>>>>> it would greatly help me (and likely others) if patches likes this
>>>>> would
>>>>> come with a somewhat more descriptive commit message.
>>>>
>>>>
>>>>
>>>> Duly noted. I normally try to say a bit, but have gotten lazy of late.
>>>> Ben's pretty terse too :) How about something like
>>>>
>>>> """
>>>> Currently OF bios load fails for a few reasons:
>>>>    - checksum failure
>>>>    - bios size too small
>>>>    - no PCIR header
>>>>    - bios length not a multiple of 4
>>>>
>>>> In this change, we resolve all of the above by ignoring any checksum
>>>> failures, and faking the PCIR data when loading from OF.
>>>> """
>>>
>>>
>>>
>>> Much better, I must say that now that you spell it out that the
>>> ignoring of the checksum sort of worries me, but I guess the
>>> checksum is part of the missing PCIR header ?
>>
>>
>> Nope, the two things are unrelated. The PCIR section is normally
>> embedded in the vbios and tells you how long the vbios is, and allows
>> you to have multiple elements appended at the end (which nouveau has
>> recently acquired some support for). The checksum failures just
>> happen,
>
>
>> "pretty sure that the OF-sourced vbioses just don't have a
>>  checksum byte at all."
>
>
> You may want to add something along those lines to the commit message
> too...

Ben, I see you already committed my change to your tree, could you
amend the commit message to read something like

"""
Currently OF bios load fails for a few reasons:
 - checksum failure
 - bios size too small
 - no PCIR header
 - bios length not a multiple of 4

In this change, we resolve all of the above by ignoring any checksum
failures (since OF VBIOS tends not to have a checksum), and faking the
PCIR data when loading from OF.
"""

I think Hans has a very valid point about the terseness of nouveau
commit messages... I'm often a bit annoyed at the terseness of yours
esp for larger changes, but apparently I go and do the same thing
myself :)

Thanks,

  -ilia
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau