Message ID | 20230609170941.1150941-3-javierm@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp1070894vqr; Fri, 9 Jun 2023 10:27:57 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ47YzdIIDPHJB5RMiTkAzlPYzJUmZHiQxZEi4WRpqPNyBpvBYAHeA53z5xzrtQvYWJekKAe X-Received: by 2002:a05:6a20:a11a:b0:114:af27:41dd with SMTP id q26-20020a056a20a11a00b00114af2741ddmr1721068pzk.34.1686331677000; Fri, 09 Jun 2023 10:27:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686331676; cv=none; d=google.com; s=arc-20160816; b=uHn8+aq2GsvADKSAU8fjy4dh6DLVwhCYsum/oM3ho1oRrbIImHd1K+7sMYCkkGGZbj 1yM9Xc8t4K/VO038KS6PlfPp+L8QLFvLNcAhNLG3/VKk44YT4iRT/n9Vgl3k9Eh93k6G sOdBGd6YuxhhT4MlYBn9+Qa3YlbrSvkp3vO/3UYkNOIteyPnerm82TT3LZbzuPCoNG12 es3jpNOvBWvUcsKEb3SXk7K8MpxxSzH3So4DBocXHY0GrFQgV3iuINkJ07jFbaT3B0gd pS4pBC+riA5VEZMcLhQXu29lQxIha9fXb9IZmm13MILoiQ11BlLz+NYLxtbwD5WoY2Z5 ajdA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=rXfuy13isQUH9meYMWRMilnarujeOibtxjp5Z2EnlHc=; b=OidGVLv3hWpASltqHBv74Y+WuC8kHcX4i9ZUptrFm/+QztU/j+KxpjqCYp8cviPjXz uRjMLmP9dY6FpKD02u82Rpl572gnYc+xsZK720QW0mgjJzHgsrD3bzRLbNYAJ2m3Zzen NXN45DCj1Wmr1j/ZE4oFKdi/jzzETDbtaIr85sUdpLmBQrXrGP8LeGsjeHlZ7fyT2K6m qbPBReCAfaSsZ4Fpb4W+mS3nKOfDFuzKeGqzVeC3TFkY/AjdvjtXbyCjShQKWu6DWQMZ RGMU7pKyGhHgeI6ClKqxSv40jEdbIRu/JCQ3K3W3uaBSGIIUB4vjykSnew1edPveG6CE 3otw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=QueaOLzf; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id lg16-20020a170902fb9000b001ae0691dfdbsi2911982plb.158.2023.06.09.10.27.44; Fri, 09 Jun 2023 10:27:56 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=QueaOLzf; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230191AbjFIRKn (ORCPT <rfc822;liningstudo@gmail.com> + 99 others); Fri, 9 Jun 2023 13:10:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58610 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229488AbjFIRKi (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 9 Jun 2023 13:10:38 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8B7323598 for <linux-kernel@vger.kernel.org>; Fri, 9 Jun 2023 10:09:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686330591; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rXfuy13isQUH9meYMWRMilnarujeOibtxjp5Z2EnlHc=; b=QueaOLzfaq011vSgiz156s3ulnthgprx2xESQE3VEgagBGxtKEyruk3Gn9LAuS92KseXnD 15acIupvEoqFZHtDRtu1aCL80+WMwnn9Wyb5ICDDLdHZSsoCmGBQqkMxOpNFcqfOfO4hDk yCMxxMHjYzFVlgkGum34ecoQtV5EVXw= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-235-SnrMtooqNIqdV3lWPxTuTA-1; Fri, 09 Jun 2023 13:09:49 -0400 X-MC-Unique: SnrMtooqNIqdV3lWPxTuTA-1 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-3f725f64b46so32544525e9.1 for <linux-kernel@vger.kernel.org>; Fri, 09 Jun 2023 10:09:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686330588; x=1688922588; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rXfuy13isQUH9meYMWRMilnarujeOibtxjp5Z2EnlHc=; b=hWLN7r8dAEL+cDS2WsLC4l7nCvV3exEOb9eSK1urleI29aV1QqG3gAxO6g2EGUXGxq HK9TDLVojQAlBamfMNvNox15x3CG6E/dchIjqL5i2HXjF0dx4LQhTa4Ykk2t+fgdPYaz y75fdzrm/6X3O3OG3zvRJ094tedfbVNTvpsuzf13nBsn+ofNJtwJ9TfDJWno8C+UT9V1 keqWSFe7V6tYxnnrDQ1m1kYKcvGtxK/Av1loY9bmYtmCB2fyam2Gw605oFyRwFIDSO/W b9koocOlPTf/HSKWZAPoGIMmrZdm3psZVrqkFTmKILzHZFpFE3N+R4N6Z6yVJMlRRr40 dDsQ== X-Gm-Message-State: AC+VfDzCzlQaxfBLHTieuw8l+PegAPcGXkI1K8+1BAv0myeoUy1rrZiU NLFE2Pf6TVYOxrfoVLxEjcIM6nMj91x7L2qzCaMa7/SlHuWwzjd25BxKwL6SVs0yo6C0ZsqxnBt 0VZTkYqejNrtMaDwfNNf+sC0ay8sJnFBp8phVj4z21XWx0NgZfTV/mI3JSmFzvlK4YjQc1cl9SE lbR9LNP/E= X-Received: by 2002:a05:600c:2297:b0:3f7:f544:4993 with SMTP id 23-20020a05600c229700b003f7f5444993mr1713202wmf.20.1686330588102; Fri, 09 Jun 2023 10:09:48 -0700 (PDT) X-Received: by 2002:a05:600c:2297:b0:3f7:f544:4993 with SMTP id 23-20020a05600c229700b003f7f5444993mr1713180wmf.20.1686330587766; Fri, 09 Jun 2023 10:09:47 -0700 (PDT) Received: from minerva.home (205.pool92-176-231.dynamic.orange.es. [92.176.231.205]) by smtp.gmail.com with ESMTPSA id c21-20020a05600c0ad500b003f7310a3ffasm3265526wmr.2.2023.06.09.10.09.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Jun 2023 10:09:47 -0700 (PDT) From: Javier Martinez Canillas <javierm@redhat.com> To: linux-kernel@vger.kernel.org Cc: Geert Uytterhoeven <geert@linux-m68k.org>, Thomas Zimmermann <tzimmermann@suse.de>, Maxime Ripard <mripard@kernel.org>, Javier Martinez Canillas <javierm@redhat.com>, Conor Dooley <conor+dt@kernel.org>, Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@gmail.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Remove default width and height values Date: Fri, 9 Jun 2023 19:09:37 +0200 Message-Id: <20230609170941.1150941-3-javierm@redhat.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230609170941.1150941-1-javierm@redhat.com> References: <20230609170941.1150941-1-javierm@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1768246924577689061?= X-GMAIL-MSGID: =?utf-8?q?1768246924577689061?= |
Series |
drm/ssd130x: A few enhancements and cleanups
|
|
Commit Message
Javier Martinez Canillas
June 9, 2023, 5:09 p.m. UTC
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. The datasheets for the chips describes the following display resolutions: - SH1106: 132 x 64 Dot Matrix OLED/PLED - SSD1305: 132 x 64 Dot Matrix OLED/PLED - SSD1306: 128 x 64 Dot Matrix OLED/PLED - SSD1307: 128 x 39 Dot Matrix OLED/PLED - SSD1309: 128 x 64 Dot Matrix OLED/PLED Update DT schema to reflect what the driver does and make its users aware. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> --- Changes in v2: - List per controller default width/height values in DT schema (Maxime Ripard). .../bindings/display/solomon,ssd1307fb.yaml | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-)
Comments
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? Cheers, Conor. > > The datasheets for the chips describes the following display resolutions: > > - SH1106: 132 x 64 Dot Matrix OLED/PLED > - SSD1305: 132 x 64 Dot Matrix OLED/PLED > - SSD1306: 128 x 64 Dot Matrix OLED/PLED > - SSD1307: 128 x 39 Dot Matrix OLED/PLED > - SSD1309: 128 x 64 Dot Matrix OLED/PLED > > Update DT schema to reflect what the driver does and make its users aware. > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > > Changes in v2: > - List per controller default width/height values in DT schema (Maxime Ripard). > > .../bindings/display/solomon,ssd1307fb.yaml | 28 ++++++++++++++++--- > 1 file changed, 24 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > index 94bb5ef567c6..20e2bd15d4d2 100644 > --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > @@ -49,15 +49,15 @@ properties: > > solomon,height: > $ref: /schemas/types.yaml#/definitions/uint32 > - default: 16 > description: > - Height in pixel of the screen driven by the controller > + Height in pixel of the screen driven by the controller. > + The default value is controller-dependent. > > solomon,width: > $ref: /schemas/types.yaml#/definitions/uint32 > - default: 96 > description: > - Width in pixel of the screen driven by the controller > + Width in pixel of the screen driven by the controller. > + The default value is controller-dependent. > > solomon,page-offset: > $ref: /schemas/types.yaml#/definitions/uint32 > @@ -157,6 +157,10 @@ allOf: > const: sinowealth,sh1106 > then: > properties: > + width: > + default: 132 > + height: > + default: 64 > solomon,dclk-div: > default: 1 > solomon,dclk-frq: > @@ -171,6 +175,10 @@ allOf: > - solomon,ssd1305 > then: > properties: > + width: > + default: 132 > + height: > + default: 64 > solomon,dclk-div: > default: 1 > solomon,dclk-frq: > @@ -185,6 +193,10 @@ allOf: > - solomon,ssd1306 > then: > properties: > + width: > + default: 128 > + height: > + default: 64 > solomon,dclk-div: > default: 1 > solomon,dclk-frq: > @@ -199,6 +211,10 @@ allOf: > - solomon,ssd1307 > then: > properties: > + width: > + default: 128 > + height: > + default: 39 > solomon,dclk-div: > default: 2 > solomon,dclk-frq: > @@ -215,6 +231,10 @@ allOf: > - solomon,ssd1309 > then: > properties: > + width: > + default: 128 > + height: > + default: 64 > solomon,dclk-div: > default: 1 > solomon,dclk-frq: > -- > 2.40.1 >
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. [1]: https://github.com/raspberrypi/linux/blob/rpi-6.1.y/arch/arm/boot/dts/overlays/ssd1306-overlay.dts > Cheers, > Conor. >
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. 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.
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.
Hi Am 11.06.23 um 01:18 schrieb Javier Martinez Canillas: > 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. > From my understanding, fixing the resolution is the correct thing to do here. Userspace needs to be able to handle these differences. Best regards Thomas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
On Mon, Jun 12, 2023 at 09:47:12AM +0200, Thomas Zimmermann wrote: > Am 11.06.23 um 01:18 schrieb Javier Martinez Canillas: > > 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. If there are different compatibles, then it can always be sorted out later iff it turns out to be a problem, since new devicetrees should not be using the deprecated compatibles anyway. I didn't realise that those deprecated compatibles existed, thanks for your patience. > > 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. > > > > From my understanding, fixing the resolution is the correct thing to do > here. Userspace needs to be able to handle these differences. Fixing meaning correcting, or fixing meaning using a fixed resolution? Not clear to me what you mean, sorry.
Conor Dooley <conor@kernel.org> writes: > On Mon, Jun 12, 2023 at 09:47:12AM +0200, Thomas Zimmermann wrote: >> Am 11.06.23 um 01:18 schrieb Javier Martinez Canillas: > >> > 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. > > If there are different compatibles, then it can always be sorted out > later iff it turns out to be a problem, since new devicetrees should not > be using the deprecated compatibles anyway. I didn't realise that those > deprecated compatibles existed, thanks for your patience. > No worries, thanks for raising this question. >> > 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. >> > >> >> From my understanding, fixing the resolution is the correct thing to do >> here. Userspace needs to be able to handle these differences. > > Fixing meaning correcting, or fixing meaning using a fixed resolution? > Not clear to me what you mean, sorry. > Fixing meaning using as a default the correct maximum resolution for each OLED controller, rather than an arbitrary 96x16 resolution that was added just to be compatible with the panel that was tested the original driver. But after talking with Thomas and Maxime about this issue, I realized that it won't even cause an issue for theoretical users that may be relying on the previous default. Changing the default resolution to something smaller could cause an issue since a user expecting a bigger default would get their display output cut but changing to something bigger just means user-space being able to write more pixels than those that will be displayed. Because there isn't really a "resolution" configured in the chip, but just how many pixels a particular controller can drive. The new default is the maximum that each controller supports according to their documentation.
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml index 94bb5ef567c6..20e2bd15d4d2 100644 --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml @@ -49,15 +49,15 @@ properties: solomon,height: $ref: /schemas/types.yaml#/definitions/uint32 - default: 16 description: - Height in pixel of the screen driven by the controller + Height in pixel of the screen driven by the controller. + The default value is controller-dependent. solomon,width: $ref: /schemas/types.yaml#/definitions/uint32 - default: 96 description: - Width in pixel of the screen driven by the controller + Width in pixel of the screen driven by the controller. + The default value is controller-dependent. solomon,page-offset: $ref: /schemas/types.yaml#/definitions/uint32 @@ -157,6 +157,10 @@ allOf: const: sinowealth,sh1106 then: properties: + width: + default: 132 + height: + default: 64 solomon,dclk-div: default: 1 solomon,dclk-frq: @@ -171,6 +175,10 @@ allOf: - solomon,ssd1305 then: properties: + width: + default: 132 + height: + default: 64 solomon,dclk-div: default: 1 solomon,dclk-frq: @@ -185,6 +193,10 @@ allOf: - solomon,ssd1306 then: properties: + width: + default: 128 + height: + default: 64 solomon,dclk-div: default: 1 solomon,dclk-frq: @@ -199,6 +211,10 @@ allOf: - solomon,ssd1307 then: properties: + width: + default: 128 + height: + default: 39 solomon,dclk-div: default: 2 solomon,dclk-frq: @@ -215,6 +231,10 @@ allOf: - solomon,ssd1309 then: properties: + width: + default: 128 + height: + default: 64 solomon,dclk-div: default: 1 solomon,dclk-frq: