All of lore.kernel.org
 help / color / mirror / Atom feed
* mgag200: Wrong looking code in mga_probe_vram
@ 2017-10-11  9:27 Michal Srb
  2017-10-18 11:49 ` Mathieu Larouche
  0 siblings, 1 reply; 2+ messages in thread
From: Michal Srb @ 2017-10-11  9:27 UTC (permalink / raw)
  To: dri-devel; +Cc: Dave Airlie, Mathieu Larouche

Hi,

While reading the mgag200 driver I noticed something that looks wrong.

In mgag200_main.c mga_probe_vram function:

    orig1 = ioread8(mem + offset);
    orig2 = ioread8(mem + offset + 0x100);

    ...

    iowrite16(orig1, mem + offset);
    iowrite16(orig2, mem + offset + 0x100);

I do not understand how exactly the probing works, but ioread8 followed by 
iowrite16 does not look correct.

The corresponding function in xf86-video-mga UMS driver is MGACountRam. It 
does the equivalent of read 16, write 16. So in case you can confirm that the 
kernel driver is correct, it would probably mean the user space driver is 
wrong.

Thanks,
Michal Srb
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: mgag200: Wrong looking code in mga_probe_vram
  2017-10-11  9:27 mgag200: Wrong looking code in mga_probe_vram Michal Srb
@ 2017-10-18 11:49 ` Mathieu Larouche
  0 siblings, 0 replies; 2+ messages in thread
From: Mathieu Larouche @ 2017-10-18 11:49 UTC (permalink / raw)
  To: Michal Srb, dri-devel; +Cc: Dave Airlie, Matt Russo

On 11/10/2017 5:27 AM, Michal Srb wrote:
> Hi,
>
> While reading the mgag200 driver I noticed something that looks wrong.
>
> In mgag200_main.c mga_probe_vram function:
>
>      orig1 = ioread8(mem + offset);
>      orig2 = ioread8(mem + offset + 0x100);
>
>      ...
>
>      iowrite16(orig1, mem + offset);
>      iowrite16(orig2, mem + offset + 0x100);
>
> I do not understand how exactly the probing works, but ioread8 followed by
> iowrite16 does not look correct.
>
> The corresponding function in xf86-video-mga UMS driver is MGACountRam. It
> does the equivalent of read 16, write 16. So in case you can confirm that the
> kernel driver is correct, it would probably mean the user space driver is
> wrong.
>
> Thanks,
> Michal Srb
>

Hi,

I agree with you, there's an issue there. The first two lines should be 
ioread16(). The goal of this routine is to detect the amount of memory 
available since it varies in some products. Basically, it stores the 
initial value, write a value and read it back, then it restores the 
initial value. In this case, only half of the value will be restored.

Mathieu
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-10-18 11:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-11  9:27 mgag200: Wrong looking code in mga_probe_vram Michal Srb
2017-10-18 11:49 ` Mathieu Larouche

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.