On Thu, 20 Aug 2009 14:40:24 +0300
Pekka Paalanen <[email protected]> wrote:

> Hi,
> 
> questions will follow.

This patch builds, but I have not tried to run it.
I'm hoping Stuart or someone could comment on it.

> ---
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c 
> b/drivers/gpu/drm/nouveau/nouveau_bios.c
> index 99f7bd4..13b3fb1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> @@ -40,8 +40,6 @@
>  #define BIOSLOG(sip, fmt, arg...) NV_DEBUG(sip->dev, fmt, ##arg)
>  #define LOG_OLD_VALUE(x) //x
>  
> -#define BIOS_USLEEP(n) mdelay((n)/1000)
> -
>  #define ROM16(x) le16_to_cpu(*(uint16_t *)&(x))
>  #define ROM32(x) le32_to_cpu(*(uint32_t *)&(x))
>  
> @@ -50,6 +48,15 @@ struct init_exec {
>       bool repeat;
>  };
>  
> +static inline void bios_usleep(unsigned usecs)
> +{
> +     might_sleep();
> +     if (usecs < 1000 * MAX_UDELAY_MS)
> +             udelay(usecs);
> +     else
> +             msleep(usecs / 1000 + 1);
> +}

This function is to replace the BIOS_USLEEP() macro with something,
that works both for short and long delays. BIOS_USLEEP would truncate
all sleeps to milliseconds, meaning delays less than 1000 us would
be zero. Also, mdelay() is a busyloop delay burning CPU cycles for
nothing. msleep() is not, it will reschedule until the delay has
passed. The change raises two questions:

- Is the bios code ever called in a situation, where scheduling
is not allowed (interrupt context, pre-emption disabled, inside
spinlocks, etc.)?

- Is the delay accuracy on the msleep path critical?

>  static bool nv_cksum(const uint8_t *data, unsigned int length)
>  {
>       /* there's a few checksums in the BIOS, so here's a generic checking 
> function */
> @@ -262,7 +269,7 @@ static int parse_init_table(struct nvbios *, unsigned 
> int, struct init_exec *);
>  static void still_alive(void)
>  {
>  //   sync();
> -//   BIOS_USLEEP(2000);
> +//   bios_usleep(2000);
>  }
>  
>  static uint32_t
> @@ -1608,17 +1615,18 @@ init_condition_time(struct nvbios *bios, uint16_t 
> offset,
>  
>       for (; retries > 0; retries--)
>               if (bios_condition_met(bios, offset, cond)) {
> -                     BIOSLOG(bios, "0x%04X: Condition met, continuing\n", 
> offset);
> +                     BIOSLOG(bios, "0x%04X: Condition met, continuing\n",
> +                                                             offset);
>                       break;
>               } else {
>                       BIOSLOG(bios, "0x%04X: Condition not met, sleeping for 
> 20ms\n", offset);
> -                     BIOS_USLEEP(20000);
> +                     bios_usleep(20000);

It so happens, that 20000 was the maximum allowed udelay() constant time
for x86. Did the number come from that?

>               }
>  
>       if (!bios_condition_met(bios, offset, cond)) {
>               NV_WARN(bios->dev,
>                       "0x%04X: Condition still not met after %dms, "
> -                     "skiping following opcodes\n", offset, 20 * retries);
> +                     "skipping following opcodes\n", offset, 20 * retries);
>               iexec->execute = false;
>       }
>  
> @@ -1851,7 +1859,7 @@ init_reset(struct nvbios *bios, uint16_t offset, struct 
> init_exec *iexec)
>       bios_wr32(bios, NV_PBUS_PCI_NV_19, 0);
>       bios_wr32(bios, reg, value1);
>  
> -     BIOS_USLEEP(10);
> +     bios_usleep(10);

With the old BIOS_USLEEP, this delay would vanish.

>  
>       bios_wr32(bios, reg, value2);
>       bios_wr32(bios, NV_PBUS_PCI_NV_19, pci_nv_19);
> @@ -2233,8 +2241,7 @@ init_time(struct nvbios *bios, uint16_t offset, struct 
> init_exec *iexec)
>       BIOSLOG(bios, "0x%04X: Sleeping for 0x%04X microseconds\n",
>               offset, time);
>  
> -     BIOS_USLEEP(time);
> -
> +     bios_usleep(time);

Can here be delay accuracy issues? How much are we allowed to exceed the
specified delay?

>       return true;
>  }
>  
> @@ -2872,9 +2879,11 @@ static int call_lvds_manufacturer_script(struct 
> drm_device *dev, struct dcb_entr
>  
>       run_digital_op_script(dev, scriptofs, dcbent, head, bios->fp.dual_link);
>  
> -     if (script == LVDS_PANEL_OFF)
> +     if (script == LVDS_PANEL_OFF) {
>               /* off-on delay in ms */
> -             BIOS_USLEEP(ROM16(bios->data[bios->fp.xlated_entry + 7]));
> +             bios_usleep(ROM16(bios->data[bios->fp.xlated_entry + 7]));
> +     }

The comment and code disagree here. Is it microseconds or milliseconds?

-- 
Pekka Paalanen
http://www.iki.fi/pq/
_______________________________________________
Nouveau mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/nouveau

Reply via email to