Re: [PATCH v2 56/60] drm/omap: Store CRTC lookup by channel table in omap_drm_private

[Sebastian Reichel]

Hi,

On Sat, May 26, 2018 at 08:25:14PM +0300, Laurent Pinchart wrote:
> The omap_crtcs global array is used to store pointers to omap_crtc
> indexed by DISPC channel number, in order to look them up in the dss_mgr
> operations. Store the information in the omap_drm_private structure in
> the form of an array of omap_drm_pipeline pointers.
> 
> Signed-off-by: Laurent Pinchart 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/gpu/drm/omapdrm/omap_crtc.c | 22 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c  | 19 +++
>  drivers/gpu/drm/omapdrm/omap_drv.h  |  1 +
>  3 files changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c 
> b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index f5bdb8de98f4..9742d9f49a7c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -109,7 +109,6 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
>   */
>  
>  /* ovl-mgr-id -> crtc */
> -static struct omap_crtc *omap_crtcs[8];
>  static struct omap_dss_device *omap_crtc_output[8];
>  
>  /* we can probably ignore these until we support command-mode panels: */
> @@ -215,7 +214,8 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, 
> bool enable)
>  static int omap_crtc_dss_enable(struct omap_drm_private *priv,
>   enum omap_channel channel)
>  {
> - struct omap_crtc *omap_crtc = omap_crtcs[channel];
> + struct drm_crtc *crtc = priv->channels[channel]->crtc;
> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>  
>   priv->dispc_ops->mgr_set_timings(priv->dispc, omap_crtc->channel,
>_crtc->vm);
> @@ -227,7 +227,8 @@ static int omap_crtc_dss_enable(struct omap_drm_private 
> *priv,
>  static void omap_crtc_dss_disable(struct omap_drm_private *priv,
> enum omap_channel channel)
>  {
> - struct omap_crtc *omap_crtc = omap_crtcs[channel];
> + struct drm_crtc *crtc = priv->channels[channel]->crtc;
> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>  
>   omap_crtc_set_enabled(_crtc->base, false);
>  }
> @@ -236,7 +237,9 @@ static void omap_crtc_dss_set_timings(struct 
> omap_drm_private *priv,
>   enum omap_channel channel,
>   const struct videomode *vm)
>  {
> - struct omap_crtc *omap_crtc = omap_crtcs[channel];
> + struct drm_crtc *crtc = priv->channels[channel]->crtc;
> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +
>   DBG("%s", omap_crtc->name);
>   omap_crtc->vm = *vm;
>  }
> @@ -245,7 +248,8 @@ static void omap_crtc_dss_set_lcd_config(struct 
> omap_drm_private *priv,
>   enum omap_channel channel,
>   const struct dss_lcd_mgr_config *config)
>  {
> - struct omap_crtc *omap_crtc = omap_crtcs[channel];
> + struct drm_crtc *crtc = priv->channels[channel]->crtc;
> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>  
>   DBG("%s", omap_crtc->name);
>   priv->dispc_ops->mgr_set_lcd_config(priv->dispc, omap_crtc->channel,
> @@ -681,8 +685,6 @@ static const char *channel_names[] = {
>  
>  void omap_crtc_pre_init(struct omap_drm_private *priv)
>  {
> - memset(omap_crtcs, 0, sizeof(omap_crtcs));
> -
>   dss_install_mgr_ops(priv->dss, _ops, priv);
>  }
>  
> @@ -706,10 +708,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>  
>   DBG("%s", channel_names[channel]);
>  
> - /* Multiple displays on same channel is not allowed */
> - if (WARN_ON(omap_crtcs[channel] != NULL))
> - return ERR_PTR(-EINVAL);
> -
>   omap_crtc = kzalloc(sizeof(*omap_crtc), GFP_KERNEL);
>   if (!omap_crtc)
>   return ERR_PTR(-ENOMEM);
> @@ -748,7 +746,5 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>  
>   omap_plane_install_properties(crtc->primary, >base);
>  
> - omap_crtcs[channel] = omap_crtc;
> -
>   return crtc;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
> b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 6bc826515684..c3c657d90029 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -167,6 +167,8 @@ static void omap_disconnect_pipelines(struct drm_device 
> *ddev)
>   pipe->display = NULL;
>   }
>  
> + memset(>channels, 0, sizeof(priv->channels));
> +
>   priv->num_pipes = 0;
>  }
>  
> @@ -186,6 +188,7 @@ static int omap_connect_pipelines(struct drm_device *ddev)
>  {
>   struct omap_drm_private *priv = ddev->dev_private;
>   struct omap_dss_device *output = NULL;
> + unsigned int i;
>   int r;
>  
>   if (!omapdss_stack_is_ready())
> @@ -218,6 +221,22 @@ static int omap_connect_pipelines(struct drm_device 
> *ddev)
>   sort(priv->pipes, priv->num_pipes, sizeof(priv->pipes[0]),
>omap_compare_pipes, NULL);
>  
> + /*
> +  * Populate the pipeline lookup table by DISPC channel. Only 

[PATCH v2 56/60] drm/omap: Store CRTC lookup by channel table in omap_drm_private

[Laurent Pinchart]

The omap_crtcs global array is used to store pointers to omap_crtc
indexed by DISPC channel number, in order to look them up in the dss_mgr
operations. Store the information in the omap_drm_private structure in
the form of an array of omap_drm_pipeline pointers.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 22 +-
 drivers/gpu/drm/omapdrm/omap_drv.c  | 19 +++
 drivers/gpu/drm/omapdrm/omap_drv.h  |  1 +
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c 
b/drivers/gpu/drm/omapdrm/omap_crtc.c
index f5bdb8de98f4..9742d9f49a7c 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -109,7 +109,6 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
  */
 
 /* ovl-mgr-id -> crtc */
-static struct omap_crtc *omap_crtcs[8];
 static struct omap_dss_device *omap_crtc_output[8];
 
 /* we can probably ignore these until we support command-mode panels: */
@@ -215,7 +214,8 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, 
bool enable)
 static int omap_crtc_dss_enable(struct omap_drm_private *priv,
enum omap_channel channel)
 {
-   struct omap_crtc *omap_crtc = omap_crtcs[channel];
+   struct drm_crtc *crtc = priv->channels[channel]->crtc;
+   struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 
priv->dispc_ops->mgr_set_timings(priv->dispc, omap_crtc->channel,
 _crtc->vm);
@@ -227,7 +227,8 @@ static int omap_crtc_dss_enable(struct omap_drm_private 
*priv,
 static void omap_crtc_dss_disable(struct omap_drm_private *priv,
  enum omap_channel channel)
 {
-   struct omap_crtc *omap_crtc = omap_crtcs[channel];
+   struct drm_crtc *crtc = priv->channels[channel]->crtc;
+   struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 
omap_crtc_set_enabled(_crtc->base, false);
 }
@@ -236,7 +237,9 @@ static void omap_crtc_dss_set_timings(struct 
omap_drm_private *priv,
enum omap_channel channel,
const struct videomode *vm)
 {
-   struct omap_crtc *omap_crtc = omap_crtcs[channel];
+   struct drm_crtc *crtc = priv->channels[channel]->crtc;
+   struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
DBG("%s", omap_crtc->name);
omap_crtc->vm = *vm;
 }
@@ -245,7 +248,8 @@ static void omap_crtc_dss_set_lcd_config(struct 
omap_drm_private *priv,
enum omap_channel channel,
const struct dss_lcd_mgr_config *config)
 {
-   struct omap_crtc *omap_crtc = omap_crtcs[channel];
+   struct drm_crtc *crtc = priv->channels[channel]->crtc;
+   struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 
DBG("%s", omap_crtc->name);
priv->dispc_ops->mgr_set_lcd_config(priv->dispc, omap_crtc->channel,
@@ -681,8 +685,6 @@ static const char *channel_names[] = {
 
 void omap_crtc_pre_init(struct omap_drm_private *priv)
 {
-   memset(omap_crtcs, 0, sizeof(omap_crtcs));
-
dss_install_mgr_ops(priv->dss, _ops, priv);
 }
 
@@ -706,10 +708,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
DBG("%s", channel_names[channel]);
 
-   /* Multiple displays on same channel is not allowed */
-   if (WARN_ON(omap_crtcs[channel] != NULL))
-   return ERR_PTR(-EINVAL);
-
omap_crtc = kzalloc(sizeof(*omap_crtc), GFP_KERNEL);
if (!omap_crtc)
return ERR_PTR(-ENOMEM);
@@ -748,7 +746,5 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
omap_plane_install_properties(crtc->primary, >base);
 
-   omap_crtcs[channel] = omap_crtc;
-
return crtc;
 }
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
b/drivers/gpu/drm/omapdrm/omap_drv.c
index 6bc826515684..c3c657d90029 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -167,6 +167,8 @@ static void omap_disconnect_pipelines(struct drm_device 
*ddev)
pipe->display = NULL;
}
 
+   memset(>channels, 0, sizeof(priv->channels));
+
priv->num_pipes = 0;
 }
 
@@ -186,6 +188,7 @@ static int omap_connect_pipelines(struct drm_device *ddev)
 {
struct omap_drm_private *priv = ddev->dev_private;
struct omap_dss_device *output = NULL;
+   unsigned int i;
int r;
 
if (!omapdss_stack_is_ready())
@@ -218,6 +221,22 @@ static int omap_connect_pipelines(struct drm_device *ddev)
sort(priv->pipes, priv->num_pipes, sizeof(priv->pipes[0]),
 omap_compare_pipes, NULL);
 
+   /*
+* Populate the pipeline lookup table by DISPC channel. Only one display
+* is allowed per channel.
+*/
+   for (i = 0; i < priv->num_pipes; ++i) {
+   struct omap_drm_pipeline *pipe = >pipes[i];
+   enum omap_channel channel = pipe->output->dispc_channel;
+
+