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 X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4466C43381 for ; Sun, 24 Feb 2019 13:25:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8689C206B6 for ; Sun, 24 Feb 2019 13:25:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728369AbfBXNZ5 (ORCPT ); Sun, 24 Feb 2019 08:25:57 -0500 Received: from honk.sigxcpu.org ([24.134.29.49]:55403 "EHLO honk.sigxcpu.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728121AbfBXNZ4 (ORCPT ); Sun, 24 Feb 2019 08:25:56 -0500 Received: from localhost (localhost [127.0.0.1]) by honk.sigxcpu.org (Postfix) with ESMTP id 64ADDFB03; Sun, 24 Feb 2019 14:25:52 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at honk.sigxcpu.org Received: from honk.sigxcpu.org ([127.0.0.1]) by localhost (honk.sigxcpu.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yJyajUATgFw1; Sun, 24 Feb 2019 14:25:49 +0100 (CET) Received: by bogon.sigxcpu.org (Postfix, from userid 1000) id 9532F43558; Sun, 24 Feb 2019 14:25:48 +0100 (CET) Date: Sun, 24 Feb 2019 14:25:48 +0100 From: Guido =?iso-8859-1?Q?G=FCnther?= To: Sam Ravnborg Cc: Thierry Reding , David Airlie , Daniel Vetter , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH 3/3] drm/panel: Add Rocktech jh057n00900 panel driver Message-ID: <20190224132548.GA6007@bogon.m.sigxcpu.org> References: <79858664af8ee2f725e53d902ce46b447ac8fcfc.1550943220.git.agx@sigxcpu.org> <20190223220304.GA20290@ravnborg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190223220304.GA20290@ravnborg.org> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sam, On Sat, Feb 23, 2019 at 11:03:04PM +0100, Sam Ravnborg wrote: > Hi Guido. > > Thanks for this patch. > See below for some review feedback. Thanks for having look! This is what I've folded in for v2: Changes from v1 * As per review comments from Sam Ravnborg * Make SPDX-License-Identifier match MODULE_LICENSE * Sort include files alphabetically * Drop drmP.h and use individual includes * Drop superfuous 'x' in mode printout on error path * Allpixelson_set: Add proper space around '*' * Drop superfluous put_device(&ctx->backlight->dev); * Add /* Sentinel */ in jh057n_of_match * Drop jh057n->enabled * Drop drm_display_info_set_bus_formats * Kconfig: Depend on BACKLIGHT_CLASS_DEVICE which somehow got lost * Move jh057n_enable close to jh057n_disable I'll hold off a v2 in case there are further comments. > The driver includes the "allpixelson_set" debugfs feature. > Is this a debug leftover, or is there a real need for this? > If this is a debug feature that is no logner needed then no > need to add it to the mainline driver. That debugfs entry is a life saver when staring at a blank screen and trying to figure out what is broken. If a echo 1 > /sys/kernel/debug/jh057n00900/allpixelson lights up the screen then DSI LP mode communication is working. One can then focus on the video mode (pixel clock, polarity, ...). So if at all possible I'd leave that in since it might help to diagnose problems not only in the driver but in upper DSI layers (phy, dsi host). Cheers, -- Guido > > Sam > > On Sat, Feb 23, 2019 at 06:39:44PM +0100, Guido Günther wrote: > > Support Rocktech jh057n00900 5.5" 720x1440 TFT LCD panel. It is a MIPI > > DSI video mode panel. > > > > The panel seems to use a Sitronix ST7703 look alike (most of the > > commands look similar to the ST7703's data sheet but use a different > > number of parameters). The initial version of the DSI init sequence > > (including sleeps) were provided by the vendor. Sleeps were reduced > > considerably though to speed up initialization. > > > > Signed-off-by: Guido Günther > > --- > > +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c > > @@ -0,0 +1,414 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Rockteck jh057n00900 5.5" MIPI-DSI panel driver > > + * > > + * Copyright (C) Purism SPC 2019 > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include