All of lore.kernel.org
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javierm@redhat.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Maxime Ripard <mripard@kernel.org>
Subject: Re: [PATCH 0/8] drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1
Date: Mon, 17 Jul 2023 11:13:00 +0200	[thread overview]
Message-ID: <87o7kakijn.fsf@minerva.mail-host-address-is-not-set> (raw)
In-Reply-To: <CAMuHMdV_qNfytroBEfqDMe04POt27s9o=FevXht7N4YXRmBz8Q@mail.gmail.com>

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

Hello Geert,

> Hi Javier,
>
> On Sun, Jul 16, 2023 at 3:30 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> > The native display format of ssd1306 OLED displays is monochrome
>> > light-on-dark (R1).  This patch series adds support for the R1 buffer
>> > format to both the ssd130x DRM driver and the FB helpers, so monochrome
>> > applications (including fbdev emulation and the text console) can avoid
>> > the overhead of back-and-forth conversions between R1 and XR24.
>>
>> I've tested your series on a ssd1306 I2C OLED panel and fbcon did work for
>> me, but had some issues when trying to run your fbtest suite. Because the
>
> Thanks, due to the limited userspace environment on my RV32 test system,
> I didn't run fbtest myself.
>

You are welcome.

>> test005 gets killed with a SIGSEGV.
>>
>> $ ./fbtest -d
>> fb_init()
>> fb_open()
>> fb_get_var()
>> fb_get_fix()
>> fb_map()
>> fb_start = 0, fb_offset = 0, fb_len = 1000
>
>    [...]
>
>> Running test test005
>> Caught signal 11. Exiting
>
> Strange.
>
>> Maybe more tests are missing the minimum num_colors requirement? Also, the
>
> On monochrome, test005 should make the left half of the screen black,
> and the right half white.  It works on ARAnyM, and there don't seem
> to be off-by-one errors in the call to fill_rect().
> Can you please run this under gdb or valgrind?
>

Sure. I only spent my free time on these panels though so likely will do
during the week or more likely the weekend. I believe the bug is somewhere
in the test though and not in your kernel patches.

>> penguin in test004 is not displayed correctly. I was expecting that to be
>> working correctly since you mentioned to be using the Linux logo on boot.
>
> Linux has logos for displays using 2, 16, and 256 colors. Note that the
> default logos are 80x80, which is larger than your display, so no logo
> is drawn.
> Fbtest has only the full color logo, so it will look bad on a monochrome
> display.
>

I see. Should the test check for minimum num_colors and skip that test then?

>> Another question, do you know if is possible to change the default format?
>
> AFAIK DRM does not support that.
>
>> I believe that fbset won't work because the DRM fbdev emulation layer does
>> not implement mode settings but I thought that changing the mode using the
>
> Correct.
>

[...]

>> Maybe I'm doing something wrong or misunderstading about how should work?
>
> Do you need the "-d" option (drop master after mode set) of modetest?
> Still, that would only impact DRM.  The fbdev emulation on top of DRM has
> already been initialized before, and is never reinitialized.
>

Ah, that explains and makes sense now. I tested other user-space fbdev
programs that only support XRGB8888 and those were working correctly so I
guess that user-space that supports R1 be defaulted to that and not able
to change is an acceptable behaviour.

When you post v2, feel free to add:

Tested-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


WARNING: multiple messages have this Message-ID (diff)
From: Javier Martinez Canillas <javierm@redhat.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/8] drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1
Date: Mon, 17 Jul 2023 11:13:00 +0200	[thread overview]
Message-ID: <87o7kakijn.fsf@minerva.mail-host-address-is-not-set> (raw)
In-Reply-To: <CAMuHMdV_qNfytroBEfqDMe04POt27s9o=FevXht7N4YXRmBz8Q@mail.gmail.com>

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

Hello Geert,

> Hi Javier,
>
> On Sun, Jul 16, 2023 at 3:30 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> > The native display format of ssd1306 OLED displays is monochrome
>> > light-on-dark (R1).  This patch series adds support for the R1 buffer
>> > format to both the ssd130x DRM driver and the FB helpers, so monochrome
>> > applications (including fbdev emulation and the text console) can avoid
>> > the overhead of back-and-forth conversions between R1 and XR24.
>>
>> I've tested your series on a ssd1306 I2C OLED panel and fbcon did work for
>> me, but had some issues when trying to run your fbtest suite. Because the
>
> Thanks, due to the limited userspace environment on my RV32 test system,
> I didn't run fbtest myself.
>

You are welcome.

>> test005 gets killed with a SIGSEGV.
>>
>> $ ./fbtest -d
>> fb_init()
>> fb_open()
>> fb_get_var()
>> fb_get_fix()
>> fb_map()
>> fb_start = 0, fb_offset = 0, fb_len = 1000
>
>    [...]
>
>> Running test test005
>> Caught signal 11. Exiting
>
> Strange.
>
>> Maybe more tests are missing the minimum num_colors requirement? Also, the
>
> On monochrome, test005 should make the left half of the screen black,
> and the right half white.  It works on ARAnyM, and there don't seem
> to be off-by-one errors in the call to fill_rect().
> Can you please run this under gdb or valgrind?
>

Sure. I only spent my free time on these panels though so likely will do
during the week or more likely the weekend. I believe the bug is somewhere
in the test though and not in your kernel patches.

>> penguin in test004 is not displayed correctly. I was expecting that to be
>> working correctly since you mentioned to be using the Linux logo on boot.
>
> Linux has logos for displays using 2, 16, and 256 colors. Note that the
> default logos are 80x80, which is larger than your display, so no logo
> is drawn.
> Fbtest has only the full color logo, so it will look bad on a monochrome
> display.
>

I see. Should the test check for minimum num_colors and skip that test then?

>> Another question, do you know if is possible to change the default format?
>
> AFAIK DRM does not support that.
>
>> I believe that fbset won't work because the DRM fbdev emulation layer does
>> not implement mode settings but I thought that changing the mode using the
>
> Correct.
>

[...]

>> Maybe I'm doing something wrong or misunderstading about how should work?
>
> Do you need the "-d" option (drop master after mode set) of modetest?
> Still, that would only impact DRM.  The fbdev emulation on top of DRM has
> already been initialized before, and is never reinitialized.
>

Ah, that explains and makes sense now. I tested other user-space fbdev
programs that only support XRGB8888 and those were working correctly so I
guess that user-space that supports R1 be defaulted to that and not able
to change is an acceptable behaviour.

When you post v2, feel free to add:

Tested-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


  reply	other threads:[~2023-07-17  9:13 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-13 13:17 [PATCH 0/8] drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1 Geert Uytterhoeven
2023-07-13 13:17 ` Geert Uytterhoeven
2023-07-13 13:17 ` [PATCH 1/8] drm/ssd130x: Fix pitch calculation in ssd130x_fb_blit_rect() Geert Uytterhoeven
2023-07-13 13:17   ` Geert Uytterhoeven
2023-07-14  9:34   ` Javier Martinez Canillas
2023-07-14  9:41     ` Geert Uytterhoeven
2023-07-14  9:41       ` Geert Uytterhoeven
2023-07-14  9:48       ` Javier Martinez Canillas
2023-07-14  9:48         ` Javier Martinez Canillas
2023-07-21 22:39         ` Javier Martinez Canillas
2023-07-21 22:39           ` Javier Martinez Canillas
2023-07-13 13:17 ` [PATCH 2/8] drm/dumb-buffers: Fix drm_mode_create_dumb() for bpp < 8 Geert Uytterhoeven
2023-07-13 13:17   ` Geert Uytterhoeven
2023-07-14  9:50   ` Javier Martinez Canillas
2023-07-14  9:50     ` Javier Martinez Canillas
2023-07-13 13:17 ` [PATCH/RFC 3/8] drm/ssd130x: Bail out early if data_array is not yet available Geert Uytterhoeven
2023-07-13 13:17   ` Geert Uytterhoeven
2023-07-14  9:53   ` Javier Martinez Canillas
2023-07-14  9:53     ` Javier Martinez Canillas
2023-07-13 13:17 ` [PATCH 4/8] drm/ssd130x: Add support for DRM_FORMAT_R1 Geert Uytterhoeven
2023-07-13 13:17   ` Geert Uytterhoeven
2023-07-14 10:14   ` Javier Martinez Canillas
2023-07-14 10:14     ` Javier Martinez Canillas
2023-07-14 11:26     ` Geert Uytterhoeven
2023-07-14 11:26       ` Geert Uytterhoeven
2023-07-14 12:35       ` Javier Martinez Canillas
2023-07-14 12:35         ` Javier Martinez Canillas
2023-07-14 12:43         ` Geert Uytterhoeven
2023-07-14 12:43           ` Geert Uytterhoeven
2023-07-14 13:08           ` Javier Martinez Canillas
2023-07-14 13:08             ` Javier Martinez Canillas
2023-07-13 13:17 ` [PATCH 5/8] drm/client: Convert drm_mode_create_dumb() to drm_mode_addfb2() Geert Uytterhoeven
2023-07-13 13:17   ` Geert Uytterhoeven
2023-07-14 10:16   ` Javier Martinez Canillas
2023-07-14 10:16     ` Javier Martinez Canillas
2023-07-14 11:01   ` Simon Ser
2023-07-14 11:01     ` Simon Ser
2023-07-14 11:29     ` Geert Uytterhoeven
2023-07-14 11:29       ` Geert Uytterhoeven
2023-07-13 13:17 ` [PATCH 6/8] drm/fb-helper: Pass buffer format via drm_fb_helper_surface_size Geert Uytterhoeven
2023-07-13 13:17   ` Geert Uytterhoeven
2023-07-14 10:25   ` Javier Martinez Canillas
2023-07-14 10:25     ` Javier Martinez Canillas
2023-07-14 11:32     ` Geert Uytterhoeven
2023-07-14 11:32       ` Geert Uytterhoeven
2023-07-13 13:17 ` [PATCH 7/8] drm/fb-helper: Add support for DRM_FORMAT_R1 Geert Uytterhoeven
2023-07-13 13:17   ` Geert Uytterhoeven
2023-07-14 10:31   ` Javier Martinez Canillas
2023-07-14 10:31     ` Javier Martinez Canillas
2023-07-13 13:17 ` [PATCH 8/8] drm/ssd130x: Switch preferred_bpp/depth to 1 Geert Uytterhoeven
2023-07-13 13:17   ` Geert Uytterhoeven
2023-07-14 10:32   ` Javier Martinez Canillas
2023-07-14 10:32     ` Javier Martinez Canillas
2023-07-16 13:30 ` [PATCH 0/8] drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1 Javier Martinez Canillas
2023-07-16 13:30   ` Javier Martinez Canillas
2023-07-17  8:07   ` Geert Uytterhoeven
2023-07-17  8:07     ` Geert Uytterhoeven
2023-07-17  9:13     ` Javier Martinez Canillas [this message]
2023-07-17  9:13       ` Javier Martinez Canillas
2023-07-17  9:19       ` Geert Uytterhoeven
2023-07-17  9:19         ` Geert Uytterhoeven
2023-07-17  9:33         ` Javier Martinez Canillas
2023-07-17  9:33           ` Javier Martinez Canillas
2023-07-17 10:03           ` Geert Uytterhoeven
2023-07-17 10:03             ` Geert Uytterhoeven
2023-07-17 10:11             ` Javier Martinez Canillas
2023-07-17 10:11               ` Javier Martinez Canillas

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=87o7kakijn.fsf@minerva.mail-host-address-is-not-set \
    --to=javierm@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=geert@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mripard@kernel.org \
    --cc=tzimmermann@suse.de \
    /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.