From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xinliang Liu Subject: Re: [PATCH v2 10/10] drm/hisilicon: Add support for external bridge Date: Tue, 1 Dec 2015 22:50:53 +0800 Message-ID: References: <1448707145-69348-1-git-send-email-xinliang.liu@linaro.org> <1448707145-69348-11-git-send-email-xinliang.liu@linaro.org> <565D62A3.2030301@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <565D62A3.2030301@codeaurora.org> Sender: linux-doc-owner@vger.kernel.org To: Archit Taneja Cc: dri-devel , devicetree@vger.kernel.org, Daniel Vetter , Rob Herring , Daniel Stone , David Airlie , Jon Corbet , Catalin Marinas , Will Deacon , linux-doc@vger.kernel.org, LAKML , linuxarm@huawei.com, Andy Green , Haojian Zhuang , "Liguozhu (Kenneth)" , Xu Wei , Wang Fei , Feng Chen , Bintian Wang , Benjamin Gaignard , Yiping Xu , XinWei Kong , zourongrong@huawei.com, lijianhua@huawei.com, Sumit Semwal List-Id: devicetree@vger.kernel.org On 1 December 2015 at 17:04, Archit Taneja wrote: > > > On 11/28/2015 04:09 PM, Xinliang Liu wrote: >> >> Add support for external HDMI bridge. >> >> Signed-off-by: Xinliang Liu >> Signed-off-by: Xinwei Kong >> Signed-off-by: Andy Green >> --- >> drivers/gpu/drm/hisilicon/hisi_drm_dsi.c | 51 >> ++++++++++++++++++++++++++++++++ >> 1 file changed, 51 insertions(+) >> >> diff --git a/drivers/gpu/drm/hisilicon/hisi_drm_dsi.c >> b/drivers/gpu/drm/hisilicon/hisi_drm_dsi.c >> index 066e08d..9e056db 100644 >> --- a/drivers/gpu/drm/hisilicon/hisi_drm_dsi.c >> +++ b/drivers/gpu/drm/hisilicon/hisi_drm_dsi.c >> @@ -78,6 +78,7 @@ struct dsi_hw_ctx { >> >> struct hisi_dsi { >> struct drm_encoder encoder; >> + struct drm_bridge *bridge; >> struct mipi_dsi_host host; >> struct drm_display_mode cur_mode; >> struct dsi_hw_ctx *ctx; >> @@ -671,6 +672,25 @@ static int dsi_host_init(struct device *dev, struct >> hisi_dsi *dsi) >> return 0; >> } >> >> +static int dsi_bridge_init(struct drm_device *dev, struct hisi_dsi *dsi) >> +{ >> + struct drm_encoder *encoder = &dsi->encoder; >> + struct drm_bridge *bridge = dsi->bridge; >> + int ret; >> + >> + /* associate the bridge to dsi encoder */ >> + encoder->bridge = bridge; >> + bridge->encoder = encoder; >> + >> + ret = drm_bridge_attach(dev, bridge); >> + if (ret) { >> + DRM_ERROR("failed to attach exteranl bridge\n"); > > > s/exteranl/external > will be fixed in v3. > >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> static int dsi_bind(struct device *dev, struct device *master, void >> *data) >> { >> struct dsi_data *ddata = dev_get_drvdata(dev); >> @@ -686,6 +706,10 @@ static int dsi_bind(struct device *dev, struct device >> *master, void *data) >> if (ret) >> return ret; >> >> + ret = dsi_bridge_init(drm_dev, dsi); >> + if (ret) >> + return ret; >> + >> return 0; >> } >> >> @@ -702,8 +726,35 @@ static const struct component_ops dsi_ops = { >> static int dsi_parse_dt(struct platform_device *pdev, struct hisi_dsi >> *dsi) >> { >> struct dsi_hw_ctx *ctx = dsi->ctx; >> + struct device_node *np = pdev->dev.of_node; >> + struct device_node *endpoint, *bridge_node; >> + struct drm_bridge *bridge; >> struct resource *res; >> >> + /* >> + * Get the endpoint node. In our case, dsi has one output port >> + * to which the external HDMI bridge is connected. >> + */ >> + endpoint = of_graph_get_next_endpoint(np, NULL); >> + if (!endpoint) { >> + DRM_ERROR("no valid endpoint node\n"); >> + return -ENODEV; >> + } >> + of_node_put(endpoint); >> + >> + bridge_node = of_graph_get_remote_port_parent(endpoint); >> + if (!bridge_node) { >> + DRM_ERROR("no valid bridge node\n"); >> + return -ENODEV; >> + } >> + of_node_put(bridge_node); >> + >> + bridge = of_drm_find_bridge(bridge_node); >> + if (!bridge) { >> + DRM_INFO("wait for external HDMI bridge driver.\n"); >> + return -EPROBE_DEFER; >> + } >> + dsi->bridge = bridge; > > > This could be left for later, but it would be better if the dsi driver > registers even if the bridge driver module isn't inserted yet, or > happens much later in boot. > > We could achieve this by trying to attach the bridge (done in > dsi_bridge_init) in the dsi_host_attach callback instead of having it > in dsi_bind. Do you mean that it is the right time or place to attach the bridge in dsi_host_attach callback. Why? Because at this time bridge driver must be register? Thanks, -xinliang > > Thanks, > Archit > >> >> ctx->dsi_cfg_clk = devm_clk_get(&pdev->dev, "pclk_dsi"); >> if (IS_ERR(ctx->dsi_cfg_clk)) { >> > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > hosted by The Linux Foundation