All of lore.kernel.org
 help / color / mirror / Atom feed
From: siarhei.siamashka@gmail.com (Siarhei Siamashka)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/5] DRM: Armada: Add support for ARGB 32x64 or 64x32 hardware cursors
Date: Mon, 7 Oct 2013 15:29:15 +0300	[thread overview]
Message-ID: <20131007152915.352cc4b2@i7> (raw)
In-Reply-To: <20131007103250.GJ12758@n2100.arm.linux.org.uk>

On Mon, 7 Oct 2013 11:32:50 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Mon, Oct 07, 2013 at 12:09:22PM +0200, Jean-Francois Moine wrote:
> > On Mon, 7 Oct 2013 10:40:08 +0100
> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > 
> > > > > This patch adds ARGB hardware cursor support to the DRM driver for the
> > > > > Marvell Armada SoCs.  ARGB cursors are supported at either 32x64 or
> > > > > 64x32 resolutions.  
> > > > 	[snip]
> > > > 
> > > > I don't see the interest of such cursors. Actually, most often, the
> > > > cursors are 64x64 and 'A' is either 0 or 0xff. As the Armada 510
> > > > supports 64x64 cursors with transparency, it would be more useful to
> > > > implement these ones...  
> > > 
> > > Sorry, you're completely wrong there.  Xorg uses an alphablended cursor.
> > > This patch is a result of trialling each of the Armada's cursor options
> > > and this is the only one which results in a reasonable looking cursor.
> > 
> > Strange. I am using the 64x64 cursor with transparency of the 510 for
> > many months and I never saw any problem.
> 
> When I tried it, all cursors had variable alpha, and converting the
> alpha to a single transparency bit made the cursor look rubbish against
> certain backgrounds.
> 
> > If you absolutely want to have all transparency shades, you should
> > accept 64x64 cursors and test if they may be rendered as 64x32 or
> > 32x64, i.e. test that there is only pure transparency in the non-
> > rendered rectangles...
> 
> That is policy, and that is something which can be done by the X server
> rather than the kernel.  The kernel exposes the hardware capabilities,
> which are to allow a 64x32 or a 32x64 cursor.  Userspace can make the
> decision dynamically which it wishes to use.
> 
> However, what you're suggesting is dangerous.  What do you do when you're
> presented with a cursor which is 64x64 ?  Do you:
> 
> (a) not display it
> (b) crash the X server
> 
> There isn't a fallback to using software cursors available in the X server
> because there's no error reporting for a failed hardware cursor update.

Hmm, maybe I'm missing something, but doesn't returning FALSE from
UseHWCursorARGB just make the X server fallback to using a software
cursor?

https://github.com/ssvb/xf86-video-fbturbo/blob/689c08256555/src/sunxi_disp_hwcursor.c#L72

I'm just doing this. And also have hooks to notify the DRI2 code about
the status of the cursor. In the case if the hardware cursor is not
enabled, hardware overlays can't be safely used with the software
cursor and we need to fallback to CPU/GPU copy instead of zero-copy
buffers swapping.

And I fully agree that it is the responsibility of the kernel to expose
as much of the hardware capabilities as possible (without compromising
reliability and/or security).

-- 
Best regards,
Siarhei Siamashka

WARNING: multiple messages have this Message-ID (diff)
From: Siarhei Siamashka <siarhei.siamashka@gmail.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH 3/5] DRM: Armada: Add support for ARGB 32x64 or 64x32 hardware cursors
Date: Mon, 7 Oct 2013 15:29:15 +0300	[thread overview]
Message-ID: <20131007152915.352cc4b2@i7> (raw)
In-Reply-To: <20131007103250.GJ12758@n2100.arm.linux.org.uk>

On Mon, 7 Oct 2013 11:32:50 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Mon, Oct 07, 2013 at 12:09:22PM +0200, Jean-Francois Moine wrote:
> > On Mon, 7 Oct 2013 10:40:08 +0100
> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > 
> > > > > This patch adds ARGB hardware cursor support to the DRM driver for the
> > > > > Marvell Armada SoCs.  ARGB cursors are supported at either 32x64 or
> > > > > 64x32 resolutions.  
> > > > 	[snip]
> > > > 
> > > > I don't see the interest of such cursors. Actually, most often, the
> > > > cursors are 64x64 and 'A' is either 0 or 0xff. As the Armada 510
> > > > supports 64x64 cursors with transparency, it would be more useful to
> > > > implement these ones...  
> > > 
> > > Sorry, you're completely wrong there.  Xorg uses an alphablended cursor.
> > > This patch is a result of trialling each of the Armada's cursor options
> > > and this is the only one which results in a reasonable looking cursor.
> > 
> > Strange. I am using the 64x64 cursor with transparency of the 510 for
> > many months and I never saw any problem.
> 
> When I tried it, all cursors had variable alpha, and converting the
> alpha to a single transparency bit made the cursor look rubbish against
> certain backgrounds.
> 
> > If you absolutely want to have all transparency shades, you should
> > accept 64x64 cursors and test if they may be rendered as 64x32 or
> > 32x64, i.e. test that there is only pure transparency in the non-
> > rendered rectangles...
> 
> That is policy, and that is something which can be done by the X server
> rather than the kernel.  The kernel exposes the hardware capabilities,
> which are to allow a 64x32 or a 32x64 cursor.  Userspace can make the
> decision dynamically which it wishes to use.
> 
> However, what you're suggesting is dangerous.  What do you do when you're
> presented with a cursor which is 64x64 ?  Do you:
> 
> (a) not display it
> (b) crash the X server
> 
> There isn't a fallback to using software cursors available in the X server
> because there's no error reporting for a failed hardware cursor update.

Hmm, maybe I'm missing something, but doesn't returning FALSE from
UseHWCursorARGB just make the X server fallback to using a software
cursor?

https://github.com/ssvb/xf86-video-fbturbo/blob/689c08256555/src/sunxi_disp_hwcursor.c#L72

I'm just doing this. And also have hooks to notify the DRI2 code about
the status of the cursor. In the case if the hardware cursor is not
enabled, hardware overlays can't be safely used with the software
cursor and we need to fallback to CPU/GPU copy instead of zero-copy
buffers swapping.

And I fully agree that it is the responsibility of the kernel to expose
as much of the hardware capabilities as possible (without compromising
reliability and/or security).

-- 
Best regards,
Siarhei Siamashka

  reply	other threads:[~2013-10-07 12:29 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-06 22:07 [PATCH 0/5] Armada DRM stuff Russell King - ARM Linux
2013-10-06 22:07 ` Russell King - ARM Linux
2013-10-06 22:07 ` [PATCH 1/5] drm/i2c: tda998x: set VIF for full range, underscanned display Russell King
2013-10-06 22:07   ` Russell King
2013-10-07  8:59   ` Jean-Francois Moine
2013-10-07  8:59     ` Jean-Francois Moine
2013-10-18 15:00     ` Rob Clark
2013-10-18 15:00       ` Rob Clark
2013-10-06 22:08 ` [PATCH 2/5] DRM: Armada: Add Armada DRM driver Russell King
2013-10-06 22:08   ` Russell King
2013-10-10 21:25   ` Rob Clark
2013-10-10 21:25     ` Rob Clark
2013-10-10 21:59     ` Russell King - ARM Linux
2013-10-10 21:59       ` Russell King - ARM Linux
2013-10-10 22:23       ` Rob Clark
2013-10-10 22:23         ` Rob Clark
2013-10-10 22:53         ` Russell King - ARM Linux
2013-10-10 22:53           ` Russell King - ARM Linux
2013-10-11  0:10           ` Rob Clark
2013-10-11  0:10             ` Rob Clark
2013-10-06 22:09 ` [PATCH 3/5] DRM: Armada: Add support for ARGB 32x64 or 64x32 hardware cursors Russell King
2013-10-06 22:09   ` Russell King
2013-10-07  9:01   ` Jean-Francois Moine
2013-10-07  9:01     ` Jean-Francois Moine
2013-10-07  9:40     ` Russell King - ARM Linux
2013-10-07  9:40       ` Russell King - ARM Linux
2013-10-07 10:09       ` Jean-Francois Moine
2013-10-07 10:09         ` Jean-Francois Moine
2013-10-07 10:32         ` Russell King - ARM Linux
2013-10-07 10:32           ` Russell King - ARM Linux
2013-10-07 12:29           ` Siarhei Siamashka [this message]
2013-10-07 12:29             ` Siarhei Siamashka
2013-10-07 12:50             ` Russell King - ARM Linux
2013-10-07 12:50               ` Russell King - ARM Linux
2013-10-17 23:58               ` Rob Clark
2013-10-17 23:58                 ` Rob Clark
2013-10-18 14:31                 ` Alex Deucher
2013-10-18 14:31                   ` Alex Deucher
2013-10-06 22:10 ` [PATCH 4/5] DRM: Armada: start of MMP2/MMP3 support Russell King
2013-10-06 22:10   ` Russell King
2013-10-18  0:11   ` Rob Clark
2013-10-18  0:11     ` Rob Clark
2013-10-06 22:11 ` [PATCH 5/5] DRM: Armada: add support for drm tda19988 driver Russell King
2013-10-06 22:11   ` Russell King
2013-10-07  9:18   ` Jean-Francois Moine
2013-10-07  9:18     ` Jean-Francois Moine
2013-10-07  9:44     ` Russell King - ARM Linux
2013-10-07  9:44       ` Russell King - ARM Linux
2013-10-07 10:48       ` Jean-Francois Moine
2013-10-07 10:48         ` Jean-Francois Moine
2013-10-07 11:09         ` Russell King - ARM Linux
2013-10-07 11:09           ` Russell King - ARM Linux
2013-10-07 11:29           ` Sebastian Hesselbarth
2013-10-07 11:29             ` Sebastian Hesselbarth
2013-10-07 15:53             ` Mark Brown
2013-10-07 15:53               ` Mark Brown
2013-10-07 16:08               ` Sebastian Hesselbarth
2013-10-07 16:08                 ` Sebastian Hesselbarth
2013-10-07 17:05                 ` Mark Brown
2013-10-07 17:05                   ` Mark Brown
2013-10-07 12:03           ` Jean-Francois Moine
2013-10-07 12:03             ` Jean-Francois Moine
2013-10-07 12:36             ` Russell King - ARM Linux
2013-10-07 12:36               ` Russell King - ARM Linux
2013-10-07 14:59           ` Rob Clark
2013-10-07 14:59             ` Rob Clark
2013-10-08  9:19             ` Jean-Francois Moine
2013-10-08  9:19               ` Jean-Francois Moine
2013-10-08  9:49               ` Russell King - ARM Linux
2013-10-08  9:49                 ` Russell King - ARM Linux
2013-10-08 15:34                 ` Jean-Francois Moine
2013-10-08 15:34                   ` Jean-Francois Moine
2013-10-18  0:20                   ` Rob Clark
2013-10-18  0:20                     ` Rob Clark
2013-10-08 12:07               ` Rob Clark
2013-10-08 12:07                 ` Rob Clark
2013-10-07 21:47 ` [PATCH 0/5] Armada DRM stuff Sebastian Hesselbarth
2013-10-07 21:47   ` Sebastian Hesselbarth
2013-10-09 14:31   ` Russell King - ARM Linux
2013-10-09 14:31     ` Russell King - ARM Linux
2013-10-09 14:48     ` Rob Clark
2013-10-09 14:48       ` Rob Clark
2013-10-18 15:15 ` [GIT PULL] Armada DRM support Russell King - ARM Linux
2013-10-18 15:15   ` Russell King - ARM Linux
2013-10-22 13:36   ` Russell King - ARM Linux
2013-10-22 13:36     ` Russell King - ARM Linux

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=20131007152915.352cc4b2@i7 \
    --to=siarhei.siamashka@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.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.