[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-19 Thread Daniel Vetter 
On Wed, Nov 19, 2014 at 10:04:13AM +0100, Boris Brezillon wrote:
> AFAIU, the suggestion was to split drm_connector_init and
> drm_connector_register calls:
>  - drm_connector_init call should still be part of the load procedure
>(this function adds the connector to the connector list which is used
>by drm_mode_group_init_legacy_group)
>  - drm_connector_register should be called after the device has been
>registered
> 
> Here what I've done and it seems to work:
> 
> static int atmel_hlcdc_dc_connector_plug_all(struct drm_device *dev)
> {
>   struct drm_connector *connector, *failed;
>   int ret;
> 
>   mutex_lock(>mode_config.mutex);
>   list_for_each_entry(connector,
>   >mode_config.connector_list, head) { ret =
>   drm_connector_register(connector); if (ret) {
>   failed = connector;
>   goto err;
>   }
>   }
>   mutex_unlock(>mode_config.mutex);
>   return 0;
> 
> err:
>   list_for_each_entry(connector, >mode_config.connector_list, head) {
>   if (failed == connector)
>   break;
> 
>   drm_connector_unregister(connector);
>   }
>   mutex_unlock(>mode_config.mutex);
> 
>   return ret;
> }
> 
> [...]
> 
> static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev)
> {
>   struct drm_device *ddev;
>   int ret;
> 
>   ddev = drm_dev_alloc(_hlcdc_dc_driver, >dev);
>   if (!ddev)
>   return -ENOMEM;
> 
>   ret = drm_dev_set_unique(ddev, dev_name(ddev->dev));
>   if (ret)
>   goto err_unref;
> 
>   ret = atmel_hlcdc_dc_load(ddev);
>   if (ret)
>   goto err_unref;
> 
>   ret = drm_dev_register(ddev, 0);
>   if (ret)
>   goto err_unload;
> 
>   ret = atmel_hlcdc_dc_connector_plug_all(ddev);
>   if (ret)
>   goto err_unregister;
> 
>   return 0;
> 
> err_unregister:
>   drm_dev_unregister(ddev);
> 
> err_unload:
>   atmel_hlcdc_dc_unload(ddev);
> 
> err_unref:
>   drm_dev_unref(ddev);
> 
>   return ret;
> }
> 
> Daniel, can you confirm that's what you had in mind ?

Yup. To be able to have race-free driver load we need to split object
into an _init step (allocates structs and links to kernel-internal lists)
and _register (makes the object userspace-visible through sysfs and
dev-node kms object lookup idr).

This entire mess is all still fallout from the dark ages of the drm
midlayer and we'll probably have fun with this for another few years ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-19 Thread Boris Brezillon 
On Wed, 19 Nov 2014 10:02:53 +0800
Mark yao  wrote:

> On 2014年11月19日 09:09, Mark yao wrote:
> > On 2014年11月18日 22:24, Daniel Vetter wrote:
> >> On Tue, Nov 18, 2014 at 02:21:30PM +0100, Boris Brezillon wrote:
> >>> Hi Daniel,
> >>>
> >>> On Tue, 18 Nov 2014 09:32:34 +0100
> >>> Daniel Vetter  wrote:
> >>>
>  On Tue, Nov 18, 2014 at 04:00:29PM +0800, Mark Yao wrote:
> > From: Mark yao 
> >
> > This patch adds the basic structure of a DRM Driver for Rockchip 
> > Socs.
> >
> > Signed-off-by: Mark Yao 
> > Signed-off-by: Daniel Kurtz 
> > Acked-by: Daniel Vetter 
> > Reviewed-by: Rob Clark 
> > ---
> > Changes in v2:
> > - use the component framework to defer main drm driver probe
> >until all VOP devices have been probed.
> > - use dma-mapping API with ARM_DMA_USE_IOMMU, create dma mapping by
> >master device and each vop device can shared the drm dma mapping.
> > - use drm_crtc_init_with_planes and drm_universal_plane_init.
> > - remove unnecessary middle layers.
> > - add cursor set, move funcs to rockchip drm crtc.
> > - use vop reset at first init
> > - reference framebuffer when used and unreference when swap out vop
> >
> > Changes in v3:
> > - change "crtc->fb" to "crtc->primary-fb"
> > Adviced by Daniel Vetter
> > - init cursor plane with universal api, remove unnecessary cursor 
> > set,move
> >
> > Changes in v4:
> > Adviced by David Herrmann
> > - remove drm_platform_*() usage, use register drm device directly.
>  Minor fixup for that part below.
> 
>  [snip]
> 
> > +static int rockchip_drm_bind(struct device *dev)
> > +{
> > +struct drm_device *drm;
> > +int ret;
> > +
> > +drm = drm_dev_alloc(_drm_driver, dev);
> > +if (!drm)
> > +return -ENOMEM;
> > +
> > +ret = drm_dev_set_unique(drm, "%s", dev_name(dev));
> > +if (ret)
> > +goto err_free;
>  Please call rockchip_drm_load here directly and don't put it as the 
>  ->load
>  function into the driver vtable. The point of the alloc/register 
>  split is
>  that the driver can be completely set up _before_ we register 
>  anything.
>  But for backwards compat and historical reasons ->load is called 
>  somewhere
>  in the middle (so that you could access the minor nodes if needed, 
>  since
>  some drivers do that).
> >>> I tried to do the same in the atmel-hlcdc DRM driver, but I need the
> >>> primary drm_minor to register the connectors (see this kernel
> >>> backtrace [1]), which means I either initialize the connector in the
> >>> wrong place (currently part of the drm load process), or I just can't
> >>> call atmel_hlcdc_dc_load before registering the drm device...
> >> Hm right, wonder who that works with rockchip tbh.
> >>
> >> We did split up the drm_connector setup into _init and register, so 
> >> if you
> >> want to do this then you need to move the call to drm_connector_register
> >> below the call to drm_dev_register.
> >>
> >> We should probably have a drm_connectors_register_all helper which does
> >> this for all connectors on the connector list. And also grabs the
> >> appropriate lock.
> >>
> >> I guess it's somewhat obvious that no one yet actually tried this ;-)
> >> -Daniel
> > right, I re-test the driver with it, get the same problem with Boris.
> > I should move drm_connector_register below the drm_dev_register.
> I try move connector_register below drm_dev_register, but unfortunately, 
> drm_dev_register call
> drm_mode_group_init_legacy_group to save crtc, connector into list, it 
> need below  connector_register.
> so that problem is that now: connector_register need bewteen minor node 
> create and
> drm_mode_group_init_legacy_group.

AFAIU, the suggestion was to split drm_connector_init and
drm_connector_register calls:
 - drm_connector_init call should still be part of the load procedure
   (this function adds the connector to the connector list which is used
   by drm_mode_group_init_legacy_group)
 - drm_connector_register should be called after the device has been
   registered

Here what I've done and it seems to work:

static int atmel_hlcdc_dc_connector_plug_all(struct drm_device *dev)
{
struct drm_connector *connector, *failed;
int ret;

mutex_lock(>mode_config.mutex);
list_for_each_entry(connector,
>mode_config.connector_list, head) { ret =
drm_connector_register(connector); if (ret) {
failed = connector;
goto err;
}
}
mutex_unlock(>mode_config.mutex);
return 0;

err:
list_for_each_entry(connector, >mode_config.connector_list, head) {
if (failed == connector)
break;

drm_connector_unregister(connector);
}

[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-19 Thread Mark yao 
On 2014年11月19日 09:09, Mark yao wrote:
> On 2014年11月18日 22:24, Daniel Vetter wrote:
>> On Tue, Nov 18, 2014 at 02:21:30PM +0100, Boris Brezillon wrote:
>>> Hi Daniel,
>>>
>>> On Tue, 18 Nov 2014 09:32:34 +0100
>>> Daniel Vetter  wrote:
>>>
 On Tue, Nov 18, 2014 at 04:00:29PM +0800, Mark Yao wrote:
> From: Mark yao 
>
> This patch adds the basic structure of a DRM Driver for Rockchip 
> Socs.
>
> Signed-off-by: Mark Yao 
> Signed-off-by: Daniel Kurtz 
> Acked-by: Daniel Vetter 
> Reviewed-by: Rob Clark 
> ---
> Changes in v2:
> - use the component framework to defer main drm driver probe
>until all VOP devices have been probed.
> - use dma-mapping API with ARM_DMA_USE_IOMMU, create dma mapping by
>master device and each vop device can shared the drm dma mapping.
> - use drm_crtc_init_with_planes and drm_universal_plane_init.
> - remove unnecessary middle layers.
> - add cursor set, move funcs to rockchip drm crtc.
> - use vop reset at first init
> - reference framebuffer when used and unreference when swap out vop
>
> Changes in v3:
> - change "crtc->fb" to "crtc->primary-fb"
> Adviced by Daniel Vetter
> - init cursor plane with universal api, remove unnecessary cursor 
> set,move
>
> Changes in v4:
> Adviced by David Herrmann
> - remove drm_platform_*() usage, use register drm device directly.
 Minor fixup for that part below.

 [snip]

> +static int rockchip_drm_bind(struct device *dev)
> +{
> +struct drm_device *drm;
> +int ret;
> +
> +drm = drm_dev_alloc(_drm_driver, dev);
> +if (!drm)
> +return -ENOMEM;
> +
> +ret = drm_dev_set_unique(drm, "%s", dev_name(dev));
> +if (ret)
> +goto err_free;
 Please call rockchip_drm_load here directly and don't put it as the 
 ->load
 function into the driver vtable. The point of the alloc/register 
 split is
 that the driver can be completely set up _before_ we register 
 anything.
 But for backwards compat and historical reasons ->load is called 
 somewhere
 in the middle (so that you could access the minor nodes if needed, 
 since
 some drivers do that).
>>> I tried to do the same in the atmel-hlcdc DRM driver, but I need the
>>> primary drm_minor to register the connectors (see this kernel
>>> backtrace [1]), which means I either initialize the connector in the
>>> wrong place (currently part of the drm load process), or I just can't
>>> call atmel_hlcdc_dc_load before registering the drm device...
>> Hm right, wonder who that works with rockchip tbh.
>>
>> We did split up the drm_connector setup into _init and register, so 
>> if you
>> want to do this then you need to move the call to drm_connector_register
>> below the call to drm_dev_register.
>>
>> We should probably have a drm_connectors_register_all helper which does
>> this for all connectors on the connector list. And also grabs the
>> appropriate lock.
>>
>> I guess it's somewhat obvious that no one yet actually tried this ;-)
>> -Daniel
> right, I re-test the driver with it, get the same problem with Boris.
> I should move drm_connector_register below the drm_dev_register.
I try move connector_register below drm_dev_register, but unfortunately, 
drm_dev_register call
drm_mode_group_init_legacy_group to save crtc, connector into list, it 
need below  connector_register.
so that problem is that now: connector_register need bewteen minor node 
create and
drm_mode_group_init_legacy_group.
Ｉ'm consider to revert the drm/rockchip v13.

-- next part --
An HTML attachment was scrubbed...
URL:

[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-19 Thread Mark yao 
On 2014年11月18日 22:24, Daniel Vetter wrote:
> On Tue, Nov 18, 2014 at 02:21:30PM +0100, Boris Brezillon wrote:
>> Hi Daniel,
>>
>> On Tue, 18 Nov 2014 09:32:34 +0100
>> Daniel Vetter  wrote:
>>
>>> On Tue, Nov 18, 2014 at 04:00:29PM +0800, Mark Yao wrote:
 From: Mark yao 

 This patch adds the basic structure of a DRM Driver for Rockchip Socs.

 Signed-off-by: Mark Yao 
 Signed-off-by: Daniel Kurtz 
 Acked-by: Daniel Vetter 
 Reviewed-by: Rob Clark 
 ---
 Changes in v2:
 - use the component framework to defer main drm driver probe
until all VOP devices have been probed.
 - use dma-mapping API with ARM_DMA_USE_IOMMU, create dma mapping by
master device and each vop device can shared the drm dma mapping.
 - use drm_crtc_init_with_planes and drm_universal_plane_init.
 - remove unnecessary middle layers.
 - add cursor set, move funcs to rockchip drm crtc.
 - use vop reset at first init
 - reference framebuffer when used and unreference when swap out vop

 Changes in v3:
 - change "crtc->fb" to "crtc->primary-fb"
 Adviced by Daniel Vetter
 - init cursor plane with universal api, remove unnecessary cursor set,move

 Changes in v4:
 Adviced by David Herrmann
 - remove drm_platform_*() usage, use register drm device directly.
>>> Minor fixup for that part below.
>>>
>>> [snip]
>>>
 +static int rockchip_drm_bind(struct device *dev)
 +{
 +  struct drm_device *drm;
 +  int ret;
 +
 +  drm = drm_dev_alloc(_drm_driver, dev);
 +  if (!drm)
 +  return -ENOMEM;
 +
 +  ret = drm_dev_set_unique(drm, "%s", dev_name(dev));
 +  if (ret)
 +  goto err_free;
>>> Please call rockchip_drm_load here directly and don't put it as the ->load
>>> function into the driver vtable. The point of the alloc/register split is
>>> that the driver can be completely set up _before_ we register anything.
>>> But for backwards compat and historical reasons ->load is called somewhere
>>> in the middle (so that you could access the minor nodes if needed, since
>>> some drivers do that).
>> I tried to do the same in the atmel-hlcdc DRM driver, but I need the
>> primary drm_minor to register the connectors (see this kernel
>> backtrace [1]), which means I either initialize the connector in the
>> wrong place (currently part of the drm load process), or I just can't
>> call atmel_hlcdc_dc_load before registering the drm device...
> Hm right, wonder who that works with rockchip tbh.
>
> We did split up the drm_connector setup into _init and register, so if you
> want to do this then you need to move the call to drm_connector_register
> below the call to drm_dev_register.
>
> We should probably have a drm_connectors_register_all helper which does
> this for all connectors on the connector list. And also grabs the
> appropriate lock.
>
> I guess it's somewhat obvious that no one yet actually tried this ;-)
> -Daniel
right, I re-test the driver with it, get the same problem with Boris.
I should move drm_connector_register below the drm_dev_register.

[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-18 Thread Mark yao 
On 2014年11月18日 16:32, Daniel Vetter wrote:
> On Tue, Nov 18, 2014 at 04:00:29PM +0800, Mark Yao wrote:
>> From: Mark yao 
>>
>> This patch adds the basic structure of a DRM Driver for Rockchip Socs.
>>
>> Signed-off-by: Mark Yao 
>> Signed-off-by: Daniel Kurtz 
>> Acked-by: Daniel Vetter 
>> Reviewed-by: Rob Clark 
>> ---
>> Changes in v2:
>> - use the component framework to defer main drm driver probe
>>until all VOP devices have been probed.
>> - use dma-mapping API with ARM_DMA_USE_IOMMU, create dma mapping by
>>master device and each vop device can shared the drm dma mapping.
>> - use drm_crtc_init_with_planes and drm_universal_plane_init.
>> - remove unnecessary middle layers.
>> - add cursor set, move funcs to rockchip drm crtc.
>> - use vop reset at first init
>> - reference framebuffer when used and unreference when swap out vop
>>
>> Changes in v3:
>> - change "crtc->fb" to "crtc->primary-fb"
>> Adviced by Daniel Vetter
>> - init cursor plane with universal api, remove unnecessary cursor set,move
>>
>> Changes in v4:
>> Adviced by David Herrmann
>> - remove drm_platform_*() usage, use register drm device directly.
> Minor fixup for that part below.
>
> [snip]
>
>> +static int rockchip_drm_bind(struct device *dev)
>> +{
>> +struct drm_device *drm;
>> +int ret;
>> +
>> +drm = drm_dev_alloc(_drm_driver, dev);
>> +if (!drm)
>> +return -ENOMEM;
>> +
>> +ret = drm_dev_set_unique(drm, "%s", dev_name(dev));
>> +if (ret)
>> +goto err_free;
> Please call rockchip_drm_load here directly and don't put it as the ->load
> function into the driver vtable. The point of the alloc/register split is
> that the driver can be completely set up _before_ we register anything.
> But for backwards compat and historical reasons ->load is called somewhere
> in the middle (so that you could access the minor nodes if needed, since
> some drivers do that).
>
> Cheers, Daniel
OK, got it.

[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-18 Thread Mark Yao 
From: Mark yao 

This patch adds the basic structure of a DRM Driver for Rockchip Socs.

Signed-off-by: Mark Yao 
Signed-off-by: Daniel Kurtz 
Acked-by: Daniel Vetter 
Reviewed-by: Rob Clark 
---
Changes in v2:
- use the component framework to defer main drm driver probe
  until all VOP devices have been probed.
- use dma-mapping API with ARM_DMA_USE_IOMMU, create dma mapping by
  master device and each vop device can shared the drm dma mapping.
- use drm_crtc_init_with_planes and drm_universal_plane_init.
- remove unnecessary middle layers.
- add cursor set, move funcs to rockchip drm crtc.
- use vop reset at first init
- reference framebuffer when used and unreference when swap out vop

Changes in v3:
- change "crtc->fb" to "crtc->primary-fb"
Adviced by Daniel Vetter
- init cursor plane with universal api, remove unnecessary cursor set,move 

Changes in v4:
Adviced by David Herrmann
- remove drm_platform_*() usage, use register drm device directly.
Adviced by Rob Clark
- remove special mmap ioctl, do userspace mmap with normal mmap() or mmap offset

Changes in v5:
Adviced by Arnd Bergmann
- doing DMA start with a 32-bit masks with dma_mask and dma_coherent_mark
- fix some incorrect dependencies.
Adviced by Boris BREZILLON
- fix some mistake and bugs. 
Adviced by Daniel Vetter
- drop all special ioctl and use generic kms ioctl instead.
Adviced by Rob Clark
- use unlocked api for drm_fb_helper_restore_fbdev_mode.
- remove unused rockchip_gem_prime_import_sg_table.

Changes in v6:
- set gem buffer pitch 64 bytes align, needed by mali gpu.
Adviced by Daniel Kurtz
- fix some mistake, bugs, remove unused define, more better code style etc. 
- use clk_prepare()/unprepare() at probe()/remove() and clk_enable()/disable()
  at runtime instead of clk_prepare_enable().
- provide a help function from vop for encoder to do mode config, instead of
  using drm_diaplay_mode private method.
- change vop mode_set timing to make it more safely. 

Changes in v7:
- fix memory leakage problem

Changes in v8:
- fix iommu crash when use dual crtc.
- use frame start interrupt for vsync instead of line flag interrupt,
because the win config take affect at frame start time, if we use ling flag
interrupt, the address check often failed.
Adviced by Daniel Kurtz
- fix some bugs, mistake, remove unused function
- keep clock and vop disabled when probe end
- use drm_plane_helper_check_update to check update_plane if vaild

Changes in v9:
- fix suspend and resume bug, make iommu attach and detach safely.

Changes in v10:
Adviced by Andrzej Hajda
- check drm_dev if it's NULL at PM suspend/resume
Adviced by Sean Paul
- use drm_fb_helper_prepare to init fb_helper funcs
- Optimized code structure and remove some unnecessary variables.

Changes in v11:
- fix mistake that use wrong variable at rockchip sys_resume/sys_suspend.

Changes in v12:
- fix compile problem with drm-next
Adviced by Daniel Kurtz
- Optimization framebuffer reference/unreference
- Optimization Code structure
- fix pm suspend/resume problem
- fix vblank irq can't close problem

 drivers/gpu/drm/Kconfig   |2 +
 drivers/gpu/drm/Makefile  |1 +
 drivers/gpu/drm/rockchip/Kconfig  |   17 +
 drivers/gpu/drm/rockchip/Makefile |8 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  482 
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h   |   55 +
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c|  200 
 drivers/gpu/drm/rockchip/rockchip_drm_fb.h|   28 +
 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c |  210 
 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h |   20 +
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c   |  294 +
 drivers/gpu/drm/rockchip/rockchip_drm_gem.h   |   54 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   | 1459 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h   |  201 
 14 files changed, 3031 insertions(+)
 create mode 100644 drivers/gpu/drm/rockchip/Kconfig
 create mode 100644 drivers/gpu/drm/rockchip/Makefile
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_drv.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_drv.h
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fb.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fb.h
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_gem.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_gem.h
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e3b4b0f..3f1624b 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -167,6 +167,8 @@ config DRM_SAVAGE

 source "drivers/gpu/drm/exynos/Kconfig"

+source "drivers/gpu/drm/rockchip/Kconfig"
+
 source

[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-18 Thread Daniel Vetter 
On Tue, Nov 18, 2014 at 02:21:30PM +0100, Boris Brezillon wrote:
> Hi Daniel,
> 
> On Tue, 18 Nov 2014 09:32:34 +0100
> Daniel Vetter  wrote:
> 
> > On Tue, Nov 18, 2014 at 04:00:29PM +0800, Mark Yao wrote:
> > > From: Mark yao 
> > > 
> > > This patch adds the basic structure of a DRM Driver for Rockchip Socs.
> > > 
> > > Signed-off-by: Mark Yao 
> > > Signed-off-by: Daniel Kurtz 
> > > Acked-by: Daniel Vetter 
> > > Reviewed-by: Rob Clark 
> > > ---
> > > Changes in v2:
> > > - use the component framework to defer main drm driver probe
> > >   until all VOP devices have been probed.
> > > - use dma-mapping API with ARM_DMA_USE_IOMMU, create dma mapping by
> > >   master device and each vop device can shared the drm dma mapping.
> > > - use drm_crtc_init_with_planes and drm_universal_plane_init.
> > > - remove unnecessary middle layers.
> > > - add cursor set, move funcs to rockchip drm crtc.
> > > - use vop reset at first init
> > > - reference framebuffer when used and unreference when swap out vop
> > > 
> > > Changes in v3:
> > > - change "crtc->fb" to "crtc->primary-fb"
> > > Adviced by Daniel Vetter
> > > - init cursor plane with universal api, remove unnecessary cursor 
> > > set,move 
> > > 
> > > Changes in v4:
> > > Adviced by David Herrmann
> > > - remove drm_platform_*() usage, use register drm device directly.
> > 
> > Minor fixup for that part below.
> > 
> > [snip]
> > 
> > > +static int rockchip_drm_bind(struct device *dev)
> > > +{
> > > + struct drm_device *drm;
> > > + int ret;
> > > +
> > > + drm = drm_dev_alloc(_drm_driver, dev);
> > > + if (!drm)
> > > + return -ENOMEM;
> > > +
> > > + ret = drm_dev_set_unique(drm, "%s", dev_name(dev));
> > > + if (ret)
> > > + goto err_free;
> > 
> > Please call rockchip_drm_load here directly and don't put it as the ->load
> > function into the driver vtable. The point of the alloc/register split is
> > that the driver can be completely set up _before_ we register anything.
> > But for backwards compat and historical reasons ->load is called somewhere
> > in the middle (so that you could access the minor nodes if needed, since
> > some drivers do that).
> 
> I tried to do the same in the atmel-hlcdc DRM driver, but I need the
> primary drm_minor to register the connectors (see this kernel
> backtrace [1]), which means I either initialize the connector in the
> wrong place (currently part of the drm load process), or I just can't
> call atmel_hlcdc_dc_load before registering the drm device...

Hm right, wonder who that works with rockchip tbh.

We did split up the drm_connector setup into _init and register, so if you
want to do this then you need to move the call to drm_connector_register
below the call to drm_dev_register.

We should probably have a drm_connectors_register_all helper which does
this for all connectors on the connector list. And also grabs the
appropriate lock.

I guess it's somewhat obvious that no one yet actually tried this ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-18 Thread Boris Brezillon 
Hi Daniel,

On Tue, 18 Nov 2014 09:32:34 +0100
Daniel Vetter  wrote:

> On Tue, Nov 18, 2014 at 04:00:29PM +0800, Mark Yao wrote:
> > From: Mark yao 
> > 
> > This patch adds the basic structure of a DRM Driver for Rockchip Socs.
> > 
> > Signed-off-by: Mark Yao 
> > Signed-off-by: Daniel Kurtz 
> > Acked-by: Daniel Vetter 
> > Reviewed-by: Rob Clark 
> > ---
> > Changes in v2:
> > - use the component framework to defer main drm driver probe
> >   until all VOP devices have been probed.
> > - use dma-mapping API with ARM_DMA_USE_IOMMU, create dma mapping by
> >   master device and each vop device can shared the drm dma mapping.
> > - use drm_crtc_init_with_planes and drm_universal_plane_init.
> > - remove unnecessary middle layers.
> > - add cursor set, move funcs to rockchip drm crtc.
> > - use vop reset at first init
> > - reference framebuffer when used and unreference when swap out vop
> > 
> > Changes in v3:
> > - change "crtc->fb" to "crtc->primary-fb"
> > Adviced by Daniel Vetter
> > - init cursor plane with universal api, remove unnecessary cursor set,move 
> > 
> > Changes in v4:
> > Adviced by David Herrmann
> > - remove drm_platform_*() usage, use register drm device directly.
> 
> Minor fixup for that part below.
> 
> [snip]
> 
> > +static int rockchip_drm_bind(struct device *dev)
> > +{
> > +   struct drm_device *drm;
> > +   int ret;
> > +
> > +   drm = drm_dev_alloc(_drm_driver, dev);
> > +   if (!drm)
> > +   return -ENOMEM;
> > +
> > +   ret = drm_dev_set_unique(drm, "%s", dev_name(dev));
> > +   if (ret)
> > +   goto err_free;
> 
> Please call rockchip_drm_load here directly and don't put it as the ->load
> function into the driver vtable. The point of the alloc/register split is
> that the driver can be completely set up _before_ we register anything.
> But for backwards compat and historical reasons ->load is called somewhere
> in the middle (so that you could access the minor nodes if needed, since
> some drivers do that).

I tried to do the same in the atmel-hlcdc DRM driver, but I need the
primary drm_minor to register the connectors (see this kernel
backtrace [1]), which means I either initialize the connector in the
wrong place (currently part of the drm load process), or I just can't
call atmel_hlcdc_dc_load before registering the drm device...

Regards,

Boris

[1]http://code.bulix.org/fi7trt-87436


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v12 1/3] drm: rockchip: Add basic drm driver

2014-11-18 Thread Daniel Vetter 
On Tue, Nov 18, 2014 at 04:00:29PM +0800, Mark Yao wrote:
> From: Mark yao 
> 
> This patch adds the basic structure of a DRM Driver for Rockchip Socs.
> 
> Signed-off-by: Mark Yao 
> Signed-off-by: Daniel Kurtz 
> Acked-by: Daniel Vetter 
> Reviewed-by: Rob Clark 
> ---
> Changes in v2:
> - use the component framework to defer main drm driver probe
>   until all VOP devices have been probed.
> - use dma-mapping API with ARM_DMA_USE_IOMMU, create dma mapping by
>   master device and each vop device can shared the drm dma mapping.
> - use drm_crtc_init_with_planes and drm_universal_plane_init.
> - remove unnecessary middle layers.
> - add cursor set, move funcs to rockchip drm crtc.
> - use vop reset at first init
> - reference framebuffer when used and unreference when swap out vop
> 
> Changes in v3:
> - change "crtc->fb" to "crtc->primary-fb"
> Adviced by Daniel Vetter
> - init cursor plane with universal api, remove unnecessary cursor set,move 
> 
> Changes in v4:
> Adviced by David Herrmann
> - remove drm_platform_*() usage, use register drm device directly.

Minor fixup for that part below.

[snip]

> +static int rockchip_drm_bind(struct device *dev)
> +{
> + struct drm_device *drm;
> + int ret;
> +
> + drm = drm_dev_alloc(_drm_driver, dev);
> + if (!drm)
> + return -ENOMEM;
> +
> + ret = drm_dev_set_unique(drm, "%s", dev_name(dev));
> + if (ret)
> + goto err_free;

Please call rockchip_drm_load here directly and don't put it as the ->load
function into the driver vtable. The point of the alloc/register split is
that the driver can be completely set up _before_ we register anything.
But for backwards compat and historical reasons ->load is called somewhere
in the middle (so that you could access the minor nodes if needed, since
some drivers do that).

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch