linux-kernel - Re: [PATCH 1/8] drm/ssd130x: Fix pitch calculation in ssd130x_fb_blit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874jm69631.fsf@minerva.mail-host-address-is-not-set>
Date:   Fri, 14 Jul 2023 11:48:34 +0200
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 1/8] drm/ssd130x: Fix pitch calculation in
 ssd130x_fb_blit_rect()

Geert Uytterhoeven <geert@...ux-m68k.org> writes:

> Hi Javier,
>
> On Fri, Jul 14, 2023 at 11:34 AM Javier Martinez Canillas
> <javierm@...hat.com> wrote:
>> Geert Uytterhoeven <geert@...ux-m68k.org> 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@...ux-m68k.org>
>
>> > --- 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.
>

Ah, I see what you mean. Yes, you are right. We can later add a
different variable when adding support for controllers using R4.

Reviewed-by: Javier Martinez Canillas <javierm@...hat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat