[PATCH] [RFC PATCH] DRM: add DRM Driver for Samsung SoC EXYNOS4210.

[Inki Dae]

> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> Sent: Thursday, August 04, 2011 12:47 AM
> To: daeinki
> Cc: dri-devel at lists.freedesktop.org; kyungmin.park at samsung.com
> Subject: Re: [PATCH] [RFC PATCH] DRM: add DRM Driver for Samsung SoC
> EXYNOS4210.
> 
> On Mon, Aug 01, 2011 at 12:52:37PM +0900, daeinki wrote:
> > Hi, Sascha Hauer.
> > thank you for your comments and below is my answer.
> >
> > Sascha Hauer wrote:
> > >Hi,
> > >
> > >On Fri, Jul 29, 2011 at 04:24:35PM +0900, Inki Dae wrote:
> > >>This patch is a DRM Driver(only including FIMD Driver yet)
> > >>for Samsung SoC Exynos4210. and as RFC, I am sending only DRM driver
> part.
> > >>
> > >>this patch is based on git repository below:
> > >>git://git.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git,
> > >>branch: drm-next
> > >>commit-id: 5a96a899bbdee86024ab9ea6d02b9e242faacbed
> > >>
> > >>We tried to re-use lowlevel codes of the FIMD driver(s3c-fb.c
> > >>based on Linux framebuffer) but couldn't so because lowlevel codes
> > >>of s3c-fb.c are included internally and so this driver shares only
> > >>platform device.
> > >>
> > >>Sub drivers such as fimd or hdmi have indenpendent platform device and
> > >>Platform driver and when driver's probe is called, the driver object
> > >>including callbacks(for hardware control) would be registered to
> > >>Samsung drm driver. and then when samsung drm driver is probed,
> > >>each probe callback of the driver object registered is called so that
> > >>additional callbacks for drm framework would be set at this time.
> > >>
> > >>We used GEM framework for buffer management and this driver supports
> > >>only physically continuous memory yet(non-iommu). and for buffer
> allocation,
> > >>we used DMA APIs(dma_alloc_writecombine) but we will change it to CMA
> instead
> > >>of DMA APIs later.
> > >>
> > >>Refer to this link for CMA(Continuous Memory Allocator):
> > >>http://lkml.org/lkml/2011/7/20/45
> > >>
> > >>Future works:
> > >>- HDMI support.
> > >>- drm plane feature support.
> > >>  refer to this link for drm plane feature:
> > >>  http://www.spinics.net/lists/dri-devel/msg11778.html
> > >>- change the allocator to CMA.
> > >>- iommu support.(for non-continuous physical memory usage)
> > >>- fimd driver update.
> > >>- add exception codes and code clean.
> > >>
> > >>to support all features above, we need long time and hard work.
> > >>so we wish that only some features(fimd and non-iommu) are applied to
> > >>mainline first.
> > >>
> > >>We would be pleased you to give us your comments.
> > >
> > >So far I only had a quick look over the driver. You might know that
> > >I wrote a Freescale i.MX drm driver which I posted to the list some
> time
> > >ago.
> > >
> > >My driver lacks GEM support which your driver has, so I specifically
> > >looked at this part. It seems we could reuse the GEM code on i.MX and
> > >probably on most other ARMs aswell. Can you split out this code
> > >and remove the samsung_ namespace?
> > >
> > By any chance, you mean drm_gem_free_mmap_offset()?
> > this patch had already been posted by Rob Clark, Omap TI as RFC.
> >
> > you can refer to this link for it.
> > http://www.spinics.net/lists/dri-devel/msg13018.html
> >
> > if so, I am aware of it but his patch isn't applied to drm-next yet
> > and so my drm driver doesn't include his patch. of course I will
> > reuse it and remove the samsung_ namespace as you pointed out if the
> > patch is applied to drm-next.
> 
> Not only. I meant the full content of samsaung_drm_gem.c,
> samsung_drm_buf.c and maybe even samsung_drm_fb.c.
> 
> Except for the samsung namespace these functions could look the same
> on i.MX and other dumb framebuffer drm drivers.
> 
> Sascha
> 
I agree with you but I think it remains to be seem yet if I understood what
you mean.
many drm drivers(i915, radeon and so on) in mainline are using their own
prefix yet.
Sometime, these duplicated codes should be used as common part.

Thank you for your comments.

> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |