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 jsarha@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