Re: drm/exynos: g2d userptr memory corruption

2015-08-19 Thread Rob Clark
On Wed, Aug 19, 2015 at 10:08 AM, Jerome Glisse jgli...@redhat.com wrote:
 On Wed, Aug 19, 2015 at 03:53:44PM +0200, Tobias Jakobi wrote:
 Adding Jérôme to Cc. I think he looked the userptr code before, so maybe
 he has some idea what is going wrong here.

 I also had a look at the code, but my knowledge about the DMA API is
 almost nonexistant. However I can see that before doing any DMA via the
 G2D on the buffer the code calls dma_map() on it, and also unmaps it
 when the commandlist is finished.


 With best wishes,
 Tobias


 Tobias Jakobi wrote:
  Thanks Lucas for the explanation!
 
 
  Lucas Stach wrote:
  Hi Tobias,
 
  Am Sonntag, den 16.08.2015, 14:48 +0200 schrieb Tobias Jakobi:
  Hello,
 
  some time ago I checked whether I could use the userptr functionality to
  do zero-copy from userspace allocated buffers via the G2D. This didn't
  work out so well, so kinda put this to the bottom of my TODO list.
 
  Now that IOMMU support has landed and Jan Kara has rewrote page pinning
  using frame vectors (see [1]) I gave userptr another try.
 
  The results are much better. I'm not experiencing any kernel lockups or
  sysmmu pagefaults anymore. However the image now suffers from visual
  artifacts. These images show the nature of the artifacts:
  http://i.imgur.com/nzT6g3Y.jpg
  http://i.imgur.com/wkuYI6X.jpg
 
  The corruption always manifests itself in these pixel lines of fixed
  size and wrong color.
 
  I have written a testcase as part of libdrm for this issue:
  https://github.com/tobiasjakobi/libdrm/commit/db8bf6844436598251f67a71fc334b929bfb2b71
 
  It allocates N (N an even number) buffers which are aligned to the
  system pagesize. Then it does this each iteration:
  1) Fill the first N/2 buffers with random data
  2) Copy the first half to the second half of the buffers
  3) memcmp() first and second half (verification pass)
 
  Usually this verification already fails on the first iteration. An
  interesting observation is that increasing (!) the buffer size (so the
  amount of pixels that have to copied per buffer grows) makes this issue
  less likely to happen.
 
  With the default 512x512 buffers however it happens, like I said above,
  almost immediately.
 
  This is obviously a cache flush missing. The memory you get from
  userspace is normal cached memory, so to make it visible to the GPU you
  need to flush parts of the cache out to main memory.
 
  The corruption you are seeing is just unflushed cachelines. This also
  explains why increasing the buffer size helps: the more memory the CPU
  touches the more cachelines will be flushed out to be replaced with new
  data.
  I should point out that the snapshots I uploaded were done with a
  different setup. There only the source memory of the G2D operation is a
  userspace allocated buffer. The destination is a GEM buffer allocated
  through libdrm, which is then used as framebuffer. So the issue already
  appears when just the source is userspace allocated.
 

 This is still consistent with cachelines issue. Is your GPU  IOMMU cache
 coherent with the CPU ? If not then it means you need to cache flush the
 buffer before you use it with the GPU. The dma API provide few helpers for
 that.

although I suspect dma-api probably not aware of any device caches
(and I suspect a bit weak when it comes to devices that support mix of
coherent and non-coherent mappings)..

BR,
-R

 Cheers,
 Jérôme
 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/21] On-demand device registration

2015-06-03 Thread Rob Clark
On Mon, May 25, 2015 at 10:53 AM, Tomeu Vizoso
tomeu.viz...@collabora.com wrote:
 Hello,

 I have a problem with the panel on my Tegra Chromebook taking longer than
 expected to be ready during boot (Stéphane Marchesin reported what is
 basically the same issue in [0]), and have looked into ordered probing as a
 better way of solving this than moving nodes around in the DT or playing with
 initcall levels.

 While reading the thread [1] that Alexander Holler started with his series to
 make probing order deterministic, it occurred to me that it should be possible
 to achieve the same by registering devices as they are referenced by other
 devices.

 This basically reuses the information that is already implicit in the probe()
 implementations, saving us from refactoring existing drivers or adding
 information to DTBs.

 Something I'm not completely happy with is that I have had to move the call to
 of_platform_populate after all platform drivers have been registered.
 Otherwise I don't see how I could register drivers on demand as we don't have
 yet each driver's compatible strings.

 For machs that don't move of_platform_populate() to a later point, these
 patches shouldn't cause any problems but it's not guaranteed that we'll avoid
 all the deferred probes as some drivers may not be registered yet.

 I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
 patches were enough to eliminate all the deferred probes.

 With this series I get the kernel to output to the panel in 0.5s, instead of 
 2.8s.

So, complete drive-by comment (and I won't claim to be a DT expert,
etc, etc, so take this with a few grains of salt), but why not push
the problem to the DT compiler (or a pre-process step that could be
run on existing DT blobs), which generates an optional DT node that is
the recommended probe order?  That seems like it avoids adding
complexity into the early boot code (which seems like a good thing)..

As a bonus, a bootarg (or something like that) which runs through the
recommended probe order in reverse (to continue our current state of
ensuring that EPROBE_DEFER error paths are well tested)

At any rate, for a device like a drm driver that has multiple
sub-components, and depends on various other clk/gpio/regulator/etc
drivers, the current EPROBE_DEFER situation is pretty comical, so any
solution that improves on things is very much welcome :-)

BR,
-R

 Regards,

 Tomeu

 [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html

 [1] https://lkml.org/lkml/2014/5/12/452

 Tomeu Vizoso (21):
   regulator: core: Reduce critical area in _regulator_get
   ARM: tegra: Add gpio-ranges property
   ARM: tegra: Register drivers before devices
   ARM: EXYNOS: Register drivers before devices
   ARM i.MX6q: Register drivers before devices
   of/platform: Add of_platform_device_ensure()
   of/platform: Ensure device registration on lookup
   gpio: Probe GPIO drivers on demand
   gpio: Probe pinctrl devices on demand
   regulator: core: Probe regulators on demand
   drm: Probe panels on demand
   drm/tegra: Probe dpaux devices on demand
   i2c: core: Probe i2c master devices on demand
   pwm: Probe PWM chip devices on demand
   backlight: Probe backlight devices on demand
   usb: phy: Probe phy devices on demand
   clk: Probe clk providers on demand
   pinctrl: Probe pinctrl devices on demand
   phy: core: Probe phy providers on demand
   dma: of: Probe DMA controllers on demand
   power-supply: Probe power supplies on demand

  arch/arm/boot/dts/tegra124.dtsi |  1 +
  arch/arm/mach-exynos/exynos.c   |  4 +--
  arch/arm/mach-imx/mach-imx6q.c  | 12 -
  arch/arm/mach-tegra/tegra.c | 21 ++-
  drivers/clk/clk.c   |  3 +++
  drivers/dma/of-dma.c|  3 +++
  drivers/gpio/gpiolib-of.c   |  5 
  drivers/gpu/drm/drm_panel.c |  3 +++
  drivers/gpu/drm/tegra/dpaux.c   |  3 +++
  drivers/i2c/i2c-core.c  |  3 +++
  drivers/of/platform.c   | 53 
 +
  drivers/phy/phy-core.c  |  3 +++
  drivers/pinctrl/devicetree.c|  2 ++
  drivers/power/power_supply_core.c   |  3 +++
  drivers/pwm/core.c  |  3 +++
  drivers/regulator/core.c| 45 +++
  drivers/usb/phy/phy.c   |  3 +++
  drivers/video/backlight/backlight.c |  3 +++
  include/linux/of_platform.h |  2 ++
  19 files changed, 130 insertions(+), 45 deletions(-)

 --
 2.4.1


 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/5] exynos: add EXYNOS_G2D_EVENT to drmHandleEvent

2015-03-29 Thread Rob Clark
so, iirc, vmwgfx also has some custom events..  not really sure if
they have their own hand-rolled drmHandleEvent() or if they have
another way of catching those.

Maybe we need some more flexible way to register handlers for driver
custom events?  But everyone adding their own thing to
drmHandleEvent() doesn't really seem so nice..  that said, I'm not
sure how much to care.  If it is just exynos and vmwgfx, then telling
them to use there own version of of drmHandleEvent() might be ok.  But
if driver custom events somehow become more popular, maybe we want a
better solution..

BR,
-R

On Fri, Mar 20, 2015 at 6:25 PM, Tobias Jakobi
tjak...@math.uni-bielefeld.de wrote:
 This event is specific to Exynos G2D DRM driver. Only
 process it when Exynos support is enabled.

 Signed-off-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  exynos/exynos_drm.h | 12 
  xf86drm.h   |  7 ++-
  xf86drmMode.c   | 18 ++
  3 files changed, 36 insertions(+), 1 deletion(-)

 diff --git a/exynos/exynos_drm.h b/exynos/exynos_drm.h
 index 256c02f..c3af0ac 100644
 --- a/exynos/exynos_drm.h
 +++ b/exynos/exynos_drm.h
 @@ -157,4 +157,16 @@ struct drm_exynos_g2d_exec {
  #define DRM_IOCTL_EXYNOS_G2D_EXEC  DRM_IOWR(DRM_COMMAND_BASE + \
 DRM_EXYNOS_G2D_EXEC, struct drm_exynos_g2d_exec)

 +/* EXYNOS specific events */
 +#define DRM_EXYNOS_G2D_EVENT   0x8000
 +
 +struct drm_exynos_g2d_event {
 +   struct drm_eventbase;
 +   __u64   user_data;
 +   __u32   tv_sec;
 +   __u32   tv_usec;
 +   __u32   cmdlist_no;
 +   __u32   reserved;
 +};
 +
  #endif
 diff --git a/xf86drm.h b/xf86drm.h
 index 40c55c9..6b1c9b4 100644
 --- a/xf86drm.h
 +++ b/xf86drm.h
 @@ -719,7 +719,7 @@ extern void drmMsg(const char *format, ...) 
 DRM_PRINTFLIKE(1, 2);
  extern int drmSetMaster(int fd);
  extern int drmDropMaster(int fd);

 -#define DRM_EVENT_CONTEXT_VERSION 2
 +#define DRM_EVENT_CONTEXT_VERSION 3

  typedef struct _drmEventContext {

 @@ -739,6 +739,11 @@ typedef struct _drmEventContext {
   unsigned int tv_usec,
   void *user_data);

 +   void (*g2d_event_handler)(int fd,
 + unsigned int cmdlist_no,
 + unsigned int tv_sec,
 + unsigned int tv_usec,
 + void *user_data);
  } drmEventContext, *drmEventContextPtr;

  extern int drmHandleEvent(int fd, drmEventContextPtr evctx);
 diff --git a/xf86drmMode.c b/xf86drmMode.c
 index 61d5e01..d9f2898 100644
 --- a/xf86drmMode.c
 +++ b/xf86drmMode.c
 @@ -53,6 +53,10 @@
  #include unistd.h
  #include errno.h

 +#ifdef HAVE_EXYNOS
 +#include exynos/exynos_drm.h
 +#endif
 +
  #ifdef HAVE_VALGRIND
  #include valgrind.h
  #include memcheck.h
 @@ -846,6 +850,7 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx)
 int len, i;
 struct drm_event *e;
 struct drm_event_vblank *vblank;
 +   struct drm_exynos_g2d_event *g2d;

 /* The DRM read semantics guarantees that we always get only
  * complete events. */
 @@ -882,6 +887,19 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx)
  vblank-tv_usec,
  U642VOID 
 (vblank-user_data));
 break;
 +#ifdef HAVE_EXYNOS
 +   case DRM_EXYNOS_G2D_EVENT:
 +   if (evctx-version  3 ||
 +   evctx-g2d_event_handler == NULL)
 +   break;
 +   g2d = (struct drm_exynos_g2d_event *) e;
 +   evctx-g2d_event_handler(fd,
 +g2d-cmdlist_no,
 +g2d-tv_sec,
 +g2d-tv_usec,
 +U642VOID (g2d-user_data));
 +   break;
 +#endif
 default:
 break;
 }
 --
 2.0.5

 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 04/14] drm/exynos: remove struct *_win_data abstraction on planes

2015-02-05 Thread Rob Clark
On Thu, Feb 5, 2015 at 4:15 AM, Daniel Vetter dan...@ffwll.ch wrote:
 On Thu, Feb 05, 2015 at 11:37:13AM +0900, Joonyoung Shim wrote:
 Hi Daniel,

 On 02/04/2015 11:28 PM, Daniel Vetter wrote:
  On Wed, Feb 04, 2015 at 04:44:12PM +0900, Joonyoung Shim wrote:
  Hi,
 
  On 02/04/2015 04:14 AM, Gustavo Padovan wrote:
  From: Gustavo Padovan gustavo.pado...@collabora.co.uk
 
  struct {fimd,mixer,vidi}_win_data was just keeping the same data
  as struct exynos_drm_plane thus get ride of it and use exynos_drm_plane
  directly.
 
  It changes how planes are created and remove .win_mode_set() callback
  that was only filling all *_win_data structs.
 
 
  I commented already on prior patch.
 
  I think you don't quite understand how this primary/overlay plane stuff
  works in drm core. The entire point of the drm core primary plane is to
  work _exactly_ like an overlay plane and allow userspace to mangle the
  primary plane configuration through the overlay plane. The only reason we
  have primary planes is so that old userspace keeps working.
 

 Right, i misunderstood a bit because exynos hw drivers have dependency
 of zpos(hw overlay position).

 Current exynos drm driver has each primary plane of hw drivers and five
 overlay planes. The primary plane is fixed on default hw overlay and all
 overlay plane can map to all hw overlays using specific zpos property of
 exynos drm plane.

 Gustavo approach will include specific hw overlay data in overlay plane
 and hw driver keeps overlay planes to array by zpos order. But current
 zpos of overlay plane is 0 always if user doesn't modify it, so hw
 driver will use only hw overlay data of primary plane always even if
 user want to use overlay plane.

 If user is modified zpos of overlay plane, hw driver can get wrong hw
 overlay data from different overlay plane because hw driver keeps
 overlay planes by zpos order.

 Yeah I noticed the zpos fun when hacking around too. Exynos should
 probably switch defaults so that overlays are visible by default. And we
 need to standardize the zpos property so that other drivers can use it
 too.

jfyi, I have a bit of logic in mdp5_crtc_atomic_check() (and really
mdp4 probably needs the same) to sort attached planes and derive the
actual hw zpos (with each layer having a unique zpos)..

I suspect most hw will end up needing the same (ie. dislike having two
overlays at same zpos), so might not be a horrible idea to make the
atomic helpers do this sorting for us..

BR,
-R

 But that doesn't change anything with the primary plane just being a
 special plane from the sw side (backwards compat), for exynos hw they all
 look the same.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

2015-01-30 Thread Rob Clark
On Tue, Jan 20, 2015 at 11:38 AM, Ajay Kumar ajaykumar...@samsung.com wrote:
 Currently, third party bridge drivers(ptn3460) are dependent
 on the corresponding encoder driver init, since bridge driver
 needs a drm_device pointer to finish drm initializations.
 The encoder driver passes the drm_device pointer to the
 bridge driver. Because of this dependency, third party drivers
 like ptn3460 doesn't adhere to the driver model.

 In this patch, we reframe the bridge registration framework
 so that bridge initialization is split into 2 steps, and
 bridge registration happens independent of drm flow:
 --Step 1: gather all the bridge settings independent of drm and
   add the bridge onto a global list of bridges.
 --Step 2: when the encoder driver is probed, call drm_bridge_attach
   for the corresponding bridge so that the bridge receives
   drm_device pointer and continues with connector and other
   drm initializations.

 The old set of bridge helpers are removed, and a set of new helpers
 are added to accomplish the 2 step initialization.

 The bridge devices register themselves onto global list of bridges
 when they get probed by calling drm_bridge_add.

 The parent encoder driver waits till the bridge is available
 in the lookup table(by calling of_drm_find_bridge) and then
 continues with its initialization.

 The encoder driver should also call drm_bridge_attach to pass
 on the drm_device to the bridge object.

 drm_bridge_attach inturn calls bridge-funcs-attach so that
 bridge can continue with drm related initializations.

ok, so I probably should have had a closer look at this before it
landed in drm-next, so if it is too late to revert (and deal w/
untangling subsequent patches that depend on this) some of these
issues we'll just have to fix with follow-on patches.

1) needs headerdoc for new fxns in drm_bridge.c, and needs to be added
in drm.tmpl
2) as far as I can tell, the new approach to cleaning up bridge
objects is to just let them leak !?!

I'll also need to update the new bridge in the msm edp code..
although that isn't such a big deal if I knew how this was *supposed*
to work.. since what is there now at least doesn't look right..

BR,
-R



 Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 Tested-by: Rahul Sharma rahul.sha...@samsung.com
 Tested-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
 Tested-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Tested-by: Sjoerd Simons sjoerd.sim...@collabora.co.uk
 ---
  drivers/gpu/drm/Makefile   |2 +-
  drivers/gpu/drm/bridge/ptn3460.c   |   27 +-
  drivers/gpu/drm/drm_bridge.c   |   91 
 
  drivers/gpu/drm/drm_crtc.c |   67 ---
  drivers/gpu/drm/msm/hdmi/hdmi.c|4 +-
  drivers/gpu/drm/msm/hdmi/hdmi.h|1 +
  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |6 +--
  drivers/gpu/drm/sti/sti_hda.c  |   10 +---
  drivers/gpu/drm/sti/sti_hdmi.c |   10 +---
  include/drm/bridge/ptn3460.h   |8 +++
  include/drm/drm_crtc.h |   26 -
  11 files changed, 133 insertions(+), 119 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_bridge.c

 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index e620807..c83ef2d 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -14,7 +14,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
 drm_info.o drm_debugfs.o drm_encoder_slave.o \
 drm_trace_points.o drm_global.o drm_prime.o \
 drm_rect.o drm_vma_manager.o drm_flip_work.o \
 -   drm_modeset_lock.o drm_atomic.o
 +   drm_modeset_lock.o drm_atomic.o drm_bridge.o

  drm-$(CONFIG_COMPAT) += drm_ioc32.o
  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index a2ddc8d..4a818c1 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -176,24 +176,11 @@ static void ptn3460_post_disable(struct drm_bridge 
 *bridge)
  {
  }

 -static void ptn3460_bridge_destroy(struct drm_bridge *bridge)
 -{
 -   struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
 -
 -   drm_bridge_cleanup(bridge);
 -   if (gpio_is_valid(ptn_bridge-gpio_pd_n))
 -   gpio_free(ptn_bridge-gpio_pd_n);
 -   if (gpio_is_valid(ptn_bridge-gpio_rst_n))
 -   gpio_free(ptn_bridge-gpio_rst_n);
 -   /* Nothing else to free, we've got devm allocated memory */
 -}
 -
  static struct drm_bridge_funcs ptn3460_bridge_funcs = {
 .pre_enable = ptn3460_pre_enable,
 .enable = ptn3460_enable,
 .disable = ptn3460_disable,
 .post_disable = ptn3460_post_disable,
 -   .destroy = ptn3460_bridge_destroy,
  };

  static int ptn3460_get_modes(struct 

Re: [RFC V2 0/3] drm/bridge: panel and chaining

2014-05-12 Thread Rob Clark
On Mon, May 12, 2014 at 3:06 AM, Andrzej Hajda a.ha...@samsung.com wrote:
 On 05/09/2014 05:05 PM, Ajay kumar wrote:
 On Fri, May 9, 2014 at 7:29 PM, Rob Clark robdcl...@gmail.com wrote:
 On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
 On 05/08/2014 08:24 PM, Rob Clark wrote:
 On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote:
 On 05/05/2014 09:52 PM, Ajay Kumar wrote:
 This patchset is based on exynos-drm-next-todo branch of Inki Dae's 
 tree at:
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

 I have just put up Rob's and Sean's idea of chaining up the bridges
 in code, and have implemented basic panel controls as a chained bridge.
 This works well with ptn3460 bridge chip on exynos5250-snow board.

 Still need to make use of standard list calls and figure out proper way
 of deleting the bridge chain. So, this is just a rough version.
 As I understand this patchset tries to solve two things:
 1. Implement panel as drm_bridge, to ease support for hardware chains:
 Crtc - Encoder - Bridge - Panel
 2. Add support to drm_bridge chaining, to allow software chains:
 drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel

 It is done using Russian doll schema, ops from the bridge calls the same
 ops from the next bridge and the next bridge ops can do the same.

 This schema means that all the bridges including the last one are seen
 from the drm core point of view as a one big drm_bridge. Additionally in
 this particular case, the first bridge (ptn3460) implements connector
 so it is hard to guess what is the location of the 2nd bridge in video
 stream chain, sometimes it is after the connector, sometimes before.
 All this is quite confusing.

 But if you look at the bridge from upstream video interface point of
 view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
 video input side acts as a panel. On the output side it expects a panel,
 lvds panel in this case.
 tbh, this is mostly about what we call it.  Perhaps bridge isn't the
 best name.. I wouldn't object to changing it.

 But my thinking was to leave in drm_panel_funcs things that are just
 needed by the connector (get_modes().. and maybe some day we need
 detect/etc).  Then leave everything else in drm_bridge_funcs.  A panel
 could (if needed) implement both interfaces.

 That is basically the same as what you are proposing, but without
 renaming bridge to panel ;-)
 Good to hear that. However there are points which are not clear for me.
 But first lets clarify names, I will use panel and bridge words
 to describe the hardware, and drm_panel, drm_bridge to describe the
 software interfaces.

 What bothers me:
 1. You want to leave connector related callbacks in drm_panel and
 the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460
 must implement connector internally because of this limitation. I guess
 it is quite typical bridge. This problem does not exists in case
 of using drm_panel for ptn3460.

 2. drm_bridge is attached to the encoder, this and the callback order
 suggests the video data flow should be as below:
 drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel]

 ptn3460 implements drm_bridge and drm_connector so it suggests its
 drm_bridge should be the last one, so there should be no place to add
 lvds panel implemented as a drm_bridge after it, as it is done in this
 patchset.

 Additionally it clearly shows that there should be two categories of
 drm_bridges - non-terminal and terminal.

 3. drm_dev uses all-or-nothing approach, ie. it will start only when all
 its components are up. It simplifies synchronization but is quite
 fragile - the whole drm will be down due to error in some of its 
 components.
 For this reason I prefer drm_panel as it is not real drm component
 it can be attached/detached to/from drm_connector anytime. I am not
 really sure but drm_bridge does not allow for that.
 So I do think we need to stick to this all-or-nothing approach for
 anything that is visible to userspace
 (drm_{plane,crtc,encoder,connector}).  We don't currently have a way
 to hotplug those so I don't see a real smooth upgrade path to add
 that in a backwards compatible way that won't cause problems with old
 userspace.

 But, that said, we have more flexibility with things not visible to
 userspace (drm_{panel,bridge}).  I'm not sure how much we want to
 allow things to be completely dynamic (we already have some hard
 enough locking fun).  But proposals/rfcs/etc welcome.

 I guess I'm not completely familiar w/ ptn3460, but the fact that it
 needs to implement drm_connector makes me a bit suspicious.  Seems
 like a symptom of missing things in drm_panel_funcs.  It would be
 better to always create the connector statically, and just have
 _detect() - disconnected if panel==NULL.

 ptn3460 has been implemented using drm_bridge and drm_connector, not by
 me, to be clear :)

sure, and afaiu it was adapted from a pre-bridge

Re: [RFC V2 0/3] drm/bridge: panel and chaining

2014-05-09 Thread Rob Clark
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
 On 05/08/2014 08:24 PM, Rob Clark wrote:
 On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote:
 On 05/05/2014 09:52 PM, Ajay Kumar wrote:
 This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree 
 at:
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

 I have just put up Rob's and Sean's idea of chaining up the bridges
 in code, and have implemented basic panel controls as a chained bridge.
 This works well with ptn3460 bridge chip on exynos5250-snow board.

 Still need to make use of standard list calls and figure out proper way
 of deleting the bridge chain. So, this is just a rough version.

 As I understand this patchset tries to solve two things:
 1. Implement panel as drm_bridge, to ease support for hardware chains:
 Crtc - Encoder - Bridge - Panel
 2. Add support to drm_bridge chaining, to allow software chains:
 drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel

 It is done using Russian doll schema, ops from the bridge calls the same
 ops from the next bridge and the next bridge ops can do the same.

 This schema means that all the bridges including the last one are seen
 from the drm core point of view as a one big drm_bridge. Additionally in
 this particular case, the first bridge (ptn3460) implements connector
 so it is hard to guess what is the location of the 2nd bridge in video
 stream chain, sometimes it is after the connector, sometimes before.
 All this is quite confusing.

 But if you look at the bridge from upstream video interface point of
 view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
 video input side acts as a panel. On the output side it expects a panel,
 lvds panel in this case.

 tbh, this is mostly about what we call it.  Perhaps bridge isn't the
 best name.. I wouldn't object to changing it.

 But my thinking was to leave in drm_panel_funcs things that are just
 needed by the connector (get_modes().. and maybe some day we need
 detect/etc).  Then leave everything else in drm_bridge_funcs.  A panel
 could (if needed) implement both interfaces.

 That is basically the same as what you are proposing, but without
 renaming bridge to panel ;-)

 Good to hear that. However there are points which are not clear for me.
 But first lets clarify names, I will use panel and bridge words
 to describe the hardware, and drm_panel, drm_bridge to describe the
 software interfaces.

 What bothers me:
 1. You want to leave connector related callbacks in drm_panel and
 the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460
 must implement connector internally because of this limitation. I guess
 it is quite typical bridge. This problem does not exists in case
 of using drm_panel for ptn3460.

 2. drm_bridge is attached to the encoder, this and the callback order
 suggests the video data flow should be as below:
 drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel]

 ptn3460 implements drm_bridge and drm_connector so it suggests its
 drm_bridge should be the last one, so there should be no place to add
 lvds panel implemented as a drm_bridge after it, as it is done in this
 patchset.

 Additionally it clearly shows that there should be two categories of
 drm_bridges - non-terminal and terminal.

 3. drm_dev uses all-or-nothing approach, ie. it will start only when all
 its components are up. It simplifies synchronization but is quite
 fragile - the whole drm will be down due to error in some of its components.
 For this reason I prefer drm_panel as it is not real drm component
 it can be attached/detached to/from drm_connector anytime. I am not
 really sure but drm_bridge does not allow for that.

So I do think we need to stick to this all-or-nothing approach for
anything that is visible to userspace
(drm_{plane,crtc,encoder,connector}).  We don't currently have a way
to hotplug those so I don't see a real smooth upgrade path to add
that in a backwards compatible way that won't cause problems with old
userspace.

But, that said, we have more flexibility with things not visible to
userspace (drm_{panel,bridge}).  I'm not sure how much we want to
allow things to be completely dynamic (we already have some hard
enough locking fun).  But proposals/rfcs/etc welcome.

I guess I'm not completely familiar w/ ptn3460, but the fact that it
needs to implement drm_connector makes me a bit suspicious.  Seems
like a symptom of missing things in drm_panel_funcs.  It would be
better to always create the connector statically, and just have
_detect() - disconnected if panel==NULL.

 Real life example to show importance of it: I have a phone with MIPI-DSI
 panel and HDMI. Due to initialization issues HDMI bridge driver
 sometimes fails during probe and the drmdev do not start. Of course this
 is development stage so I have serial console I can diagnose the
 problem, disable HDMI, fix the problem, etc...
 But what happens

Re: [RFC V2 0/3] drm/bridge: panel and chaining

2014-05-08 Thread Rob Clark
On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote:
 On 05/05/2014 09:52 PM, Ajay Kumar wrote:
 This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at:
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

 I have just put up Rob's and Sean's idea of chaining up the bridges
 in code, and have implemented basic panel controls as a chained bridge.
 This works well with ptn3460 bridge chip on exynos5250-snow board.

 Still need to make use of standard list calls and figure out proper way
 of deleting the bridge chain. So, this is just a rough version.

 As I understand this patchset tries to solve two things:
 1. Implement panel as drm_bridge, to ease support for hardware chains:
 Crtc - Encoder - Bridge - Panel
 2. Add support to drm_bridge chaining, to allow software chains:
 drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel

 It is done using Russian doll schema, ops from the bridge calls the same
 ops from the next bridge and the next bridge ops can do the same.

 This schema means that all the bridges including the last one are seen
 from the drm core point of view as a one big drm_bridge. Additionally in
 this particular case, the first bridge (ptn3460) implements connector
 so it is hard to guess what is the location of the 2nd bridge in video
 stream chain, sometimes it is after the connector, sometimes before.
 All this is quite confusing.

 But if you look at the bridge from upstream video interface point of
 view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
 video input side acts as a panel. On the output side it expects a panel,
 lvds panel in this case.

tbh, this is mostly about what we call it.  Perhaps bridge isn't the
best name.. I wouldn't object to changing it.

But my thinking was to leave in drm_panel_funcs things that are just
needed by the connector (get_modes().. and maybe some day we need
detect/etc).  Then leave everything else in drm_bridge_funcs.  A panel
could (if needed) implement both interfaces.

That is basically the same as what you are proposing, but without
renaming bridge to panel ;-)

BR,
-R

 So why not implement ptn3460 bridge as drm_panel which internally uses
 another drm_panel. With this approach everything fits much better.
 You do not need those (pre|post)_(enable|disable) calls, you do not need
 to implement connector in the bridge and you have a driver following
 linux driver model. And no single bit changed in drm core.

 I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2].
 It was not accepted as Inki preferred drm_bridge but as I see the
 problems with drm_bridges I have decide to attract attention to much
 more cleaner solution.

 [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559
 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044

 Regards
 Andrzej



 Ajay Kumar (3):
   [RFC V2 1/3] drm: implement chaining of drm bridges
   [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges
   [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining

  .../bindings/drm/bridge/bridge_panel.txt   |  45 
  drivers/gpu/drm/bridge/Kconfig |   6 +
  drivers/gpu/drm/bridge/Makefile|   1 +
  drivers/gpu/drm/bridge/bridge_panel.c  | 240 
 +
  drivers/gpu/drm/bridge/ptn3460.c   |  21 +-
  drivers/gpu/drm/drm_crtc.c |  13 +-
  include/drm/bridge/bridge_panel.h  |  37 
  include/drm/drm_crtc.h |   2 +
  8 files changed, 360 insertions(+), 5 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt
  create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c
  create mode 100644 include/drm/bridge/bridge_panel.h


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC V2 0/3] drm/bridge: panel and chaining

2014-05-08 Thread Rob Clark
On Thu, May 8, 2014 at 7:57 AM, Inki Dae inki@samsung.com wrote:
 On 2014년 05월 08일 19:52, Ajay kumar wrote:
 +Dave
 +Thierry

 On Thu, May 8, 2014 at 1:14 PM, Inki Dae inki@samsung.com wrote:

 Just re-sending with text mode. Sorry for this.


 On 2014년 05월 08일 15:41, Andrzej Hajda wrote:
 On 05/05/2014 09:52 PM, Ajay Kumar wrote:
 This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree 
 at:
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

 I have just put up Rob's and Sean's idea of chaining up the bridges
 in code, and have implemented basic panel controls as a chained bridge.
 This works well with ptn3460 bridge chip on exynos5250-snow board.

 Still need to make use of standard list calls and figure out proper way
 of deleting the bridge chain. So, this is just a rough version.

 As I understand this patchset tries to solve two things:
 1. Implement panel as drm_bridge, to ease support for hardware chains:
 Crtc - Encoder - Bridge - Panel
 2. Add support to drm_bridge chaining, to allow software chains:
 drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel

 It is done using Russian doll schema, ops from the bridge calls the same
 ops from the next bridge and the next bridge ops can do the same.

 This schema means that all the bridges including the last one are seen
 from the drm core point of view as a one big drm_bridge. Additionally in
 this particular case, the first bridge (ptn3460) implements connector
 so it is hard to guess what is the location of the 2nd bridge in video
 stream chain, sometimes it is after the connector, sometimes before.
 All this is quite confusing.

 But if you look at the bridge from upstream video interface point of
 view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
 video input side acts as a panel. On the output side it expects a panel,
 lvds panel in this case.

 So why not implement ptn3460 bridge as drm_panel which internally uses
 another drm_panel. With this approach everything fits much better.
 You do not need those (pre|post)_(enable|disable) calls, you do not need
 to implement connector in the bridge and you have a driver following
 linux driver model. And no single bit changed in drm core.

 I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2].
 It was not accepted as Inki preferred drm_bridge but as I see the

 Yes, in above email threads, I disagreed to using drm_panel framework
 for bridge device, especially, to implement connector/encoder to crtc
 driver.

 However, I thought that it'd be more generic way that lvds drivers use
 driver-model, and the use of drm_panel infrastructure would be suitable
 to doing this.

 So my intention in turn, was that LVDS driver uses integrated drm_bridge
 based on drm_panel infrastructure[1], and RFC patch[2] for it. This way
 is same as your proposal in the point that LVDS and Panel drivers use
 driver-model. The only different point is that LVDS driver has some ops
 specific to LVDS device, not using existing ops of drm_panel commonly:
 we may need to consider the characteristic of LVDS device.

 [1]:http://www.spinics.net/lists/dri-devel/msg5.html
 [2]:http://www.spinics.net/lists/dri-devel/msg55658.html

 Thanks,
 Inki Dae
 I am just consolidating the discussion happening here.
 1) This patchset started from a discussion wherein I tried to combine
 drm_panel with drm_bridge.
 
 https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg28943.html
 2) Sean and Rob suggested to implement a chain of bridges and then
 consider adding
 basic panel controls as a bridge.
 3) Andrej's idea is to drop the existing bridge infrastructure and
 implement ptn3460 using drm_panel,
 the same way he has implemented
 http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559.
 4) Inki's suggestion is to combine drm_bridge, drm_panel and
 drm_enhance into a single
 drm_hw_block.


 And more thing, we would need to consider image enhancer device placed
 between crtc and connector/encoder devices. And it'd better to rename
 drm_hw_block to drm_bridge, and existing drm_bridge relevant codes
 should be removed.

I don't object to changing the name to hw_block or something else.
Although to avoid introducing too much confusion in this discussion,
for now I am calling it bridge/drm_bridge_funcs/etc for now ;-)

BR,
-R

 Thanks,
 Inki Dae

 I am currently trying to implement (2):chaining of bridges, and I
 think we have not yet
 reached to a consensus. So adding few other people from drm community
 to comment.

 Regards,
 Ajay

 problems with drm_bridges I have decide to attract attention to much
 more cleaner solution.

 [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559
 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044

 Regards
 Andrzej



 Ajay Kumar (3):
   [RFC V2 1/3] drm: implement chaining of drm bridges
   [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds 
 

Re: [RFC V2 1/3] drm: implement chaining of drm bridges

2014-05-06 Thread Rob Clark
On Tue, May 6, 2014 at 11:55 AM, Sean Paul seanp...@chromium.org wrote:
 On Mon, May 5, 2014 at 12:52 PM, Ajay Kumar ajaykumar...@samsung.com wrote:
 As of now, we can have only one bridge hanging off the encoder.
 With this patch, we allow multiple bridges to hang onto the same encoder
 with the use of a simple next_bridge ptr.

 The drm core calls bridge-funcs for the first bridge which
 is attached to it, and its upto the individual bridge drivers
 to call bridge-funcs for the next bridge in the chain.

 Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
 ---
  drivers/gpu/drm/drm_crtc.c | 13 -
  include/drm/drm_crtc.h |  2 ++
  2 files changed, 14 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
 index d8b7099..fe9905f 100644
 --- a/drivers/gpu/drm/drm_crtc.c
 +++ b/drivers/gpu/drm/drm_crtc.c
 @@ -918,8 +918,10 @@ EXPORT_SYMBOL(drm_connector_unplug_all);
   * Zero on success, error code on failure.
   */
  int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge,
 -   const struct drm_bridge_funcs *funcs)
 +   struct drm_encoder *encoder,
 +   const struct drm_bridge_funcs *funcs)


 IMO, we should let whoever is spawning the bridges chain them
 together, instead of passing encoder in init().

that plus, might be a good time to start adding some static-inline
helper fxns for fxn ptr calls like we have for drm_panel..  not a big
deal, but I guess it would be a good time to do it now before we start
adding chained bridge calls in all the bridges.

but yeah, in general this is what I had in mind

BR,
-R

 Sean


  {
 +   struct drm_bridge *temp;
 int ret;

 drm_modeset_lock_all(dev);
 @@ -931,6 +933,15 @@ int drm_bridge_init(struct drm_device *dev, struct 
 drm_bridge *bridge,
 bridge-dev = dev;
 bridge-funcs = funcs;

 +   if (encoder-bridge) {
 +   temp = encoder-bridge;
 +   while (temp-next_bridge)
 +   temp = temp-next_bridge;
 +
 +   temp-next_bridge = bridge;
 +   } else
 +   encoder-bridge = bridge;
 +
 list_add_tail(bridge-head, dev-mode_config.bridge_list);
 dev-mode_config.num_bridge++;

 diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
 index e55fccb..bb6ed88 100644
 --- a/include/drm/drm_crtc.h
 +++ b/include/drm/drm_crtc.h
 @@ -619,6 +619,7 @@ struct drm_bridge_funcs {
  struct drm_bridge {
 struct drm_device *dev;
 struct list_head head;
 +   struct drm_bridge *next_bridge;

 struct drm_mode_object base;

 @@ -862,6 +863,7 @@ extern void drm_connector_cleanup(struct drm_connector 
 *connector);
  extern void drm_connector_unplug_all(struct drm_device *dev);

  extern int drm_bridge_init(struct drm_device *dev, struct drm_bridge 
 *bridge,
 +  struct drm_encoder *encoder,
const struct drm_bridge_funcs *funcs);
  extern void drm_bridge_cleanup(struct drm_bridge *bridge);

 --
 1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-29 Thread Rob Clark
On Mon, Apr 28, 2014 at 9:08 AM, Ajay kumar ajayn...@gmail.com wrote:
 Daniel,

 On Sun, Apr 27, 2014 at 6:25 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Fri, Apr 25, 2014 at 8:17 AM, Ajay kumar ajayn...@gmail.com wrote:
 We can call panel_enable/disable at the right point. Even the bridge chips
 expect the same. So, I am not ok with combining the bridge and panel and
 calling the fxn pointers from crtc_helpers.
 So, either we leave it the way it is in this patch (call drm_panel
 functions at right points) or
 we don't use drm_panel to control the panel sequence and instead use few DT
 properties and regulators, inside the bridge chip driver.

 That wasn't really suggested, but instead the idea was to provide a
 default drm_bridge which wraps the drm_panel so that you could more
 easily chain them up.
 What do you mean by default drm_bridge?
 I just want to clear few things. Let me explain what I have understood:
 What I understand is that Rob wants me to create a common
 structure which wraps up drm_panel and drm_bridge into a single structure:
   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */  == Really not sure why
 this is needed!
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 And now, the above function becomes a common function
 (also with an ordering issue btw panel and bridge).
 Where do we keep it? And, where do we call it from?

my proposal involves no change to crtc helpers.  The later variation
(with composite_bridge so you could choose the order) would have a
sequence along the lines of:

crtc helpers:
+ bridge_funcs-enable(): - composite_bridge_enable()
   + cbridge-b1-funcs-enable(): - ptn3460_bridge_enable()
   + cbridge-b2-funcs-enable(): - panel_bridge_enable();
   + pbridge-panel-funcs-enable(): - foo_panel_enable()

or if the order needs to be swapped you can use ptn3460_bridge as b2
and panel bridge as b1.

BR,
-R

 Also I'm not really happy that the bridge callbacks have been added to
 the drm helpers (I'd prefer if driver callbacks would bear that
 responsibility). But you can always create your own drm_bridge
 integration.
 Do you mean, the bridge calls should be moved out of common drm_helper
 functions and called from platform specific drivers instead?

 In any case your concern that drivers need to control
 when certain callbacks are called is shared by everyone here afaics.
 And I also don't see any issues with Rob's proposal in this regard.
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch

 Please let me know if my understanding is correct and correct me if
 there is a misconception.

 Regards,
 Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-24 Thread Rob Clark
On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote:
 Sorry for the previous reply,

 Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 We cannot call them like this from crtc helpers in the order you mentioned,
 since each individual bridge chip expects the panel controls at
 different places.
 I mean,
 -- sometimes panel controls needs to be done before bridge
 controls(ptn3460: before RST_N and PD_N)

well, this is why bridge has pre-enable/enable/disable/post-disable
hooks, so you can choose before or after..

 -- sometimes in between the bridge controls (ps8622: one panel control
 before SLP_N and one after SLP_N)
 -- sometimes panel controls needs to be done after bridge controls.

I am not convinced that a generic panel/bridge adapter is not
possible.  Maybe we need more fine grained fxn ptr callbacks, although
seems like pre+post should give you enough.  It would certainly be
easier than having to add panel support in each individual bridge
driver (which seems like it will certainly result that only certain
combinations of panel+bridge actually work as intended)

 So, putting these drm_panel controls inside individual bridge drivers will 
 work,
 but keeping them in crtc_helpers might break stuff.

I'm not talking about crtc changing helpers.

BR,
-R

 Thanks and regards,
 Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-24 Thread Rob Clark
On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote:
 Rob,

 On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote:
 On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote:
 Sorry for the previous reply,

 Here goes the full explaination:

 Rob,

 On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote:
 So what about, rather than adding drm_panel support to each bridge
 individually, we introduce a drm_panel_bridge (with a form of
 chaining).. ie:

   struct drm_panel_bridge {
 struct drm_bridge base;
 struct drm_panel *panel;
 struct drm_bridge *bridge; /* optional */
   };

   static void drm_panel_bridge_enable(struct drm_bridge *bridge)
   {
 struct drm_panel_bridge *pb = to_panel_bridge(bridge);
 if (pb-bridge)
   pb-bridge-funcs-enable(pb-bridge);
 pb-panel-funcs-enable(pb-panel);
   }

 We cannot call them like this from crtc helpers in the order you mentioned,
 since each individual bridge chip expects the panel controls at
 different places.
 I mean,
 -- sometimes panel controls needs to be done before bridge
 controls(ptn3460: before RST_N and PD_N)

 well, this is why bridge has pre-enable/enable/disable/post-disable
 hooks, so you can choose before or after..
 These calls are for a bridge to sync with the encoder calls.
 We might end up defining pre-enable/enable/disable/post-disable for a
 panel to sync
 with the bridge calls! I have explained below.

 -- sometimes in between the bridge controls (ps8622: one panel control
 before SLP_N and one after SLP_N)
 -- sometimes panel controls needs to be done after bridge controls.

 I am not convinced that a generic panel/bridge adapter is not
 possible.  Maybe we need more fine grained fxn ptr callbacks, although
 seems like pre+post should give you enough.  It would certainly be
 easier than having to add panel support in each individual bridge
 driver (which seems like it will certainly result that only certain
 combinations of panel+bridge actually work as intended)
 Ok. Consider this case:
 Currently, we have the sequence as bridge-pre_enable,
 encoder_enable, bridge-enable
 And, the bridge should obviously be enabled in bridge-pre_enable.
 Now, where do you choose to call panel-pre_enable?
 -- as the first step of bridge-pre_enable, before we pull up/down bridge 
 GPIOs.
 -- at the last step of bridge-pre_enable, after we pull up/down the
 bridge GPIOs.

 Ideally, PTN3460 expects it to be the second case, and PS8625 expects
 it to be the first case.
 So, we will end up adding pre_pre_enable, post_pre_enable to
 accomodate such scenarios.

ok, well I think we can deal with this with a slight modification of
my original proposal.  Split drm_panel_bridge into
drm_composite_bridge and drm_panel_bridge:

  struct drm_composite_bridge {
struct drm_bridge base;
struct drm_bridge *b1, *b2;
  }

  static void drm_composite_bridge_enable(struct drm_bridge *bridge)
  {
struct drm_composite_bridge *cbridge = to_composite_bridge(bridge);
cbridge-b1-funcs-enable(cbridge-b1);
cbridge-b2-funcs-enable(cbridge-b2);
  }

  .. and so on ..

  struct drm_panel_bridge {
struct drm_bridge base;
struct drm_panel *panel;
  }

  static void drm_panel_bridge_enable(struct drm_bridge *bridge)
  {
struct drm_panel_bridge *pbridge = to_panel_bridge(bridge);
pbridge-panel-funcs-enable(pbridge-panel);
  }

  .. and so on..


then in initialization, order can be managed like:

  if (panel_first)
bridge = drm_composite_bridge_init(panel_bridge, actual_bridge)
  else
bridge = drm_composite_bridge_init(actual_bridge, panel_bridge);

  possibly parametrize drm_panel_bridge if we need to map
panel-enable() to bridge-pre_enable() vs bridge-enable(), etc..

BR,
-R

 So, we leave the choice for the individual bridge chip drivers to
 implement panel
 power up/down controls in the place where they wish to.


 Thanks and regards,
  Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls

2014-04-22 Thread Rob Clark
So what about, rather than adding drm_panel support to each bridge
individually, we introduce a drm_panel_bridge (with a form of
chaining).. ie:

  struct drm_panel_bridge {
struct drm_bridge base;
struct drm_panel *panel;
struct drm_bridge *bridge; /* optional */
  };

  static void drm_panel_bridge_enable(struct drm_bridge *bridge)
  {
struct drm_panel_bridge *pb = to_panel_bridge(bridge);
if (pb-bridge)
  pb-bridge-funcs-enable(pb-bridge);
pb-panel-funcs-enable(pb-panel);
  }

... and so on.

If you don't need a real bridge, just create one of these w/
pb-bridge==NULL, otherwise create it as a wrapper for your real
bridge.

BR,
-R

On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar...@samsung.com wrote:
 attach ptn3460 connector to drm_panel and support drm_panel routines,
 if a valid drm_panel object is passed to ptn3460_init.

 Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
 ---
 Changes since V1:
 Address few coding style comments from Jingoo Han

  drivers/gpu/drm/bridge/Kconfig  |1 +
  drivers/gpu/drm/bridge/ptn3460.c|   20 +++-
  drivers/gpu/drm/exynos/exynos_dp_core.c |   16 
  include/drm/bridge/ptn3460.h|6 --
  4 files changed, 36 insertions(+), 7 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
 index 884923f..3bc6845 100644
 --- a/drivers/gpu/drm/bridge/Kconfig
 +++ b/drivers/gpu/drm/bridge/Kconfig
 @@ -2,4 +2,5 @@ config DRM_PTN3460
 tristate PTN3460 DP/LVDS bridge
 depends on DRM
 select DRM_KMS_HELPER
 +   select DRM_PANEL
 ---help---
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index f1d2afc..3920202 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -19,6 +19,7 @@
  #include linux/i2c.h
  #include linux/gpio.h
  #include linux/delay.h
 +#include drm/drm_panel.h

  #include drmP.h
  #include drm_edid.h
 @@ -38,6 +39,7 @@ struct ptn3460_bridge {
 struct i2c_client *client;
 struct drm_encoder *encoder;
 struct drm_bridge *bridge;
 +   struct drm_panel *panel;
 struct edid *edid;
 int gpio_pd_n;
 int gpio_rst_n;
 @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
 gpio_set_value(ptn_bridge-gpio_rst_n, 1);
 }

 +   drm_panel_pre_enable(ptn_bridge-panel);
 +
 /*
  * There's a bug in the PTN chip where it falsely asserts hotplug 
 before
  * it is fully functional. We're forced to wait for the maximum start 
 up
 @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)

  static void ptn3460_enable(struct drm_bridge *bridge)
  {
 +   struct ptn3460_bridge *ptn_bridge = bridge-driver_private;
 +
 +   if (ptn_bridge-enabled)
 +   drm_panel_enable(ptn_bridge-panel);
  }

  static void ptn3460_disable(struct drm_bridge *bridge)
 @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge)

 ptn_bridge-enabled = false;

 +   drm_panel_disable(ptn_bridge-panel);
 +   drm_panel_post_disable(ptn_bridge-panel);
 +
 if (gpio_is_valid(ptn_bridge-gpio_rst_n))
 gpio_set_value(ptn_bridge-gpio_rst_n, 1);

 @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector)

 power_off = !ptn_bridge-enabled;
 ptn3460_pre_enable(ptn_bridge-bridge);
 +   ptn3460_enable(ptn_bridge-bridge);

 edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
 if (!edid) {
 @@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = {
  };

  int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
 -   struct i2c_client *client, struct device_node *node)
 +   struct i2c_client *client, struct device_node *node,
 +   struct drm_panel *panel)
  {
 int ret;
 struct drm_bridge *bridge;
 @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct 
 drm_encoder *encoder,
 goto err;
 }

 +   if (panel) {
 +   ptn_bridge-panel = panel;
 +   drm_panel_attach(ptn_bridge-panel, ptn_bridge-connector);
 +   }
 +
 bridge-driver_private = ptn_bridge;
 encoder-bridge = bridge;
 ptn_bridge-connector.polled = DRM_CONNECTOR_POLL_HPD;
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index dbc5ccc..4853f31 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -989,13 +989,14 @@ static bool find_bridge(const char *compat, struct 
 bridge_init *bridge)

  /* returns the number of bridges attached */
  static int exynos_drm_attach_lcd_bridge(struct drm_device *dev,
 -   struct drm_encoder *encoder)
 +   struct drm_encoder *encoder, struct 

Re: [PATCH] dma-buf: avoid using IS_ERR_OR_NULL

2013-12-21 Thread Rob Clark
On Fri, Dec 20, 2013 at 7:43 PM, Colin Cross ccr...@android.com wrote:
 dma_buf_map_attachment and dma_buf_vmap can return NULL or
 ERR_PTR on a error.  This encourages a common buggy pattern in
 callers:
 sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
 if (IS_ERR_OR_NULL(sgt))
 return PTR_ERR(sgt);

 This causes the caller to return 0 on an error.  IS_ERR_OR_NULL
 is almost always a sign of poorly-defined error handling.

 This patch converts dma_buf_map_attachment to always return
 ERR_PTR, and fixes the callers that incorrectly handled NULL.
 There are a few more callers that were not checking for NULL
 at all, which would have dereferenced a NULL pointer later.
 There are also a few more callers that correctly handled NULL
 and ERR_PTR differently, I left those alone but they could also
 be modified to delete the NULL check.

 This patch also converts dma_buf_vmap to always return NULL.
 All the callers to dma_buf_vmap only check for NULL, and would
 have dereferenced an ERR_PTR and panic'd if one was ever
 returned. This is not consistent with the rest of the dma buf
 APIs, but matches the expectations of all of the callers.

 Signed-off-by: Colin Cross ccr...@android.com
 ---
  drivers/base/dma-buf.c | 18 +++---
  drivers/gpu/drm/drm_prime.c|  2 +-
  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |  2 +-
  drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
  4 files changed, 14 insertions(+), 10 deletions(-)

 diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
 index 1e16cbd61da2..cfe1d8bc7bb8 100644
 --- a/drivers/base/dma-buf.c
 +++ b/drivers/base/dma-buf.c
 @@ -251,9 +251,8 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
   * @dmabuf:[in]buffer to attach device to.
   * @dev:   [in]device to be attached.
   *
 - * Returns struct dma_buf_attachment * for this attachment; may return 
 negative
 - * error codes.
 - *
 + * Returns struct dma_buf_attachment * for this attachment; returns ERR_PTR 
 on
 + * error.
   */
  struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
   struct device *dev)
 @@ -319,9 +318,8 @@ EXPORT_SYMBOL_GPL(dma_buf_detach);
   * @attach:[in]attachment whose scatterlist is to be returned
   * @direction: [in]direction of DMA transfer
   *
 - * Returns sg_table containing the scatterlist to be returned; may return 
 NULL
 - * or ERR_PTR.
 - *
 + * Returns sg_table containing the scatterlist to be returned; returns 
 ERR_PTR
 + * on error.
   */
  struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
 enum dma_data_direction direction)
 @@ -334,6 +332,8 @@ struct sg_table *dma_buf_map_attachment(struct 
 dma_buf_attachment *attach,
 return ERR_PTR(-EINVAL);

 sg_table = attach-dmabuf-ops-map_dma_buf(attach, direction);
 +   if (!sg_table)
 +   sg_table = ERR_PTR(-ENOMEM);

 return sg_table;
  }
 @@ -544,6 +544,8 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
   * These calls are optional in drivers. The intended use for them
   * is for mapping objects linear in kernel space for high use objects.
   * Please attempt to use kmap/kunmap before thinking about these interfaces.
 + *
 + * Returns NULL on error.
   */
  void *dma_buf_vmap(struct dma_buf *dmabuf)
  {
 @@ -566,7 +568,9 @@ void *dma_buf_vmap(struct dma_buf *dmabuf)
 BUG_ON(dmabuf-vmap_ptr);

 ptr = dmabuf-ops-vmap(dmabuf);
 -   if (IS_ERR_OR_NULL(ptr))
 +   if (WARN_ON_ONCE(IS_ERR(ptr)))

since vmap is optional, the WARN_ON might be a bit strong..  although
it would be a bit strange for an exporter to supply a vmap fxn which
always returned NULL, not sure about that.  Just thought I'd mention
it in case anyone else had an opinion about that.

But either way:

Reviewed-by: Rob Clark robdcl...@gmail.com


 +   ptr = NULL;
 +   if (!ptr)
 goto out_unlock;

 dmabuf-vmap_ptr = ptr;
 diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
 index 56805c39c906..bb516fdd195d 100644
 --- a/drivers/gpu/drm/drm_prime.c
 +++ b/drivers/gpu/drm/drm_prime.c
 @@ -471,7 +471,7 @@ struct drm_gem_object *drm_gem_prime_import(struct 
 drm_device *dev,
 get_dma_buf(dma_buf);

 sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
 -   if (IS_ERR_OR_NULL(sgt)) {
 +   if (IS_ERR(sgt)) {
 ret = PTR_ERR(sgt);
 goto fail_detach;
 }
 diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c 
 b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
 index 59827cc5e770..c786cd4f457b 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
 @@ -224,7 +224,7 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct 
 drm_device *drm_dev,
 get_dma_buf(dma_buf);

 sgt

Re: [PATCH V2] drm/exynos: Add fallback option to get non physically continous memory for fb

2013-08-05 Thread Rob Clark
On Mon, Aug 5, 2013 at 5:44 AM, Vikas Sajjan vikas.saj...@linaro.org wrote:
 While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
 connected with resolution 2560x1600, following error occured even with
 IOMMU enabled:
 [0.88] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate buffer.
 [0.89] [drm] Initialized exynos 1.0.0 20110530 on minor 0

 To address the case where physically continous memory MAY NOT be a
 mandatory requirement for fb, the patch adds a feature to get non physically
 continous memory for fb if IOMMU is supported and if CONTIG memory allocation
 fails.


Reviewed-by: Rob Clark robdcl...@gmail.com


 Signed-off-by: Vikas Sajjan vikas.saj...@linaro.org
 Signed-off-by: Arun Kumar arun...@samsung.com
 ---
 changes since v1:
  - Modified to add the fallback patch if CONTIG alloc fails as 
 suggested
  by Rob Clark robdcl...@gmail.com and Tomasz Figa 
 tomasz.f...@gmail.com.

  - changed the commit message.
 ---
  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   19 +++
  1 file changed, 15 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c 
 b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
 index 8e60bd6..9a4b886 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
 @@ -16,6 +16,7 @@
  #include drm/drm_crtc.h
  #include drm/drm_fb_helper.h
  #include drm/drm_crtc_helper.h
 +#include drm/exynos_drm.h

  #include exynos_drm_drv.h
  #include exynos_drm_fb.h
 @@ -165,11 +166,21 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper 
 *helper,

 size = mode_cmd.pitches[0] * mode_cmd.height;

 -   /* 0 means to allocate physically continuous memory */
 -   exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
 +   exynos_gem_obj = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG, size);
 if (IS_ERR(exynos_gem_obj)) {
 -   ret = PTR_ERR(exynos_gem_obj);
 -   goto err_release_framebuffer;
 +   /*
 +* If IOMMU is supported then try to get buffer from
 +* non-continous memory area
 +*/
 +   if (is_drm_iommu_supported(dev))
 +   exynos_gem_obj = exynos_drm_gem_create(dev,
 +   EXYNOS_BO_NONCONTIG, size);
 +   if (IS_ERR(exynos_gem_obj)) {
 +   ret = PTR_ERR(exynos_gem_obj);
 +   goto err_release_framebuffer;
 +   }
 +   dev_warn(pdev-dev, exynos_gem_obj for FB is allocated 
 with\n
 +   non physically continuous memory\n);
 }

 exynos_fbdev-exynos_gem_obj = exynos_gem_obj;
 --
 1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] drm/exynos: Add check for IOMMU while passing physically continous memory flag

2013-08-01 Thread Rob Clark
On Thu, Aug 1, 2013 at 7:20 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
 Hi Vikas,

 On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote:
 While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
 connected with resolution 2560x1600, following error occured even with
 IOMMU enabled:
 [0.88] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate
 buffer. [0.89] [drm] Initialized exynos 1.0.0 20110530 on minor 0

 This patch fixes the issue by adding a check for IOMMU.

 Signed-off-by: Vikas Sajjan vikas.saj...@linaro.org
 Signed-off-by: Arun Kumar arun...@samsung.com
 ---
  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
 b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 8e60bd6..2a8
 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
 @@ -16,6 +16,7 @@
  #include drm/drm_crtc.h
  #include drm/drm_fb_helper.h
  #include drm/drm_crtc_helper.h
 +#include drm/exynos_drm.h

  #include exynos_drm_drv.h
  #include exynos_drm_fb.h
 @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct
 drm_fb_helper *helper, struct platform_device *pdev = dev-platformdev;
   unsigned long size;
   int ret;
 + unsigned int flag;

   DRM_DEBUG_KMS(surface width(%d), height(%d) and bpp(%d\n,
   sizes-surface_width, sizes-surface_height,
 @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct
 drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height;

   /* 0 means to allocate physically continuous memory */
 - exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
 + if (!is_drm_iommu_supported(dev))
 + flag = 0;
 + else
 + flag = EXYNOS_BO_NONCONTIG;

 While noncontig memory might be used for devices that support IOMMU, there
 should be no problem with using contig memory for them, so this seems more
 like masking the original problem rather than tracking it down.

it is probably a good idea to not require contig memory when it is not
needed for performance or functionality (and if it is only
performance, then fallback gracefully to non-contig).. but yeah, would
be good to know if this is masking another issue all the same

BR,
-R

 Could you check why the allocation fails when requesting contiguous
 memory?

 Best regards,
 Tomasz

 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: exynos-drm-next todo work.

2013-07-07 Thread Rob Clark
On Fri, Jul 5, 2013 at 2:12 PM, Mark Brown broo...@kernel.org wrote:
 On Wed, Jun 19, 2013 at 01:48:15PM +0900, Inki Dae wrote:

  Related to HDMI sound support in Alsa, I have posted a working RFC
  exynos-hdmi to CDF complaint display driver. It registers a separate
  sound card by registering hdmi audio codec dai and cpu-dai, as you
  mentioned. But there is a problem with this approach that I2S being the
  single cpu dai for HDMI and Speaker sound card, only one card can
  support playback at a time.

  I would like to work on this to provide a more refined solution.

 Thanks. However, I think it's not good to use the CDF framework to interface
 between ALSA SoC DAI driver and DRM HDMI driver. The purpose of the CDF
 framework is to provide common interfaces to display bus drivers to Linux
 framebuffer and DRM KMS based display controller drivers as long as I know.
 So my opinion is to implement ALSA Soc DAI driver-it seems like possible to
 use as is-based on a new common framework that the other can also use it.

 You guys really want to be discussing this on the ALSA list and CCing
 the ASoC maintainers...  I do note that OMAP and SH both have HDMI
 support in mainline, whatever they're doing seems like a good place to
 start.

I don't know about SH, but at least OMAP (and I've seen same on MSM
non-upstream driver, and I guess it is the same elsewhere), there end
up being some calls from audio driver to functions exported by the
display driver.  I guess there is probably some better way to do it
compared to how the existing drivers work.

BR,
-R

 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html