Hi,

On Tue, Dec 08, 2020 at 02:28:55PM +0200, Tomi Valkeinen wrote:
> Panel drivers can send DSI commands in panel's prepare(), which happens
> before the bridge's enable() is called. The OMAP DSI driver currently
> only sets up the DSI interface at bridge's enable(), so prepare() cannot
> be used to send DSI commands.
> 
> This patch fixes the issue by making it possible to enable the DSI
> interface any time a command is about to be sent. Disabling the
> interface is be done via delayed work.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkei...@ti.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reic...@collabora.com>

Would be nice to include the information why locking is ok from your
reply mails to the patch description. It was helpful for reviewing
the patch :)

-- Sebastian

>  drivers/gpu/drm/omapdrm/dss/dsi.c | 49 +++++++++++++++++++++++++++----
>  drivers/gpu/drm/omapdrm/dss/dsi.h |  3 ++
>  2 files changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
> b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 53a64bc91867..34f665aa9a59 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -3503,6 +3503,9 @@ static void dsi_enable(struct dsi_data *dsi)
>  
>       WARN_ON(!dsi_bus_is_locked(dsi));
>  
> +     if (WARN_ON(dsi->iface_enabled))
> +             return;
> +
>       mutex_lock(&dsi->lock);
>  
>       r = dsi_runtime_get(dsi);
> @@ -3515,6 +3518,8 @@ static void dsi_enable(struct dsi_data *dsi)
>       if (r)
>               goto err_init_dsi;
>  
> +     dsi->iface_enabled = true;
> +
>       mutex_unlock(&dsi->lock);
>  
>       return;
> @@ -3530,6 +3535,9 @@ static void dsi_disable(struct dsi_data *dsi)
>  {
>       WARN_ON(!dsi_bus_is_locked(dsi));
>  
> +     if (WARN_ON(!dsi->iface_enabled))
> +             return;
> +
>       mutex_lock(&dsi->lock);
>  
>       dsi_sync_vc(dsi, 0);
> @@ -3541,6 +3549,8 @@ static void dsi_disable(struct dsi_data *dsi)
>  
>       dsi_runtime_put(dsi);
>  
> +     dsi->iface_enabled = false;
> +
>       mutex_unlock(&dsi->lock);
>  }
>  
> @@ -4229,10 +4239,12 @@ static ssize_t omap_dsi_host_transfer(struct 
> mipi_dsi_host *host,
>  
>       dsi_bus_lock(dsi);
>  
> -     if (dsi->video_enabled)
> -             r = _omap_dsi_host_transfer(dsi, vc, msg);
> -     else
> -             r = -EIO;
> +     if (!dsi->iface_enabled) {
> +             dsi_enable(dsi);
> +             schedule_delayed_work(&dsi->dsi_disable_work, 
> msecs_to_jiffies(2000));
> +     }
> +
> +     r = _omap_dsi_host_transfer(dsi, vc, msg);
>  
>       dsi_bus_unlock(dsi);
>  
> @@ -4397,6 +4409,14 @@ static int omap_dsi_host_detach(struct mipi_dsi_host 
> *host,
>       if (WARN_ON(dsi->dsidev != client))
>               return -EINVAL;
>  
> +     cancel_delayed_work_sync(&dsi->dsi_disable_work);
> +
> +     if (dsi->iface_enabled) {
> +             dsi_bus_lock(dsi);
> +             dsi_disable(dsi);
> +             dsi_bus_unlock(dsi);
> +     }
> +
>       omap_dsi_unregister_te_irq(dsi);
>       dsi->dsidev = NULL;
>       return 0;
> @@ -4632,9 +4652,12 @@ static void dsi_bridge_enable(struct drm_bridge 
> *bridge)
>       struct dsi_data *dsi = drm_bridge_to_dsi(bridge);
>       struct omap_dss_device *dssdev = &dsi->output;
>  
> +     cancel_delayed_work_sync(&dsi->dsi_disable_work);
> +
>       dsi_bus_lock(dsi);
>  
> -     dsi_enable(dsi);
> +     if (!dsi->iface_enabled)
> +             dsi_enable(dsi);
>  
>       dsi_enable_video_output(dssdev, VC_VIDEO);
>  
> @@ -4648,6 +4671,8 @@ static void dsi_bridge_disable(struct drm_bridge 
> *bridge)
>       struct dsi_data *dsi = drm_bridge_to_dsi(bridge);
>       struct omap_dss_device *dssdev = &dsi->output;
>  
> +     cancel_delayed_work_sync(&dsi->dsi_disable_work);
> +
>       dsi_bus_lock(dsi);
>  
>       dsi->video_enabled = false;
> @@ -4840,6 +4865,18 @@ static const struct soc_device_attribute 
> dsi_soc_devices[] = {
>       { /* sentinel */ }
>  };
>  
> +static void omap_dsi_disable_work_callback(struct work_struct *work)
> +{
> +     struct dsi_data *dsi = container_of(work, struct dsi_data, 
> dsi_disable_work.work);
> +
> +     dsi_bus_lock(dsi);
> +
> +     if (dsi->iface_enabled && !dsi->video_enabled)
> +             dsi_disable(dsi);
> +
> +     dsi_bus_unlock(dsi);
> +}
> +
>  static int dsi_probe(struct platform_device *pdev)
>  {
>       const struct soc_device_attribute *soc;
> @@ -4873,6 +4910,8 @@ static int dsi_probe(struct platform_device *pdev)
>       INIT_DEFERRABLE_WORK(&dsi->framedone_timeout_work,
>                            dsi_framedone_timeout_work_callback);
>  
> +     INIT_DEFERRABLE_WORK(&dsi->dsi_disable_work, 
> omap_dsi_disable_work_callback);
> +
>  #ifdef DSI_CATCH_MISSING_TE
>       timer_setup(&dsi->te_timer, dsi_te_timeout, 0);
>  #endif
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.h 
> b/drivers/gpu/drm/omapdrm/dss/dsi.h
> index de9411067ba2..601707c0ecc4 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.h
> @@ -394,6 +394,7 @@ struct dsi_data {
>       atomic_t do_ext_te_update;
>  
>       bool te_enabled;
> +     bool iface_enabled;
>       bool video_enabled;
>  
>       struct delayed_work framedone_timeout_work;
> @@ -443,6 +444,8 @@ struct dsi_data {
>  
>       struct omap_dss_device output;
>       struct drm_bridge bridge;
> +
> +     struct delayed_work dsi_disable_work;
>  };
>  
>  struct dsi_packet_sent_handler_data {
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

Attachment: signature.asc
Description: PGP signature

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to