From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D235CC43217 for ; Wed, 2 Feb 2022 13:53:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242323AbiBBNxj (ORCPT ); Wed, 2 Feb 2022 08:53:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33454 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233386AbiBBNxj (ORCPT ); Wed, 2 Feb 2022 08:53:39 -0500 Received: from smtp.domeneshop.no (smtp.domeneshop.no [IPv6:2a01:5b40:0:3005::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A10FFC061714 for ; Wed, 2 Feb 2022 05:53:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tronnes.org ; s=ds202112; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=gyLaoh+vsDn3ZLnoONwAHHTpsR6J1Z9CuQ+OvRZ9enE=; b=flM2BnQ5KQAexOGKaVlzvwcnfr vn5QMPloAshCuKdKrAiW6O4odl5ey1mMlBIRh+7KLZt0IUL1ViO+OUpjE6S8Ep+iDXeTpkaAIRyuF owQUeJHuHHlW+4lFG6fymhacCupI+huRQzQvew+pkIhpMaHyNt/E2d/wpGh34rsC5/hB+IW5aaKCA QKIrVaihQYD7bX4zDxBJD9HxJu16lqG1173E7DKk2Pzp/2u15QZzs1lKwd4pv2OSkP3ZNLOMLimyQ Cnmi7tidaMjs6jeTdID7D7OPsZW7ZrXqb1WiwvJQhkBtCwRvVlAj4p5Mc/wY/GrGHzP4fVgbsP+iG oQ39TnWQ==; Received: from [2a01:799:95e:a400:cca0:57ac:c55d:a485] (port=54700) by smtp.domeneshop.no with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nFG4l-0003AS-OS; Wed, 02 Feb 2022 14:53:35 +0100 Message-ID: <261c76d6-6bb9-1fe1-6560-58af5ba240dd@tronnes.org> Date: Wed, 2 Feb 2022 14:53:32 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v2 3/3] drm/panel: Add MIPI DBI compatible SPI driver To: Maxime Ripard Cc: robh+dt@kernel.org, thierry.reding@gmail.com, sam@ravnborg.org, dave.stevenson@raspberrypi.com, david@lechnology.com, devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org References: <20220125175700.37408-1-noralf@tronnes.org> <20220125175700.37408-4-noralf@tronnes.org> <20220127100452.bmxcgf6ye3nxrgtq@houat> <20220202100953.pymb6blsqlaw7jv6@houat> From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= In-Reply-To: <20220202100953.pymb6blsqlaw7jv6@houat> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Den 02.02.2022 11.09, skrev Maxime Ripard: > On Thu, Jan 27, 2022 at 06:53:48PM +0100, Noralf Trønnes wrote: >>>> +struct panel_mipi_dbi_config { >>>> + /* Magic string: panel_mipi_dbi_magic */ >>>> + u8 magic[15]; >>>> + >>>> + /* Config file format version */ >>>> + u8 file_format_version; >>>> + >>>> + /* Width in pixels */ >>>> + __be16 width; >>>> + /* Height in pixels */ >>>> + __be16 height; >>>> + >>>> + /* Width in millimeters (optional) */ >>>> + __be16 width_mm; >>>> + /* Height in millimeters (optional) */ >>>> + __be16 height_mm; >>>> + >>>> + /* X-axis panel offset */ >>>> + __be16 x_offset; >>>> + /* Y-axis panel offset */ >>>> + __be16 y_offset; >>>> + >>>> + /* 4 pad bytes, must be zero */ >>>> + u8 pad[4]; >>>> + >>>> + /* >>>> + * Optional MIPI commands to execute when the display pipeline is enabled. >>>> + * This can be used to configure the display controller. >>>> + * >>>> + * The commands are stored in a byte array with the format: >>>> + * command, num_parameters, [ parameter, ...], command, ... >>>> + * >>>> + * Some commands require a pause before the next command can be received. >>>> + * Inserting a delay in the command sequence is done by using the NOP command with one >>>> + * parameter: delay in miliseconds (the No Operation command is part of the MIPI Display >>>> + * Command Set where it has no parameters). >>>> + * >>>> + * Example: >>>> + * command 0x11 >>>> + * sleep 120ms >>>> + * command 0xb1 parameters 0x01, 0x2c, 0x2d >>>> + * command 0x29 >>>> + * >>>> + * Byte sequence: >>>> + * 0x11 0x00 >>>> + * 0x00 0x01 0x78 >>>> + * 0xb1 0x03 0x01 0x2c 0x2d >>>> + * 0x29 0x00 >>>> + */ >>>> + u8 commands[]; >>>> +}; >>> >>> I'm not really a fan of parsing raw data in the kernel. I guess we can't >>> really avoid the introduction of a special case to sleep, but we already >>> have dt properties for all of the other properties (but X and Y offset, >>> maybe?) >>> >>> Maybe we should use those instead? >> >> I don't understand your reluctance to parsing data, lots of ioctls do >> it. > > The reluctance comes from the parsing itself: you need to have input > validation, and it's hard to get right. The less we have, the easier it > gets. > >> And this data can only be loaded by root. What I like about having >> these properties in the config file is that the binding becomes a >> fallback binding that can actually be made to work without changing the >> Device Tree. >> >> For arguments sake let's say tiny/st7735r.c was not built and we had >> this node: >> >> display@0{ >> compatible = "jianda,jd-t18003-t01", "sitronix,st7735r", >> "panel-mipi-dbi-spi"; >> }; >> >> It will still be possible to use this display without changing the >> Device Tree. Just add a firmware/config file. >> >> Having the properties in DT it would have to look like this for the >> fallback to work: >> >> display@0{ >> compatible = "jianda,jd-t18003-t01", "sitronix,st7735r", >> "panel-mipi-dbi-spi"; >> panel-timing = { >> hactive = <128>; >> vactive = <128>; >> }; >> width-mm = <25>; >> height-mm = <26>; >> x-offset = <2>; >> y-offset = <3>; >> }; >> >> Is this important, I'm not sure. What do you think? > > Parts of it is ergonomics I guess. We're used to having all those > properties either in the DT or the driver, but here we introduce a new > way that isn't done anywhere else. > > And I don't see any real downside to putting it in the DT? It's going to > be in an overlay, under the user's control anyway, right? > Ok, I'll spin a new version using DT properties. Noralf.