From: Javier Martinez Canillas <javierm@redhat.com>
To: Conor Dooley <conor@kernel.org>
Cc: devicetree@vger.kernel.org, Conor Dooley <conor+dt@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
linux-kernel@vger.kernel.org, Maxime Ripard <mripard@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Remove default width and height values
Date: Sun, 11 Jun 2023 01:18:28 +0200 [thread overview]
Message-ID: <87jzwa29ff.fsf@minerva.mail-host-address-is-not-set> (raw)
In-Reply-To: <20230610-unused-engaged-c1f4119cff08@spud>
Conor Dooley <conor@kernel.org> writes:
> On Sat, Jun 10, 2023 at 07:51:35PM +0200, Javier Martinez Canillas wrote:
>> Conor Dooley <conor@kernel.org> writes:
>>
>> > On Fri, Jun 09, 2023 at 07:09:37PM +0200, Javier Martinez Canillas wrote:
>> >> A default resolution in the ssd130x driver isn't set to an arbitrary 96x16
>> >> anymore. Instead is set to a width and height that's controller dependent.
>> >
>> > Did that change to the driver not break backwards compatibility with
>> > existing devicetrees that relied on the default values to get 96x16?
>> >
>>
>> It would but I don't think it is an issue in pratice. Most users of these
>> panels use one of the multiple libraries on top of the spidev interface.
>>
>> For the small userbase that don't, I believe that they will use the rpif
>> kernel and ssd1306-overlay.dtbo DTB overlay, which defaults to width=128
>> and height=64 [1]. So those users will have to explicitly set a width and
>> height for a 96x16 panel anyways.
>>
>> The intersection of users that have a 96x16 panel, assumed that default
>> and consider the DTB a stable ABI, and only update their kernel but not
>> the DTB should be very small IMO.
>
> It's the adding of new defaults that makes it a bit messier, since you
> can't even revert without potentially breaking a newer user. I'd be more
> inclined to require the properties, rather change their defaults in the
> binding, lest there are people relying on them.
I think that's OK, the old drivers/video/fbdev/ssd1307fb.c fbdev driver
still has the old behaviour so it will only be a problem for users that
want to move to the new ssd130x driver as well.
By looking at the git log history, the 96x16 resolution was chosen when
the driver was merged because Maxime tested it on a CFA10036 board [1]
that has a 96x16 panel that uses an SSD1307 controller.
But as mentioned, that chip can drive up to 128x39 pixels so the maximum
makes more sense as default to me.
[1]: https://www.crystalfontz.com/product/cfa10036
> If you and the other knowledgeable folk in the area really do know such
> users do not exist then I suppose it is fine to do.
I believe is fine, since as explained above that change was only done in
the ssd130x DRM driver, not the ssd1307fb fbdev driver whose default is
still 96x16. Both drivers share the same DT binding scheme, I was asked
to do that to make it as much backward compatible as possible with the
old fbdev driver.
But I will be OK to drop the "solomon,ssd130?fb-i2c" compatible strings
from the DRM driver and only match against the new "solomon,ssd130?-i2c"
compatible strings. And add a different DT binding schema for the ssd130x
driver, if that would mean being able to fix things like the one mentioned
in this patch.
In my opinion, trying to always make the drivers backward compatible with
old DTBs only makes the drivers code more complicated for unclear benefit.
Usually this just ends being code that is neither used nor tested. Because
in practice most people update the DTBs and kernels, instead of trying to
make the DTB a stable ABI like firmware.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
next prev parent reply other threads:[~2023-06-10 23:19 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-09 17:09 [PATCH v2 0/5] drm/ssd130x: A few enhancements and cleanups Javier Martinez Canillas
2023-06-09 17:09 ` [PATCH v2 1/5] drm/ssd130x: Make default width and height to be controller dependent Javier Martinez Canillas
2023-06-09 17:09 ` [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Remove default width and height values Javier Martinez Canillas
2023-06-10 15:10 ` Conor Dooley
2023-06-10 17:51 ` Javier Martinez Canillas
2023-06-10 18:14 ` Conor Dooley
2023-06-10 23:18 ` Javier Martinez Canillas [this message]
2023-06-12 7:47 ` Thomas Zimmermann
2023-06-12 17:44 ` Conor Dooley
2023-06-12 18:30 ` Javier Martinez Canillas
2023-06-09 17:09 ` [PATCH v2 3/5] drm/ssd130x: Set the page height value in the device info data Javier Martinez Canillas
2023-06-09 17:09 ` [PATCH v2 4/5] drm/ssd130x: Don't allocate buffers on each plane update Javier Martinez Canillas
2023-07-13 12:44 ` Geert Uytterhoeven
2023-07-13 13:15 ` Javier Martinez Canillas
2023-07-13 13:25 ` Geert Uytterhoeven
2023-07-13 14:12 ` Javier Martinez Canillas
2023-07-13 14:53 ` Geert Uytterhoeven
2023-07-13 16:34 ` Javier Martinez Canillas
2023-06-09 17:09 ` [PATCH v2 5/5] drm/ssd130x: Remove hardcoded bits-per-pixel in ssd130x_buf_alloc() Javier Martinez Canillas
2023-07-12 10:24 ` Geert Uytterhoeven
2023-06-15 21:53 ` [PATCH v2 0/5] drm/ssd130x: A few enhancements and cleanups 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=87jzwa29ff.fsf@minerva.mail-host-address-is-not-set \
--to=javierm@redhat.com \
--cc=conor+dt@kernel.org \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=geert@linux-m68k.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mripard@kernel.org \
--cc=robh+dt@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).