Re: Another SIGFPE in display code, now in cirrus

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/12/2010 04:45 PM, Stefano Stabellini wrote:

Note it's just during mode changes.  During normal operation I'm sure
the pitches are equal.

The source blt pitch as set by the driver is always equal to the display
pitch (apart from the case reported above).
However cirrus_blt_srcpitch is not always equal to the display pitch
because of CIRRUS_BLTMODE_BACKWARDS: cirrus_blt_srcpitch can also be
negative and equal to -display_pitch.

Yes.

I suggest to start using the display pitch (with the proper sign)
instead of cirrus_blt_srcpitch in cirrus_do_copy at least when
cirrus_blt_srcpitch doesn't have a proper value.

Why switch from one bug to the other?

It's perfectly possible to take into account both values. Of course when abs(blt_pitch) != display pitch we can't use console_copy, but so what.

I think the code should be something like

      if bitblt destination intersects display memory:
          if bitblt pitch == display pitch
             use console_copy
         else
             invalidate entire display

I think the following should be enough:

if bitblt destination intersects display memory:
     qemu_console_copy
else
     invalidate region

why do we need if bitblt pitch == display pitch or to invalidate
everything?

Because the region is not a rectangle anymore. We could compute exactly what needs to be invalidated, but since it will never happen, there's no point in optimizing it.

31c05501c says this breaks bitblt, but I can't see why this is true.
The copy should update the display.  This is probably due to a
miscalculation of the affected region, and now we have two invalidates
instead of one, reducing performance.


I agree with you: qemu_console_copy does imply that the copied portion
of the screen changed, so there is no reason to invalidate it again if
qemu_console_copy is called.

Well, we can't just revert 31c05501c.  There's probably another bug
involved.

Sure, we have to fix the other one first :)

And find it.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux