[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c6a911c9-64e4-46aa-a6da-3311907b1c8b@linaro.org>
Date: Mon, 8 Jul 2024 09:44:29 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Hironori KIKUCHI <kikuchan98@...il.com>, linux-kernel@...r.kernel.org
Cc: Jagan Teki <jagan@...rulasolutions.com>,
Jessica Zhang <quic_jesszhan@...cinc.com>, Sam Ravnborg <sam@...nborg.org>,
David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, dri-devel@...ts.freedesktop.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v3 2/5] drm/panel: st7701: Decouple DSI and DRM parts
On 06/07/2024 12:23, Hironori KIKUCHI wrote:
> Split into a DSI-specific part and a DRM-specific part.
>
> Additionally, use devm_add_action_or_reset() to simplify the flow,
> and disable and unprepare the panel on cleanup.
>
> Signed-off-by: Hironori KIKUCHI <kikuchan98@...il.com>
> ---
> drivers/gpu/drm/panel/panel-sitronix-st7701.c | 72 ++++++++++++-------
> 1 file changed, 45 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7701.c b/drivers/gpu/drm/panel/panel-sitronix-st7701.c
> index a9a8fd85057..a0644f7a4c8 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7701.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7701.c
> @@ -539,7 +539,7 @@ static int st7701_get_modes(struct drm_panel *panel,
>
> mode = drm_mode_duplicate(connector->dev, desc_mode);
> if (!mode) {
> - dev_err(&st7701->dsi->dev, "failed to add mode %ux%u@%u\n",
> + dev_err(panel->dev, "failed to add mode %ux%u@%u\n",
> desc_mode->hdisplay, desc_mode->vdisplay,
> drm_mode_vrefresh(desc_mode));
> return -ENOMEM;
> @@ -974,42 +974,48 @@ static const struct st7701_panel_desc rg_arc_desc = {
> .gip_sequence = rg_arc_gip_sequence,
> };
>
> -static int st7701_dsi_probe(struct mipi_dsi_device *dsi)
> +static void st7701_cleanup(void *data)
> +{
> + struct st7701 *st7701 = (struct st7701 *)data;
> +
> + drm_panel_remove(&st7701->panel);
> + drm_panel_disable(&st7701->panel);
> + drm_panel_unprepare(&st7701->panel);
> +}
> +
> +static int st7701_probe(struct device *dev, int connector_type)
> {
> const struct st7701_panel_desc *desc;
> struct st7701 *st7701;
> int ret;
>
> - st7701 = devm_kzalloc(&dsi->dev, sizeof(*st7701), GFP_KERNEL);
> + st7701 = devm_kzalloc(dev, sizeof(*st7701), GFP_KERNEL);
> if (!st7701)
> return -ENOMEM;
>
> - desc = of_device_get_match_data(&dsi->dev);
> - dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> - MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS;
> - dsi->format = desc->format;
> - dsi->lanes = desc->lanes;
> + desc = of_device_get_match_data(dev);
> + if (!desc)
> + return -ENODEV;
>
> st7701->supplies[0].supply = "VCC";
> st7701->supplies[1].supply = "IOVCC";
>
> - ret = devm_regulator_bulk_get(&dsi->dev, ARRAY_SIZE(st7701->supplies),
> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(st7701->supplies),
> st7701->supplies);
> if (ret < 0)
> return ret;
>
> - st7701->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
> + st7701->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> if (IS_ERR(st7701->reset)) {
> - dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
> + dev_err(dev, "Couldn't get our reset GPIO\n");
> return PTR_ERR(st7701->reset);
> }
>
> - ret = of_drm_get_panel_orientation(dsi->dev.of_node, &st7701->orientation);
> + ret = of_drm_get_panel_orientation(dev->of_node, &st7701->orientation);
> if (ret < 0)
> - return dev_err_probe(&dsi->dev, ret, "Failed to get orientation\n");
> + return dev_err_probe(dev, ret, "Failed to get orientation\n");
>
> - drm_panel_init(&st7701->panel, &dsi->dev, &st7701_funcs,
> - DRM_MODE_CONNECTOR_DSI);
> + drm_panel_init(&st7701->panel, dev, &st7701_funcs, connector_type);
>
> /**
> * Once sleep out has been issued, ST7701 IC required to wait 120ms
> @@ -1028,27 +1034,39 @@ static int st7701_dsi_probe(struct mipi_dsi_device *dsi)
>
> drm_panel_add(&st7701->panel);
>
> - mipi_dsi_set_drvdata(dsi, st7701);
> - st7701->dsi = dsi;
> + dev_set_drvdata(dev, st7701);
> st7701->desc = desc;
>
> - ret = mipi_dsi_attach(dsi);
> - if (ret)
> - goto err_attach;
> + return devm_add_action_or_reset(dev, st7701_cleanup, st7701);
> +}
> +
> +static int st7701_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> + struct st7701 *st7701;
> + int err;
> +
> + err = st7701_probe(&dsi->dev, DRM_MODE_CONNECTOR_DSI);
> + if (err)
> + return err;
> +
> + st7701 = dev_get_drvdata(&dsi->dev);
> + st7701->dsi = dsi;
> +
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> + MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS;
> + dsi->format = st7701->desc->format;
> + dsi->lanes = st7701->desc->lanes;
> +
> + err = mipi_dsi_attach(dsi);
> + if (err)
> + return dev_err_probe(&dsi->dev, err, "Failed to init MIPI DSI\n");
>
> return 0;
> -
> -err_attach:
> - drm_panel_remove(&st7701->panel);
> - return ret;
> }
>
> static void st7701_dsi_remove(struct mipi_dsi_device *dsi)
> {
> - struct st7701 *st7701 = mipi_dsi_get_drvdata(dsi);
> -
> mipi_dsi_detach(dsi);
> - drm_panel_remove(&st7701->panel);
> }
>
> static const struct of_device_id st7701_of_match[] = {
Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>
Powered by blists - more mailing lists