Hi Tomi and Sebastian,
Thank you for the patch.
On Thu, Nov 05, 2020 at 02:03:08PM +0200, Tomi Valkeinen wrote:
> From: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
>
> Complete the direction reversal of the DSS device enable/disable
> operations started by 19b4200d8f4b ("drm/omap: Reverse direction
> of the DSS device enable/disable operations").
>
> This effectively drops the requirement of calling DSS specific
> code from the DSI panel driver moving it a bit further to a
> standard drm_panel driver.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> ---
> .../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 74 +++++++++----------
> drivers/gpu/drm/omapdrm/omap_encoder.c | 24 ++----
> 2 files changed, 45 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> index 1582960f9e90..45d417870498 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> @@ -288,27 +288,6 @@ static int dsicm_power_on(struct panel_drv_data *ddata)
> struct omap_dss_device *src = ddata->src;
> u8 id1, id2, id3;
> int r;
> - struct omap_dss_dsi_config dsi_config = {
> - .vm = &ddata->vm,
> - .hs_clk_min = 150000000,
> - .hs_clk_max = 300000000,
> - .lp_clk_min = 7000000,
> - .lp_clk_max = 10000000,
> - };
> -
> - r = regulator_bulk_enable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
> - if (r) {
> - dev_err(&ddata->dsi->dev, "failed to enable supplies: %d\n", r);
> - return r;
> - }
> -
> - r = src->ops->dsi.set_config(src, &dsi_config);
> - if (r) {
> - dev_err(&ddata->dsi->dev, "failed to configure DSI\n");
> - goto err_regulators;
> - }
> -
> - src->ops->enable(src);
>
> dsicm_hw_reset(ddata);
>
> @@ -363,12 +342,6 @@ static int dsicm_power_on(struct panel_drv_data *ddata)
>
> dsicm_hw_reset(ddata);
>
> - src->ops->disable(src);
> -err_regulators:
> - r = regulator_bulk_disable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
> - if (r)
> - dev_err(&ddata->dsi->dev, "failed to disable supplies: %d\n", r);
> -
> return r;
> }
>
> @@ -377,6 +350,8 @@ static void dsicm_power_off(struct panel_drv_data *ddata)
> struct omap_dss_device *src = ddata->src;
> int r;
>
> + ddata->enabled = false;
> +
> src->ops->dsi.disable_video_output(src, ddata->dsi->channel);
>
> r = mipi_dsi_dcs_set_display_off(ddata->dsi);
> @@ -388,14 +363,6 @@ static void dsicm_power_off(struct panel_drv_data *ddata)
> "error disabling panel, issuing HW reset\n");
> dsicm_hw_reset(ddata);
> }
> -
> - src->ops->disable(src);
> -
> - r = regulator_bulk_disable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
> - if (r)
> - dev_err(&ddata->dsi->dev, "failed to disable supplies: %d\n", r);
> -
> - ddata->enabled = false;
> }
>
> static int dsicm_connect(struct omap_dss_device *src,
> @@ -415,6 +382,29 @@ static void dsicm_disconnect(struct omap_dss_device *src,
> ddata->src = NULL;
> }
>
> +static void dsicm_pre_enable(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + struct omap_dss_device *src = ddata->src;
> + int r;
> + struct omap_dss_dsi_config dsi_config = {
> + .vm = &ddata->vm,
> + .hs_clk_min = 150000000,
> + .hs_clk_max = 300000000,
> + .lp_clk_min = 7000000,
> + .lp_clk_max = 10000000,
> + };
> +
> + r = regulator_bulk_enable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
> + if (r)
> + dev_err(&ddata->dsi->dev, "failed to enable supplies: %d\n", r);
> +
> + r = src->ops->dsi.set_config(src, &dsi_config);
> + if (r) {
> + dev_err(&ddata->dsi->dev, "failed to configure DSI\n");
> + }
> +}
> +
> static void dsicm_enable(struct omap_dss_device *dssdev)
> {
> struct panel_drv_data *ddata = to_panel_data(dssdev);
> @@ -449,6 +439,16 @@ static void dsicm_disable(struct omap_dss_device *dssdev)
> mutex_unlock(&ddata->lock);
> }
>
> +static void dsicm_post_disable(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + int r;
> +
> + r = regulator_bulk_disable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
> + if (r)
> + dev_err(&ddata->dsi->dev, "failed to disable supplies: %d\n", r);
> +}
> +
> static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable)
> {
> struct mipi_dsi_device *dsi = ddata->dsi;
> @@ -502,8 +502,10 @@ static const struct omap_dss_device_ops dsicm_ops = {
> .connect = dsicm_connect,
> .disconnect = dsicm_disconnect,
>
> + .pre_enable = dsicm_pre_enable,
> .enable = dsicm_enable,
> .disable = dsicm_disable,
> + .post_disable = dsicm_post_disable,
>
> .get_modes = dsicm_get_modes,
> .check_timings = dsicm_check_timings,
> @@ -664,8 +666,6 @@ static int __exit dsicm_remove(struct mipi_dsi_device *dsi)
>
> omapdss_device_unregister(dssdev);
>
> - if (omapdss_device_is_enabled(dssdev))
> - dsicm_disable(dssdev);
> omapdss_device_disconnect(ddata->src, dssdev);
>
> sysfs_remove_group(&dsi->dev.kobj, &dsicm_attr_group);
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 18a79dde6815..10abe4d01b0b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -137,15 +137,11 @@ static void omap_encoder_disable(struct drm_encoder *encoder)
> omapdss_device_disable(dssdev->next);
>
> /*
> - * Disable the internal encoder. This will disable the DSS output. The
> - * DSI is treated as an exception as DSI pipelines still use the legacy
> - * flow where the pipeline output controls the encoder.
> + * Disable the internal encoder. This will disable the DSS output.
> */
You could write this
/* Disable the internal encoder. This will disable the DSS output. */
Same below.
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> - if (dssdev->type != OMAP_DISPLAY_TYPE_DSI) {
> - if (dssdev->ops && dssdev->ops->disable)
> - dssdev->ops->disable(dssdev);
> - dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
> - }
> + if (dssdev->ops && dssdev->ops->disable)
> + dssdev->ops->disable(dssdev);
> + dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>
> /*
> * Perform the post-disable operations on the chain of external devices
> @@ -166,15 +162,11 @@ static void omap_encoder_enable(struct drm_encoder *encoder)
> omapdss_device_pre_enable(dssdev->next);
>
> /*
> - * Enable the internal encoder. This will enable the DSS output. The
> - * DSI is treated as an exception as DSI pipelines still use the legacy
> - * flow where the pipeline output controls the encoder.
> + * Enable the internal encoder. This will enable the DSS output.
> */
> - if (dssdev->type != OMAP_DISPLAY_TYPE_DSI) {
> - if (dssdev->ops && dssdev->ops->enable)
> - dssdev->ops->enable(dssdev);
> - dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> - }
> + if (dssdev->ops && dssdev->ops->enable)
> + dssdev->ops->enable(dssdev);
> + dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
>
> /*
> * Enable the chain of external devices, starting at the one at the
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
