Re: [PATCH] omap2+: add drm device

2012-07-03 Thread Tony Lindgren
* Gross, Andy andy.gr...@ti.com [120619 14:17]:
 Tony,
 
 Please queue this patch at your earliest convenience.
 
 We had some discussion on the splitting out of the DMM/Tiler driver
 from the omapdrm driver.  There might be some interest in leveraging
 the Tiler for omapfb.  However, we agreed this can be deferred until
 some other device (omapfb or otherwise) needs to use the Tiler.

OK I'll apply this into devel-board as that's seems to fit the platform
data area.

Regards,

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


Re: [PATCH] omap2+: add drm device

2012-06-19 Thread Gross, Andy
Tony,

Please queue this patch at your earliest convenience.

We had some discussion on the splitting out of the DMM/Tiler driver
from the omapdrm driver.  There might be some interest in leveraging
the Tiler for omapfb.  However, we agreed this can be deferred until
some other device (omapfb or otherwise) needs to use the Tiler.

Regards,

Andy Gross


On Mon, Jun 11, 2012 at 10:51 AM, Rob Clark rob.cl...@linaro.org wrote:

 On Wed, May 23, 2012 at 3:08 PM, Andy Gross andy.gr...@ti.com wrote:
  Register OMAP DRM/KMS platform device.  DMM is split into a
  separate device using hwmod.
 
  Signed-off-by: Andy Gross andy.gr...@ti.com

 Signed-off-by: Rob Clark rob.cl...@linaro.org

  ---
   arch/arm/mach-omap2/Makefile           |    4 ++
   arch/arm/mach-omap2/drm.c              |   61 
  
   drivers/staging/omapdrm/omap_drv.h     |    2 +-
   drivers/staging/omapdrm/omap_priv.h    |   55 
   include/linux/platform_data/omap_drm.h |   52 +++
   5 files changed, 118 insertions(+), 56 deletions(-)
   create mode 100644 arch/arm/mach-omap2/drm.c
   delete mode 100644 drivers/staging/omapdrm/omap_priv.h
   create mode 100644 include/linux/platform_data/omap_drm.h
 
  diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
  index 49f92bc..c301ab7 100644
  --- a/arch/arm/mach-omap2/Makefile
  +++ b/arch/arm/mach-omap2/Makefile
  @@ -187,6 +187,10 @@ ifneq ($(CONFIG_TIDSPBRIDGE),)
   obj-y                                  += dsp.o
   endif
 
  +ifneq ($(CONFIG_DRM_OMAP),)
  +obj-y                                  += drm.o
  +endif
  +
   # Specific board support
   obj-$(CONFIG_MACH_OMAP_GENERIC)                += board-generic.o
   obj-$(CONFIG_MACH_OMAP_H4)             += board-h4.o
  diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
  new file mode 100644
  index 000..72e0f01b
  --- /dev/null
  +++ b/arch/arm/mach-omap2/drm.c
  @@ -0,0 +1,61 @@
  +/*
  + * DRM/KMS device registration for TI OMAP platforms
  + *
  + * Copyright (C) 2012 Texas Instruments
  + * Author: Rob Clark rob.cl...@linaro.org
  + *
  + * This program is free software; you can redistribute it and/or modify it
  + * under the terms of the GNU General Public License version 2 as 
  published by
  + * the Free Software Foundation.
  + *
  + * This program is distributed in the hope that it will be useful, but 
  WITHOUT
  + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
  + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
  for
  + * more details.
  + *
  + * You should have received a copy of the GNU General Public License along 
  with
  + * this program.  If not, see http://www.gnu.org/licenses/.
  + */
  +
  +#include linux/module.h
  +#include linux/kernel.h
  +#include linux/mm.h
  +#include linux/init.h
  +#include linux/platform_device.h
  +#include linux/dma-mapping.h
  +
  +#include plat/omap_device.h
  +#include plat/omap_hwmod.h
  +
  +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
  +
  +static struct platform_device omap_drm_device = {
  +       .dev = {
  +               .coherent_dma_mask = DMA_BIT_MASK(32),
  +       },
  +       .name = omapdrm,
  +       .id = 0,
  +};
  +
  +static int __init omap_init_drm(void)
  +{
  +       struct omap_hwmod *oh = NULL;
  +       struct platform_device *pdev;
  +
  +       /* lookup and populate the DMM information, if present - OMAP4+ */
  +       oh = omap_hwmod_lookup(dmm);
  +
  +       if (oh) {
  +               pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
  +                                       false);
  +               WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
  +                       oh-name);
  +       }
  +
  +       return platform_device_register(omap_drm_device);
  +
  +}
  +
  +arch_initcall(omap_init_drm);
  +
  +#endif
  diff --git a/drivers/staging/omapdrm/omap_drv.h 
  b/drivers/staging/omapdrm/omap_drv.h
  index b7e0f07..96296e0 100644
  --- a/drivers/staging/omapdrm/omap_drv.h
  +++ b/drivers/staging/omapdrm/omap_drv.h
  @@ -25,8 +25,8 @@
   #include linux/types.h
   #include drm/drmP.h
   #include drm/drm_crtc_helper.h
  +#include linux/platform_data/omap_drm.h
   #include omap_drm.h
  -#include omap_priv.h
 
   #define DBG(fmt, ...) DRM_DEBUG(fmt\n, ##__VA_ARGS__)
   #define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose 
  debug */
  diff --git a/drivers/staging/omapdrm/omap_priv.h 
  b/drivers/staging/omapdrm/omap_priv.h
  deleted file mode 100644
  index ef64414..000
  --- a/drivers/staging/omapdrm/omap_priv.h
  +++ /dev/null
  @@ -1,55 +0,0 @@
  -/*
  - * include/drm/omap_priv.h
  - *
  - * Copyright (C) 2011 Texas Instruments
  - * Author: Rob Clark r...@ti.com
  - *
  - * This program is free software; you can redistribute it and/or modify it
  - * under the terms of the GNU General Public License version 2 as 
  published by
  

Re: [PATCH] omap2+: add drm device

2012-06-11 Thread Gross, Andy
Tomi,

So at this point, are you OK with deferring a split of the DMM until it
necessary to do so (if ever)?  I'd like to get this patch in so that people
have a working omapdrm device when they enable the config options.

Regards,

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


Re: [PATCH] omap2+: add drm device

2012-06-11 Thread Tomi Valkeinen
On Mon, 2012-06-11 at 09:51 -0500, Gross, Andy wrote:
 Tomi,
 
 
 So at this point, are you OK with deferring a split of the DMM until
 it necessary to do so (if ever)?  I'd like to get this patch in so
 that people have a working omapdrm device when they enable the config
 options.

Yes, I'm ok with it.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-06-11 Thread Rob Clark
On Wed, May 23, 2012 at 3:08 PM, Andy Gross andy.gr...@ti.com wrote:
 Register OMAP DRM/KMS platform device.  DMM is split into a
 separate device using hwmod.

 Signed-off-by: Andy Gross andy.gr...@ti.com

Signed-off-by: Rob Clark rob.cl...@linaro.org

 ---
  arch/arm/mach-omap2/Makefile           |    4 ++
  arch/arm/mach-omap2/drm.c              |   61 
 
  drivers/staging/omapdrm/omap_drv.h     |    2 +-
  drivers/staging/omapdrm/omap_priv.h    |   55 
  include/linux/platform_data/omap_drm.h |   52 +++
  5 files changed, 118 insertions(+), 56 deletions(-)
  create mode 100644 arch/arm/mach-omap2/drm.c
  delete mode 100644 drivers/staging/omapdrm/omap_priv.h
  create mode 100644 include/linux/platform_data/omap_drm.h

 diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
 index 49f92bc..c301ab7 100644
 --- a/arch/arm/mach-omap2/Makefile
 +++ b/arch/arm/mach-omap2/Makefile
 @@ -187,6 +187,10 @@ ifneq ($(CONFIG_TIDSPBRIDGE),)
  obj-y                                  += dsp.o
  endif

 +ifneq ($(CONFIG_DRM_OMAP),)
 +obj-y                                  += drm.o
 +endif
 +
  # Specific board support
  obj-$(CONFIG_MACH_OMAP_GENERIC)                += board-generic.o
  obj-$(CONFIG_MACH_OMAP_H4)             += board-h4.o
 diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
 new file mode 100644
 index 000..72e0f01b
 --- /dev/null
 +++ b/arch/arm/mach-omap2/drm.c
 @@ -0,0 +1,61 @@
 +/*
 + * DRM/KMS device registration for TI OMAP platforms
 + *
 + * Copyright (C) 2012 Texas Instruments
 + * Author: Rob Clark rob.cl...@linaro.org
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License version 2 as published 
 by
 + * the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 + * more details.
 + *
 + * You should have received a copy of the GNU General Public License along 
 with
 + * this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/module.h
 +#include linux/kernel.h
 +#include linux/mm.h
 +#include linux/init.h
 +#include linux/platform_device.h
 +#include linux/dma-mapping.h
 +
 +#include plat/omap_device.h
 +#include plat/omap_hwmod.h
 +
 +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
 +
 +static struct platform_device omap_drm_device = {
 +       .dev = {
 +               .coherent_dma_mask = DMA_BIT_MASK(32),
 +       },
 +       .name = omapdrm,
 +       .id = 0,
 +};
 +
 +static int __init omap_init_drm(void)
 +{
 +       struct omap_hwmod *oh = NULL;
 +       struct platform_device *pdev;
 +
 +       /* lookup and populate the DMM information, if present - OMAP4+ */
 +       oh = omap_hwmod_lookup(dmm);
 +
 +       if (oh) {
 +               pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
 +                                       false);
 +               WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
 +                       oh-name);
 +       }
 +
 +       return platform_device_register(omap_drm_device);
 +
 +}
 +
 +arch_initcall(omap_init_drm);
 +
 +#endif
 diff --git a/drivers/staging/omapdrm/omap_drv.h 
 b/drivers/staging/omapdrm/omap_drv.h
 index b7e0f07..96296e0 100644
 --- a/drivers/staging/omapdrm/omap_drv.h
 +++ b/drivers/staging/omapdrm/omap_drv.h
 @@ -25,8 +25,8 @@
  #include linux/types.h
  #include drm/drmP.h
  #include drm/drm_crtc_helper.h
 +#include linux/platform_data/omap_drm.h
  #include omap_drm.h
 -#include omap_priv.h

  #define DBG(fmt, ...) DRM_DEBUG(fmt\n, ##__VA_ARGS__)
  #define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug 
 */
 diff --git a/drivers/staging/omapdrm/omap_priv.h 
 b/drivers/staging/omapdrm/omap_priv.h
 deleted file mode 100644
 index ef64414..000
 --- a/drivers/staging/omapdrm/omap_priv.h
 +++ /dev/null
 @@ -1,55 +0,0 @@
 -/*
 - * include/drm/omap_priv.h
 - *
 - * Copyright (C) 2011 Texas Instruments
 - * Author: Rob Clark r...@ti.com
 - *
 - * This program is free software; you can redistribute it and/or modify it
 - * under the terms of the GNU General Public License version 2 as published 
 by
 - * the Free Software Foundation.
 - *
 - * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 - * more details.
 - *
 - * You should have received a copy of the GNU General Public License along 
 with
 - * this program.  If not, see http://www.gnu.org/licenses/.
 - */
 -
 -#ifndef __OMAP_PRIV_H__
 -#define __OMAP_PRIV_H__
 -
 -/* Non-userspace facing APIs
 - */
 -
 -/* optional 

Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Tomi Valkeinen
Hi,

On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
 Register OMAP DRM/KMS platform device.  DMM is split into a
 separate device using hwmod.
 
 Signed-off-by: Andy Gross andy.gr...@ti.com

snip

 +static int __init omap_init_drm(void)
 +{
 + struct omap_hwmod *oh = NULL;
 + struct platform_device *pdev;
 +
 + /* lookup and populate the DMM information, if present - OMAP4+ */
 + oh = omap_hwmod_lookup(dmm);
 +
 + if (oh) {
 + pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
 + false);
 + WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
 + oh-name);
 + }
 +
 + return platform_device_register(omap_drm_device);
 +
 +}

I still don't like fixing the tiler to drm. I would like to have basic
tiler support in omapfb also, but with this approach I'll need to
duplicate the code. And even if we disregard omapfb, wouldn't it be
architecturally better to have the tiler as a separate independent
library/driver?

 +struct omap_drm_platform_data {
 + struct omap_kms_platform_data *kms_pdata;
 +};

This one is missing struct omap_dmm_platform_data *dmm_pdata, so you
didn't just move the struct. Is that on purpose?

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Clark, Rob
On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 Hi,

 On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
 Register OMAP DRM/KMS platform device.  DMM is split into a
 separate device using hwmod.

 Signed-off-by: Andy Gross andy.gr...@ti.com

 snip

 +static int __init omap_init_drm(void)
 +{
 +     struct omap_hwmod *oh = NULL;
 +     struct platform_device *pdev;
 +
 +     /* lookup and populate the DMM information, if present - OMAP4+ */
 +     oh = omap_hwmod_lookup(dmm);
 +
 +     if (oh) {
 +             pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
 +                                     false);
 +             WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
 +                     oh-name);
 +     }
 +
 +     return platform_device_register(omap_drm_device);
 +
 +}

 I still don't like fixing the tiler to drm. I would like to have basic
 tiler support in omapfb also, but with this approach I'll need to
 duplicate the code. And even if we disregard omapfb, wouldn't it be
 architecturally better to have the tiler as a separate independent
 library/driver?

Not easily, at least not if we want to manage to use tiler/dmm in a
more dynamic way, or to enable some additional features which are
still on the roadmap (like reprogramming dmm synchronized w/ scanout,
or some things which are coming if future hw generations).  We need
one place to keep track of which buffers are potentially evictable to
make room for mapping a new buffer.  And if you look at the tricks
that go on with mmap'ing tiled buffers to userspace, you *really*
don't want to duplicate that in N different drivers.

Fortunately with dmabuf there is not really a need for N different
drivers to need to use tiler/dmm directly.  The dmabuf mechanism
provides what they need to import GEM buffers from omapdrm.  That may
not really help omapfb because fbdev doesn't have a concept of
importing buffers.  But OTOH this is unnecessary, because drm provides
an fbdev interface for legacy apps.  The best thing I'd recommend is,
if you miss some features of omapfb in the drm fbdev implementation,
is to send some patches to add this missing features.

 +struct omap_drm_platform_data {
 +     struct omap_kms_platform_data *kms_pdata;
 +};

 This one is missing struct omap_dmm_platform_data *dmm_pdata, so you
 didn't just move the struct. Is that on purpose?

the dmm pdata is no longer needed because we get what we need from
hwmod via platform_get_resource()

BR,
-R

  Tomi

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


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Tomi Valkeinen
On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
 On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  Hi,
 
  On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
  Register OMAP DRM/KMS platform device.  DMM is split into a
  separate device using hwmod.
 
  Signed-off-by: Andy Gross andy.gr...@ti.com
 
  snip
 
  +static int __init omap_init_drm(void)
  +{
  + struct omap_hwmod *oh = NULL;
  + struct platform_device *pdev;
  +
  + /* lookup and populate the DMM information, if present - OMAP4+ */
  + oh = omap_hwmod_lookup(dmm);
  +
  + if (oh) {
  + pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
  + false);
  + WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
  + oh-name);
  + }
  +
  + return platform_device_register(omap_drm_device);
  +
  +}
 
  I still don't like fixing the tiler to drm. I would like to have basic
  tiler support in omapfb also, but with this approach I'll need to
  duplicate the code. And even if we disregard omapfb, wouldn't it be
  architecturally better to have the tiler as a separate independent
  library/driver?
 
 Not easily, at least not if we want to manage to use tiler/dmm in a
 more dynamic way, or to enable some additional features which are
 still on the roadmap (like reprogramming dmm synchronized w/ scanout,
 or some things which are coming if future hw generations).  We need
 one place to keep track of which buffers are potentially evictable to
 make room for mapping a new buffer.  And if you look at the tricks
 that go on with mmap'ing tiled buffers to userspace, you *really*
 don't want to duplicate that in N different drivers.

So why can't all that code be in a tiler library/driver?

 Fortunately with dmabuf there is not really a need for N different
 drivers to need to use tiler/dmm directly.  The dmabuf mechanism
 provides what they need to import GEM buffers from omapdrm.  That may
 not really help omapfb because fbdev doesn't have a concept of
 importing buffers.  But OTOH this is unnecessary, because drm provides
 an fbdev interface for legacy apps.  The best thing I'd recommend is,
 if you miss some features of omapfb in the drm fbdev implementation,
 is to send some patches to add this missing features.

Well, at least currently omapfb and omapdrm work quite differently, if
I've understood right. Can we make a full omapfb layer on top of
omapdrm? With multiple framebuffers mapped to one or more overlays,
support for all the ioctls, etc?

I guess we'd still need to have omapfb driver to keep the module
parameters and behavior the same. Can omapdrm be used from inside the
kernel by another driver?

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Tomi Valkeinen
On Thu, 2012-05-24 at 10:05 +0300, Tomi Valkeinen wrote:
 On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
  On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
   Hi,
  
   On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
   Register OMAP DRM/KMS platform device.  DMM is split into a
   separate device using hwmod.
  
   Signed-off-by: Andy Gross andy.gr...@ti.com
  
   snip
  
   +static int __init omap_init_drm(void)
   +{
   + struct omap_hwmod *oh = NULL;
   + struct platform_device *pdev;
   +
   + /* lookup and populate the DMM information, if present - OMAP4+ */
   + oh = omap_hwmod_lookup(dmm);
   +
   + if (oh) {
   + pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 
   0,
   + false);
   + WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
   + oh-name);
   + }
   +
   + return platform_device_register(omap_drm_device);
   +
   +}
  
   I still don't like fixing the tiler to drm. I would like to have basic
   tiler support in omapfb also, but with this approach I'll need to
   duplicate the code. And even if we disregard omapfb, wouldn't it be
   architecturally better to have the tiler as a separate independent
   library/driver?
  
  Not easily, at least not if we want to manage to use tiler/dmm in a
  more dynamic way, or to enable some additional features which are
  still on the roadmap (like reprogramming dmm synchronized w/ scanout,
  or some things which are coming if future hw generations).  We need
  one place to keep track of which buffers are potentially evictable to
  make room for mapping a new buffer.  And if you look at the tricks
  that go on with mmap'ing tiled buffers to userspace, you *really*
  don't want to duplicate that in N different drivers.
 
 So why can't all that code be in a tiler library/driver?

And I think we've discussed about this before, so sorry if I'm repeating
myself. I just find it odd that we are not able to create a nice
separate lib/driver for the tiler, which is a separate piece of HW that
multiple drivers might want to use.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Rob Clark
On Thu, May 24, 2012 at 1:05 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
 On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  Hi,
 
  On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
  Register OMAP DRM/KMS platform device.  DMM is split into a
  separate device using hwmod.
 
  Signed-off-by: Andy Gross andy.gr...@ti.com
 
  snip
 
  +static int __init omap_init_drm(void)
  +{
  +     struct omap_hwmod *oh = NULL;
  +     struct platform_device *pdev;
  +
  +     /* lookup and populate the DMM information, if present - OMAP4+ */
  +     oh = omap_hwmod_lookup(dmm);
  +
  +     if (oh) {
  +             pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
  +                                     false);
  +             WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
  +                     oh-name);
  +     }
  +
  +     return platform_device_register(omap_drm_device);
  +
  +}
 
  I still don't like fixing the tiler to drm. I would like to have basic
  tiler support in omapfb also, but with this approach I'll need to
  duplicate the code. And even if we disregard omapfb, wouldn't it be
  architecturally better to have the tiler as a separate independent
  library/driver?

 Not easily, at least not if we want to manage to use tiler/dmm in a
 more dynamic way, or to enable some additional features which are
 still on the roadmap (like reprogramming dmm synchronized w/ scanout,
 or some things which are coming if future hw generations).  We need
 one place to keep track of which buffers are potentially evictable to
 make room for mapping a new buffer.  And if you look at the tricks
 that go on with mmap'ing tiled buffers to userspace, you *really*
 don't want to duplicate that in N different drivers.

 So why can't all that code be in a tiler library/driver?

Possibly the placement stuff could be in a library..  the
mmap/faulting stuff I think would be harder to split.  With it split
out in a separate lib, it becomes logistically a bit more of a
headache to change APIs, etc.  Basically it just makes more work and
is unnecessary.

 Fortunately with dmabuf there is not really a need for N different
 drivers to need to use tiler/dmm directly.  The dmabuf mechanism
 provides what they need to import GEM buffers from omapdrm.  That may
 not really help omapfb because fbdev doesn't have a concept of
 importing buffers.  But OTOH this is unnecessary, because drm provides
 an fbdev interface for legacy apps.  The best thing I'd recommend is,
 if you miss some features of omapfb in the drm fbdev implementation,
 is to send some patches to add this missing features.

 Well, at least currently omapfb and omapdrm work quite differently, if
 I've understood right. Can we make a full omapfb layer on top of
 omapdrm? With multiple framebuffers mapped to one or more overlays,
 support for all the ioctls, etc?

Well, there is still room to add your own fb_ioctl() fxn, so I guess
in principle it should be possible to add whatever custom ioctls are
required.

Typically you have a single fbdev device for a single drm device,
although I suppose nothing prevents creating more.  I'd probably want
to encourage users more towards using KMS directly for multi-display
cases because you have a lot more options/flexibility that way.

 I guess we'd still need to have omapfb driver to keep the module
 parameters and behavior the same. Can omapdrm be used from inside the
 kernel by another driver?

Hmm, I'm not quite sure what you have in mind, but it sounds a bit
hacky..  I'd guess if you need 100% backwards compatibility even down
to kernel cmdline / module params, then you probably want to use
omapfb.  But there isn't really need to add new features to omapfb in
that case.

Off the top of my head, I guess that 80-90% compatibility would
probably be reasonable to add to omapdrm's fbdev..  and that the last
10-20% would be too hacky/invasive to justify adding to omapdrm.

BR,
-R

  Tomi


 ___
 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Rob Clark
On Thu, May 24, 2012 at 1:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-05-24 at 10:05 +0300, Tomi Valkeinen wrote:
 On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
  On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
   Hi,
  
   On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
   Register OMAP DRM/KMS platform device.  DMM is split into a
   separate device using hwmod.
  
   Signed-off-by: Andy Gross andy.gr...@ti.com
  
   snip
  
   +static int __init omap_init_drm(void)
   +{
   +     struct omap_hwmod *oh = NULL;
   +     struct platform_device *pdev;
   +
   +     /* lookup and populate the DMM information, if present - OMAP4+ */
   +     oh = omap_hwmod_lookup(dmm);
   +
   +     if (oh) {
   +             pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 
   0,
   +                                     false);
   +             WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
   +                     oh-name);
   +     }
   +
   +     return platform_device_register(omap_drm_device);
   +
   +}
  
   I still don't like fixing the tiler to drm. I would like to have basic
   tiler support in omapfb also, but with this approach I'll need to
   duplicate the code. And even if we disregard omapfb, wouldn't it be
   architecturally better to have the tiler as a separate independent
   library/driver?
 
  Not easily, at least not if we want to manage to use tiler/dmm in a
  more dynamic way, or to enable some additional features which are
  still on the roadmap (like reprogramming dmm synchronized w/ scanout,
  or some things which are coming if future hw generations).  We need
  one place to keep track of which buffers are potentially evictable to
  make room for mapping a new buffer.  And if you look at the tricks
  that go on with mmap'ing tiled buffers to userspace, you *really*
  don't want to duplicate that in N different drivers.

 So why can't all that code be in a tiler library/driver?

 And I think we've discussed about this before, so sorry if I'm repeating
 myself. I just find it odd that we are not able to create a nice
 separate lib/driver for the tiler, which is a separate piece of HW that
 multiple drivers might want to use.

but other drivers *can* use tiler, thanks to dmabuf.. I have omap4iss
v4l2 camera working w/ tiler buffers on my pandaboard, for example.

Maybe fbdev is an exception to the rule because it has no way for
userspace to pass it a buffer to use.  But on the other hand it is a
legacy API so I'm not sure if it is worth loosing too much sleep over
that.

BR,
-R

  Tomi


 ___
 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Tomi Valkeinen
On Thu, 2012-05-24 at 02:35 -0600, Rob Clark wrote:
 On Thu, May 24, 2012 at 1:05 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
  On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
   Hi,
  
   On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
   Register OMAP DRM/KMS platform device.  DMM is split into a
   separate device using hwmod.
  
   Signed-off-by: Andy Gross andy.gr...@ti.com
  
   snip
  
   +static int __init omap_init_drm(void)
   +{
   + struct omap_hwmod *oh = NULL;
   + struct platform_device *pdev;
   +
   + /* lookup and populate the DMM information, if present - OMAP4+ */
   + oh = omap_hwmod_lookup(dmm);
   +
   + if (oh) {
   + pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 
   0,
   + false);
   + WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
   + oh-name);
   + }
   +
   + return platform_device_register(omap_drm_device);
   +
   +}
  
   I still don't like fixing the tiler to drm. I would like to have basic
   tiler support in omapfb also, but with this approach I'll need to
   duplicate the code. And even if we disregard omapfb, wouldn't it be
   architecturally better to have the tiler as a separate independent
   library/driver?
 
  Not easily, at least not if we want to manage to use tiler/dmm in a
  more dynamic way, or to enable some additional features which are
  still on the roadmap (like reprogramming dmm synchronized w/ scanout,
  or some things which are coming if future hw generations).  We need
  one place to keep track of which buffers are potentially evictable to
  make room for mapping a new buffer.  And if you look at the tricks
  that go on with mmap'ing tiled buffers to userspace, you *really*
  don't want to duplicate that in N different drivers.
 
  So why can't all that code be in a tiler library/driver?
 
 Possibly the placement stuff could be in a library..  the
 mmap/faulting stuff I think would be harder to split.  With it split
 out in a separate lib, it becomes logistically a bit more of a
 headache to change APIs, etc.  Basically it just makes more work and
 is unnecessary.

Unnecessary for you, but maybe not for those who want to use omapfb.

  Fortunately with dmabuf there is not really a need for N different
  drivers to need to use tiler/dmm directly.  The dmabuf mechanism
  provides what they need to import GEM buffers from omapdrm.  That may
  not really help omapfb because fbdev doesn't have a concept of
  importing buffers.  But OTOH this is unnecessary, because drm provides
  an fbdev interface for legacy apps.  The best thing I'd recommend is,
  if you miss some features of omapfb in the drm fbdev implementation,
  is to send some patches to add this missing features.
 
  Well, at least currently omapfb and omapdrm work quite differently, if
  I've understood right. Can we make a full omapfb layer on top of
  omapdrm? With multiple framebuffers mapped to one or more overlays,
  support for all the ioctls, etc?
 
 Well, there is still room to add your own fb_ioctl() fxn, so I guess
 in principle it should be possible to add whatever custom ioctls are
 required.
 
 Typically you have a single fbdev device for a single drm device,
 although I suppose nothing prevents creating more.  I'd probably want
 to encourage users more towards using KMS directly for multi-display
 cases because you have a lot more options/flexibility that way.

Sure, but we can't force people to use omapdrm instead of omapfb. And
omapfb is not going to disappear. So obviously we should recommend using
omapdrm, but on the other hand, I don't see any problem in adding new
features to omapfb if they are easily implemented (using, for example, a
separate tiler driver).

  I guess we'd still need to have omapfb driver to keep the module
  parameters and behavior the same. Can omapdrm be used from inside the
  kernel by another driver?
 
 Hmm, I'm not quite sure what you have in mind, but it sounds a bit
 hacky..  I'd guess if you need 100% backwards compatibility even down
 to kernel cmdline / module params, then you probably want to use
 omapfb.  But there isn't really need to add new features to omapfb in
 that case.

I was thinking of making omapfb use omapdrm, instead of omapdss. I mean,
not planning to do that, just wondering if that would be possible.

 Off the top of my head, I guess that 80-90% compatibility would
 probably be reasonable to add to omapdrm's fbdev..  and that the last
 10-20% would be too hacky/invasive to justify adding to omapdrm.

I think it should be 99.9% - 100% or nothing. If it's only 80-90%
compatible, then it's not compatible =).

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Gross, Andy
On Thu, May 24, 2012 at 1:01 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 +struct omap_drm_platform_data {
 +     struct omap_kms_platform_data *kms_pdata;
 +};

 This one is missing struct omap_dmm_platform_data *dmm_pdata, so you
 didn't just move the struct. Is that on purpose?

Good point.  I can clean that up.


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


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Gross, Andy
On Thu, May 24, 2012 at 7:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-05-24 at 02:44 -0600, Rob Clark wrote:

 but other drivers *can* use tiler, thanks to dmabuf.. I have omap4iss
 v4l2 camera working w/ tiler buffers on my pandaboard, for example.

 Maybe fbdev is an exception to the rule because it has no way for
 userspace to pass it a buffer to use.  But on the other hand it is a
 legacy API so I'm not sure if it is worth loosing too much sleep over
 that.

 I'm not that familiar with dmabuf, but are you saying it's not possible
 for a kernel driver to request the buffers? They _must_ come from the
 userspace?

 Anyway, even if it would be possible, if the tiler is a part of omapdrm
 we need omapdrm to get and use the tiler buffers. And we can't have
 omapdrm running when using omapfb, because they both use omapdss.

And that is a good point.  The DSS is kind of a sticking point to anyone
having to enable DRM to get Tiler.  However, omapfb doesn't currently utilize
DMM/Tiler features.  Can't we defer generalizing until there is a
requirement for it?

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


Re: [PATCH] omap2+: add drm device

2012-05-24 Thread Tomi Valkeinen
On Thu, 2012-05-24 at 10:09 -0500, Gross, Andy wrote:
 On Thu, May 24, 2012 at 7:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Thu, 2012-05-24 at 02:44 -0600, Rob Clark wrote:
 
  but other drivers *can* use tiler, thanks to dmabuf.. I have omap4iss
  v4l2 camera working w/ tiler buffers on my pandaboard, for example.
 
  Maybe fbdev is an exception to the rule because it has no way for
  userspace to pass it a buffer to use.  But on the other hand it is a
  legacy API so I'm not sure if it is worth loosing too much sleep over
  that.
 
  I'm not that familiar with dmabuf, but are you saying it's not possible
  for a kernel driver to request the buffers? They _must_ come from the
  userspace?
 
  Anyway, even if it would be possible, if the tiler is a part of omapdrm
  we need omapdrm to get and use the tiler buffers. And we can't have
  omapdrm running when using omapfb, because they both use omapdss.
 
 And that is a good point.  The DSS is kind of a sticking point to anyone
 having to enable DRM to get Tiler.  However, omapfb doesn't currently utilize
 DMM/Tiler features.  Can't we defer generalizing until there is a
 requirement for it?

Sure. I just brought it up because it'd be nice and it'd be better
architecturally. However, as I said, I'm not familiar with the related
problems, so I take your word that it's not simple =).

 Tomi



signature.asc
Description: This is a digitally signed message part


[PATCH] omap2+: add drm device

2012-05-23 Thread Andy Gross
Register OMAP DRM/KMS platform device.  DMM is split into a
separate device using hwmod.

Signed-off-by: Andy Gross andy.gr...@ti.com
---
 arch/arm/mach-omap2/Makefile   |4 ++
 arch/arm/mach-omap2/drm.c  |   61 
 drivers/staging/omapdrm/omap_drv.h |2 +-
 drivers/staging/omapdrm/omap_priv.h|   55 
 include/linux/platform_data/omap_drm.h |   52 +++
 5 files changed, 118 insertions(+), 56 deletions(-)
 create mode 100644 arch/arm/mach-omap2/drm.c
 delete mode 100644 drivers/staging/omapdrm/omap_priv.h
 create mode 100644 include/linux/platform_data/omap_drm.h

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 49f92bc..c301ab7 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -187,6 +187,10 @@ ifneq ($(CONFIG_TIDSPBRIDGE),)
 obj-y  += dsp.o
 endif
 
+ifneq ($(CONFIG_DRM_OMAP),)
+obj-y  += drm.o
+endif
+
 # Specific board support
 obj-$(CONFIG_MACH_OMAP_GENERIC)+= board-generic.o
 obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o
diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
new file mode 100644
index 000..72e0f01b
--- /dev/null
+++ b/arch/arm/mach-omap2/drm.c
@@ -0,0 +1,61 @@
+/*
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Rob Clark rob.cl...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#include linux/module.h
+#include linux/kernel.h
+#include linux/mm.h
+#include linux/init.h
+#include linux/platform_device.h
+#include linux/dma-mapping.h
+
+#include plat/omap_device.h
+#include plat/omap_hwmod.h
+
+#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
+
+static struct platform_device omap_drm_device = {
+   .dev = {
+   .coherent_dma_mask = DMA_BIT_MASK(32),
+   },
+   .name = omapdrm,
+   .id = 0,
+};
+
+static int __init omap_init_drm(void)
+{
+   struct omap_hwmod *oh = NULL;
+   struct platform_device *pdev;
+
+   /* lookup and populate the DMM information, if present - OMAP4+ */
+   oh = omap_hwmod_lookup(dmm);
+
+   if (oh) {
+   pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
+   false);
+   WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
+   oh-name);
+   }
+
+   return platform_device_register(omap_drm_device);
+
+}
+
+arch_initcall(omap_init_drm);
+
+#endif
diff --git a/drivers/staging/omapdrm/omap_drv.h 
b/drivers/staging/omapdrm/omap_drv.h
index b7e0f07..96296e0 100644
--- a/drivers/staging/omapdrm/omap_drv.h
+++ b/drivers/staging/omapdrm/omap_drv.h
@@ -25,8 +25,8 @@
 #include linux/types.h
 #include drm/drmP.h
 #include drm/drm_crtc_helper.h
+#include linux/platform_data/omap_drm.h
 #include omap_drm.h
-#include omap_priv.h
 
 #define DBG(fmt, ...) DRM_DEBUG(fmt\n, ##__VA_ARGS__)
 #define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
diff --git a/drivers/staging/omapdrm/omap_priv.h 
b/drivers/staging/omapdrm/omap_priv.h
deleted file mode 100644
index ef64414..000
--- a/drivers/staging/omapdrm/omap_priv.h
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * include/drm/omap_priv.h
- *
- * Copyright (C) 2011 Texas Instruments
- * Author: Rob Clark r...@ti.com
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published by
- * the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program.  If not, see http://www.gnu.org/licenses/.
- */
-
-#ifndef __OMAP_PRIV_H__
-#define __OMAP_PRIV_H__
-
-/* Non-userspace facing APIs
- */
-
-/* optional platform data to configure the default configuration of which
- * pipes/overlays/CRTCs are used.. if this is not provided, then instead the
- * first CONFIG_DRM_OMAP_NUM_CRTCS are used, and they are each connected to
- * one manager, with priority given to managers that are 

Re: [PATCH] omap2+: add drm device

2012-03-16 Thread Tomi Valkeinen
On Thu, 2012-03-15 at 07:32 -0500, Rob Clark wrote:
 On Thu, Mar 15, 2012 at 3:46 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Wed, 2012-03-14 at 10:06 -0500, Rob Clark wrote:
 
   Well, as I said, it's not an issue for me and from my side it can be
   improved later.
 
  yeah, when CMA is actually merged, there are a few other things I'd
  like to do to, incl converting omapfb over to use CMA and remove
  omap_vram.. but I guess those will be other patches.
 
  Right, I just realized CMA is not in the kernel, nor does it seem to be
  in the linux-next. Is there a reason why you want it already merged?
  Wouldn't it be easier to get it in only when it can actually be used.
  Especially if there's room for improvement.
 
 Some folks are already pulling CMA into product kernels for various
 reasons.. keeping this w/ #ifdef CONFIG_CMA guards seemed like it
 would make their life a bit easier.
 
 But if people feel strongly about it, I can strip that out.

Well, I wouldn't say feel strongly, but... I think the mainline kernel
should have code only for the mainline kernel, not for some custom
kernels. And the code is not testable in any way, not even compilable. I
think all code going in should be tested and compiled. Also, if the CMA
code is not in, who says it won't change. Perhaps the CMA function won't
even exist in the version going into mainline.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-03-15 Thread Tomi Valkeinen
On Wed, 2012-03-14 at 10:06 -0500, Rob Clark wrote:

  Well, as I said, it's not an issue for me and from my side it can be
  improved later.
 
 yeah, when CMA is actually merged, there are a few other things I'd
 like to do to, incl converting omapfb over to use CMA and remove
 omap_vram.. but I guess those will be other patches.

Right, I just realized CMA is not in the kernel, nor does it seem to be
in the linux-next. Is there a reason why you want it already merged?
Wouldn't it be easier to get it in only when it can actually be used.
Especially if there's room for improvement.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-03-15 Thread Rob Clark
On Thu, Mar 15, 2012 at 3:46 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2012-03-14 at 10:06 -0500, Rob Clark wrote:

  Well, as I said, it's not an issue for me and from my side it can be
  improved later.

 yeah, when CMA is actually merged, there are a few other things I'd
 like to do to, incl converting omapfb over to use CMA and remove
 omap_vram.. but I guess those will be other patches.

 Right, I just realized CMA is not in the kernel, nor does it seem to be
 in the linux-next. Is there a reason why you want it already merged?
 Wouldn't it be easier to get it in only when it can actually be used.
 Especially if there's room for improvement.

Some folks are already pulling CMA into product kernels for various
reasons.. keeping this w/ #ifdef CONFIG_CMA guards seemed like it
would make their life a bit easier.

But if people feel strongly about it, I can strip that out.

BR,
-R


  Tomi


 ___
 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap2+: add drm device

2012-03-14 Thread Tomi Valkeinen
Hi,

On Tue, 2012-03-13 at 15:34 -0500, Rob Clark wrote:
 From: Andy Gross andy.gr...@ti.com
 
 Register OMAP DRM/KMS platform device, and reserve a CMA region for
 the device to use for buffer allocation.  DMM is split into a
 separate device using hwmod.

What's the diff with this and the previous one?

I see you still have the platform data there. You didn't comment on
that. Where is it used after the board files are gone when we use DT?

And how about the size of the contiguous memory area, it was left a bit
unclear to me why it cannot be dynamic.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-03-14 Thread Rob Clark
On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 Hi,

 On Tue, 2012-03-13 at 15:34 -0500, Rob Clark wrote:
 From: Andy Gross andy.gr...@ti.com

 Register OMAP DRM/KMS platform device, and reserve a CMA region for
 the device to use for buffer allocation.  DMM is split into a
 separate device using hwmod.

 What's the diff with this and the previous one?

Moving drm.c to mach-omap2 (header could not move because
omap_reserve() is in plat-omap.. but this seems to be the same as what
is done for dspbridge).

 I see you still have the platform data there. You didn't comment on
 that. Where is it used after the board files are gone when we use DT?

I was kind-of thinking that there would be some DT-data-structure
glue somewhere.. not sure if this goes in mach-omap2 or in the driver
itself, but presumably it is needed somewhere.

It is only really special cases where some board specific device-tree
data is needed, so it seems like this is ok to handle later.

 And how about the size of the contiguous memory area, it was left a bit
 unclear to me why it cannot be dynamic.

I don't think there is anything preventing adding a bootarg, but I
think it is not essential so ok to add later

BR,
-R

  Tomi

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


Re: [PATCH] omap2+: add drm device

2012-03-14 Thread Tomi Valkeinen
On Wed, 2012-03-14 at 07:55 -0500, Rob Clark wrote:
 On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  Hi,
 
  On Tue, 2012-03-13 at 15:34 -0500, Rob Clark wrote:
  From: Andy Gross andy.gr...@ti.com
 
  Register OMAP DRM/KMS platform device, and reserve a CMA region for
  the device to use for buffer allocation.  DMM is split into a
  separate device using hwmod.
 
  What's the diff with this and the previous one?
 
 Moving drm.c to mach-omap2 (header could not move because
 omap_reserve() is in plat-omap.. but this seems to be the same as what
 is done for dspbridge).
 
  I see you still have the platform data there. You didn't comment on
  that. Where is it used after the board files are gone when we use DT?
 
 I was kind-of thinking that there would be some DT-data-structure
 glue somewhere.. not sure if this goes in mach-omap2 or in the driver
 itself, but presumably it is needed somewhere.
 
 It is only really special cases where some board specific device-tree
 data is needed, so it seems like this is ok to handle later.

Afaik DT data should only contain information about the hardware. This
is SW configuration, so I think DT people won't accept things like that.

  And how about the size of the contiguous memory area, it was left a bit
  unclear to me why it cannot be dynamic.
 
 I don't think there is anything preventing adding a bootarg, but I
 think it is not essential so ok to add later

Well, maybe not essential to you =). But you are reserving 32MB memory,
which is quite a big amount. Even if the reserved memory can be used for
some other purposes, it's still a big chunk of special memory being
reserved even if the user doesn't use or have a display at all.

Well, it's not an issue for me either, but I just feel that doing things
like that without allowing the user to avoid it is a bit bad thing.

Btw, do you know why the dma_declare_contiguous() takes a dev pointer as
an argument, if the memory is not private to that device? (at least I
understood from you that the memory can be used for other purposes).

 Tomi


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


Re: [PATCH] omap2+: add drm device

2012-03-14 Thread Rob Clark
On Wed, Mar 14, 2012 at 8:07 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2012-03-14 at 07:55 -0500, Rob Clark wrote:
 On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  Hi,
 
  On Tue, 2012-03-13 at 15:34 -0500, Rob Clark wrote:
  From: Andy Gross andy.gr...@ti.com
 
  Register OMAP DRM/KMS platform device, and reserve a CMA region for
  the device to use for buffer allocation.  DMM is split into a
  separate device using hwmod.
 
  What's the diff with this and the previous one?

 Moving drm.c to mach-omap2 (header could not move because
 omap_reserve() is in plat-omap.. but this seems to be the same as what
 is done for dspbridge).

  I see you still have the platform data there. You didn't comment on
  that. Where is it used after the board files are gone when we use DT?

 I was kind-of thinking that there would be some DT-data-structure
 glue somewhere.. not sure if this goes in mach-omap2 or in the driver
 itself, but presumably it is needed somewhere.

 It is only really special cases where some board specific device-tree
 data is needed, so it seems like this is ok to handle later.

 Afaik DT data should only contain information about the hardware. This
 is SW configuration, so I think DT people won't accept things like that.

hmm, then where *does* it go.. it is a bit too much for bootargs..

  And how about the size of the contiguous memory area, it was left a bit
  unclear to me why it cannot be dynamic.

 I don't think there is anything preventing adding a bootarg, but I
 think it is not essential so ok to add later

 Well, maybe not essential to you =). But you are reserving 32MB memory,
 which is quite a big amount. Even if the reserved memory can be used for
 some other purposes, it's still a big chunk of special memory being
 reserved even if the user doesn't use or have a display at all.

If you didn't have display, and were tight on memory, wouldn't you
just disable the display device in your kernel config?

 Well, it's not an issue for me either, but I just feel that doing things
 like that without allowing the user to avoid it is a bit bad thing.

 Btw, do you know why the dma_declare_contiguous() takes a dev pointer as
 an argument, if the memory is not private to that device? (at least I
 understood from you that the memory can be used for other purposes).

contiguous use of the memory is private to the device.  There is also
a global CMA pool, from which all dma_alloc_xyz() which is not
associated w/ a per-device pool comes from.. but the advantage of a
per-device-pool is it puts an upper limit on how much dma memory that
device can allocate so that it cannot starve other devices.

BR,
-R

  Tomi


 ___
 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap2+: add drm device

2012-03-14 Thread Tomi Valkeinen
On Wed, 2012-03-14 at 08:16 -0500, Rob Clark wrote:
 On Wed, Mar 14, 2012 at 8:07 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Wed, 2012-03-14 at 07:55 -0500, Rob Clark wrote:
  On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
   Hi,
  
   On Tue, 2012-03-13 at 15:34 -0500, Rob Clark wrote:
   From: Andy Gross andy.gr...@ti.com
  
   Register OMAP DRM/KMS platform device, and reserve a CMA region for
   the device to use for buffer allocation.  DMM is split into a
   separate device using hwmod.
  
   What's the diff with this and the previous one?
 
  Moving drm.c to mach-omap2 (header could not move because
  omap_reserve() is in plat-omap.. but this seems to be the same as what
  is done for dspbridge).
 
   I see you still have the platform data there. You didn't comment on
   that. Where is it used after the board files are gone when we use DT?
 
  I was kind-of thinking that there would be some DT-data-structure
  glue somewhere.. not sure if this goes in mach-omap2 or in the driver
  itself, but presumably it is needed somewhere.
 
  It is only really special cases where some board specific device-tree
  data is needed, so it seems like this is ok to handle later.
 
  Afaik DT data should only contain information about the hardware. This
  is SW configuration, so I think DT people won't accept things like that.
 
 hmm, then where *does* it go.. it is a bit too much for bootargs..

I have no idea =). And I may be wrong, but my understanding is the the
only thing DT data should have is information about the HW
configuration.

But is that kind of configuration needed at boot time? Why cannot the
userspace do the config? What configs are actually needed at boot time,
before userspace takes control? The only thing coming to my mind is to
define the display used for the console.

   And how about the size of the contiguous memory area, it was left a bit
   unclear to me why it cannot be dynamic.
 
  I don't think there is anything preventing adding a bootarg, but I
  think it is not essential so ok to add later
 
  Well, maybe not essential to you =). But you are reserving 32MB memory,
  which is quite a big amount. Even if the reserved memory can be used for
  some other purposes, it's still a big chunk of special memory being
  reserved even if the user doesn't use or have a display at all.
 
 If you didn't have display, and were tight on memory, wouldn't you
 just disable the display device in your kernel config?

Sure, if you want to modify your kernel. But you could as well use the
same kernel binary, and just say vram=0M which disables the vram
allocation. For DRM you would _have_ to modify your kernel.

Actually, I just realized vram=0M doesn't work, as the code checks for !
= 0, and just skips the vram argument when it's 0 =). So my code sucks
also...

  Well, it's not an issue for me either, but I just feel that doing things
  like that without allowing the user to avoid it is a bit bad thing.
 
  Btw, do you know why the dma_declare_contiguous() takes a dev pointer as
  an argument, if the memory is not private to that device? (at least I
  understood from you that the memory can be used for other purposes).
 
 contiguous use of the memory is private to the device.  There is also
 a global CMA pool, from which all dma_alloc_xyz() which is not
 associated w/ a per-device pool comes from.. but the advantage of a
 per-device-pool is it puts an upper limit on how much dma memory that
 device can allocate so that it cannot starve other devices.

Ah, I see. So the 32MB you reserve there is not visible as contiguous
memory to anyone else than omapdrm, but userspace can still use the 32MB
area for pages that can be moved out as needed.

But then, if it's private, it's also rather bad. If I have a kernel with
omapfb and omapdrm enabled as modules, but I never use omapdrm, the 32MB
is still always reserver and away from other contiguous memory use.

Also, I just realized that besides the memory reservation being fixed,
it's also hidden. If I enable omapdrm in the kernel config, I get no
indication that 32MB of mem is being reserved. Perhaps a Kconfig option
for it, like with OMAP VRAM, and then a boot arg which will override the
Kconfig value.

Well, as I said, it's not an issue for me and from my side it can be
improved later.

 Tomi


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


Re: [PATCH] omap2+: add drm device

2012-03-14 Thread Rob Clark
On Wed, Mar 14, 2012 at 8:43 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2012-03-14 at 08:16 -0500, Rob Clark wrote:
 On Wed, Mar 14, 2012 at 8:07 AM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  On Wed, 2012-03-14 at 07:55 -0500, Rob Clark wrote:
  On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
   Hi,
  
   On Tue, 2012-03-13 at 15:34 -0500, Rob Clark wrote:
   From: Andy Gross andy.gr...@ti.com
  
   Register OMAP DRM/KMS platform device, and reserve a CMA region for
   the device to use for buffer allocation.  DMM is split into a
   separate device using hwmod.
  
   What's the diff with this and the previous one?
 
  Moving drm.c to mach-omap2 (header could not move because
  omap_reserve() is in plat-omap.. but this seems to be the same as what
  is done for dspbridge).
 
   I see you still have the platform data there. You didn't comment on
   that. Where is it used after the board files are gone when we use DT?
 
  I was kind-of thinking that there would be some DT-data-structure
  glue somewhere.. not sure if this goes in mach-omap2 or in the driver
  itself, but presumably it is needed somewhere.
 
  It is only really special cases where some board specific device-tree
  data is needed, so it seems like this is ok to handle later.
 
  Afaik DT data should only contain information about the hardware. This
  is SW configuration, so I think DT people won't accept things like that.

 hmm, then where *does* it go.. it is a bit too much for bootargs..

 I have no idea =). And I may be wrong, but my understanding is the the
 only thing DT data should have is information about the HW
 configuration.

 But is that kind of configuration needed at boot time? Why cannot the
 userspace do the config? What configs are actually needed at boot time,
 before userspace takes control? The only thing coming to my mind is to
 define the display used for the console.

drm builds up list of resources at startup, and attempts to light up
any connected displays at boot..  the decision about what resources to
use must be taken before userspace starts.

Anyways, if it is too controversial, maybe I just remove it.  It is
really only very special cases (possibly multi-seat setups) where you
might want to do something other than the default (which is for a
single omapdrm instance to use all dss video pipes).  It is just code
I had written previously for a certain product, and it seemed
potentially useful enough to not strip out of the upstream driver.

   And how about the size of the contiguous memory area, it was left a bit
   unclear to me why it cannot be dynamic.
 
  I don't think there is anything preventing adding a bootarg, but I
  think it is not essential so ok to add later
 
  Well, maybe not essential to you =). But you are reserving 32MB memory,
  which is quite a big amount. Even if the reserved memory can be used for
  some other purposes, it's still a big chunk of special memory being
  reserved even if the user doesn't use or have a display at all.

 If you didn't have display, and were tight on memory, wouldn't you
 just disable the display device in your kernel config?

 Sure, if you want to modify your kernel. But you could as well use the
 same kernel binary, and just say vram=0M which disables the vram
 allocation. For DRM you would _have_ to modify your kernel.

 Actually, I just realized vram=0M doesn't work, as the code checks for !
 = 0, and just skips the vram argument when it's 0 =). So my code sucks
 also...

well, I suppose there is always something somewhere to improve..

anyways, at this point omapdrm isn't enabled by default in
omap2plus_defconfig.. I just want to get the platform device
registration merged in some form so folks don't have to go poking
around to find some out of tree patch in order to enable it.

  Well, it's not an issue for me either, but I just feel that doing things
  like that without allowing the user to avoid it is a bit bad thing.
 
  Btw, do you know why the dma_declare_contiguous() takes a dev pointer as
  an argument, if the memory is not private to that device? (at least I
  understood from you that the memory can be used for other purposes).

 contiguous use of the memory is private to the device.  There is also
 a global CMA pool, from which all dma_alloc_xyz() which is not
 associated w/ a per-device pool comes from.. but the advantage of a
 per-device-pool is it puts an upper limit on how much dma memory that
 device can allocate so that it cannot starve other devices.

 Ah, I see. So the 32MB you reserve there is not visible as contiguous
 memory to anyone else than omapdrm, but userspace can still use the 32MB
 area for pages that can be moved out as needed.

 But then, if it's private, it's also rather bad. If I have a kernel with
 omapfb and omapdrm enabled as modules, but I never use omapdrm, the 32MB
 is still always reserver and away from other contiguous memory use.

 Also, I just realized that besides 

[PATCH] omap2+: add drm device

2012-03-13 Thread Rob Clark
From: Andy Gross andy.gr...@ti.com

Register OMAP DRM/KMS platform device, and reserve a CMA region for
the device to use for buffer allocation.  DMM is split into a
separate device using hwmod.

Signed-off-by: Andy Gross andy.gr...@ti.com
Signed-off-by: Rob Clark r...@ti.com
---
 arch/arm/mach-omap2/Makefile  |4 ++
 arch/arm/mach-omap2/drm.c |   83 +
 arch/arm/plat-omap/common.c   |3 +-
 arch/arm/plat-omap/include/plat/drm.h |   64 +
 4 files changed, 153 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-omap2/drm.c
 create mode 100644 arch/arm/plat-omap/include/plat/drm.h

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index bd76394..9e6065b 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -189,6 +189,10 @@ ifneq ($(CONFIG_TIDSPBRIDGE),)
 obj-y  += dsp.o
 endif
 
+ifneq ($(CONFIG_DRM_OMAP),)
+obj-y  += drm.o
+endif
+
 # Specific board support
 obj-$(CONFIG_MACH_OMAP_GENERIC)+= board-generic.o
 obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o
diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
new file mode 100644
index 000..779ae02
--- /dev/null
+++ b/arch/arm/mach-omap2/drm.c
@@ -0,0 +1,83 @@
+/*
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Rob Clark rob.cl...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#include linux/module.h
+#include linux/kernel.h
+#include linux/mm.h
+#include linux/init.h
+#include linux/platform_device.h
+#include linux/dma-mapping.h
+#ifdef CONFIG_CMA
+#  include linux/dma-contiguous.h
+#endif
+
+#include plat/omap_device.h
+#include plat/omap_hwmod.h
+
+#include plat/drm.h
+
+#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
+
+static struct omap_drm_platform_data omapdrm_platdata;
+
+static struct platform_device omap_drm_device = {
+   .dev = {
+   .coherent_dma_mask = DMA_BIT_MASK(32),
+   .platform_data = omapdrm_platdata,
+   },
+   .name = omapdrm,
+   .id = 0,
+};
+
+static int __init omap_init_drm(void)
+{
+   struct omap_hwmod *oh = NULL;
+   struct platform_device *pdev;
+
+   /* lookup and populate the DMM information, if present - OMAP4+ */
+   oh = omap_hwmod_lookup(dmm);
+
+   if (oh) {
+   pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
+   false);
+   WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
+   oh-name);
+   }
+
+   return platform_device_register(omap_drm_device);
+
+}
+
+arch_initcall(omap_init_drm);
+
+void omapdrm_reserve_vram(void)
+{
+#ifdef CONFIG_CMA
+   /*
+* Create private 32MiB contiguous memory area for omapdrm.0 device
+* TODO revisit size.. if uc/wc buffers are allocated from CMA pages
+* then the amount of memory we need goes up..
+*/
+   dma_declare_contiguous(omap_drm_device.dev, 32 * SZ_1M, 0, 0);
+#else
+#  warning CMA is not enabled, there may be limitations about scanout buffer 
allocations on OMAP3 and earlier
+#endif
+}
+
+#endif
diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
index 4de7d1e..e027cc7 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -21,10 +21,10 @@
 #include plat/board.h
 #include plat/vram.h
 #include plat/dsp.h
+#include plat/drm.h
 
 #include plat/omap-secure.h
 
-
 #define NO_LENGTH_CHECK 0x
 
 struct omap_board_config_kernel *omap_board_config __initdata;
@@ -65,6 +65,7 @@ const void *__init omap_get_var_config(u16 tag, size_t *len)
 
 void __init omap_reserve(void)
 {
+   omapdrm_reserve_vram();
omapfb_reserve_sdram_memblock();
omap_vram_reserve_sdram_memblock();
omap_dsp_reserve_sdram_memblock();
diff --git a/arch/arm/plat-omap/include/plat/drm.h 
b/arch/arm/plat-omap/include/plat/drm.h
new file mode 100644
index 000..df9bc41
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/drm.h
@@ -0,0 +1,64 @@
+/*
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Rob Clark rob.cl...@linaro.org
+ *
+ * This program is free 

Re: [PATCH] omap2+: add drm device

2012-03-07 Thread Tomi Valkeinen
On Tue, 2012-03-06 at 09:29 -0600, Rob Clark wrote:
 On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Tue, 2012-03-06 at 08:01 -0600, Rob Clark wrote:
  On Tue, Mar 6, 2012 at 7:26 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
 
   Can there be more than one omapdrm device? I guess not. If so, the id
   should be -1.
 
  in the past, we have used multiple devices (using the platform-data to
  divide up the dss resources), although this was sort of a
  special-case.  But in theory you could have multiple.  (Think of a
  multi-seat scenario, where multiple compositors need to run and each
  need to be drm-master of their own device to do mode-setting on their
  corresponding display.)
 
  I think if we use .id = -1, then we'd end up with a funny looking
  bus-id for the device: platform:omapdrm:-1
 
  I don't know about that, but it's the way platform devices are meant to
  be used if there can be only one instance. If the case where there are
  multiple omapdrm devices is rather theoretical, or only used for certain
  debugging scenarios or such, I think having the id as -1 in the mainline
  is cleaner.
 
 well, I don't care much one way or another, but need to check if there
 is a small patch needed in drm core code that generates the bus-id for
 .id = -1..

Hmm, why does the drm core care about it?

And generally, I think if the bus id is -1, there is no bus id in a
sense. For example, with bus id 0 you'll get a device omapdrm.0. With
bus id -1 you'll get a device omapdrm.

   +arch_initcall(omap_init_gpu);
   +
   +void omapdrm_reserve_vram(void)
   +{
   +#ifdef CONFIG_CMA
   + /*
   +  * Create private 32MiB contiguous memory area for omapdrm.0 
   device
   +  * TODO revisit size.. if uc/wc buffers are allocated from CMA 
   pages
   +  * then the amount of memory we need goes up..
   +  */
   + dma_declare_contiguous(omap_drm_device.dev, 32 * SZ_1M, 0, 0);
  
   What does this actually do? Does it reserve the memory, i.e. it's not
   usable for others? If so, shouldn't there be a way for the user to
   configure it?
 
  It can be re-used by others.. see http://lwn.net/Articles/479297/
 
  The link didn't tell much. I looked at the patches, and
  dma_declare_contiguous allocates the memory with memblock_alloc. So is
  that allocated memory available for any users? If so, why does the
  function want a device pointer as an argument...
 
  Is the memory available for normal kmalloc, or only available via the
  CMA functions? Surely there must be some downside to the above? =) And
  if so, it should be configurable. You could have a board with no display
  at all, and you probably don't want to have 32MB allocated for DRM in
  that case.
 
 Basically the short version is memory from a CMA carveout can be
 re-used for unpinnable memory.  Not kmalloc, but it can be used for
 anon userspace pages, for example.  Userspace needs memory too.

Okay, let me ask the other way. Is 32MB enough for everyone? Hardcoding
a value like that without any possibility to adjust it just sounds like
a rather bad thing.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-03-07 Thread Tomi Valkeinen
On Tue, 2012-03-06 at 09:50 -0600, Gross, Andy wrote:
 
 
 On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen tomi.valkei...@ti.com
 wrote:
 
 
 I have to say I don't know much about DMM, but my
 understanding is that
 DMM is a bigger entity, of which TILER is only a small part,
 and DMM
 manages all memory accesses.
 
 Can there be other users for the DMM than DRM? I know there
 could be
 other users for the TILER, and I know you want to combine that
 with the
 DRM driver, but I'm wondering about the other parts of DMM
 than the
 TILER.
 
 Somehow having a DMM driver inside omapdrm sounds strange.
 
 
 The DMM does indeed contain a number of entities.  However, the TILER
 portion is the only part that requires a driver.  All other register
 modifications (LISA map settings, EMIF, etc) are done statically in
 the loader or u-boot and never changed again.  As such, DMM has become
 synonymous with TILER.

Ok. Well, as I said, I don't know much about that, just sounds rather
strange to me =).

Does this DMM has become synonymous mean that people just started
calling TILER DMM, and thus the name has stuck, or are there technical
reasons to handle it as DMM in the kernel? If the former, and if TILER
is the technically exact name for it, perhaps it would make sense to
call it TILER, as that's what (at least I) would be looking for if I
read the TRM and try to find the code for the TILER.

 Tomi




signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-03-07 Thread Rob Clark
On Wed, Mar 7, 2012 at 5:59 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-03-06 at 09:29 -0600, Rob Clark wrote:
 On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Tue, 2012-03-06 at 08:01 -0600, Rob Clark wrote:
  On Tue, Mar 6, 2012 at 7:26 AM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
 
   Can there be more than one omapdrm device? I guess not. If so, the id
   should be -1.
 
  in the past, we have used multiple devices (using the platform-data to
  divide up the dss resources), although this was sort of a
  special-case.  But in theory you could have multiple.  (Think of a
  multi-seat scenario, where multiple compositors need to run and each
  need to be drm-master of their own device to do mode-setting on their
  corresponding display.)
 
  I think if we use .id = -1, then we'd end up with a funny looking
  bus-id for the device: platform:omapdrm:-1
 
  I don't know about that, but it's the way platform devices are meant to
  be used if there can be only one instance. If the case where there are
  multiple omapdrm devices is rather theoretical, or only used for certain
  debugging scenarios or such, I think having the id as -1 in the mainline
  is cleaner.

 well, I don't care much one way or another, but need to check if there
 is a small patch needed in drm core code that generates the bus-id for
 .id = -1..

 Hmm, why does the drm core care about it?

Because it is the one generating the bus-id.. see
drivers/gpu/drm/drm_platform.c drm_platform_set_busid()

Anyways, it's just a detail about how libdrm/drmOpen() can
differentiate between multiple instances of the same driver, similar
to how PCI bus-id is used in the desktop world.  It is not difficult
to change in drm_platform_set_busid(), although not sure if that would
be considered an ABI change to userspace.  (Maybe it is less critical,
I'm under the impression that other platform-drm users didn't even
realize we had a bus-id.)

 And generally, I think if the bus id is -1, there is no bus id in a
 sense. For example, with bus id 0 you'll get a device omapdrm.0. With
 bus id -1 you'll get a device omapdrm.

   +arch_initcall(omap_init_gpu);
   +
   +void omapdrm_reserve_vram(void)
   +{
   +#ifdef CONFIG_CMA
   +     /*
   +      * Create private 32MiB contiguous memory area for omapdrm.0 
   device
   +      * TODO revisit size.. if uc/wc buffers are allocated from CMA 
   pages
   +      * then the amount of memory we need goes up..
   +      */
   +     dma_declare_contiguous(omap_drm_device.dev, 32 * SZ_1M, 0, 0);
  
   What does this actually do? Does it reserve the memory, i.e. it's not
   usable for others? If so, shouldn't there be a way for the user to
   configure it?
 
  It can be re-used by others.. see http://lwn.net/Articles/479297/
 
  The link didn't tell much. I looked at the patches, and
  dma_declare_contiguous allocates the memory with memblock_alloc. So is
  that allocated memory available for any users? If so, why does the
  function want a device pointer as an argument...
 
  Is the memory available for normal kmalloc, or only available via the
  CMA functions? Surely there must be some downside to the above? =) And
  if so, it should be configurable. You could have a board with no display
  at all, and you probably don't want to have 32MB allocated for DRM in
  that case.

 Basically the short version is memory from a CMA carveout can be
 re-used for unpinnable memory.  Not kmalloc, but it can be used for
 anon userspace pages, for example.  Userspace needs memory too.

 Okay, let me ask the other way. Is 32MB enough for everyone? Hardcoding
 a value like that without any possibility to adjust it just sounds like
 a rather bad thing.

The main requirement is that, on omap3 or before (platforms without
DMM) you need enough to allocate all of your scanout buffers.

I'm not against having a bootarg to adjust, although I strongly prefer
sane defaults and not requiring a million bootargs just to boot in
some usable fashion.

BR,
-R

  Tomi


 ___
 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap2+: add drm device

2012-03-07 Thread Tomi Valkeinen
On Wed, 2012-03-07 at 07:06 -0600, Rob Clark wrote:
 On Wed, Mar 7, 2012 at 5:59 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:

  Hmm, why does the drm core care about it?
 
 Because it is the one generating the bus-id.. see
 drivers/gpu/drm/drm_platform.c drm_platform_set_busid()
 
 Anyways, it's just a detail about how libdrm/drmOpen() can
 differentiate between multiple instances of the same driver, similar
 to how PCI bus-id is used in the desktop world.  It is not difficult
 to change in drm_platform_set_busid(), although not sure if that would
 be considered an ABI change to userspace.  (Maybe it is less critical,
 I'm under the impression that other platform-drm users didn't even
 realize we had a bus-id.)

Ok. Well, I'm fine with id 0 also, if it makes sense in the DRM side. It
was just something that caught my eye.

  Okay, let me ask the other way. Is 32MB enough for everyone? Hardcoding
  a value like that without any possibility to adjust it just sounds like
  a rather bad thing.
 
 The main requirement is that, on omap3 or before (platforms without
 DMM) you need enough to allocate all of your scanout buffers.
 
 I'm not against having a bootarg to adjust, although I strongly prefer
 sane defaults and not requiring a million bootargs just to boot in
 some usable fashion.

Well, those are two different things. I'm fine with a default of 32MB.
My point was, what if I need more, or I don't need any.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-03-07 Thread Rob Clark
On Wed, Mar 7, 2012 at 6:05 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-03-06 at 09:50 -0600, Gross, Andy wrote:


 On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen tomi.valkei...@ti.com
 wrote:


         I have to say I don't know much about DMM, but my
         understanding is that
         DMM is a bigger entity, of which TILER is only a small part,
         and DMM
         manages all memory accesses.

         Can there be other users for the DMM than DRM? I know there
         could be
         other users for the TILER, and I know you want to combine that
         with the
         DRM driver, but I'm wondering about the other parts of DMM
         than the
         TILER.

         Somehow having a DMM driver inside omapdrm sounds strange.


 The DMM does indeed contain a number of entities.  However, the TILER
 portion is the only part that requires a driver.  All other register
 modifications (LISA map settings, EMIF, etc) are done statically in
 the loader or u-boot and never changed again.  As such, DMM has become
 synonymous with TILER.

 Ok. Well, as I said, I don't know much about that, just sounds rather
 strange to me =).

 Does this DMM has become synonymous mean that people just started
 calling TILER DMM, and thus the name has stuck, or are there technical
 reasons to handle it as DMM in the kernel? If the former, and if TILER
 is the technically exact name for it, perhaps it would make sense to
 call it TILER, as that's what (at least I) would be looking for if I
 read the TRM and try to find the code for the TILER.

Well, dmm is the name in hwmod, so either way we are sort of stuck
with that.. but if you look in the TRM, you'd be looking for dynamic
memory manager, so I tend to think dmm is the better name.  But for
some reason the old driver (in some product kernels but never made it
upstream) was called tiler, which might be part of the reason that
people have started using the names interchangeably.

At any rate, neither tiler nor dmm appear in any userspace facing
API (instead you just have some bits you can set in the flags you
specify if you want tiled buffer or not when allocating a gem buffer
object).  So I guess it isn't as critical.

BR,
-R

  Tomi



 ___
 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap2+: add drm device

2012-03-07 Thread Gross, Andy
On Wed, Mar 7, 2012 at 6:05 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:

 Does this DMM has become synonymous mean that people just started
 calling TILER DMM, and thus the name has stuck, or are there technical
 reasons to handle it as DMM in the kernel? If the former, and if TILER
 is the technically exact name for it, perhaps it would make sense to
 call it TILER, as that's what (at least I) would be looking for if I
 read the TRM and try to find the code for the TILER.

  Tomi

Well the breakdown of the DMM/Tiler kind of goes like this.  The DMM
defines a set of registers used to manipulate the PAT table that backs
the address space with physical memory.  The Tiler portion of the DMM
really just describes the address space that is exposed.  Depending on
what address range you access, you'll get a different mode of access
and rotation.  Management of the address space is attributed to the
Tiler as well and that is where we have managed the address space and
provided APIs to reserve address space and pin/unpin memory to those
regions.

From a hardware perspective, we need access to the resources provided
by the DMM so that we can do the PAT programming and that can only be
gotten from the hwmod entry.  And if you use the hwmod device entry,
you have to match the name used for that entry.

Regards,

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


Re: [PATCH] omap2+: add drm device

2012-03-07 Thread Tomi Valkeinen
On Wed, 2012-03-07 at 09:59 -0600, Gross, Andy wrote:
 On Wed, Mar 7, 2012 at 6:05 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 
  Does this DMM has become synonymous mean that people just started
  calling TILER DMM, and thus the name has stuck, or are there technical
  reasons to handle it as DMM in the kernel? If the former, and if TILER
  is the technically exact name for it, perhaps it would make sense to
  call it TILER, as that's what (at least I) would be looking for if I
  read the TRM and try to find the code for the TILER.
 
   Tomi
 
 Well the breakdown of the DMM/Tiler kind of goes like this.  The DMM
 defines a set of registers used to manipulate the PAT table that backs
 the address space with physical memory.  The Tiler portion of the DMM
 really just describes the address space that is exposed.  Depending on
 what address range you access, you'll get a different mode of access
 and rotation.  Management of the address space is attributed to the
 Tiler as well and that is where we have managed the address space and
 provided APIs to reserve address space and pin/unpin memory to those
 regions.
 
 From a hardware perspective, we need access to the resources provided
 by the DMM so that we can do the PAT programming and that can only be
 gotten from the hwmod entry.  And if you use the hwmod device entry,
 you have to match the name used for that entry.

Ok. Thanks for the detailed explanation =).

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-03-06 Thread Tomi Valkeinen
On Mon, 2012-03-05 at 10:54 -0600, Rob Clark wrote:
 From: Andy Gross andy.gr...@ti.com
 
 Register OMAP DRM/KMS platform device, and reserve a CMA region for
 the device to use for buffer allocation.  DMM is split into a
 separate device using hwmod.
 
 Signed-off-by: Andy Gross andy.gr...@ti.com
 Signed-off-by: Rob Clark r...@ti.com
 ---
  arch/arm/plat-omap/Makefile   |2 +-
  arch/arm/plat-omap/common.c   |3 +-
  arch/arm/plat-omap/drm.c  |   83 
 +
  arch/arm/plat-omap/include/plat/drm.h |   64 +
  4 files changed, 150 insertions(+), 2 deletions(-)
  create mode 100644 arch/arm/plat-omap/drm.c
  create mode 100644 arch/arm/plat-omap/include/plat/drm.h
 
 diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
 index 9a58461..b86e6cb 100644
 --- a/arch/arm/plat-omap/Makefile
 +++ b/arch/arm/plat-omap/Makefile
 @@ -4,7 +4,7 @@
  
  # Common support
  obj-y := common.o sram.o clock.o devices.o dma.o mux.o \
 -  usb.o fb.o counter_32k.o
 +  usb.o fb.o counter_32k.o drm.o
  obj-m :=
  obj-n :=
  obj-  :=
 diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
 index 06383b5..0d87dab 100644
 --- a/arch/arm/plat-omap/common.c
 +++ b/arch/arm/plat-omap/common.c
 @@ -21,10 +21,10 @@
  #include plat/board.h
  #include plat/vram.h
  #include plat/dsp.h
 +#include plat/drm.h
  
  #include plat/omap-secure.h
  
 -
  #define NO_LENGTH_CHECK 0x
  
  struct omap_board_config_kernel *omap_board_config __initdata;
 @@ -65,6 +65,7 @@ const void *__init omap_get_var_config(u16 tag, size_t *len)
  
  void __init omap_reserve(void)
  {
 + omapdrm_reserve_vram();
   omapfb_reserve_sdram_memblock();
   omap_vram_reserve_sdram_memblock();
   omap_dsp_reserve_sdram_memblock();
 diff --git a/arch/arm/plat-omap/drm.c b/arch/arm/plat-omap/drm.c

As Tony said, mach-omap2 is probably a better place. fb.c is in
plat-omap because it supports both OMAP1 and OMAP2+.

 new file mode 100644
 index 000..28279df
 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.c
 @@ -0,0 +1,83 @@
 +/*
 + * DRM/KMS device registration for TI OMAP platforms
 + *
 + * Copyright (C) 2012 Texas Instruments
 + * Author: Rob Clark rob.cl...@linaro.org
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License version 2 as published 
 by
 + * the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 + * more details.
 + *
 + * You should have received a copy of the GNU General Public License along 
 with
 + * this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/module.h
 +#include linux/kernel.h
 +#include linux/mm.h
 +#include linux/init.h
 +#include linux/platform_device.h
 +#include linux/dma-mapping.h
 +#ifdef CONFIG_CMA
 +#  include linux/dma-contiguous.h
 +#endif
 +
 +#include plat/omap_device.h
 +#include plat/omap_hwmod.h
 +
 +#include plat/drm.h
 +
 +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
 +
 +static struct omap_drm_platform_data omapdrm_platdata;
 +
 +static struct platform_device omap_drm_device = {
 + .dev = {
 + .coherent_dma_mask = DMA_BIT_MASK(32),
 + .platform_data = omapdrm_platdata,
 + },
 + .name = omapdrm,
 + .id = 0,

Can there be more than one omapdrm device? I guess not. If so, the id
should be -1.

 +};
 +
 +static int __init omap_init_gpu(void)
 +{
 + struct omap_hwmod *oh = NULL;
 + struct platform_device *pdev;
 +
 + /* lookup and populate the DMM information, if present - OMAP4+ */
 + oh = omap_hwmod_lookup(dmm);
 +
 + if (oh) {
 + pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
 + false);
 + WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
 + oh-name);
 + }
 +
 + return platform_device_register(omap_drm_device);
 +
 +}

The function is called omap_init_gpu(), but it doesn't do anything
related to the gpu... And aren't DMM and DRM totally separate things,
why are they bunched together like that?

 +arch_initcall(omap_init_gpu);
 +
 +void omapdrm_reserve_vram(void)
 +{
 +#ifdef CONFIG_CMA
 + /*
 +  * Create private 32MiB contiguous memory area for omapdrm.0 device
 +  * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
 +  * then the amount of memory we need goes up..
 +  */
 + dma_declare_contiguous(omap_drm_device.dev, 32 * SZ_1M, 0, 0);

What does this actually do? Does it reserve the memory, i.e. it's not
usable for others? If so, shouldn't there be a way for the user to
configure it?

 +#else
 

Re: [PATCH] omap2+: add drm device

2012-03-06 Thread Rob Clark
On Tue, Mar 6, 2012 at 7:26 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2012-03-05 at 10:54 -0600, Rob Clark wrote:
 From: Andy Gross andy.gr...@ti.com

 Register OMAP DRM/KMS platform device, and reserve a CMA region for
 the device to use for buffer allocation.  DMM is split into a
 separate device using hwmod.

 Signed-off-by: Andy Gross andy.gr...@ti.com
 Signed-off-by: Rob Clark r...@ti.com
 ---
  arch/arm/plat-omap/Makefile           |    2 +-
  arch/arm/plat-omap/common.c           |    3 +-
  arch/arm/plat-omap/drm.c              |   83 
 +
  arch/arm/plat-omap/include/plat/drm.h |   64 +
  4 files changed, 150 insertions(+), 2 deletions(-)
  create mode 100644 arch/arm/plat-omap/drm.c
  create mode 100644 arch/arm/plat-omap/include/plat/drm.h

 diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
 index 9a58461..b86e6cb 100644
 --- a/arch/arm/plat-omap/Makefile
 +++ b/arch/arm/plat-omap/Makefile
 @@ -4,7 +4,7 @@

  # Common support
  obj-y := common.o sram.o clock.o devices.o dma.o mux.o \
 -      usb.o fb.o counter_32k.o
 +      usb.o fb.o counter_32k.o drm.o
  obj-m :=
  obj-n :=
  obj-  :=
 diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
 index 06383b5..0d87dab 100644
 --- a/arch/arm/plat-omap/common.c
 +++ b/arch/arm/plat-omap/common.c
 @@ -21,10 +21,10 @@
  #include plat/board.h
  #include plat/vram.h
  #include plat/dsp.h
 +#include plat/drm.h

  #include plat/omap-secure.h

 -
  #define NO_LENGTH_CHECK 0x

  struct omap_board_config_kernel *omap_board_config __initdata;
 @@ -65,6 +65,7 @@ const void *__init omap_get_var_config(u16 tag, size_t 
 *len)

  void __init omap_reserve(void)
  {
 +     omapdrm_reserve_vram();
       omapfb_reserve_sdram_memblock();
       omap_vram_reserve_sdram_memblock();
       omap_dsp_reserve_sdram_memblock();
 diff --git a/arch/arm/plat-omap/drm.c b/arch/arm/plat-omap/drm.c

 As Tony said, mach-omap2 is probably a better place. fb.c is in
 plat-omap because it supports both OMAP1 and OMAP2+.

 new file mode 100644
 index 000..28279df
 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.c
 @@ -0,0 +1,83 @@
 +/*
 + * DRM/KMS device registration for TI OMAP platforms
 + *
 + * Copyright (C) 2012 Texas Instruments
 + * Author: Rob Clark rob.cl...@linaro.org
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License version 2 as published 
 by
 + * the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 + * more details.
 + *
 + * You should have received a copy of the GNU General Public License along 
 with
 + * this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/module.h
 +#include linux/kernel.h
 +#include linux/mm.h
 +#include linux/init.h
 +#include linux/platform_device.h
 +#include linux/dma-mapping.h
 +#ifdef CONFIG_CMA
 +#  include linux/dma-contiguous.h
 +#endif
 +
 +#include plat/omap_device.h
 +#include plat/omap_hwmod.h
 +
 +#include plat/drm.h
 +
 +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
 +
 +static struct omap_drm_platform_data omapdrm_platdata;
 +
 +static struct platform_device omap_drm_device = {
 +             .dev = {
 +                     .coherent_dma_mask = DMA_BIT_MASK(32),
 +                     .platform_data = omapdrm_platdata,
 +             },
 +             .name = omapdrm,
 +             .id = 0,

 Can there be more than one omapdrm device? I guess not. If so, the id
 should be -1.

in the past, we have used multiple devices (using the platform-data to
divide up the dss resources), although this was sort of a
special-case.  But in theory you could have multiple.  (Think of a
multi-seat scenario, where multiple compositors need to run and each
need to be drm-master of their own device to do mode-setting on their
corresponding display.)

I think if we use .id = -1, then we'd end up with a funny looking
bus-id for the device: platform:omapdrm:-1

 +};
 +
 +static int __init omap_init_gpu(void)
 +{
 +     struct omap_hwmod *oh = NULL;
 +     struct platform_device *pdev;
 +
 +     /* lookup and populate the DMM information, if present - OMAP4+ */
 +     oh = omap_hwmod_lookup(dmm);
 +
 +     if (oh) {
 +             pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
 +                                     false);
 +             WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
 +                     oh-name);
 +     }
 +
 +     return platform_device_register(omap_drm_device);
 +
 +}

 The function is called omap_init_gpu(), but it doesn't do anything
 related to the gpu... And aren't DMM and DRM totally separate things,
 why are they bunched together like that?

only 

Re: [PATCH] omap2+: add drm device

2012-03-06 Thread Tomi Valkeinen
On Tue, 2012-03-06 at 08:01 -0600, Rob Clark wrote:
 On Tue, Mar 6, 2012 at 7:26 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:

  Can there be more than one omapdrm device? I guess not. If so, the id
  should be -1.
 
 in the past, we have used multiple devices (using the platform-data to
 divide up the dss resources), although this was sort of a
 special-case.  But in theory you could have multiple.  (Think of a
 multi-seat scenario, where multiple compositors need to run and each
 need to be drm-master of their own device to do mode-setting on their
 corresponding display.)
 
 I think if we use .id = -1, then we'd end up with a funny looking
 bus-id for the device: platform:omapdrm:-1

I don't know about that, but it's the way platform devices are meant to
be used if there can be only one instance. If the case where there are
multiple omapdrm devices is rather theoretical, or only used for certain
debugging scenarios or such, I think having the id as -1 in the mainline
is cleaner.

  +};
  +
  +static int __init omap_init_gpu(void)
  +{
  + struct omap_hwmod *oh = NULL;
  + struct platform_device *pdev;
  +
  + /* lookup and populate the DMM information, if present - OMAP4+ */
  + oh = omap_hwmod_lookup(dmm);
  +
  + if (oh) {
  + pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
  + false);
  + WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
  + oh-name);
  + }
  +
  + return platform_device_register(omap_drm_device);
  +
  +}
 
  The function is called omap_init_gpu(), but it doesn't do anything
  related to the gpu... And aren't DMM and DRM totally separate things,
  why are they bunched together like that?
 
 only legacy.. product branches also have sgx initialization here.  But
 I can change that to omap_init_drm()
 
 DMM is managed by the drm driver (and exposed to userspace as drm/gem
 buffers, shared with other devices via dma-buf, etc).  It is only
 split out to a separate device to play along with hwmod.

I have to say I don't know much about DMM, but my understanding is that
DMM is a bigger entity, of which TILER is only a small part, and DMM
manages all memory accesses.

Can there be other users for the DMM than DRM? I know there could be
other users for the TILER, and I know you want to combine that with the
DRM driver, but I'm wondering about the other parts of DMM than the
TILER.

Somehow having a DMM driver inside omapdrm sounds strange.

  +arch_initcall(omap_init_gpu);
  +
  +void omapdrm_reserve_vram(void)
  +{
  +#ifdef CONFIG_CMA
  + /*
  +  * Create private 32MiB contiguous memory area for omapdrm.0 device
  +  * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
  +  * then the amount of memory we need goes up..
  +  */
  + dma_declare_contiguous(omap_drm_device.dev, 32 * SZ_1M, 0, 0);
 
  What does this actually do? Does it reserve the memory, i.e. it's not
  usable for others? If so, shouldn't there be a way for the user to
  configure it?
 
 It can be re-used by others.. see http://lwn.net/Articles/479297/

The link didn't tell much. I looked at the patches, and
dma_declare_contiguous allocates the memory with memblock_alloc. So is
that allocated memory available for any users? If so, why does the
function want a device pointer as an argument...

Is the memory available for normal kmalloc, or only available via the
CMA functions? Surely there must be some downside to the above? =) And
if so, it should be configurable. You could have a board with no display
at all, and you probably don't want to have 32MB allocated for DRM in
that case.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [PATCH] omap2+: add drm device

2012-03-06 Thread Rob Clark
On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-03-06 at 08:01 -0600, Rob Clark wrote:
 On Tue, Mar 6, 2012 at 7:26 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:

  Can there be more than one omapdrm device? I guess not. If so, the id
  should be -1.

 in the past, we have used multiple devices (using the platform-data to
 divide up the dss resources), although this was sort of a
 special-case.  But in theory you could have multiple.  (Think of a
 multi-seat scenario, where multiple compositors need to run and each
 need to be drm-master of their own device to do mode-setting on their
 corresponding display.)

 I think if we use .id = -1, then we'd end up with a funny looking
 bus-id for the device: platform:omapdrm:-1

 I don't know about that, but it's the way platform devices are meant to
 be used if there can be only one instance. If the case where there are
 multiple omapdrm devices is rather theoretical, or only used for certain
 debugging scenarios or such, I think having the id as -1 in the mainline
 is cleaner.

well, I don't care much one way or another, but need to check if there
is a small patch needed in drm core code that generates the bus-id for
.id = -1..

  +};
  +
  +static int __init omap_init_gpu(void)
  +{
  +     struct omap_hwmod *oh = NULL;
  +     struct platform_device *pdev;
  +
  +     /* lookup and populate the DMM information, if present - OMAP4+ */
  +     oh = omap_hwmod_lookup(dmm);
  +
  +     if (oh) {
  +             pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
  +                                     false);
  +             WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
  +                     oh-name);
  +     }
  +
  +     return platform_device_register(omap_drm_device);
  +
  +}
 
  The function is called omap_init_gpu(), but it doesn't do anything
  related to the gpu... And aren't DMM and DRM totally separate things,
  why are they bunched together like that?

 only legacy.. product branches also have sgx initialization here.  But
 I can change that to omap_init_drm()

 DMM is managed by the drm driver (and exposed to userspace as drm/gem
 buffers, shared with other devices via dma-buf, etc).  It is only
 split out to a separate device to play along with hwmod.

 I have to say I don't know much about DMM, but my understanding is that
 DMM is a bigger entity, of which TILER is only a small part, and DMM
 manages all memory accesses.

when most people refer to TILER they actually refer to DMM..

DMM is the piece which is basically a GART, it is what omapdrm is
programming and managing.

 Can there be other users for the DMM than DRM? I know there could be
 other users for the TILER, and I know you want to combine that with the
 DRM driver, but I'm wondering about the other parts of DMM than the
 TILER.

yes, clearly there are other users.. we pass gem buffers allocated
from omapdrm to (for example, video decoder).  But it is omapdrm which
is managing the allocation, keeping track of which buffers are pinned
and which can be evicted, dealing with the complications of userspace
mmap'ing of tiled buffers, etc.  That stuff you don't want littered
throughout the kernel.

 Somehow having a DMM driver inside omapdrm sounds strange.

  +arch_initcall(omap_init_gpu);
  +
  +void omapdrm_reserve_vram(void)
  +{
  +#ifdef CONFIG_CMA
  +     /*
  +      * Create private 32MiB contiguous memory area for omapdrm.0 device
  +      * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
  +      * then the amount of memory we need goes up..
  +      */
  +     dma_declare_contiguous(omap_drm_device.dev, 32 * SZ_1M, 0, 0);
 
  What does this actually do? Does it reserve the memory, i.e. it's not
  usable for others? If so, shouldn't there be a way for the user to
  configure it?

 It can be re-used by others.. see http://lwn.net/Articles/479297/

 The link didn't tell much. I looked at the patches, and
 dma_declare_contiguous allocates the memory with memblock_alloc. So is
 that allocated memory available for any users? If so, why does the
 function want a device pointer as an argument...

 Is the memory available for normal kmalloc, or only available via the
 CMA functions? Surely there must be some downside to the above? =) And
 if so, it should be configurable. You could have a board with no display
 at all, and you probably don't want to have 32MB allocated for DRM in
 that case.

Basically the short version is memory from a CMA carveout can be
re-used for unpinnable memory.  Not kmalloc, but it can be used for
anon userspace pages, for example.  Userspace needs memory too.

BR,
-R

  Tomi


 ___
 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

[PATCH] omap2+: add drm device

2012-03-05 Thread Rob Clark
From: Andy Gross andy.gr...@ti.com

Register OMAP DRM/KMS platform device, and reserve a CMA region for
the device to use for buffer allocation.  DMM is split into a
separate device using hwmod.

Signed-off-by: Andy Gross andy.gr...@ti.com
Signed-off-by: Rob Clark r...@ti.com
---
 arch/arm/plat-omap/Makefile   |2 +-
 arch/arm/plat-omap/common.c   |3 +-
 arch/arm/plat-omap/drm.c  |   83 +
 arch/arm/plat-omap/include/plat/drm.h |   64 +
 4 files changed, 150 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/plat-omap/drm.c
 create mode 100644 arch/arm/plat-omap/include/plat/drm.h

diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 9a58461..b86e6cb 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -4,7 +4,7 @@
 
 # Common support
 obj-y := common.o sram.o clock.o devices.o dma.o mux.o \
-usb.o fb.o counter_32k.o
+usb.o fb.o counter_32k.o drm.o
 obj-m :=
 obj-n :=
 obj-  :=
diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
index 06383b5..0d87dab 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -21,10 +21,10 @@
 #include plat/board.h
 #include plat/vram.h
 #include plat/dsp.h
+#include plat/drm.h
 
 #include plat/omap-secure.h
 
-
 #define NO_LENGTH_CHECK 0x
 
 struct omap_board_config_kernel *omap_board_config __initdata;
@@ -65,6 +65,7 @@ const void *__init omap_get_var_config(u16 tag, size_t *len)
 
 void __init omap_reserve(void)
 {
+   omapdrm_reserve_vram();
omapfb_reserve_sdram_memblock();
omap_vram_reserve_sdram_memblock();
omap_dsp_reserve_sdram_memblock();
diff --git a/arch/arm/plat-omap/drm.c b/arch/arm/plat-omap/drm.c
new file mode 100644
index 000..28279df
--- /dev/null
+++ b/arch/arm/plat-omap/drm.c
@@ -0,0 +1,83 @@
+/*
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Rob Clark rob.cl...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#include linux/module.h
+#include linux/kernel.h
+#include linux/mm.h
+#include linux/init.h
+#include linux/platform_device.h
+#include linux/dma-mapping.h
+#ifdef CONFIG_CMA
+#  include linux/dma-contiguous.h
+#endif
+
+#include plat/omap_device.h
+#include plat/omap_hwmod.h
+
+#include plat/drm.h
+
+#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
+
+static struct omap_drm_platform_data omapdrm_platdata;
+
+static struct platform_device omap_drm_device = {
+   .dev = {
+   .coherent_dma_mask = DMA_BIT_MASK(32),
+   .platform_data = omapdrm_platdata,
+   },
+   .name = omapdrm,
+   .id = 0,
+};
+
+static int __init omap_init_gpu(void)
+{
+   struct omap_hwmod *oh = NULL;
+   struct platform_device *pdev;
+
+   /* lookup and populate the DMM information, if present - OMAP4+ */
+   oh = omap_hwmod_lookup(dmm);
+
+   if (oh) {
+   pdev = omap_device_build(oh-name, -1, oh, NULL, 0, NULL, 0,
+   false);
+   WARN(IS_ERR(pdev), Could not build omap_device for %s\n,
+   oh-name);
+   }
+
+   return platform_device_register(omap_drm_device);
+
+}
+
+arch_initcall(omap_init_gpu);
+
+void omapdrm_reserve_vram(void)
+{
+#ifdef CONFIG_CMA
+   /*
+* Create private 32MiB contiguous memory area for omapdrm.0 device
+* TODO revisit size.. if uc/wc buffers are allocated from CMA pages
+* then the amount of memory we need goes up..
+*/
+   dma_declare_contiguous(omap_drm_device.dev, 32 * SZ_1M, 0, 0);
+#else
+#  warning CMA is not enabled, there may be limitations about scanout buffer 
allocations on OMAP3 and earlier
+#endif
+}
+
+#endif
diff --git a/arch/arm/plat-omap/include/plat/drm.h 
b/arch/arm/plat-omap/include/plat/drm.h
new file mode 100644
index 000..df9bc41
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/drm.h
@@ -0,0 +1,64 @@
+/*
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Rob Clark rob.cl...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software 

Re: [PATCH] omap2+: add drm device

2012-03-05 Thread Tony Lindgren
* Rob Clark rob.cl...@linaro.org [120305 08:24]:
 From: Andy Gross andy.gr...@ti.com
 
 Register OMAP DRM/KMS platform device, and reserve a CMA region for
 the device to use for buffer allocation.  DMM is split into a
 separate device using hwmod.
 --- a/arch/arm/plat-omap/common.c
 +++ b/arch/arm/plat-omap/common.c
 @@ -21,10 +21,10 @@
  #include plat/board.h
  #include plat/vram.h
  #include plat/dsp.h
 +#include plat/drm.h
  
  #include plat/omap-secure.h
  
 -
  #define NO_LENGTH_CHECK 0x
  
  struct omap_board_config_kernel *omap_board_config __initdata;
 @@ -65,6 +65,7 @@ const void *__init omap_get_var_config(u16 tag, size_t *len)
  
  void __init omap_reserve(void)
  {
 + omapdrm_reserve_vram();
   omapfb_reserve_sdram_memblock();
   omap_vram_reserve_sdram_memblock();
   omap_dsp_reserve_sdram_memblock();

Tomi needs to look at the vram changes here.

 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.c

This file would be better in mach-omap2/drm.c. I doubt that this will
ever be usable for omap1. But other than that, up to Tomi.

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


Re: [PATCH] omap2+: add drm device

2012-03-05 Thread Rob Clark
On Mon, Mar 5, 2012 at 6:10 PM, Tony Lindgren t...@atomide.com wrote:
 * Rob Clark rob.cl...@linaro.org [120305 08:24]:
 From: Andy Gross andy.gr...@ti.com

 Register OMAP DRM/KMS platform device, and reserve a CMA region for
 the device to use for buffer allocation.  DMM is split into a
 separate device using hwmod.
 --- a/arch/arm/plat-omap/common.c
 +++ b/arch/arm/plat-omap/common.c
 @@ -21,10 +21,10 @@
  #include plat/board.h
  #include plat/vram.h
  #include plat/dsp.h
 +#include plat/drm.h

  #include plat/omap-secure.h

 -
  #define NO_LENGTH_CHECK 0x

  struct omap_board_config_kernel *omap_board_config __initdata;
 @@ -65,6 +65,7 @@ const void *__init omap_get_var_config(u16 tag, size_t 
 *len)

  void __init omap_reserve(void)
  {
 +     omapdrm_reserve_vram();
       omapfb_reserve_sdram_memblock();
       omap_vram_reserve_sdram_memblock();
       omap_dsp_reserve_sdram_memblock();

 Tomi needs to look at the vram changes here.

it is just setting up a CMA region (if CMA is enabled.. basically for
once CMA is merged)

after CMA is merged, other drivers should be migrated to use CMA, and
omap_vram removed.. but that is a topic for another patch

 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.c

 This file would be better in mach-omap2/drm.c. I doubt that this will
 ever be usable for omap1. But other than that, up to Tomi.

I was attempting to copy what omapfb was doing (plat-omap/fb.c)
because I wasn't really sure about plat-omap vs mach-omap2, although
on closer inspection I think the file I was looking at is actually for
the legacy omapdss1..  So I'll move this stuff under mach-omap2 and
resubmit this patch.

BR,
-R

 Tony
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] omap2+: add drm device

2012-01-13 Thread Rob Clark
From: Rob Clark r...@ti.com

Register OMAP DRM/KMS platform device, and reserve a CMA region for
the device to use for buffer allocation.

Signed-off-by: Rob Clark r...@ti.com
---
 arch/arm/plat-omap/Makefile |2 +-
 arch/arm/plat-omap/common.c |2 +
 arch/arm/plat-omap/drm.c|   88 +++
 arch/arm/plat-omap/drm.h|   37 ++
 4 files changed, 128 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/plat-omap/drm.c
 create mode 100644 arch/arm/plat-omap/drm.h

diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 9a58461..b86e6cb 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -4,7 +4,7 @@
 
 # Common support
 obj-y := common.o sram.o clock.o devices.o dma.o mux.o \
-usb.o fb.o counter_32k.o
+usb.o fb.o counter_32k.o drm.o
 obj-m :=
 obj-n :=
 obj-  :=
diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
index 06383b5..caf6082 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -24,6 +24,7 @@
 
 #include plat/omap-secure.h
 
+#include drm.h
 
 #define NO_LENGTH_CHECK 0x
 
@@ -65,6 +66,7 @@ const void *__init omap_get_var_config(u16 tag, size_t *len)
 
 void __init omap_reserve(void)
 {
+   omapdrm_reserve_vram();
omapfb_reserve_sdram_memblock();
omap_vram_reserve_sdram_memblock();
omap_dsp_reserve_sdram_memblock();
diff --git a/arch/arm/plat-omap/drm.c b/arch/arm/plat-omap/drm.c
new file mode 100644
index 000..5d8588f
--- /dev/null
+++ b/arch/arm/plat-omap/drm.c
@@ -0,0 +1,88 @@
+/*
+ * File: arch/arm/plat-omap/drm.c
+ *
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2011 Texas Instruments
+ * Author: Rob Clark rob.cl...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#include linux/module.h
+#include linux/kernel.h
+#include linux/mm.h
+#include linux/init.h
+#include linux/platform_device.h
+#include linux/dma-mapping.h
+#ifdef CONFIG_CMA
+#  include linux/dma-contiguous.h
+#endif
+
+#include plat/omap_device.h
+#include plat/omap_hwmod.h
+
+#include drm.h
+
+/* files from staging that contain platform data structure definitions */
+#include ../../../drivers/staging/omapdrm/omap_priv.h
+#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h
+
+#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
+
+static struct omap_drm_platform_data omapdrm_platdata;
+static struct omap_dmm_platform_data dmm_platdata;
+
+static struct platform_device omap_drm_device = {
+   .dev = {
+   .coherent_dma_mask = DMA_BIT_MASK(32),
+   .platform_data = omapdrm_platdata,
+   },
+   .name = omapdrm,
+   .id = 0,
+};
+
+static int __init omap_init_gpu(void)
+{
+   struct omap_hwmod *oh = NULL;
+
+   /* lookup and populate the DMM information, if present - OMAP4+ */
+   oh = omap_hwmod_lookup(dmm);
+
+   if (oh) {
+   dmm_platdata.base = omap_hwmod_get_mpu_rt_va(oh);
+   dmm_platdata.irq = oh-mpu_irqs[0].irq;
+
+   if (dmm_platdata.base)
+   omapdrm_platdata.dmm_pdata = dmm_platdata;
+   }
+
+   return platform_device_register(omap_drm_device);
+}
+
+arch_initcall(omap_init_gpu);
+
+void omapdrm_reserve_vram(void)
+{
+#ifdef CONFIG_CMA
+   /* Create private 32MiB contiguous memory area for omapdrm.0 device
+* TODO revisit size.. if uc/wc buffers are allocated from CMA pages
+* then the amount of memory we need goes up..
+*/
+   dma_declare_contiguous(omap_drm_device.dev, 32*SZ_1M, 0, 0);
+#else
+#  warning CMA is not enabled, there may be limitations about scanout buffer 
allocations on OMAP3 and earlier
+#endif
+}
+
+#endif
diff --git a/arch/arm/plat-omap/drm.h b/arch/arm/plat-omap/drm.h
new file mode 100644
index 000..56e0c0e
--- /dev/null
+++ b/arch/arm/plat-omap/drm.h
@@ -0,0 +1,37 @@
+/*
+ * File: arch/arm/plat-omap/drm.c
+ *
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2011 Texas Instruments
+ * Author: Rob Clark rob.cl...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will 

Re: [PATCH] omap2+: add drm device

2012-01-13 Thread Rob Clark
On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark rob.cl...@linaro.org wrote:
 From: Rob Clark r...@ti.com

 Register OMAP DRM/KMS platform device, and reserve a CMA region for
 the device to use for buffer allocation.

 Signed-off-by: Rob Clark r...@ti.com
 ---
  arch/arm/plat-omap/Makefile |    2 +-
  arch/arm/plat-omap/common.c |    2 +
  arch/arm/plat-omap/drm.c    |   88 
 +++
  arch/arm/plat-omap/drm.h    |   37 ++
  4 files changed, 128 insertions(+), 1 deletions(-)
  create mode 100644 arch/arm/plat-omap/drm.c
  create mode 100644 arch/arm/plat-omap/drm.h

 diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
 index 9a58461..b86e6cb 100644
 --- a/arch/arm/plat-omap/Makefile
 +++ b/arch/arm/plat-omap/Makefile
 @@ -4,7 +4,7 @@

  # Common support
  obj-y := common.o sram.o clock.o devices.o dma.o mux.o \
 -        usb.o fb.o counter_32k.o
 +        usb.o fb.o counter_32k.o drm.o
  obj-m :=
  obj-n :=
  obj-  :=
 diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
 index 06383b5..caf6082 100644
 --- a/arch/arm/plat-omap/common.c
 +++ b/arch/arm/plat-omap/common.c
 @@ -24,6 +24,7 @@

  #include plat/omap-secure.h

 +#include drm.h

  #define NO_LENGTH_CHECK 0x

 @@ -65,6 +66,7 @@ const void *__init omap_get_var_config(u16 tag, size_t *len)

  void __init omap_reserve(void)
  {
 +       omapdrm_reserve_vram();
        omapfb_reserve_sdram_memblock();
        omap_vram_reserve_sdram_memblock();
        omap_dsp_reserve_sdram_memblock();
 diff --git a/arch/arm/plat-omap/drm.c b/arch/arm/plat-omap/drm.c
 new file mode 100644
 index 000..5d8588f
 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.c
 @@ -0,0 +1,88 @@
 +/*
 + * File: arch/arm/plat-omap/drm.c
 + *
 + * DRM/KMS device registration for TI OMAP platforms
 + *
 + * Copyright (C) 2011 Texas Instruments
 + * Author: Rob Clark rob.cl...@linaro.org
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License version 2 as published 
 by
 + * the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 + * more details.
 + *
 + * You should have received a copy of the GNU General Public License along 
 with
 + * this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/module.h
 +#include linux/kernel.h
 +#include linux/mm.h
 +#include linux/init.h
 +#include linux/platform_device.h
 +#include linux/dma-mapping.h
 +#ifdef CONFIG_CMA
 +#  include linux/dma-contiguous.h
 +#endif
 +
 +#include plat/omap_device.h
 +#include plat/omap_hwmod.h
 +
 +#include drm.h
 +
 +/* files from staging that contain platform data structure definitions */
 +#include ../../../drivers/staging/omapdrm/omap_priv.h
 +#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h

btw, I'm not a huge fan of doing #includes this way, so if someone has
a better suggestion (given that staging drivers should not have
headers outside of drivers/staging) please let me know

BR,
-R

 +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
 +
 +static struct omap_drm_platform_data omapdrm_platdata;
 +static struct omap_dmm_platform_data dmm_platdata;
 +
 +static struct platform_device omap_drm_device = {
 +               .dev = {
 +                       .coherent_dma_mask = DMA_BIT_MASK(32),
 +                       .platform_data = omapdrm_platdata,
 +               },
 +               .name = omapdrm,
 +               .id = 0,
 +};
 +
 +static int __init omap_init_gpu(void)
 +{
 +       struct omap_hwmod *oh = NULL;
 +
 +       /* lookup and populate the DMM information, if present - OMAP4+ */
 +       oh = omap_hwmod_lookup(dmm);
 +
 +       if (oh) {
 +               dmm_platdata.base = omap_hwmod_get_mpu_rt_va(oh);
 +               dmm_platdata.irq = oh-mpu_irqs[0].irq;
 +
 +               if (dmm_platdata.base)
 +                       omapdrm_platdata.dmm_pdata = dmm_platdata;
 +       }
 +
 +       return platform_device_register(omap_drm_device);
 +}
 +
 +arch_initcall(omap_init_gpu);
 +
 +void omapdrm_reserve_vram(void)
 +{
 +#ifdef CONFIG_CMA
 +       /* Create private 32MiB contiguous memory area for omapdrm.0 device
 +        * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
 +        * then the amount of memory we need goes up..
 +        */
 +       dma_declare_contiguous(omap_drm_device.dev, 32*SZ_1M, 0, 0);
 +#else
 +#  warning CMA is not enabled, there may be limitations about scanout 
 buffer allocations on OMAP3 and earlier
 +#endif
 +}
 +
 +#endif
 diff --git a/arch/arm/plat-omap/drm.h b/arch/arm/plat-omap/drm.h
 new file mode 100644
 index 000..56e0c0e
 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.h
 @@ -0,0 +1,37 @@
 +/*
 + * File: 

Re: [PATCH] omap2+: add drm device

2012-01-13 Thread Felipe Contreras
On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark r...@ti.com wrote:
 On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark rob.cl...@linaro.org wrote:
 +/* files from staging that contain platform data structure definitions */
 +#include ../../../drivers/staging/omapdrm/omap_priv.h
 +#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h

 btw, I'm not a huge fan of doing #includes this way, so if someone has
 a better suggestion (given that staging drivers should not have
 headers outside of drivers/staging) please let me know

Move those structs to /arch/arm/plat-omap/drm.h?

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


Re: [PATCH] omap2+: add drm device

2012-01-13 Thread Aguirre, Sergio
Hi Rob,

Minor nitpicks.

On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark rob.cl...@linaro.org wrote:
 From: Rob Clark r...@ti.com

 Register OMAP DRM/KMS platform device, and reserve a CMA region for
 the device to use for buffer allocation.

 Signed-off-by: Rob Clark r...@ti.com
 ---
  arch/arm/plat-omap/Makefile |    2 +-
  arch/arm/plat-omap/common.c |    2 +
  arch/arm/plat-omap/drm.c    |   88 
 +++
  arch/arm/plat-omap/drm.h    |   37 ++
  4 files changed, 128 insertions(+), 1 deletions(-)
  create mode 100644 arch/arm/plat-omap/drm.c
  create mode 100644 arch/arm/plat-omap/drm.h

 diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
 index 9a58461..b86e6cb 100644
 --- a/arch/arm/plat-omap/Makefile
 +++ b/arch/arm/plat-omap/Makefile
 @@ -4,7 +4,7 @@

  # Common support
  obj-y := common.o sram.o clock.o devices.o dma.o mux.o \
 -        usb.o fb.o counter_32k.o
 +        usb.o fb.o counter_32k.o drm.o
  obj-m :=
  obj-n :=
  obj-  :=
 diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
 index 06383b5..caf6082 100644
 --- a/arch/arm/plat-omap/common.c
 +++ b/arch/arm/plat-omap/common.c
 @@ -24,6 +24,7 @@

  #include plat/omap-secure.h

 +#include drm.h

  #define NO_LENGTH_CHECK 0x

 @@ -65,6 +66,7 @@ const void *__init omap_get_var_config(u16 tag, size_t *len)

  void __init omap_reserve(void)
  {
 +       omapdrm_reserve_vram();
        omapfb_reserve_sdram_memblock();
        omap_vram_reserve_sdram_memblock();
        omap_dsp_reserve_sdram_memblock();
 diff --git a/arch/arm/plat-omap/drm.c b/arch/arm/plat-omap/drm.c
 new file mode 100644
 index 000..5d8588f
 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.c
 @@ -0,0 +1,88 @@
 +/*
 + * File: arch/arm/plat-omap/drm.c

I believe keeping a file path is frowned upon.

 + *
 + * DRM/KMS device registration for TI OMAP platforms
 + *
 + * Copyright (C) 2011 Texas Instruments

Happy new year! (2012?) :)

 + * Author: Rob Clark rob.cl...@linaro.org
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License version 2 as published 
 by
 + * the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 + * more details.
 + *
 + * You should have received a copy of the GNU General Public License along 
 with
 + * this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include linux/module.h
 +#include linux/kernel.h
 +#include linux/mm.h
 +#include linux/init.h
 +#include linux/platform_device.h
 +#include linux/dma-mapping.h
 +#ifdef CONFIG_CMA
 +#  include linux/dma-contiguous.h
 +#endif
 +
 +#include plat/omap_device.h
 +#include plat/omap_hwmod.h
 +
 +#include drm.h
 +
 +/* files from staging that contain platform data structure definitions */
 +#include ../../../drivers/staging/omapdrm/omap_priv.h
 +#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h
 +
 +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
 +
 +static struct omap_drm_platform_data omapdrm_platdata;
 +static struct omap_dmm_platform_data dmm_platdata;
 +
 +static struct platform_device omap_drm_device = {
 +               .dev = {
 +                       .coherent_dma_mask = DMA_BIT_MASK(32),
 +                       .platform_data = omapdrm_platdata,
 +               },
 +               .name = omapdrm,
 +               .id = 0,
 +};
 +
 +static int __init omap_init_gpu(void)
 +{
 +       struct omap_hwmod *oh = NULL;
 +
 +       /* lookup and populate the DMM information, if present - OMAP4+ */
 +       oh = omap_hwmod_lookup(dmm);
 +
 +       if (oh) {
 +               dmm_platdata.base = omap_hwmod_get_mpu_rt_va(oh);
 +               dmm_platdata.irq = oh-mpu_irqs[0].irq;
 +
 +               if (dmm_platdata.base)
 +                       omapdrm_platdata.dmm_pdata = dmm_platdata;
 +       }
 +
 +       return platform_device_register(omap_drm_device);
 +}
 +
 +arch_initcall(omap_init_gpu);
 +
 +void omapdrm_reserve_vram(void)
 +{
 +#ifdef CONFIG_CMA
 +       /* Create private 32MiB contiguous memory area for omapdrm.0 device
 +        * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
 +        * then the amount of memory we need goes up..
 +        */
 +       dma_declare_contiguous(omap_drm_device.dev, 32*SZ_1M, 0, 0);
 +#else
 +#  warning CMA is not enabled, there may be limitations about scanout 
 buffer allocations on OMAP3 and earlier
 +#endif
 +}
 +
 +#endif
 diff --git a/arch/arm/plat-omap/drm.h b/arch/arm/plat-omap/drm.h
 new file mode 100644
 index 000..56e0c0e
 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.h
 @@ -0,0 +1,37 @@
 +/*
 + * File: arch/arm/plat-omap/drm.c

Again here. (The path is incorrect anyways)

 + *
 + * DRM/KMS device registration 

Re: [PATCH] omap2+: add drm device

2012-01-13 Thread Rob Clark
On Fri, Jan 13, 2012 at 1:49 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark r...@ti.com wrote:
 On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark rob.cl...@linaro.org wrote:
 +/* files from staging that contain platform data structure definitions */
 +#include ../../../drivers/staging/omapdrm/omap_priv.h
 +#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h

 btw, I'm not a huge fan of doing #includes this way, so if someone has
 a better suggestion (given that staging drivers should not have
 headers outside of drivers/staging) please let me know

 Move those structs to /arch/arm/plat-omap/drm.h?

Can I do that?  Maybe it depends of if you consider those structs as
part of the driver, or part of the platform?

I guess that looks like how it was handled for dspbridge, so maybe
that means it is a suitable precedent..

BR,
-R

 --
 Felipe Contreras
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap2+: add drm device

2012-01-13 Thread Rob Clark
On Fri, Jan 13, 2012 at 1:51 PM, Aguirre, Sergio saagui...@ti.com wrote:
 + *
 + * DRM/KMS device registration for TI OMAP platforms
 + *
 + * Copyright (C) 2011 Texas Instruments

 Happy new year! (2012?) :)

well, the patch did start life last year.. I can update these ;-)

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


Re: [PATCH] omap2+: add drm device

2012-01-13 Thread Felipe Contreras
On Fri, Jan 13, 2012 at 9:53 PM, Rob Clark r...@ti.com wrote:
 On Fri, Jan 13, 2012 at 1:49 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark r...@ti.com wrote:
 On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark rob.cl...@linaro.org wrote:
 +/* files from staging that contain platform data structure definitions */
 +#include ../../../drivers/staging/omapdrm/omap_priv.h
 +#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h

 btw, I'm not a huge fan of doing #includes this way, so if someone has
 a better suggestion (given that staging drivers should not have
 headers outside of drivers/staging) please let me know

 Move those structs to /arch/arm/plat-omap/drm.h?

 Can I do that?  Maybe it depends of if you consider those structs as
 part of the driver, or part of the platform?

Why not? The platform is using them in arch/arm/plat-omap/drm.c.

 I guess that looks like how it was handled for dspbridge, so maybe
 that means it is a suitable precedent..

Indeed, the way I see it is this: imagine there's another driver in
staging (or maybe even out of tree), that requires this data. It would
simply include plat-omap/drm.h to fill this. OK, this is pretty
far-fetched, but demonstrates the point: platform data should be
completely independent of the drivers.

Cheers.

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


Re: [PATCH] omap2+: add drm device

2012-01-13 Thread Rob Clark
On Fri, Jan 13, 2012 at 2:23 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Fri, Jan 13, 2012 at 9:53 PM, Rob Clark r...@ti.com wrote:
 On Fri, Jan 13, 2012 at 1:49 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark r...@ti.com wrote:
 On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark rob.cl...@linaro.org wrote:
 +/* files from staging that contain platform data structure definitions */
 +#include ../../../drivers/staging/omapdrm/omap_priv.h
 +#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h

 btw, I'm not a huge fan of doing #includes this way, so if someone has
 a better suggestion (given that staging drivers should not have
 headers outside of drivers/staging) please let me know

 Move those structs to /arch/arm/plat-omap/drm.h?

 Can I do that?  Maybe it depends of if you consider those structs as
 part of the driver, or part of the platform?

 Why not? The platform is using them in arch/arm/plat-omap/drm.c.

 I guess that looks like how it was handled for dspbridge, so maybe
 that means it is a suitable precedent..

 Indeed, the way I see it is this: imagine there's another driver in
 staging (or maybe even out of tree), that requires this data. It would
 simply include plat-omap/drm.h to fill this. OK, this is pretty
 far-fetched, but demonstrates the point: platform data should be
 completely independent of the drivers.

yeah, makes sense.. I'm about to send v2 of this patch.

BR,
-R

 Cheers.

 --
 Felipe Contreras
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap 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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap2+: add drm device

2012-01-13 Thread Felipe Contreras
On Fri, Jan 13, 2012 at 9:41 PM, Rob Clark rob.cl...@linaro.org wrote:
 +void omapdrm_reserve_vram(void)
 +{
 +#ifdef CONFIG_CMA
 +       /* Create private 32MiB contiguous memory area for omapdrm.0 device
 +        * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
 +        * then the amount of memory we need goes up..
 +        */

/*
 * Foo.
 */

 +       dma_declare_contiguous(omap_drm_device.dev, 32*SZ_1M, 0, 0);

32 * SZ1_M

 +#else
 +#  warning CMA is not enabled, there may be limitations about scanout 
 buffer allocations on OMAP3 and earlier
 +#endif
 +}
 +
 +#endif
 diff --git a/arch/arm/plat-omap/drm.h b/arch/arm/plat-omap/drm.h
 new file mode 100644
 index 000..56e0c0e
 --- /dev/null
 +++ b/arch/arm/plat-omap/drm.h

Maybe this should go to include/plat

 +#ifndef __PLAT_OMAP_DRM_H__
 +#define __PLAT_OMAP_DRM_H__

I see a lot of headers using this form:
__ARCH_OMAP_FOO_H

Cheers.

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