All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Hellstrom <thellstrom@vmware.com>
To: Christopher Friedt <chrisfriedt@gmail.com>
Cc: linux-graphics-maintainer@vmware.com,
	linux-kernel@vger.kernel.org,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 1/1] drm/vmwgfx: correct fb_fix_screeninfo.line_length
Date: Mon, 30 Jun 2014 14:39:48 +0200	[thread overview]
Message-ID: <53B15A94.3010402@vmware.com> (raw)
In-Reply-To: <CAF4BF-T4VZQtd_3wWfNQ_==KJE0z0N5XonG4jcy=Ys5x_XxJ=w@mail.gmail.com>

On 06/30/2014 02:25 PM, Christopher Friedt wrote:
> On Mon, Jun 30, 2014 at 7:48 AM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>> I don't think we can blame video-vmware for this. A kernel driver change
>> that breaks existing user-space is by definition a kernel driver bug,
>> regardless whether exisiting user-space is doing something horrendously
>> stupid.
> I wouldn't be so quick to say it's a kernel bug. The fbdev contract
> hasn't changed. Also xf86-video-vmware isn't using the fbdev driver,
> and the fbdev driver code is obviously correct (see screenshots in
> link submitted with initial patch).

As I said. A kernel change that breaks existing user-space is ALWAYS BY
DEFINITION a kernel bug.
kernel changes are NOT ALLOWED to break existing user-space. The only
exception I can see here is if someone uses
the old non-kms driver which is not intended to work if vmware fbdev is
loaded but that's not the case here, from what I can tell?
>
>> So the fix must IMO be a kernel driver fix. My initial guess is that
>> once we set the bytes per line register, it might not be automatically
>> updated when the screen width is changed, but the documentation is poor.
>> I see if I can shed some light over this.
> Having dumped all of the svga registers while hacking on vmwgfx, I
> noticed that the BYTES_PER_LINE field is initially incorrectly set to
> something way off. My initial reaction is that video-vmware doesn't
> properly compute the bytes-per-line register, and therefore that it is
> a video-vmware bug that has always existed.

xf86-video-vmware in kms mode uses the kernel driver to set these registers.
FWIW, the modesetting part of the kernel driver uses SVGA_REG_PITCHLOCK
instead of
SVGA_REG_BYTES_PER_LINE to set the pitch. That's probably where the
clash happens.

/Thomas


>
> I'm reproducing the problem and providing a fix for video-vmware as I
> write this.
>
> C

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Hellstrom <thellstrom@vmware.com>
To: Christopher Friedt <chrisfriedt@gmail.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
	linux-kernel@vger.kernel.org, Dave Airlie <airlied@gmail.com>,
	linux-graphics-maintainer@vmware.com
Subject: Re: [PATCH 1/1] drm/vmwgfx: correct fb_fix_screeninfo.line_length
Date: Mon, 30 Jun 2014 14:39:48 +0200	[thread overview]
Message-ID: <53B15A94.3010402@vmware.com> (raw)
In-Reply-To: <CAF4BF-T4VZQtd_3wWfNQ_==KJE0z0N5XonG4jcy=Ys5x_XxJ=w@mail.gmail.com>

On 06/30/2014 02:25 PM, Christopher Friedt wrote:
> On Mon, Jun 30, 2014 at 7:48 AM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>> I don't think we can blame video-vmware for this. A kernel driver change
>> that breaks existing user-space is by definition a kernel driver bug,
>> regardless whether exisiting user-space is doing something horrendously
>> stupid.
> I wouldn't be so quick to say it's a kernel bug. The fbdev contract
> hasn't changed. Also xf86-video-vmware isn't using the fbdev driver,
> and the fbdev driver code is obviously correct (see screenshots in
> link submitted with initial patch).

As I said. A kernel change that breaks existing user-space is ALWAYS BY
DEFINITION a kernel bug.
kernel changes are NOT ALLOWED to break existing user-space. The only
exception I can see here is if someone uses
the old non-kms driver which is not intended to work if vmware fbdev is
loaded but that's not the case here, from what I can tell?
>
>> So the fix must IMO be a kernel driver fix. My initial guess is that
>> once we set the bytes per line register, it might not be automatically
>> updated when the screen width is changed, but the documentation is poor.
>> I see if I can shed some light over this.
> Having dumped all of the svga registers while hacking on vmwgfx, I
> noticed that the BYTES_PER_LINE field is initially incorrectly set to
> something way off. My initial reaction is that video-vmware doesn't
> properly compute the bytes-per-line register, and therefore that it is
> a video-vmware bug that has always existed.

xf86-video-vmware in kms mode uses the kernel driver to set these registers.
FWIW, the modesetting part of the kernel driver uses SVGA_REG_PITCHLOCK
instead of
SVGA_REG_BYTES_PER_LINE to set the pitch. That's probably where the
clash happens.

/Thomas


>
> I'm reproducing the problem and providing a fix for video-vmware as I
> write this.
>
> C

  reply	other threads:[~2014-06-30 12:39 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-28  0:45 [PATCH 1/1] drm/vmwgfx: correct fb_fix_screeninfo.line_length Christopher Friedt
2014-03-28  0:48 ` Christopher Friedt
2014-03-28  1:45 ` Dave Airlie
2014-03-28  1:45   ` Dave Airlie
2014-04-01 10:00   ` Thomas Hellstrom
2014-04-01 10:00     ` Thomas Hellstrom
2014-06-30 10:19     ` Christopher Friedt
2014-06-30 11:48       ` Thomas Hellstrom
2014-06-30 11:48         ` Thomas Hellstrom
2014-06-30 12:25         ` Christopher Friedt
2014-06-30 12:39           ` Thomas Hellstrom [this message]
2014-06-30 12:39             ` Thomas Hellstrom
2014-06-30 12:49             ` Christopher Friedt
2014-06-30 13:03               ` Thomas Hellstrom
2014-06-30 13:03                 ` Thomas Hellstrom
2014-06-30 13:18               ` Christopher Friedt
2014-06-30 13:22                 ` Thomas Hellstrom
2014-06-30 13:22                   ` Thomas Hellstrom
2014-07-02  3:01                   ` Christopher Friedt
2014-07-02  3:01                     ` Christopher Friedt
2014-07-02  4:16                     ` Thomas Hellstrom
2014-07-02  4:16                       ` Thomas Hellstrom
2014-07-02 12:15                       ` Christopher Friedt
2014-07-02 22:19                         ` Christopher Friedt
2014-07-02 22:19                           ` Christopher Friedt
  -- strict thread matches above, loose matches on Subject: below --
2014-02-01 15:26 Christopher Friedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53B15A94.3010402@vmware.com \
    --to=thellstrom@vmware.com \
    --cc=chrisfriedt@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-graphics-maintainer@vmware.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.