All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 22/30] drm: omapdrm: dss: Store DSS device pointer in the omapdrm private data
Date: Wed, 14 Feb 2018 17:34:35 +0200	[thread overview]
Message-ID: <2006846.pXobRl4t5r@avalon> (raw)
In-Reply-To: <e0ffaffb-0c2c-4d66-fd46-4e67cb74824d@ti.com>

Hi Tomi,

On Wednesday, 14 February 2018 10:31:17 EET Tomi Valkeinen wrote:
> Hi,
> 
> On 13/02/18 14:00, Laurent Pinchart wrote:
> > The dss_device is the top-level component in the omapdss driver. Give
> > the omapdrm driver access to the dss_device pointer in order to obtain
> > pointers to all other components from it. This requires a new global
> > variable in the omapdss driver that will be removed when merging the
> > omapdrm and omapdss drivers, but will already allow removal of several
> > other global variables.
> 
> How can we support different DSS versions with this change? So far the
> DSS functions used from omapdrm have been designed to be free of DSS
> specific parameters, and in the TI kernel we have a separate DSS driver
> for the DSS6, and omapdrm "just works" with either DSS2-5 and DSS6.
> 
> We could have the "struct dss_device *dss" and "struct dispc_device
> *dispc" in omap_drv.h as "struct device" pointers instead.

From an omapdrm point of view the dss_device and dispc_device structures are 
opaque. We're free to do anything we want with them, from switching to struct 
device pointers to allow completely separate implementations of the DSS and 
DISPC on DSS2-5 and DSS6, or storing common data in struct dss_device and 
struct dispc_device, that we can then subclass with version-specific 
structures internally. I don't have a strong opinion at the moment as I 
haven't seen the DSS6 code and thus can't judge whether there's a need to 
share data and code. At this point what matters to me is making sure we always 
pass objects around explicitly to avoid global variables. We could go for 
struct device now and switch to something else later, or the other way around, 
or anything else that works for you.

> Alternatively, we could just create a totally new DRM driver for DSS6,
> having nothing in common with the current omapdrm. From naming
> perspective that would not be so bad, as OMAP is dead =). But if we ever
> get DSS7, perhaps that's again different enough that we don't want a
> single DSS driver for DSS6 and DSS7.

Again I haven't stupid the differences between DSS2-5 and DSS6 so I can't 
really comment, but being able to start from scratch without carrying device 
mistakes over is tempting. Of course that means we would just make different 
design mistakes :-)

-- 
Regards,

Laurent Pinchart

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

  reply	other threads:[~2018-02-14 15:34 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-13 12:00 [PATCH v2 00/30] omapdrm: Allocate objects dynamically Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 01/30] drm: omapdrm: Split init and cleanup from probe and remove functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 02/30] drm: omapdrm: dss: Expose DSS data in a dss_device structure Laurent Pinchart
2018-02-13 20:05   ` Sebastian Reichel
2018-02-13 12:00 ` [PATCH v2 03/30] drm: omapdrm: dss: Pass DSS private structure to runtime PM functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 04/30] drm: omapdrm: dss: Pass PLL pointer to dss_ctrl_pll_enable() Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 05/30] drm: omapdrm: dss: Pass DSS pointer to dss_sdi_*() functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 06/30] drm: omapdrm: dss: Pass DSS pointer to dss_ops operations Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 07/30] drm: omapdrm: dss: Pass DSS pointer to dss_get_*_clk_source() Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 08/30] drm: omapdrm: dss: Pass DSS pointer to dss clock functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 09/30] drm: omapdrm: dss: Pass DSS pointer to remaining dss functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 10/30] drm: omapdrm: dss: Allocate the DSS private data structure dynamically Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 11/30] drm: omapdrm: dss: Support passing private data to debugfs show handlers Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 12/30] drm: omapdrm: dss: Store the registered plls array in struct dss_device Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 13/30] drm: omapdrm: dss: Store the debugfs root directory " Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 14/30] drm: omapdrm: dss: Don't unnecessarily cast to dev to pdev and back Laurent Pinchart
2018-02-13 20:23   ` Sebastian Reichel
2018-02-13 12:00 ` [PATCH v2 15/30] drm: omapdrm: dsi: Pass the dsi_data pointer to internal functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 16/30] drm: omapdrm: dsi: Combine two commonly used inline functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 17/30] drm: omapdrm: dsi: Use dev pointer directly in dsi_bind() function Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 18/30] drm: omapdrm: dsi: Store the struct device pointer in struct dsi_data Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 19/30] drm: omapdrm: dsi: Don't pass channel to dispc init/uninit functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 20/30] drm: omapdrm: dss: Pass omap_dss_device pointer to dss_mgr_*() functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 21/30] drm: omapdrm: dss: Pass omap_drm_private pointer to dss_mgr_ops Laurent Pinchart
2018-02-13 20:58   ` Sebastian Reichel
2018-02-13 12:00 ` [PATCH v2 22/30] drm: omapdrm: dss: Store DSS device pointer in the omapdrm private data Laurent Pinchart
2018-02-13 21:25   ` Sebastian Reichel
2018-02-14  8:31   ` Tomi Valkeinen
2018-02-14 15:34     ` Laurent Pinchart [this message]
2018-02-13 12:00 ` [PATCH v2 23/30] drm: omapdrm: dss: Store dispc ops in dss_device structure Laurent Pinchart
2018-02-13 21:32   ` Sebastian Reichel
2018-02-13 12:00 ` [PATCH v2 24/30] drm: omapdrm: dispc: Pass DISPC pointer to dispc_ops operations Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 25/30] drm: omapdrm: dispc: Pass DISPC pointer to remaining dispc API functions Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 26/30] drm: omapdrm: dispc: Allocate the dispc private data structure dynamically Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 27/30] drm: omapdrm: hdmi4: Allocate the omap_hdmi " Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 28/30] drm: omapdrm: hdmi5: " Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 29/30] drm: omapdrm: sdi: Allocate the sdi private " Laurent Pinchart
2018-02-13 12:00 ` [PATCH v2 30/30] drm: omapdrm: venc: Allocate the venc " Laurent Pinchart
2018-02-14 13:24 ` [PATCH v2 00/30] omapdrm: Allocate objects dynamically Tomi Valkeinen
2018-02-14 15:39   ` Laurent Pinchart
2018-02-14 15:52     ` Tomi Valkeinen
2018-02-28  8:35 ` Tomi Valkeinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2006846.pXobRl4t5r@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=tomi.valkeinen@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.