All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Rob Clark <robdclark@gmail.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 02/23] drm: omapdrm: fb: Don't store format BPP for each plane
Date: Mon, 09 May 2016 23:57:19 +0300	[thread overview]
Message-ID: <3238575.Acs1L0uYI1@avalon> (raw)
In-Reply-To: <CAF6AEGtCgqM6-fLy9gW8f7=djvyKRRQnd4LGANJL=oNZ9Zs6Pg@mail.gmail.com>

Hi Rob,

On Monday 02 May 2016 17:01:23 Rob Clark wrote:
> On Mon, May 2, 2016 at 11:43 AM, Tomi Valkeinen wrote:
> > On 26/04/16 23:35, Laurent Pinchart wrote:
> >> The number of bits per pixel is identical for all planes, don't store
> >> multiple copies.
> > 
> > That's not true, as with NV12, Y has 8 bits per pixel and UV has 16 bits
> > per pixel. But as the subsampling is precalculated into the stride_bpp
> > (is it bytes or bits? bpp always confuses me =), the 'stride_bpp' ends
> > up being same for both planes.
> > 
> > To be honest, I'd rather go into more complex struct than simpler one.
> > The current one is already confusing, I think, and your version is too.
> > The main issue is that the sub_x is encoded into the stride_bpp. In
> > kmsxx I used this format:
> > 
> > { PixelFormat::NV12, { 2, { { 8, 1, 1, }, { 8, 2, 2 } }, } },
> > 
> > The first number is the number of planes, and for each plane, bitspp,
> > xsub and ysub. It's more verbose, but (I think) easier to understand.
> 
> fwiw, I guess a lot of data from that table could these days be
> replaced w/ some of the drm format helpers
> (drm_format_num_planes()/drm_format_plane_cpp()/drm_format_{horz,vert}_chrom
> a_subsampling()/etc)

I don't like those helpers as they're inefficient. Drivers often need to know 
multiple pieces of information about a format, and the API forces look-ups for 
every piece of information needed.

Would it make sense to add a drm_format_info() function that returns a pointer 
to a data structure that describes the format, and reimplement the existing 
helpers on top of that ?

-- 
Regards,

Laurent Pinchart

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

  parent reply	other threads:[~2016-05-09 20:56 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-26 20:35 [PATCH 00/23] OMAP DRM fixes and improvements Laurent Pinchart
2016-04-26 20:35 ` [PATCH 01/23] drm: omapdrm: fb: Limit number of planes per framebuffer to two Laurent Pinchart
2016-04-26 20:35 ` [PATCH 02/23] drm: omapdrm: fb: Don't store format BPP for each plane Laurent Pinchart
2016-05-02 15:43   ` Tomi Valkeinen
2016-05-02 21:01     ` Rob Clark
2016-05-03  6:08       ` Tomi Valkeinen
2016-05-03 11:22         ` Ville Syrjälä
2016-05-09 20:57       ` Laurent Pinchart [this message]
2016-05-10  7:14         ` Tomi Valkeinen
2016-06-06  1:39           ` Laurent Pinchart
2016-06-06  6:27             ` Daniel Vetter
2016-05-09 20:55     ` Laurent Pinchart
2016-05-10  7:34       ` Tomi Valkeinen
2016-04-26 20:35 ` [PATCH 03/23] drm: omapdrm: fb: Store number of planes in format structure Laurent Pinchart
2016-04-26 20:35 ` [PATCH 04/23] drm: omapdrm: fb: Simplify objects lookup when creating framebuffer Laurent Pinchart
2016-05-09 14:20   ` Tomi Valkeinen
2016-04-26 20:35 ` [PATCH 05/23] drm: omapdrm: fb: Simplify mode command checks " Laurent Pinchart
2016-05-09 15:15   ` Tomi Valkeinen
2016-06-06  0:24     ` Laurent Pinchart
2016-04-26 20:35 ` [PATCH 06/23] drm: omapdrm: fb: Turn framebuffer creation error messages into debug Laurent Pinchart
2016-05-09 14:28   ` Tomi Valkeinen
2016-04-26 20:35 ` [PATCH 07/23] drm: omapdrm: Handle FIFO underflow IRQs internally Laurent Pinchart
2016-05-09 14:42   ` Tomi Valkeinen
2016-06-05 23:21     ` Laurent Pinchart
2016-06-06 10:50       ` Tomi Valkeinen
2016-06-06 22:51         ` Laurent Pinchart
2016-04-26 20:35 ` [PATCH 08/23] drm: omapdrm: Handle CRTC error IRQs directly Laurent Pinchart
2016-05-10 12:58   ` Tomi Valkeinen
2016-04-26 20:35 ` [PATCH 09/23] drm: omapdrm: Handle OCP error IRQ directly Laurent Pinchart
2016-05-10 13:10   ` Tomi Valkeinen
2016-06-06  0:45     ` Laurent Pinchart
2016-04-26 20:35 ` [PATCH 10/23] drm: omapdrm: Use atomic state instead of local device state Laurent Pinchart
2016-05-10 13:24   ` Tomi Valkeinen
2016-05-11  7:37     ` Daniel Vetter
2016-06-06  1:14       ` Laurent Pinchart
2016-06-06 10:37         ` Tomi Valkeinen
2016-06-06 22:53           ` Laurent Pinchart
2016-09-18 10:37         ` Laurent Pinchart
2016-04-26 20:35 ` [PATCH 11/23] drm: omapdrm: Check DSS manager state in the enable/disable helpers Laurent Pinchart
2016-05-10 13:28   ` Tomi Valkeinen
2016-05-11  7:40     ` Daniel Vetter
2016-06-06  1:36       ` Laurent Pinchart
2016-06-06  1:38     ` Laurent Pinchart
2016-04-26 20:35 ` [PATCH 12/23] drm: omapdrm: Prevent processing the same event multiple times Laurent Pinchart
2016-04-26 20:35 ` [PATCH 13/23] drm: omapdrm: Use a spinlock to protect the CRTC pending flag Laurent Pinchart
2016-04-26 20:35 ` [PATCH 14/23] drm: omapdrm: Keep vblank interrupt enabled while CRTC is active Laurent Pinchart
2016-04-26 20:35 ` [PATCH 15/23] drm: omapdrm: Don't expose the omap_irq_(un)register() functions Laurent Pinchart
2016-05-11 11:05   ` Tomi Valkeinen
2016-06-06  0:53     ` Laurent Pinchart
2016-04-26 20:35 ` [PATCH 16/23] drm: omapdrm: Don't call DISPC power handling in IRQ wait functions Laurent Pinchart
2016-04-26 20:35 ` [PATCH 17/23] drm: omapdrm: Make pipe2vbl function static Laurent Pinchart
2016-05-11 11:01   ` Tomi Valkeinen
2016-06-06  0:49     ` Laurent Pinchart
2016-04-26 20:35 ` [PATCH 18/23] drm: omapdrm: Simplify IRQ wait implementation Laurent Pinchart
2016-04-26 20:35 ` [PATCH 19/23] drm: omapdrm: Remove global variables Laurent Pinchart
2016-04-26 20:35 ` [PATCH 20/23] drm: omapdrm: panel-lgphilips-lb035q02: Remove unused backlight GPIO Laurent Pinchart
2016-05-10 10:55   ` Tomi Valkeinen
2016-04-26 20:35 ` [PATCH 21/23] drm: omapdrm: Remove unused omap_framebuffer_bo function Laurent Pinchart
2016-05-10 11:00   ` Tomi Valkeinen
2016-06-06  0:29     ` Laurent Pinchart
2016-04-26 20:35 ` [PATCH 22/23] drm: omapdrm: Remove unused omap_gem_tiled_size function Laurent Pinchart
2016-05-10 10:57   ` Tomi Valkeinen
2016-04-26 20:35 ` [PATCH 23/23] drm: omapdrm: Remove buffer synchronization support Laurent Pinchart
2016-05-11 11:12   ` Tomi Valkeinen

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=3238575.Acs1L0uYI1@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=robdclark@gmail.com \
    --cc=tomi.valkeinen@ti.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.