All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Keeping <john@metanate.com>
To: Dave Airlie <airlied@gmail.com>
Cc: "Michel Dänzer" <michel.daenzer@amd.com>,
	"Maling list - DRI developers" <dri-devel@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/qxl: fix cursor position with non-zero hotspot
Date: Mon, 8 Feb 2016 11:32:32 +0000	[thread overview]
Message-ID: <20160208113232.02bc534a.john@metanate.com> (raw)
In-Reply-To: <20151203104607.1a9a757a.john@metanate.com>

I haven't seen this get picked up.  Is there something wrong with the
patch or was it just lost in the noise?

On Thu, 3 Dec 2015 10:46:07 +0000 John Keeping wrote:

> On Wed, 18 Nov 2015 13:58:23 +0100, Daniel Vetter wrote:
> 
> > On Wed, Nov 18, 2015 at 11:17:25AM +0000, John Keeping wrote:  
> > > The SPICE protocol considers the position of a cursor to be the
> > > location of its active pixel on the display, so the cursor is drawn
> > > with its top-left corner at "(x - hot_spot_x, y - hot_spot_y)" but
> > > the DRM cursor position gives the location where the top-left
> > > corner should be drawn, with the hotspot being a hint for drivers
> > > that need it.
> > > 
> > > This fixes the location of the window resize cursors when using
> > > Fluxbox with the QXL DRM driver and both the QXL and modesetting X
> > > drivers.
> > > 
> > > Signed-off-by: John Keeping <john@metanate.com>
> > > ---
> > > This comes from the discussion in the thread at [1].  My test
> > > environment is using:
> > > 
> > >     - X.Org X Server 1.17.4
> > >     - xf86-video-qxl 0.1.4
> > >     - spice-gtk3 0.30
> > > 
> > > [1]
> > > http://lists.freedesktop.org/archives/dri-devel/2015-November/thread.html#94650
> > > 
> > >  drivers/gpu/drm/qxl/qxl_display.c | 13 +++++++++----
> > >  drivers/gpu/drm/qxl/qxl_drv.h     |  2 ++
> > >  2 files changed, 11 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> > > b/drivers/gpu/drm/qxl/qxl_display.c index 183aea1..5edebf4 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_display.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_display.c
> > > @@ -375,10 +375,15 @@ static int qxl_crtc_cursor_set2(struct
> > > drm_crtc *crtc, 
> > >  	qxl_bo_kunmap(user_bo);
> > >  
> > > +	qcrtc->cur_x += qcrtc->hot_spot_x - hot_x;
> > > +	qcrtc->cur_y += qcrtc->hot_spot_y - hot_y;
> > > +	qcrtc->hot_spot_x = hot_x;
> > > +	qcrtc->hot_spot_y = hot_y;    
> > 
> > Yay for the artificial split of the legacy driver interface for
> > cursors.
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: stable@vger.kernel.org
> > 
> > Dave, can you pls pick this up if you agree?  
> 
> Ping?
> 
> > > +
> > >  	cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev,
> > > release); cmd->type = QXL_CURSOR_SET;
> > > -	cmd->u.set.position.x = qcrtc->cur_x;
> > > -	cmd->u.set.position.y = qcrtc->cur_y;
> > > +	cmd->u.set.position.x = qcrtc->cur_x + qcrtc->hot_spot_x;
> > > +	cmd->u.set.position.y = qcrtc->cur_y + qcrtc->hot_spot_y;
> > >  
> > >  	cmd->u.set.shape = qxl_bo_physical_address(qdev,
> > > cursor_bo, 0); 
> > > @@ -441,8 +446,8 @@ static int qxl_crtc_cursor_move(struct drm_crtc
> > > *crtc, 
> > >  	cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev,
> > > release); cmd->type = QXL_CURSOR_MOVE;
> > > -	cmd->u.position.x = qcrtc->cur_x;
> > > -	cmd->u.position.y = qcrtc->cur_y;
> > > +	cmd->u.position.x = qcrtc->cur_x + qcrtc->hot_spot_x;
> > > +	cmd->u.position.y = qcrtc->cur_y + qcrtc->hot_spot_y;
> > >  	qxl_release_unmap(qdev, release, &cmd->release_info);
> > >  
> > >  	qxl_push_cursor_ring_release(qdev, release,
> > > QXL_CMD_CURSOR, false); diff --git a/drivers/gpu/drm/qxl/qxl_drv.h
> > > b/drivers/gpu/drm/qxl/qxl_drv.h index 01a8694..3ab9017 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_drv.h
> > > +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> > > @@ -135,6 +135,8 @@ struct qxl_crtc {
> > >  	int index;
> > >  	int cur_x;
> > >  	int cur_y;
> > > +	int hot_spot_x;
> > > +	int hot_spot_y;
> > >  };
> > >  
> > >  struct qxl_output {
> > > -- 
> > > 2.6.3.462.gbe2c914
> > >     
> >   

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

WARNING: multiple messages have this Message-ID (diff)
From: John Keeping <john@metanate.com>
To: Dave Airlie <airlied@gmail.com>
Cc: "Daniel Vetter" <daniel@ffwll.ch>,
	"Maling list - DRI developers" <dri-devel@lists.freedesktop.org>,
	"Michel Dänzer" <michel.daenzer@amd.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/qxl: fix cursor position with non-zero hotspot
Date: Mon, 8 Feb 2016 11:32:32 +0000	[thread overview]
Message-ID: <20160208113232.02bc534a.john@metanate.com> (raw)
In-Reply-To: <20151203104607.1a9a757a.john@metanate.com>

I haven't seen this get picked up.  Is there something wrong with the
patch or was it just lost in the noise?

On Thu, 3 Dec 2015 10:46:07 +0000 John Keeping wrote:

> On Wed, 18 Nov 2015 13:58:23 +0100, Daniel Vetter wrote:
> 
> > On Wed, Nov 18, 2015 at 11:17:25AM +0000, John Keeping wrote:  
> > > The SPICE protocol considers the position of a cursor to be the
> > > location of its active pixel on the display, so the cursor is drawn
> > > with its top-left corner at "(x - hot_spot_x, y - hot_spot_y)" but
> > > the DRM cursor position gives the location where the top-left
> > > corner should be drawn, with the hotspot being a hint for drivers
> > > that need it.
> > > 
> > > This fixes the location of the window resize cursors when using
> > > Fluxbox with the QXL DRM driver and both the QXL and modesetting X
> > > drivers.
> > > 
> > > Signed-off-by: John Keeping <john@metanate.com>
> > > ---
> > > This comes from the discussion in the thread at [1].  My test
> > > environment is using:
> > > 
> > >     - X.Org X Server 1.17.4
> > >     - xf86-video-qxl 0.1.4
> > >     - spice-gtk3 0.30
> > > 
> > > [1]
> > > http://lists.freedesktop.org/archives/dri-devel/2015-November/thread.html#94650
> > > 
> > >  drivers/gpu/drm/qxl/qxl_display.c | 13 +++++++++----
> > >  drivers/gpu/drm/qxl/qxl_drv.h     |  2 ++
> > >  2 files changed, 11 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> > > b/drivers/gpu/drm/qxl/qxl_display.c index 183aea1..5edebf4 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_display.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_display.c
> > > @@ -375,10 +375,15 @@ static int qxl_crtc_cursor_set2(struct
> > > drm_crtc *crtc, 
> > >  	qxl_bo_kunmap(user_bo);
> > >  
> > > +	qcrtc->cur_x += qcrtc->hot_spot_x - hot_x;
> > > +	qcrtc->cur_y += qcrtc->hot_spot_y - hot_y;
> > > +	qcrtc->hot_spot_x = hot_x;
> > > +	qcrtc->hot_spot_y = hot_y;    
> > 
> > Yay for the artificial split of the legacy driver interface for
> > cursors.
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: stable@vger.kernel.org
> > 
> > Dave, can you pls pick this up if you agree?  
> 
> Ping?
> 
> > > +
> > >  	cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev,
> > > release); cmd->type = QXL_CURSOR_SET;
> > > -	cmd->u.set.position.x = qcrtc->cur_x;
> > > -	cmd->u.set.position.y = qcrtc->cur_y;
> > > +	cmd->u.set.position.x = qcrtc->cur_x + qcrtc->hot_spot_x;
> > > +	cmd->u.set.position.y = qcrtc->cur_y + qcrtc->hot_spot_y;
> > >  
> > >  	cmd->u.set.shape = qxl_bo_physical_address(qdev,
> > > cursor_bo, 0); 
> > > @@ -441,8 +446,8 @@ static int qxl_crtc_cursor_move(struct drm_crtc
> > > *crtc, 
> > >  	cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev,
> > > release); cmd->type = QXL_CURSOR_MOVE;
> > > -	cmd->u.position.x = qcrtc->cur_x;
> > > -	cmd->u.position.y = qcrtc->cur_y;
> > > +	cmd->u.position.x = qcrtc->cur_x + qcrtc->hot_spot_x;
> > > +	cmd->u.position.y = qcrtc->cur_y + qcrtc->hot_spot_y;
> > >  	qxl_release_unmap(qdev, release, &cmd->release_info);
> > >  
> > >  	qxl_push_cursor_ring_release(qdev, release,
> > > QXL_CMD_CURSOR, false); diff --git a/drivers/gpu/drm/qxl/qxl_drv.h
> > > b/drivers/gpu/drm/qxl/qxl_drv.h index 01a8694..3ab9017 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_drv.h
> > > +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> > > @@ -135,6 +135,8 @@ struct qxl_crtc {
> > >  	int index;
> > >  	int cur_x;
> > >  	int cur_y;
> > > +	int hot_spot_x;
> > > +	int hot_spot_y;
> > >  };
> > >  
> > >  struct qxl_output {
> > > -- 
> > > 2.6.3.462.gbe2c914
> > >     
> >   

  reply	other threads:[~2016-02-08 11:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-18 11:17 [PATCH] drm/qxl: fix cursor position with non-zero hotspot John Keeping
2015-11-18 11:17 ` John Keeping
2015-11-18 12:58 ` Daniel Vetter
2015-11-18 12:58   ` Daniel Vetter
2015-12-03 10:46   ` John Keeping
2015-12-03 10:46     ` John Keeping
2016-02-08 11:32     ` John Keeping [this message]
2016-02-08 11:32       ` John Keeping
2016-02-09 14:31       ` Jani Nikula
2016-02-09 14:31         ` Jani Nikula

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=20160208113232.02bc534a.john@metanate.com \
    --to=john@metanate.com \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michel.daenzer@amd.com \
    /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.