From: "José Expósito" <jose.exposito89@gmail.com>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: linux-hyperv@vger.kernel.org, david@lechnology.com,
airlied@linux.ie, dri-devel@lists.freedesktop.org,
javierm@redhat.com, virtualization@lists.linux-foundation.org,
drawat.floss@gmail.com, noralf@tronnes.org, kraxel@redhat.com,
airlied@redhat.com, sam@ravnborg.org
Subject: Re: [PATCH 12/12] drm/format-helper: Move destination-buffer handling into internal helper
Date: Thu, 28 Jul 2022 20:23:17 +0200 [thread overview]
Message-ID: <20220728182317.GA99136@elementary> (raw)
In-Reply-To: <f3cb2246-76e5-8e10-f7f6-3294de6709b3@suse.de>
On Thu, Jul 28, 2022 at 09:45:27AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 28.07.22 um 09:26 schrieb José Expósito:
> > Hi!
> >
> > On Wed, Jul 27, 2022 at 01:33:12PM +0200, Thomas Zimmermann wrote:
> > > The format-convertion helpers handle several cases for different
> > > values of destination buffer and pitch. Move that code into the
> > > internal helper drm_fb_xfrm() and avoid quite a bit of duplucation.
> > >
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > ---
> > > drivers/gpu/drm/drm_format_helper.c | 169 +++++++++++-----------------
> > > 1 file changed, 64 insertions(+), 105 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> > > index d296d181659d..35aebdb90165 100644
> > > --- a/drivers/gpu/drm/drm_format_helper.c
> > > +++ b/drivers/gpu/drm/drm_format_helper.c
> > > @@ -41,11 +41,11 @@ unsigned int drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info
> > > }
> > > EXPORT_SYMBOL(drm_fb_clip_offset);
> > > -/* TODO: Make this functon work with multi-plane formats. */
> > > -static int drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
> > > - const void *vaddr, const struct drm_framebuffer *fb,
> > > - const struct drm_rect *clip, bool vaddr_cached_hint,
> > > - void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > +/* TODO: Make this function work with multi-plane formats. */
> > > +static int __drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
> > > + const void *vaddr, const struct drm_framebuffer *fb,
> > > + const struct drm_rect *clip, bool vaddr_cached_hint,
> > > + void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > {
> > > unsigned long linepixels = drm_rect_width(clip);
> > > unsigned long lines = drm_rect_height(clip);
> > > @@ -84,11 +84,11 @@ static int drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_pix
> > > return 0;
> > > }
> > > -/* TODO: Make this functon work with multi-plane formats. */
> > > -static int drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
> > > - const void *vaddr, const struct drm_framebuffer *fb,
> > > - const struct drm_rect *clip, bool vaddr_cached_hint,
> > > - void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > +/* TODO: Make this function work with multi-plane formats. */
> > > +static int __drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
> > > + const void *vaddr, const struct drm_framebuffer *fb,
> > > + const struct drm_rect *clip, bool vaddr_cached_hint,
> > > + void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > {
> > > unsigned long linepixels = drm_rect_width(clip);
> > > unsigned long lines = drm_rect_height(clip);
> > > @@ -129,6 +129,29 @@ static int drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsigned
> > > return 0;
> > > }
> > > +/* TODO: Make this function work with multi-plane formats. */
> > > +static int drm_fb_xfrm(struct iosys_map *dst,
> > > + const unsigned int *dst_pitch, const u8 *dst_pixsize,
> > > + const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > + const struct drm_rect *clip, bool vaddr_cached_hint,
> > > + void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > +{
> > > + static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > + 0, 0, 0, 0
> > > + };
> > > +
> > > + if (!dst_pitch)
> > > + dst_pitch = default_dst_pitch;
> > > +
> > > + if (dst[0].is_iomem)
> > > + return __drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], dst_pixsize[0],
> > > + vmap[0].vaddr, fb, clip, false, xfrm_line);
> > > + else
> > > + return __drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], dst_pixsize[0],
> > > + vmap[0].vaddr, fb, clip, false, xfrm_line);
> > > +}
> > > +
> > > +
> >
> > Nit: Extra blank line
>
> Oh!
>
> >
> > > /**
> > > * drm_fb_memcpy - Copy clip buffer
> > > * @dst: Array of destination buffers
> > > @@ -213,14 +236,10 @@ void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip, bool cached)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > - };
> > > const struct drm_format_info *format = fb->format;
> > > - u8 cpp = format->cpp[0];
> > > void (*swab_line)(void *dbuf, const void *sbuf, unsigned int npixels);
> > > - switch (cpp) {
> > > + switch (format->cpp[0]) {
> > > case 4:
> > > swab_line = drm_fb_swab32_line;
> > > break;
> > > @@ -230,21 +249,10 @@ void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
> > > default:
> > > drm_warn_once(fb->dev, "Format %p4cc has unsupported pixel size.\n",
> > > &format->format);
> > > - swab_line = NULL;
> > > - break;
> > > - }
> > > - if (!swab_line)
> > > return;
> > > + }
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst->is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], cpp,
> > > - vmap[0].vaddr, fb, clip, cached, swab_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], cpp, vmap[0].vaddr, fb,
> > > - clip, cached, swab_line);
> > > + drm_fb_xfrm(dst, dst_pitch, format->cpp, vmap, fb, clip, cached, swab_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_swab);
> > > @@ -277,19 +285,12 @@ void drm_fb_xrgb8888_to_rgb332(struct iosys_map *dst, const unsigned int *dst_pi
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 1,
> > > };
> >
> > Could "dst_pixsize" be obtained from "drm_format_info->cpp"? (in all
> > conversion functions, not only in this one).
>
> It could and I already considered it. But it would require a pointer to the
> destination format's info, which we don't yet have here. The info lookup is
> at [1], but it has linear complexity. So I was reluctant to use it.
>
> The solution I have in mind is to pass in the dst format info from the
> calling driver. Drivers can look it up once and reuse it. But as that's a
> change to quite a few drivers, it's something for a separate patchset.
>
> In general, we should make an effort to replace uses of 4CC codes with
> pointers to a format info.
Cool, thanks for the explanation. I don't see an easy way to make
__drm_format_info O(1) without adding unnecessary complexity and
it is probably not worth it.
Jose
> Best regards
> Thomas
>
> [1] https://elixir.bootlin.com/linux/v5.18.14/source/drivers/gpu/drm/drm_fourcc.c#L132
>
> >
> > I think they are similar structures, so we might be able to reuse that
> > information.
> >
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 1, vmap[0].vaddr, fb, clip,
> > > - false, drm_fb_xrgb8888_to_rgb332_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 1, vmap[0].vaddr, fb, clip,
> > > - false, drm_fb_xrgb8888_to_rgb332_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_xrgb8888_to_rgb332_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
> > > @@ -344,9 +345,10 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip, bool swab)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 2,
> > > };
> > > +
> > > void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels);
> > > if (swab)
> > > @@ -354,15 +356,7 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
> > > else
> > > xfrm_line = drm_fb_xrgb8888_to_rgb565_line;
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 2, vmap[0].vaddr, fb, clip,
> > > - false, xfrm_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 2, vmap[0].vaddr, fb, clip,
> > > - false, xfrm_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false, xfrm_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
> > > @@ -396,19 +390,12 @@ void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pi
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 3,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 3, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_rgb888_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 3, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_rgb888_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_xrgb8888_to_rgb888_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
> > > @@ -435,19 +422,12 @@ static void drm_fb_rgb565_to_xrgb8888(struct iosys_map *dst, const unsigned int
> > > const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 4,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_rgb565_to_xrgb8888_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_rgb565_to_xrgb8888_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_rgb565_to_xrgb8888_line);
> > > }
> > > static void drm_fb_rgb888_to_xrgb8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
> > > @@ -470,19 +450,12 @@ static void drm_fb_rgb888_to_xrgb8888(struct iosys_map *dst, const unsigned int
> > > const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 4,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_rgb888_to_xrgb8888_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_rgb888_to_xrgb8888_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_rgb888_to_xrgb8888_line);
> > > }
> > > static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, unsigned int pixels)
> > > @@ -518,19 +491,12 @@ void drm_fb_xrgb8888_to_xrgb2101010(struct iosys_map *dst, const unsigned int *d
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 4,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_xrgb2101010_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_xrgb2101010_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_xrgb8888_to_xrgb2101010_line);
> > > }
> > > static void drm_fb_xrgb8888_to_gray8_line(void *dbuf, const void *sbuf, unsigned int pixels)
> > > @@ -571,19 +537,12 @@ void drm_fb_xrgb8888_to_gray8(struct iosys_map *dst, const unsigned int *dst_pit
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 1,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 1, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_gray8_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 1, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_gray8_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_xrgb8888_to_gray8_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
> > > --
> > > 2.37.1
> > >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
WARNING: multiple messages have this Message-ID (diff)
From: "José Expósito" <jose.exposito89@gmail.com>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: sam@ravnborg.org, noralf@tronnes.org, daniel@ffwll.ch,
airlied@linux.ie, mripard@kernel.org,
maarten.lankhorst@linux.intel.com, airlied@redhat.com,
javierm@redhat.com, drawat.floss@gmail.com, kraxel@redhat.com,
david@lechnology.com, dri-devel@lists.freedesktop.org,
linux-hyperv@vger.kernel.org,
virtualization@lists.linux-foundation.org
Subject: Re: [PATCH 12/12] drm/format-helper: Move destination-buffer handling into internal helper
Date: Thu, 28 Jul 2022 20:23:17 +0200 [thread overview]
Message-ID: <20220728182317.GA99136@elementary> (raw)
In-Reply-To: <f3cb2246-76e5-8e10-f7f6-3294de6709b3@suse.de>
On Thu, Jul 28, 2022 at 09:45:27AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 28.07.22 um 09:26 schrieb José Expósito:
> > Hi!
> >
> > On Wed, Jul 27, 2022 at 01:33:12PM +0200, Thomas Zimmermann wrote:
> > > The format-convertion helpers handle several cases for different
> > > values of destination buffer and pitch. Move that code into the
> > > internal helper drm_fb_xfrm() and avoid quite a bit of duplucation.
> > >
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > ---
> > > drivers/gpu/drm/drm_format_helper.c | 169 +++++++++++-----------------
> > > 1 file changed, 64 insertions(+), 105 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> > > index d296d181659d..35aebdb90165 100644
> > > --- a/drivers/gpu/drm/drm_format_helper.c
> > > +++ b/drivers/gpu/drm/drm_format_helper.c
> > > @@ -41,11 +41,11 @@ unsigned int drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info
> > > }
> > > EXPORT_SYMBOL(drm_fb_clip_offset);
> > > -/* TODO: Make this functon work with multi-plane formats. */
> > > -static int drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
> > > - const void *vaddr, const struct drm_framebuffer *fb,
> > > - const struct drm_rect *clip, bool vaddr_cached_hint,
> > > - void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > +/* TODO: Make this function work with multi-plane formats. */
> > > +static int __drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
> > > + const void *vaddr, const struct drm_framebuffer *fb,
> > > + const struct drm_rect *clip, bool vaddr_cached_hint,
> > > + void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > {
> > > unsigned long linepixels = drm_rect_width(clip);
> > > unsigned long lines = drm_rect_height(clip);
> > > @@ -84,11 +84,11 @@ static int drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_pix
> > > return 0;
> > > }
> > > -/* TODO: Make this functon work with multi-plane formats. */
> > > -static int drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
> > > - const void *vaddr, const struct drm_framebuffer *fb,
> > > - const struct drm_rect *clip, bool vaddr_cached_hint,
> > > - void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > +/* TODO: Make this function work with multi-plane formats. */
> > > +static int __drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
> > > + const void *vaddr, const struct drm_framebuffer *fb,
> > > + const struct drm_rect *clip, bool vaddr_cached_hint,
> > > + void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > {
> > > unsigned long linepixels = drm_rect_width(clip);
> > > unsigned long lines = drm_rect_height(clip);
> > > @@ -129,6 +129,29 @@ static int drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsigned
> > > return 0;
> > > }
> > > +/* TODO: Make this function work with multi-plane formats. */
> > > +static int drm_fb_xfrm(struct iosys_map *dst,
> > > + const unsigned int *dst_pitch, const u8 *dst_pixsize,
> > > + const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > + const struct drm_rect *clip, bool vaddr_cached_hint,
> > > + void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
> > > +{
> > > + static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > + 0, 0, 0, 0
> > > + };
> > > +
> > > + if (!dst_pitch)
> > > + dst_pitch = default_dst_pitch;
> > > +
> > > + if (dst[0].is_iomem)
> > > + return __drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], dst_pixsize[0],
> > > + vmap[0].vaddr, fb, clip, false, xfrm_line);
> > > + else
> > > + return __drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], dst_pixsize[0],
> > > + vmap[0].vaddr, fb, clip, false, xfrm_line);
> > > +}
> > > +
> > > +
> >
> > Nit: Extra blank line
>
> Oh!
>
> >
> > > /**
> > > * drm_fb_memcpy - Copy clip buffer
> > > * @dst: Array of destination buffers
> > > @@ -213,14 +236,10 @@ void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip, bool cached)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > - };
> > > const struct drm_format_info *format = fb->format;
> > > - u8 cpp = format->cpp[0];
> > > void (*swab_line)(void *dbuf, const void *sbuf, unsigned int npixels);
> > > - switch (cpp) {
> > > + switch (format->cpp[0]) {
> > > case 4:
> > > swab_line = drm_fb_swab32_line;
> > > break;
> > > @@ -230,21 +249,10 @@ void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
> > > default:
> > > drm_warn_once(fb->dev, "Format %p4cc has unsupported pixel size.\n",
> > > &format->format);
> > > - swab_line = NULL;
> > > - break;
> > > - }
> > > - if (!swab_line)
> > > return;
> > > + }
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst->is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], cpp,
> > > - vmap[0].vaddr, fb, clip, cached, swab_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], cpp, vmap[0].vaddr, fb,
> > > - clip, cached, swab_line);
> > > + drm_fb_xfrm(dst, dst_pitch, format->cpp, vmap, fb, clip, cached, swab_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_swab);
> > > @@ -277,19 +285,12 @@ void drm_fb_xrgb8888_to_rgb332(struct iosys_map *dst, const unsigned int *dst_pi
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 1,
> > > };
> >
> > Could "dst_pixsize" be obtained from "drm_format_info->cpp"? (in all
> > conversion functions, not only in this one).
>
> It could and I already considered it. But it would require a pointer to the
> destination format's info, which we don't yet have here. The info lookup is
> at [1], but it has linear complexity. So I was reluctant to use it.
>
> The solution I have in mind is to pass in the dst format info from the
> calling driver. Drivers can look it up once and reuse it. But as that's a
> change to quite a few drivers, it's something for a separate patchset.
>
> In general, we should make an effort to replace uses of 4CC codes with
> pointers to a format info.
Cool, thanks for the explanation. I don't see an easy way to make
__drm_format_info O(1) without adding unnecessary complexity and
it is probably not worth it.
Jose
> Best regards
> Thomas
>
> [1] https://elixir.bootlin.com/linux/v5.18.14/source/drivers/gpu/drm/drm_fourcc.c#L132
>
> >
> > I think they are similar structures, so we might be able to reuse that
> > information.
> >
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 1, vmap[0].vaddr, fb, clip,
> > > - false, drm_fb_xrgb8888_to_rgb332_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 1, vmap[0].vaddr, fb, clip,
> > > - false, drm_fb_xrgb8888_to_rgb332_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_xrgb8888_to_rgb332_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
> > > @@ -344,9 +345,10 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip, bool swab)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 2,
> > > };
> > > +
> > > void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels);
> > > if (swab)
> > > @@ -354,15 +356,7 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
> > > else
> > > xfrm_line = drm_fb_xrgb8888_to_rgb565_line;
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 2, vmap[0].vaddr, fb, clip,
> > > - false, xfrm_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 2, vmap[0].vaddr, fb, clip,
> > > - false, xfrm_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false, xfrm_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
> > > @@ -396,19 +390,12 @@ void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pi
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 3,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 3, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_rgb888_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 3, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_rgb888_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_xrgb8888_to_rgb888_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
> > > @@ -435,19 +422,12 @@ static void drm_fb_rgb565_to_xrgb8888(struct iosys_map *dst, const unsigned int
> > > const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 4,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_rgb565_to_xrgb8888_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_rgb565_to_xrgb8888_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_rgb565_to_xrgb8888_line);
> > > }
> > > static void drm_fb_rgb888_to_xrgb8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
> > > @@ -470,19 +450,12 @@ static void drm_fb_rgb888_to_xrgb8888(struct iosys_map *dst, const unsigned int
> > > const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 4,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_rgb888_to_xrgb8888_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_rgb888_to_xrgb8888_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_rgb888_to_xrgb8888_line);
> > > }
> > > static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, unsigned int pixels)
> > > @@ -518,19 +491,12 @@ void drm_fb_xrgb8888_to_xrgb2101010(struct iosys_map *dst, const unsigned int *d
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 4,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_xrgb2101010_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 4, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_xrgb2101010_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_xrgb8888_to_xrgb2101010_line);
> > > }
> > > static void drm_fb_xrgb8888_to_gray8_line(void *dbuf, const void *sbuf, unsigned int pixels)
> > > @@ -571,19 +537,12 @@ void drm_fb_xrgb8888_to_gray8(struct iosys_map *dst, const unsigned int *dst_pit
> > > const struct iosys_map *vmap, const struct drm_framebuffer *fb,
> > > const struct drm_rect *clip)
> > > {
> > > - static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > - 0, 0, 0, 0
> > > + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> > > + 1,
> > > };
> > > - if (!dst_pitch)
> > > - dst_pitch = default_dst_pitch;
> > > -
> > > - if (dst[0].is_iomem)
> > > - drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], 1, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_gray8_line);
> > > - else
> > > - drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], 1, vmap[0].vaddr, fb,
> > > - clip, false, drm_fb_xrgb8888_to_gray8_line);
> > > + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, vmap, fb, clip, false,
> > > + drm_fb_xrgb8888_to_gray8_line);
> > > }
> > > EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
> > > --
> > > 2.37.1
> > >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
next prev parent reply other threads:[~2022-07-28 18:23 UTC|newest]
Thread overview: 103+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-27 11:33 [PATCH 00/12] drm/format-helper: Move to struct iosys_map Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` [PATCH 01/12] drm/format-helper: Provide drm_fb_blit() Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 17:02 ` Sam Ravnborg
2022-08-04 17:02 ` Sam Ravnborg
2022-08-04 17:02 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 02/12] drm/format-helper: Merge drm_fb_memcpy() and drm_fb_memcpy_toio() Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 19:52 ` Sam Ravnborg
2022-08-04 19:52 ` Sam Ravnborg
2022-08-04 19:52 ` Sam Ravnborg
2022-08-08 12:07 ` Thomas Zimmermann
2022-08-08 12:07 ` Thomas Zimmermann
2022-08-08 12:07 ` Thomas Zimmermann
2022-07-27 11:33 ` [PATCH 03/12] drm/format-helper: Convert drm_fb_swab() to struct iosys_map Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 20:08 ` Sam Ravnborg
2022-08-04 20:08 ` Sam Ravnborg
2022-08-04 20:08 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 04/12] drm/format-helper: Rework XRGB8888-to-RGBG332 conversion Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-28 7:13 ` José Expósito
2022-07-28 7:13 ` José Expósito
2022-07-28 7:27 ` Thomas Zimmermann
2022-07-28 7:27 ` Thomas Zimmermann
2022-07-28 7:27 ` Thomas Zimmermann
2022-07-28 17:47 ` José Expósito
2022-07-28 17:47 ` José Expósito
2022-08-04 20:10 ` Sam Ravnborg
2022-08-04 20:10 ` Sam Ravnborg
2022-08-04 20:10 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 05/12] drm/format-helper: Rework XRGB8888-to-RGBG565 conversion Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-30 12:27 ` José Expósito
2022-07-30 12:27 ` José Expósito
2022-08-04 20:12 ` Sam Ravnborg
2022-08-04 20:12 ` Sam Ravnborg
2022-08-04 20:12 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 06/12] drm/format-helper: Rework XRGB8888-to-RGB888 conversion Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 20:14 ` Sam Ravnborg
2022-08-04 20:14 ` Sam Ravnborg
2022-08-04 20:14 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 07/12] drm/format-helper: Rework RGB565-to-XRGB8888 conversion Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 20:15 ` Sam Ravnborg
2022-08-04 20:15 ` Sam Ravnborg
2022-08-04 20:15 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 08/12] drm/format-helper: Rework RGB888-to-XRGB8888 conversion Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 20:16 ` Sam Ravnborg
2022-08-04 20:16 ` Sam Ravnborg
2022-08-04 20:16 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 09/12] drm/format-helper: Rework XRGB8888-to-XRGB2101010 conversion Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 20:17 ` Sam Ravnborg
2022-08-04 20:17 ` Sam Ravnborg
2022-08-04 20:17 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 10/12] drm/format-helper: Rework XRGB8888-to-GRAY8 conversion Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 20:19 ` Sam Ravnborg
2022-08-04 20:19 ` Sam Ravnborg
2022-08-04 20:19 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 11/12] drm/format-helper: Rework XRGB8888-to-MONO conversion Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-08-04 20:21 ` Sam Ravnborg
2022-08-04 20:21 ` Sam Ravnborg
2022-08-04 20:21 ` Sam Ravnborg
2022-07-27 11:33 ` [PATCH 12/12] drm/format-helper: Move destination-buffer handling into internal helper Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-27 11:33 ` Thomas Zimmermann
2022-07-28 7:26 ` José Expósito
2022-07-28 7:26 ` José Expósito
2022-07-28 7:45 ` Thomas Zimmermann
2022-07-28 7:45 ` Thomas Zimmermann
2022-07-28 7:45 ` Thomas Zimmermann
2022-07-28 18:23 ` José Expósito [this message]
2022-07-28 18:23 ` José Expósito
2022-08-05 17:52 ` Sam Ravnborg
2022-08-05 17:52 ` Sam Ravnborg
2022-08-05 17:52 ` Sam Ravnborg
2022-08-08 11:40 ` Thomas Zimmermann
2022-08-08 11:40 ` Thomas Zimmermann
2022-08-08 11:40 ` Thomas Zimmermann
2022-08-08 18:25 ` Sam Ravnborg
2022-08-08 18:25 ` Sam Ravnborg
2022-08-08 18:25 ` Sam Ravnborg
2022-08-05 17:59 ` [PATCH 00/12] drm/format-helper: Move to struct iosys_map Sam Ravnborg
2022-08-05 17:59 ` Sam Ravnborg
2022-08-05 17:59 ` Sam Ravnborg
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=20220728182317.GA99136@elementary \
--to=jose.exposito89@gmail.com \
--cc=airlied@linux.ie \
--cc=airlied@redhat.com \
--cc=david@lechnology.com \
--cc=drawat.floss@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=javierm@redhat.com \
--cc=kraxel@redhat.com \
--cc=linux-hyperv@vger.kernel.org \
--cc=noralf@tronnes.org \
--cc=sam@ravnborg.org \
--cc=tzimmermann@suse.de \
--cc=virtualization@lists.linux-foundation.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.