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=-13.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,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 42F64C282C5 for ; Thu, 24 Jan 2019 19:50:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0F32521726 for ; Thu, 24 Jan 2019 19:50:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732928AbfAXTum (ORCPT ); Thu, 24 Jan 2019 14:50:42 -0500 Received: from asavdk3.altibox.net ([109.247.116.14]:55906 "EHLO asavdk3.altibox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732692AbfAXTuk (ORCPT ); Thu, 24 Jan 2019 14:50:40 -0500 Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by asavdk3.altibox.net (Postfix) with ESMTPS id CA8A820056; Thu, 24 Jan 2019 20:50:37 +0100 (CET) Date: Thu, 24 Jan 2019 20:50:35 +0100 From: Sam Ravnborg To: Jagan Teki Cc: Thierry Reding , David Airlie , Daniel Vetter , Michael Trimarchi , linux-amarula@amarulasolutions.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v4 2/2] drm/panel: Add Feiyang FY07024DI26A30-D MIPI-DSI LCD panel Message-ID: <20190124195035.GB4783@ravnborg.org> References: <20190124184313.30193-1-jagan@amarulasolutions.com> <20190124184313.30193-2-jagan@amarulasolutions.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190124184313.30193-2-jagan@amarulasolutions.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=dqr19Wo4 c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=kj9zAlcOel0A:10 a=iP-xVBlJAAAA:8 a=e5mUnYsNAAAA:8 a=WZHNqt2aAAAA:8 a=g_qM3QxpcqtW7zA0w_EA:9 a=CjuIK1q_8ugA:10 a=lHLH-nfn2y1bM_0xSXwp:22 a=Vxmtnl_E_bksehYqCbjh:22 a=PrHl9onO2p7xFKlKy1af:22 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jagan. Thanks for this nice patch, a few comments below. On Fri, Jan 25, 2019 at 12:13:13AM +0530, Jagan Teki wrote: > Feiyang FY07024DI26A30-D is 1024x600, 4-lane MIPI-DSI LCD panel. > > Add panel driver for it. > > Signed-off-by: Jagan Teki > --- > Changes for v5: > - rebase on master > - adjust the hporch values to satisfy the refresh > Changes for v4: > - use simple structure for command init > - update proper comments on power, reset delay sequnce > - fix to use set_display_off in disable function > - move mode type to structure > - drop refres rate value, let drm compute > Changes for v2: > - new patch, derived from another dsi series > > MAINTAINERS | 6 + > drivers/gpu/drm/panel/Kconfig | 9 + > drivers/gpu/drm/panel/Makefile | 1 + > .../drm/panel/panel-feiyang-fy07024di26a30d.c | 286 ++++++++++++++++++ > 4 files changed, 302 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index e3f785bad68b..5c8591eb5840 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4831,6 +4831,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > S: Maintained > F: drivers/gpu/drm/tve200/ > > +DRM DRIVER FOR FEIYANG FY07024DI26A30-D MIPI-DSI LCD PANELS > +M: Jagan Teki > +S: Maintained > +F: drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > +F: Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt > + > DRM DRIVER FOR ILITEK ILI9225 PANELS > M: David Lechner > S: Maintained > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index 151ddf720b83..5304db7b7b55 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -47,6 +47,15 @@ config DRM_PANEL_SIMPLE > that it can be automatically turned off when the panel goes into a > low power state. > > +config DRM_PANEL_FEIYANG_FY07024DI26A30D > + tristate "Feiyang FY07024DI26A30-D MIPI-DSI LCD panel" > + depends on OF > + depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > + help > + Say Y if you want to enable support for panels based on the > + Feiyang FY07024DI26A30-D MIPI-DSI interface. > + > config DRM_PANEL_ILITEK_IL9322 > tristate "Ilitek ILI9322 320x240 QVGA panels" > depends on OF && SPI > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile > index 54db0921f329..c88dacf9d439 100644 > --- a/drivers/gpu/drm/panel/Makefile > +++ b/drivers/gpu/drm/panel/Makefile > @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o > obj-$(CONFIG_DRM_PANEL_BANANAPI_S070WV20_ICN6211) += panel-bananapi-s070wv20-icn6211.o > obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o > obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o > +obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o > obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o > obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o > obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o > diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > new file mode 100644 > index 000000000000..f655647aeade > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > @@ -0,0 +1,286 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018 Amarula Solutions > + * Author: Jagan Teki > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +#include Please do not use drmP.h in new drivers. We are slowly gettting rid of it and in drm-misc.git the drmP.h file no longer contains anything (other than includes/forwards). > + gpiod_set_value(ctx->reset, 1); > + msleep(50); > + > + gpiod_set_value(ctx->reset, 0); > + /* T5 + T6 (avdd rise + video & logic signal rise) > + * T5 >= 10ms, 0 < T6 <= 10ms > + */ > + msleep(20); > + > + gpiod_set_value(ctx->reset, 1); Is it really necessary to do a 1 => 0 => 1 cycle here for reset? panel-simple do not need it. > + > +static int feiyang_dsi_probe(struct mipi_dsi_device *dsi) > +{ > + struct feiyang *ctx; > + int ret; > + > + ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + mipi_dsi_set_drvdata(dsi, ctx); Add empty line atfer return -ENOMEM;? > + ctx->dsi = dsi; > + > + drm_panel_init(&ctx->panel); > + ctx->panel.dev = &dsi->dev; > + ctx->panel.funcs = &feiyang_funcs; > + > + ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd"); > + if (IS_ERR(ctx->dvdd)) { > + DRM_DEV_ERROR(&dsi->dev, "Couldn't get dvdd regulator\n"); > + return PTR_ERR(ctx->dvdd); > + } > + > + ctx->avdd = devm_regulator_get(&dsi->dev, "avdd"); > + if (IS_ERR(ctx->avdd)) { > + DRM_DEV_ERROR(&dsi->dev, "Couldn't get avdd regulator\n"); > + return PTR_ERR(ctx->avdd); > + } > + > + ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(ctx->reset)) { > + DRM_DEV_ERROR(&dsi->dev, "Couldn't get our reset GPIO\n"); > + return PTR_ERR(ctx->reset); > + } > + > + ctx->backlight = devm_of_find_backlight(&dsi->dev); > + if (IS_ERR(ctx->backlight)) > + return PTR_ERR(ctx->backlight); > + > + ret = drm_panel_add(&ctx->panel); > + if (ret < 0) > + goto put_backlight; > + > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST; > + dsi->format = MIPI_DSI_FMT_RGB888; > + dsi->lanes = 4; > + > + ret = mipi_dsi_attach(dsi); > + if (ret < 0) > + goto panel_remove; > + > + return ret; > + > +panel_remove: > + drm_panel_remove(&ctx->panel); > +put_backlight: > + if (ctx->backlight) > + put_device(&ctx->backlight->dev); As devm_of_find_backlight() is used the put_device() will be done for you. No need to do it here. I think the same is true for the panel. > + > + return ret; > +} > + > +static int feiyang_dsi_remove(struct mipi_dsi_device *dsi) > +{ > + struct feiyang *ctx = mipi_dsi_get_drvdata(dsi); > + > + mipi_dsi_detach(dsi); > + drm_panel_remove(&ctx->panel); > + > + if (ctx->backlight) > + put_device(&ctx->backlight->dev); Likewise, use of devm_of_find_backlight() will do this for you. Sam