From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752363AbdK3GrV (ORCPT ); Thu, 30 Nov 2017 01:47:21 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:33088 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752012AbdK3GrT (ORCPT ); Thu, 30 Nov 2017 01:47:19 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org B71C46020B Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=architt@codeaurora.org Subject: Re: [PATCH] drm/bridge: Fix lvds-encoder since the panel_bridge rework. To: =?UTF-8?Q?Lothar_Wa=c3=9fmann?= , Eric Anholt Cc: dri-devel@lists.freedesktop.org, Andrzej Hajda , Laurent Pinchart , linux-kernel@vger.kernel.org References: <20171114191647.22207-1-eric@anholt.net> <20171115105958.4941720c@karo-electronics.de> From: Archit Taneja Message-ID: <1e94d083-b688-912e-1f11-533ada81408e@codeaurora.org> Date: Thu, 30 Nov 2017 12:17:13 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171115105958.4941720c@karo-electronics.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/15/2017 03:29 PM, Lothar Waßmann wrote: > Hi, > > On Tue, 14 Nov 2017 11:16:47 -0800 Eric Anholt wrote: >> The panel_bridge bridge attaches to the panel's OF node, not the >> lvds-encoder's node. Put in a little no-op bridge of our own so that >> our consumers can still find a bridge where they expect. >> >> This also fixes an unintended unregistration and leak of the >> panel-bridge on module remove. >> >> Signed-off-by: Eric Anholt >> Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bri >> dge.") >> --- >> >> Note: I haven't actually tested this patch! Hope it helps, though. >> >> drivers/gpu/drm/bridge/lvds-encoder.c | 48 ++++++++++++++++++++++++++++++----- >> 1 file changed, 41 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c >> index 0903ba574f61..75b0d3f6e4de 100644 >> --- a/drivers/gpu/drm/bridge/lvds-encoder.c >> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c >> @@ -13,13 +13,37 @@ >> >> #include >> >> +struct lvds_encoder { >> + struct drm_bridge bridge; >> + struct drm_bridge *panel_bridge; >> +}; >> + >> +static int lvds_encoder_attach(struct drm_bridge *bridge) >> +{ >> + struct lvds_encoder *lvds_encoder = container_of(bridge, >> + struct lvds_encoder, >> + bridge); >> + >> + return drm_bridge_attach(bridge->encoder, lvds_encoder->panel_bridge, >> + bridge); >> +} >> + >> +static struct drm_bridge_funcs funcs = { >> + .attach = lvds_encoder_attach, >> +}; >> + >> static int lvds_encoder_probe(struct platform_device *pdev) >> { >> struct device_node *port; >> struct device_node *endpoint; >> struct device_node *panel_node; >> struct drm_panel *panel; >> - struct drm_bridge *bridge; >> + struct lvds_encoder *lvds_encoder; >> + >> + lvds_encoder = devm_kzalloc(&pdev->dev, sizeof(*lvds_encoder), >> + GFP_KERNEL); >> + if (!lvds_encoder) >> + return -ENOMEM; >> >> /* Locate the panel DT node. */ >> port = of_graph_get_port_by_id(pdev->dev.of_node, 1); >> @@ -49,20 +73,30 @@ static int lvds_encoder_probe(struct platform_device *pdev) >> return -EPROBE_DEFER; >> } >> >> - bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS); >> - if (IS_ERR(bridge)) >> - return PTR_ERR(bridge); >> + lvds_encoder->panel_bridge = >> + devm_drm_panel_bridge_add(&pdev->dev, >> + panel, DRM_MODE_CONNECTOR_LVDS); >> + if (IS_ERR(lvds_encoder->panel_bridge)) >> + return PTR_ERR(lvds_encoder->panel_bridge); >> + >> + /* The panel_bridge bridge is attached to the panel's of_node, >> + * but we need a bridge attached to our of_node for our user >> + * to look up. >> + */ >> + lvds_encoder->bridge.of_node = pdev->dev.of_node; >> + lvds_encoder->bridge.funcs = &funcs; >> + drm_bridge_add(&lvds_encoder->bridge); >> >> - platform_set_drvdata(pdev, bridge); >> + platform_set_drvdata(pdev, lvds_encoder); >> >> return 0; >> } >> >> static int lvds_encoder_remove(struct platform_device *pdev) >> { >> - struct drm_bridge *bridge = platform_get_drvdata(pdev); >> + struct lvds_encoder *lvds_encoder = platform_get_drvdata(pdev); >> >> - drm_bridge_remove(bridge); >> + drm_bridge_remove(&lvds_encoder->bridge); >> >> return 0; >> } >> > Tested-by: Lothar Waßmann queued to drm-misc-fixes. I think we should send this patch for the stable kernels (v4.12+) too. Thanks, Archit > > > Lothar Waßmann > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project