Hi Jyri, Laurent,

On 16/02/18 13:25, Jyri Sarha wrote:
> After this patch OMAP_DSS_BASE module is not including any OMAP2_DSS
> headers, only the API omapdss.h. "sturct dss_data", the piece of the
> data structure needed for base.c is defined in omapdss.h and added as
> a member to struct dss_device, and later to struct dss6_device.
> 
> The patch is still a bit hackish. The struct dispc_device declaration
> is currently shared between alternative dss implementations, with
> different internal definitions. It should be relatively simple to use
> a similar struct dispc_data as struct dss_data is for dss_device, move
> some common parts - maybe the dispc_ops itself - there and find the
> private data with container_of macro. Also the contents of struct
> dss_data in side dss_device is currently redundant. These should be
> easy enough to fix, if we decide to take this route.
> 
> Signed-off-by: Jyri Sarha <jsa...@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/base.c    | 11 +++++------
>  drivers/gpu/drm/omapdrm/dss/dispc.c   |  4 ++++
>  drivers/gpu/drm/omapdrm/dss/dss.c     |  2 +-
>  drivers/gpu/drm/omapdrm/dss/dss.h     |  2 ++
>  drivers/gpu/drm/omapdrm/dss/omapdss.h | 13 +++++++++----
>  drivers/gpu/drm/omapdrm/omap_drv.h    |  2 +-
>  6 files changed, 22 insertions(+), 12 deletions(-)

I think something in this direction would be good. I'd really like to keep it 
possible to run dss6 on top of omapdrm.

But I think this can be done slightly simpler like this:


diff --git a/drivers/gpu/drm/omapdrm/dss/base.c 
b/drivers/gpu/drm/omapdrm/dss/base.c
index 99e8cb8dc65b..08913e006e93 100644
--- a/drivers/gpu/drm/omapdrm/dss/base.c
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -19,10 +19,11 @@
 #include <linux/of_graph.h>
 #include <linux/list.h>
 
-#include "dss.h"
 #include "omapdss.h"
 
 static struct dss_device *dss_device;
+static struct dispc_device *s_dispc_device;
+static const struct dispc_ops *s_dispc_ops;
 
 static struct list_head omapdss_comp_list;
 
@@ -46,16 +47,23 @@ EXPORT_SYMBOL(omapdss_set_dss);
 
 struct dispc_device *dispc_get_dispc(struct dss_device *dss)
 {
-       return dss->dispc;
+       return s_dispc_device;
 }
 EXPORT_SYMBOL(dispc_get_dispc);
 
 const struct dispc_ops *dispc_get_ops(struct dss_device *dss)
 {
-       return dss->dispc_ops;
+       return s_dispc_ops;
 }
 EXPORT_SYMBOL(dispc_get_ops);
 
+void omapdss_set_dispc(struct dispc_device *dispc, const struct dispc_ops* 
dispc_ops)
+{
+       s_dispc_device = dispc;
+       s_dispc_ops = dispc_ops;
+}
+EXPORT_SYMBOL(omapdss_set_dispc);
+
 static bool omapdss_list_contains(const struct device_node *node)
 {
        struct omapdss_comp_node *comp;
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index ce470b51e326..b72f981d660e 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -4786,7 +4786,7 @@ static int dispc_bind(struct device *dev, struct device 
*master, void *data)
        dispc_runtime_put(dispc);
 
        dss->dispc = dispc;
-       dss->dispc_ops = &dispc_ops;
+       omapdss_set_dispc(dispc, &dispc_ops);
 
        dispc->debugfs = dss_debugfs_create_file(dss, "dispc", dispc_dump_regs,
                                                 dispc);
@@ -4807,8 +4807,8 @@ static void dispc_unbind(struct device *dev, struct 
device *master, void *data)
 
        dss_debugfs_remove_file(dispc->debugfs);
 
+       omapdss_set_dispc(NULL, NULL);
        dss->dispc = NULL;
-       dss->dispc_ops = NULL;
 
        pm_runtime_disable(dev);
 
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h 
b/drivers/gpu/drm/omapdrm/dss/dss.h
index 6f6fd3d1b159..3d23232ec1f7 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.h
+++ b/drivers/gpu/drm/omapdrm/dss/dss.h
@@ -274,7 +274,6 @@ struct dss_device {
        struct dss_pll  *video2_pll;
 
        struct dispc_device *dispc;
-       const struct dispc_ops *dispc_ops;
 };
 
 /* core */
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index a4f71e082c1c..b724dae22d7a 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -751,6 +751,7 @@ struct dispc_ops {
 
 struct dispc_device *dispc_get_dispc(struct dss_device *dss);
 const struct dispc_ops *dispc_get_ops(struct dss_device *dss);
+void omapdss_set_dispc(struct dispc_device *dispc, const struct dispc_ops* 
dispc_ops);
 
 bool omapdss_component_is_display(struct device_node *node);
 bool omapdss_component_is_output(struct device_node *node);


Yes, it adds two new globals. But I don't think those are a big issue. Note 
that I left the dss->dispc there, for dss internal use.

Laurent, what do you think? If this is fine, can you squash to your series? Or 
I can even have this on top as well. I think otherwise it's good for merging.

Can you also have a quick look at patches 2, 3, 4, 5, 6 and 7. While their aim 
is to get dss6 working, I think they're ok cleanups and shouldn't cause issues 
with the main dss rework.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to