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=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 3B6E1C433DF for ; Wed, 17 Jun 2020 15:25:16 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C115F206FA for ; Wed, 17 Jun 2020 15:25:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=agner.ch header.i=@agner.ch header.b="zPHVwzVP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C115F206FA Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=agner.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 25B066E124; Wed, 17 Jun 2020 15:25:15 +0000 (UTC) Received: from mail.kmu-office.ch (mail.kmu-office.ch [IPv6:2a02:418:6a02::a2]) by gabe.freedesktop.org (Postfix) with ESMTPS id A19F76E95A for ; Wed, 17 Jun 2020 15:25:13 +0000 (UTC) Received: from webmail.kmu-office.ch (unknown [IPv6:2a02:418:6a02::a3]) by mail.kmu-office.ch (Postfix) with ESMTPSA id DD7E45C8E51; Wed, 17 Jun 2020 17:25:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=agner.ch; s=dkim; t=1592407511; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ocFPFb2f9fysGjxdBpGbWPqBiRJzn16gDWm6LRAJlYs=; b=zPHVwzVPl9ykwbZGiOjALnKXLsufI6Xgi1gHRGKQVdIciI+y1wvshnTUjbnKLs14+9XJRM LJGIgb+kGs/ULz1K3BwHIUqPT8f14eFph80sYV8e8FD0aW/Zbaq7KAQ8PIL+4XhiefaKxU epvQh1k8FkMoJMtXWgNR1qHgu+ZR7HE= MIME-Version: 1.0 Date: Wed, 17 Jun 2020 17:25:11 +0200 From: Stefan Agner To: Laurent Pinchart Subject: Re: [PATCH v2 02/22] drm: mxsfb: Use drm_panel_bridge In-Reply-To: <20200530031015.15492-3-laurent.pinchart@ideasonboard.com> References: <20200530031015.15492-1-laurent.pinchart@ideasonboard.com> <20200530031015.15492-3-laurent.pinchart@ideasonboard.com> User-Agent: Roundcube Webmail/1.4.1 Message-ID: X-Sender: stefan@agner.ch X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Marek Vasut , dri-devel@lists.freedesktop.org, linux-imx@nxp.com, kernel@pengutronix.de, robert.chiras@nxp.com, leonard.crestez@nxp.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2020-05-30 05:09, Laurent Pinchart wrote: > Replace the manual connector implementation based on drm_panel with the > drm_panel_bridge helper. This simplifies the mxsfb driver by removing > connector-related code, and standardizing all pipeline control > operations on bridges. > > A hack is needed to get hold of the connector, as that's our only source > of bus flags and formats for now. As soon as the bridge API provides us > with that information this can be fixed. > > Signed-off-by: Laurent Pinchart As discussed in the late discussion on the v1 thread, we should add a default type to avoid warnings for some panels. Other than that, looks good to me: Reviewed-by: Stefan Agner > --- > Changes since v1: > > - Select DRM_PANEL_BRIDGE in Kconfig > --- > drivers/gpu/drm/mxsfb/Kconfig | 1 + > drivers/gpu/drm/mxsfb/Makefile | 2 +- > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 105 ++++++++++++++---------------- > drivers/gpu/drm/mxsfb/mxsfb_drv.h | 5 +- > drivers/gpu/drm/mxsfb/mxsfb_out.c | 99 ---------------------------- > 5 files changed, 54 insertions(+), 158 deletions(-) > delete mode 100644 drivers/gpu/drm/mxsfb/mxsfb_out.c > > diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig > index 0dca8f27169e..e43b326e9147 100644 > --- a/drivers/gpu/drm/mxsfb/Kconfig > +++ b/drivers/gpu/drm/mxsfb/Kconfig > @@ -13,6 +13,7 @@ config DRM_MXSFB > select DRM_KMS_FB_HELPER > select DRM_KMS_CMA_HELPER > select DRM_PANEL > + select DRM_PANEL_BRIDGE > help > Choose this option if you have an i.MX23/i.MX28/i.MX6SX MXSFB > LCD controller. > diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile > index ff6e358088fa..811584e54ad1 100644 > --- a/drivers/gpu/drm/mxsfb/Makefile > +++ b/drivers/gpu/drm/mxsfb/Makefile > @@ -1,3 +1,3 @@ > # SPDX-License-Identifier: GPL-2.0-only > -mxsfb-y := mxsfb_drv.o mxsfb_crtc.o mxsfb_out.o > +mxsfb-y := mxsfb_drv.o mxsfb_crtc.o > obj-$(CONFIG_DRM_MXSFB) += mxsfb.o > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > index 2e6068d96034..cffc70257bd3 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > @@ -29,7 +29,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -100,29 +99,11 @@ static void mxsfb_pipe_enable(struct > drm_simple_display_pipe *pipe, > struct drm_crtc_state *crtc_state, > struct drm_plane_state *plane_state) > { > - struct drm_connector *connector; > struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe); > struct drm_device *drm = pipe->plane.dev; > > - if (!mxsfb->connector) { > - list_for_each_entry(connector, > - &drm->mode_config.connector_list, > - head) > - if (connector->encoder == &mxsfb->pipe.encoder) { > - mxsfb->connector = connector; > - break; > - } > - } > - > - if (!mxsfb->connector) { > - dev_warn(drm->dev, "No connector attached, using default\n"); > - mxsfb->connector = &mxsfb->panel_connector; > - } > - > pm_runtime_get_sync(drm->dev); > - drm_panel_prepare(mxsfb->panel); > mxsfb_crtc_enable(mxsfb); > - drm_panel_enable(mxsfb->panel); > } > > static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe) > @@ -132,9 +113,7 @@ static void mxsfb_pipe_disable(struct > drm_simple_display_pipe *pipe) > struct drm_crtc *crtc = &pipe->crtc; > struct drm_pending_vblank_event *event; > > - drm_panel_disable(mxsfb->panel); > mxsfb_crtc_disable(mxsfb); > - drm_panel_unprepare(mxsfb->panel); > pm_runtime_put_sync(drm->dev); > > spin_lock_irq(&drm->event_lock); > @@ -144,9 +123,6 @@ static void mxsfb_pipe_disable(struct > drm_simple_display_pipe *pipe) > drm_crtc_send_vblank_event(crtc, event); > } > spin_unlock_irq(&drm->event_lock); > - > - if (mxsfb->connector != &mxsfb->panel_connector) > - mxsfb->connector = NULL; > } > > static void mxsfb_pipe_update(struct drm_simple_display_pipe *pipe, > @@ -190,6 +166,48 @@ static struct drm_simple_display_pipe_funcs mxsfb_funcs = { > .disable_vblank = mxsfb_pipe_disable_vblank, > }; > > +static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb) > +{ > + struct drm_device *drm = mxsfb->drm; > + struct drm_connector_list_iter iter; > + struct drm_panel *panel; > + struct drm_bridge *bridge; > + int ret; > + > + ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, &panel, > + &bridge); > + if (ret) > + return ret; > + > + if (panel) { > + bridge = devm_drm_panel_bridge_add(drm->dev, panel); > + if (IS_ERR(bridge)) > + return PTR_ERR(bridge); > + } > + > + if (!bridge) > + return -ENODEV; > + > + ret = drm_simple_display_pipe_attach_bridge(&mxsfb->pipe, bridge); > + if (ret) { > + DRM_DEV_ERROR(drm->dev, > + "failed to attach bridge: %d\n", ret); > + return ret; > + } > + > + mxsfb->bridge = bridge; > + > + /* > + * Get hold of the connector. This is a bit of a hack, until the bridge > + * API gives us bus flags and formats. > + */ > + drm_connector_list_iter_begin(drm, &iter); > + mxsfb->connector = drm_connector_list_iter_next(&iter); > + drm_connector_list_iter_end(&iter); > + > + return 0; > +} > + > static int mxsfb_load(struct drm_device *drm, unsigned long flags) > { > struct platform_device *pdev = to_platform_device(drm->dev); > @@ -201,6 +219,7 @@ static int mxsfb_load(struct drm_device *drm, > unsigned long flags) > if (!mxsfb) > return -ENOMEM; > > + mxsfb->drm = drm; > drm->dev_private = mxsfb; > mxsfb->devdata = &mxsfb_devdata[pdev->id_entry->driver_data]; > > @@ -236,41 +255,17 @@ static int mxsfb_load(struct drm_device *drm, > unsigned long flags) > /* Modeset init */ > drm_mode_config_init(drm); > > - ret = mxsfb_create_output(drm); > - if (ret < 0) { > - dev_err(drm->dev, "Failed to create outputs\n"); > - goto err_vblank; > - } > - > ret = drm_simple_display_pipe_init(drm, &mxsfb->pipe, &mxsfb_funcs, > - mxsfb_formats, ARRAY_SIZE(mxsfb_formats), NULL, > - mxsfb->connector); > + mxsfb_formats, ARRAY_SIZE(mxsfb_formats), NULL, NULL); > if (ret < 0) { > dev_err(drm->dev, "Cannot setup simple display pipe\n"); > goto err_vblank; > } > > - /* > - * Attach panel only if there is one. > - * If there is no panel attach, it must be a bridge. In this case, we > - * need a reference to its connector for a proper initialization. > - * We will do this check in pipe->enable(), since the connector won't > - * be attached to an encoder until then. > - */ > - > - if (mxsfb->panel) { > - ret = drm_panel_attach(mxsfb->panel, mxsfb->connector); > - if (ret) { > - dev_err(drm->dev, "Cannot connect panel: %d\n", ret); > - goto err_vblank; > - } > - } else if (mxsfb->bridge) { > - ret = drm_simple_display_pipe_attach_bridge(&mxsfb->pipe, > - mxsfb->bridge); > - if (ret) { > - dev_err(drm->dev, "Cannot connect bridge: %d\n", ret); > - goto err_vblank; > - } > + ret = mxsfb_attach_bridge(mxsfb); > + if (ret) { > + dev_err(drm->dev, "Cannot connect bridge: %d\n", ret); > + goto err_vblank; > } > > drm->mode_config.min_width = MXSFB_MIN_XRES; > @@ -288,7 +283,7 @@ static int mxsfb_load(struct drm_device *drm, > unsigned long flags) > > if (ret < 0) { > dev_err(drm->dev, "Failed to install IRQ handler\n"); > - goto err_irq; > + goto err_vblank; > } > > drm_kms_helper_poll_init(drm); > @@ -299,8 +294,6 @@ static int mxsfb_load(struct drm_device *drm, > unsigned long flags) > > return 0; > > -err_irq: > - drm_panel_detach(mxsfb->panel); > err_vblank: > pm_runtime_disable(drm->dev); > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h > b/drivers/gpu/drm/mxsfb/mxsfb_drv.h > index 0b65b5194a9c..0e3e5a63bbf9 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h > @@ -8,6 +8,8 @@ > #ifndef __MXSFB_DRV_H__ > #define __MXSFB_DRV_H__ > > +struct drm_device; > + > struct mxsfb_devdata { > unsigned int transfer_count; > unsigned int cur_buf; > @@ -26,10 +28,9 @@ struct mxsfb_drm_private { > struct clk *clk_axi; > struct clk *clk_disp_axi; > > + struct drm_device *drm; > struct drm_simple_display_pipe pipe; > - struct drm_connector panel_connector; > struct drm_connector *connector; > - struct drm_panel *panel; > struct drm_bridge *bridge; > }; > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_out.c > b/drivers/gpu/drm/mxsfb/mxsfb_out.c > deleted file mode 100644 > index 9eca1605d11d..000000000000 > --- a/drivers/gpu/drm/mxsfb/mxsfb_out.c > +++ /dev/null > @@ -1,99 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0-or-later > -/* > - * Copyright (C) 2016 Marek Vasut > - */ > - > -#include > - > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > - > -#include "mxsfb_drv.h" > - > -static struct mxsfb_drm_private * > -drm_connector_to_mxsfb_drm_private(struct drm_connector *connector) > -{ > - return container_of(connector, struct mxsfb_drm_private, > - panel_connector); > -} > - > -static int mxsfb_panel_get_modes(struct drm_connector *connector) > -{ > - struct mxsfb_drm_private *mxsfb = > - drm_connector_to_mxsfb_drm_private(connector); > - > - if (mxsfb->panel) > - return drm_panel_get_modes(mxsfb->panel, connector); > - > - return 0; > -} > - > -static const struct > -drm_connector_helper_funcs mxsfb_panel_connector_helper_funcs = { > - .get_modes = mxsfb_panel_get_modes, > -}; > - > -static enum drm_connector_status > -mxsfb_panel_connector_detect(struct drm_connector *connector, bool force) > -{ > - struct mxsfb_drm_private *mxsfb = > - drm_connector_to_mxsfb_drm_private(connector); > - > - if (mxsfb->panel) > - return connector_status_connected; > - > - return connector_status_disconnected; > -} > - > -static void mxsfb_panel_connector_destroy(struct drm_connector *connector) > -{ > - struct mxsfb_drm_private *mxsfb = > - drm_connector_to_mxsfb_drm_private(connector); > - > - if (mxsfb->panel) > - drm_panel_detach(mxsfb->panel); > - > - drm_connector_unregister(connector); > - drm_connector_cleanup(connector); > -} > - > -static const struct drm_connector_funcs mxsfb_panel_connector_funcs = { > - .detect = mxsfb_panel_connector_detect, > - .fill_modes = drm_helper_probe_single_connector_modes, > - .destroy = mxsfb_panel_connector_destroy, > - .reset = drm_atomic_helper_connector_reset, > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > -}; > - > -int mxsfb_create_output(struct drm_device *drm) > -{ > - struct mxsfb_drm_private *mxsfb = drm->dev_private; > - int ret; > - > - ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, > - &mxsfb->panel, &mxsfb->bridge); > - if (ret) > - return ret; > - > - if (mxsfb->panel) { > - mxsfb->connector = &mxsfb->panel_connector; > - mxsfb->connector->dpms = DRM_MODE_DPMS_OFF; > - mxsfb->connector->polled = 0; > - drm_connector_helper_add(mxsfb->connector, > - &mxsfb_panel_connector_helper_funcs); > - ret = drm_connector_init(drm, mxsfb->connector, > - &mxsfb_panel_connector_funcs, > - DRM_MODE_CONNECTOR_Unknown); > - } > - > - return ret; > -} _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel