Re: [PATCH 1/8] drm/ssd130x: Fix pitch calculation in ssd130x_fb_blit_rect()

[Geert Uytterhoeven]

Hi Javier,

On Fri, Jul 14, 2023 at 11:34 AM Javier Martinez Canillas
<javierm@xxxxxxxxxx> wrote:
> Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> writes:
> > The page height must be taken into account only for vertical coordinates
> > and heights, not for horizontal coordinates and widths.
> >
> > Fixes: 179a790aaf2a0127 ("drm/ssd130x: Set the page height value in the device info data")
> > Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>

> > --- a/drivers/gpu/drm/solomon/ssd130x.c
> > +++ b/drivers/gpu/drm/solomon/ssd130x.c
> > @@ -596,7 +596,7 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m
> >       rect->y1 = round_down(rect->y1, page_height);
> >       rect->y2 = min_t(unsigned int, round_up(rect->y2, page_height), ssd130x->height);
> >
> > -     dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), page_height);
> > +     dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);
> >
>
> That's true for ssd130x controllers that use R1, but when doing that
> change one of my goals was to prepare the driver for supporting the
> ssd132x family that use a 16-grayscale pixel format (R4).
>
> For those controllers, the pixels are encoded in 4-bit and each page
> has two pixels. So for those controllers the dst_pitch will need to
> be DIV_ROUND_UP(drm_rect_width(rect), 2) instead since the width is
> not 8 in that case.
>
> So I would prefer to skip this patch from your set, because otherwise
> we will need to revert it when adding support for SSD132x controllers.

My point is that the 8 as used here is related to the number of bits per pixel,
not to the page height.  The page height might also be impacted by the
number of bits per pixel, but that is orthogonal.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds